Bug 160199
| Summary: | CVE-2005-1768 64bit execve() race leads to buffer overflow | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Red Hat Enterprise Linux 3 | Reporter: | Mark J. Cox <mjc> | ||||||||||
| Component: | kernel | Assignee: | Dave Anderson <anderson> | ||||||||||
| Status: | CLOSED ERRATA | QA Contact: | Brian Brock <bbrock> | ||||||||||
| Severity: | high | Docs Contact: | |||||||||||
| Priority: | medium | ||||||||||||
| Version: | 3.0 | CC: | dhoward, jparadis, peterm, petrides, security-response-team | ||||||||||
| Target Milestone: | --- | Keywords: | Security | ||||||||||
| Target Release: | --- | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | Linux | ||||||||||||
| Whiteboard: | source=secalert,reported=20050612,impact=important,public=20050704 | ||||||||||||
| Fixed In Version: | RHSA-2005-663 | Doc Type: | Bug Fix | ||||||||||
| Doc Text: | Story Points: | --- | |||||||||||
| Clone Of: | Environment: | ||||||||||||
| Last Closed: | 2005-09-28 15:21:32 UTC | Type: | --- | ||||||||||
| Regression: | --- | Mount Type: | --- | ||||||||||
| Documentation: | --- | CRM: | |||||||||||
| Verified Versions: | Category: | --- | |||||||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||||||
| Embargoed: | |||||||||||||
| Bug Depends On: | |||||||||||||
| Bug Blocks: | 156320 | ||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 115347 [details]
Proposed patch from Andi Kleen
The patch posted in Comment #1 is clearly "theoretical" because it wouldn't work or apply. Ernie's reference to the need for a "signed int" is one example, and in addition, the ia64 piece shows a "cnt" variable below which doesn't exist (it should be "n"): + if (n >= (MAX_ARG_PAGES * PAGE_SIZE) / sizeof(char *) || (max >= 0 && cnt >= max)) return -E2BIG; In any case, with a patch cobbled into place, and run against a test program Ernie whipped up, there's what appears to be a regression, where the test program works OK without the patch, but fails with it -- returning E2BIG when it shouldn't. So further investigation to determine whether patch was created incorrectly is in order. So if Andi Kleen posts a public patch today, can you copy it to this bugzilla ASAP for comparison to the old one? Thanks, Dave Created attachment 116130 [details]
Update/fix of Andi Kleen's proposed patch
The attached patch addresses the two irregularities in Andi Kleen's patch
mentioned above (the "unsigned int" and the "cnt" complaints), as well as
two other fixes:
(1) ensure that the correct errno gets passed back in the x86_64 case, and
(2) fixes for both the x86_64 and ia64 nargs() "if" statements that cause E2BIG
to be returned.
We would like to compare this patch to the upstream version that get applied.
Created attachment 116183 [details]
Second proposed patch from Andi Kleen
Created attachment 116184 [details]
Update/fix of Andi Kleen's second proposed patch
Andi Kleen sent us an updated version of his original patch,
which he said was sent to Marcelo. However, it has two bugs
in it, which I have fixed in the attached patch.
I sent Andi the following response regarding the bugs:
--------------------------------------------------------------
Hi Andi,
There appears to be two problems with this patch:
(1) In the x86_64 case, if the user attempts to use too
many arguments, the intial call to nargs() will recognize
it, and return E2BIG. But then sys32_execve() ignores
the E2BIG return argument and returns EFAULT. The ia64
case works OK because it returns what nargs() returns.
(2) Also in the x86_64 case, nargs() returns the number of
arguments found, which sys32_execve() stores in "na", and
which is later passed back to nargs() as the "max" arg.
But the second call to nargs() will then allow "na+1"
arguments, and therefore the buffer overflow. So the
second set of calls to nargs() should pass "na-1" and
"ne-1" respectively. The ia64 case works OK because its
nargs() returns the number of arguments minus 1.
I verified case (1). As far as (2), I've never been able to
reproduce the the overflow, but the logic seems to make sense.
Agreed?
Dave
--------------------------------------------------------------
Public 20050704, opening embargo A fix for this problem has just been committed to the RHEL3 U6 patch pool this evening (in kernel version 2.4.21-32.10.EL). Is there a reproducer for this bug that QE can use for testing this ? Actually, no. I don't believe anybody has ever reproduced it. An advisory has been issued which should help the problem described in this bug report. This report is therefore being closed with a resolution of ERRATA. For more information on the solution and/or where to find the updated files, please follow the link below. You may reopen this bug report if the solution does not work for you. http://rhn.redhat.com/errata/RHSA-2005-663.html |
Reported to secalert by Ilja van Sprundel, a race condition in execve affecting x86_64 and ia64 which exists because kmalloc() or get_user() can block, and hence another thread can change the argv and envp pointers, causing a bufferoverflow (also on SMP without blocking) na = nargs(argv, NULL); ... ne = nargs(envp, NULL); ... len = (na + ne + 2) * sizeof(*av); ... av = kmalloc(len, GFP_KERNEL); ... r = nargs(argv, av); ... r = nargs(envp, ae); "the nargs() function is used to first count the amount of argv and envp pointers, next a kmalloc() is done, and then nargs() is called again to copy the argv and envp pointer into this newly allocated memory. the race condition exists because kmalloc() or get_user() can block, and hence another thread can change the argv and envp pointers, causing a buffer overflow." Currently embargoed