hi,
there seems to be bug in the _copy_to_user and _copy_from_user
functions, not allowing access to the last user page.
Also I tried to decipher the inline assembly in __range_not_ok,
and it seems to work properly, but the macro comment seems to
be misleading.
wbr,
jirka
---
As shown in BZ 30352 (https://bugzilla.kernel.org/show_bug.cgi?id=30352)
there's an issue with reading last allowed page on x86_64.
The _copy_to_user and _copy_from_user functions use following
check for address limit:
if (buf + size >= limit)
fail
while it should be:
if (buf + size > limit)
fail
That's because the size represents the number of bytes being
read/write from/to buf address AND including the buf address.
So the copy function will actually never touch the limit
address even if "buf + size == limit".
Following program fails to use the last page as buffer
due to the wrong limit check.
---
#include <sys/mman.h>
#include <sys/socket.h>
#include <assert.h>
#define PAGE_SIZE (4096)
#define LAST_PAGE ((void*)(0x7fffffffe000))
int main()
{
int fds[2], err;
void * ptr = mmap(LAST_PAGE, PAGE_SIZE, PROT_READ | PROT_WRITE,
MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
assert(ptr == LAST_PAGE);
err = socketpair(AF_LOCAL, SOCK_STREAM, 0, fds);
assert(err == 0);
err = send(fds[0], ptr, PAGE_SIZE, 0);
perror("send");
assert(err == PAGE_SIZE);
err = recv(fds[1], ptr, PAGE_SIZE, MSG_WAITALL);
perror("recv");
assert(err == PAGE_SIZE);
return 0;
}
---
Other place checking the addr limit is access_ok function,
which is working properly. There's just misleading comment
for the __range_not_ok macro.
Signed-off-by: Jiri Olsa <[email protected]>
---
arch/x86/include/asm/uaccess.h | 2 +-
arch/x86/lib/copy_user_64.S | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index abd3e0e..99f0ad7 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -42,7 +42,7 @@
* Returns 0 if the range is valid, nonzero otherwise.
*
* This is equivalent to the following test:
- * (u33)addr + (u33)size >= (u33)current->addr_limit.seg (u65 for x86_64)
+ * (u33)addr + (u33)size > (u33)current->addr_limit.seg (u65 for x86_64)
*
* This needs 33-bit (65-bit for x86_64) arithmetic. We have a carry...
*/
diff --git a/arch/x86/lib/copy_user_64.S b/arch/x86/lib/copy_user_64.S
index 99e4826..a73397f 100644
--- a/arch/x86/lib/copy_user_64.S
+++ b/arch/x86/lib/copy_user_64.S
@@ -72,7 +72,7 @@ ENTRY(_copy_to_user)
addq %rdx,%rcx
jc bad_to_user
cmpq TI_addr_limit(%rax),%rcx
- jae bad_to_user
+ ja bad_to_user
ALTERNATIVE_JUMP X86_FEATURE_REP_GOOD,copy_user_generic_unrolled,copy_user_generic_string
CFI_ENDPROC
ENDPROC(_copy_to_user)
@@ -85,7 +85,7 @@ ENTRY(_copy_from_user)
addq %rdx,%rcx
jc bad_from_user
cmpq TI_addr_limit(%rax),%rcx
- jae bad_from_user
+ ja bad_from_user
ALTERNATIVE_JUMP X86_FEATURE_REP_GOOD,copy_user_generic_unrolled,copy_user_generic_string
CFI_ENDPROC
ENDPROC(_copy_from_user)
--
1.7.1
* Jiri Olsa <[email protected]> wrote:
> hi,
> there seems to be bug in the _copy_to_user and _copy_from_user
> functions, not allowing access to the last user page.
>
> Also I tried to decipher the inline assembly in __range_not_ok,
> and it seems to work properly, but the macro comment seems to
> be misleading.
>
> wbr,
> jirka
>
> ---
> As shown in BZ 30352 (https://bugzilla.kernel.org/show_bug.cgi?id=30352)
> there's an issue with reading last allowed page on x86_64.
>
> The _copy_to_user and _copy_from_user functions use following
> check for address limit:
>
> if (buf + size >= limit)
> fail
>
> while it should be:
>
> if (buf + size > limit)
> fail
>
> That's because the size represents the number of bytes being
> read/write from/to buf address AND including the buf address.
> So the copy function will actually never touch the limit
> address even if "buf + size == limit".
>
> Following program fails to use the last page as buffer
> due to the wrong limit check.
>
> ---
> #include <sys/mman.h>
> #include <sys/socket.h>
> #include <assert.h>
>
> #define PAGE_SIZE (4096)
> #define LAST_PAGE ((void*)(0x7fffffffe000))
>
> int main()
> {
> int fds[2], err;
> void * ptr = mmap(LAST_PAGE, PAGE_SIZE, PROT_READ | PROT_WRITE,
> MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
> assert(ptr == LAST_PAGE);
> err = socketpair(AF_LOCAL, SOCK_STREAM, 0, fds);
> assert(err == 0);
> err = send(fds[0], ptr, PAGE_SIZE, 0);
> perror("send");
> assert(err == PAGE_SIZE);
> err = recv(fds[1], ptr, PAGE_SIZE, MSG_WAITALL);
> perror("recv");
> assert(err == PAGE_SIZE);
> return 0;
> }
> ---
>
> Other place checking the addr limit is access_ok function,
> which is working properly. There's just misleading comment
> for the __range_not_ok macro.
>
>
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
> arch/x86/include/asm/uaccess.h | 2 +-
> arch/x86/lib/copy_user_64.S | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> index abd3e0e..99f0ad7 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -42,7 +42,7 @@
> * Returns 0 if the range is valid, nonzero otherwise.
> *
> * This is equivalent to the following test:
> - * (u33)addr + (u33)size >= (u33)current->addr_limit.seg (u65 for x86_64)
> + * (u33)addr + (u33)size > (u33)current->addr_limit.seg (u65 for x86_64)
> *
> * This needs 33-bit (65-bit for x86_64) arithmetic. We have a carry...
> */
> diff --git a/arch/x86/lib/copy_user_64.S b/arch/x86/lib/copy_user_64.S
> index 99e4826..a73397f 100644
> --- a/arch/x86/lib/copy_user_64.S
> +++ b/arch/x86/lib/copy_user_64.S
> @@ -72,7 +72,7 @@ ENTRY(_copy_to_user)
> addq %rdx,%rcx
> jc bad_to_user
> cmpq TI_addr_limit(%rax),%rcx
> - jae bad_to_user
> + ja bad_to_user
> ALTERNATIVE_JUMP X86_FEATURE_REP_GOOD,copy_user_generic_unrolled,copy_user_generic_string
> CFI_ENDPROC
> ENDPROC(_copy_to_user)
> @@ -85,7 +85,7 @@ ENTRY(_copy_from_user)
> addq %rdx,%rcx
> jc bad_from_user
> cmpq TI_addr_limit(%rax),%rcx
> - jae bad_from_user
> + ja bad_from_user
> ALTERNATIVE_JUMP X86_FEATURE_REP_GOOD,copy_user_generic_unrolled,copy_user_generic_string
> CFI_ENDPROC
> ENDPROC(_copy_from_user)
Hm, something tickles me about this area that we would reintroduce a security
hole, that we really wanted to treat the last page of user-space as some sort
of guard page but i cannot quite remember it why ...
IIRC Linus wrote bits of this so i'm Cc:-ing him just in case he remembers.
Thanks,
Ingo
On Mon, May 16, 2011 at 7:42 AM, Ingo Molnar <[email protected]> wrote:
>
> * Jiri Olsa <[email protected]> wrote:
>
>> hi,
>> there seems to be bug in the _copy_to_user and _copy_from_user
>> functions, not allowing access to the last user page.
>>
>> Also I tried to decipher the inline assembly in __range_not_ok,
>> and it seems to work properly, but the macro comment seems to
>> be misleading.
>>
>> wbr,
>> jirka
>>
>> ---
>> As shown in BZ 30352 (https://bugzilla.kernel.org/show_bug.cgi?id=30352)
>> there's an issue with reading last allowed page on x86_64.
>>
>> The _copy_to_user and _copy_from_user functions use following
>> check for address limit:
>>
>> if (buf + size >= limit)
>> fail
>>
>> while it should be:
>>
>> if (buf + size > limit)
>> fail
>>
>> That's because the size represents the number of bytes being
>> read/write from/to buf address AND including the buf address.
>> So the copy function will actually never touch the limit
>> address even if "buf + size == limit".
>>
>> Following program fails to use the last page as buffer
>> due to the wrong limit check.
>>
>> ---
>> #include <sys/mman.h>
>> #include <sys/socket.h>
>> #include <assert.h>
>>
>> #define PAGE_SIZE (4096)
>> #define LAST_PAGE ((void*)(0x7fffffffe000))
>>
>> int main()
>> {
>> int fds[2], err;
>> void * ptr = mmap(LAST_PAGE, PAGE_SIZE, PROT_READ | PROT_WRITE,
>> MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
>> assert(ptr == LAST_PAGE);
>> err = socketpair(AF_LOCAL, SOCK_STREAM, 0, fds);
>> assert(err == 0);
>> err = send(fds[0], ptr, PAGE_SIZE, 0);
>> perror("send");
>> assert(err == PAGE_SIZE);
>> err = recv(fds[1], ptr, PAGE_SIZE, MSG_WAITALL);
>> perror("recv");
>> assert(err == PAGE_SIZE);
>> return 0;
>> }
>> ---
>>
>> Other place checking the addr limit is access_ok function,
>> which is working properly. There's just misleading comment
>> for the __range_not_ok macro.
>>
>>
>> Signed-off-by: Jiri Olsa <[email protected]>
>> ---
>> arch/x86/include/asm/uaccess.h | 2 +-
>> arch/x86/lib/copy_user_64.S | 4 ++--
>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
>> index abd3e0e..99f0ad7 100644
>> --- a/arch/x86/include/asm/uaccess.h
>> +++ b/arch/x86/include/asm/uaccess.h
>> @@ -42,7 +42,7 @@
>> * Returns 0 if the range is valid, nonzero otherwise.
>> *
>> * This is equivalent to the following test:
>> - * (u33)addr + (u33)size >= (u33)current->addr_limit.seg (u65 for x86_64)
>> + * (u33)addr + (u33)size > (u33)current->addr_limit.seg (u65 for x86_64)
>> *
>> * This needs 33-bit (65-bit for x86_64) arithmetic. We have a carry...
>> */
>> diff --git a/arch/x86/lib/copy_user_64.S b/arch/x86/lib/copy_user_64.S
>> index 99e4826..a73397f 100644
>> --- a/arch/x86/lib/copy_user_64.S
>> +++ b/arch/x86/lib/copy_user_64.S
>> @@ -72,7 +72,7 @@ ENTRY(_copy_to_user)
>> addq %rdx,%rcx
>> jc bad_to_user
>> cmpq TI_addr_limit(%rax),%rcx
>> - jae bad_to_user
>> + ja bad_to_user
>> ALTERNATIVE_JUMP X86_FEATURE_REP_GOOD,copy_user_generic_unrolled,copy_user_generic_string
>> CFI_ENDPROC
>> ENDPROC(_copy_to_user)
>> @@ -85,7 +85,7 @@ ENTRY(_copy_from_user)
>> addq %rdx,%rcx
>> jc bad_from_user
>> cmpq TI_addr_limit(%rax),%rcx
>> - jae bad_from_user
>> + ja bad_from_user
>> ALTERNATIVE_JUMP X86_FEATURE_REP_GOOD,copy_user_generic_unrolled,copy_user_generic_string
>> CFI_ENDPROC
>> ENDPROC(_copy_from_user)
>
> Hm, something tickles me about this area that we would reintroduce a security
> hole, that we really wanted to treat the last page of user-space as some sort
> of guard page but i cannot quite remember it why ...
>
> IIRC Linus wrote bits of this so i'm Cc:-ing him just in case he remembers.
>
> Thanks,
>
> Ingo
The guard page is apparently due to an erratum on K8 cpus (#121
Sequential Execution Across Non-Canonical Boundary
Causes Processor Hang). However, his test code is using the last
valid page before the guard page. The bug is that the last byte
before the guard page can't be read because of the off-by-one error.
--
Brian Gerst
* Brian Gerst <[email protected]> wrote:
> On Mon, May 16, 2011 at 7:42 AM, Ingo Molnar <[email protected]> wrote:
> >
> > * Jiri Olsa <[email protected]> wrote:
> >
> >> hi,
> >> there seems to be bug in the _copy_to_user and _copy_from_user
> >> functions, not allowing access to the last user page.
> >>
> >> Also I tried to decipher the inline assembly in __range_not_ok,
> >> and it seems to work properly, but the macro comment seems to
> >> be misleading.
> >>
> >> wbr,
> >> jirka
> >>
> >> ---
> >> As shown in BZ 30352 (https://bugzilla.kernel.org/show_bug.cgi?id=30352)
> >> there's an issue with reading last allowed page on x86_64.
> >>
> >> The _copy_to_user and _copy_from_user functions use following
> >> check for address limit:
> >>
> >> if (buf + size >= limit)
> >> ? ? ? fail
> >>
> >> while it should be:
> >>
> >> if (buf + size > limit)
> >> ? ? ? fail
> >>
> >> That's because the size represents the number of bytes being
> >> read/write from/to buf address AND including the buf address.
> >> So the copy function will actually never touch the limit
> >> address even if "buf + size == limit".
> >>
> >> Following program fails to use the last page as buffer
> >> due to the wrong limit check.
> >>
> >> ---
> >> #include <sys/mman.h>
> >> #include <sys/socket.h>
> >> #include <assert.h>
> >>
> >> #define PAGE_SIZE ? ? ? (4096)
> >> #define LAST_PAGE ? ? ? ((void*)(0x7fffffffe000))
> >>
> >> int main()
> >> {
> >> ? ? ? ? int fds[2], err;
> >> ? ? ? ? void * ptr = mmap(LAST_PAGE, PAGE_SIZE, PROT_READ | PROT_WRITE,
> >> ? ? ? ? ? ? ? ? ? ? ? ? ? MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
> >> ? ? ? ? assert(ptr == LAST_PAGE);
> >> ? ? ? ? err = socketpair(AF_LOCAL, SOCK_STREAM, 0, fds);
> >> ? ? ? ? assert(err == 0);
> >> ? ? ? ? err = send(fds[0], ptr, PAGE_SIZE, 0);
> >> ? ? ? ? perror("send");
> >> ? ? ? ? assert(err == PAGE_SIZE);
> >> ? ? ? ? err = recv(fds[1], ptr, PAGE_SIZE, MSG_WAITALL);
> >> ? ? ? ? perror("recv");
> >> ? ? ? ? assert(err == PAGE_SIZE);
> >> ? ? ? ? return 0;
> >> }
> >> ---
> >>
> >> Other place checking the addr limit is access_ok function,
> >> which is working properly. There's just misleading comment
> >> for the __range_not_ok macro.
> >>
> >>
> >> Signed-off-by: Jiri Olsa <[email protected]>
> >> ---
> >> ?arch/x86/include/asm/uaccess.h | ? ?2 +-
> >> ?arch/x86/lib/copy_user_64.S ? ?| ? ?4 ++--
> >> ?2 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> >> index abd3e0e..99f0ad7 100644
> >> --- a/arch/x86/include/asm/uaccess.h
> >> +++ b/arch/x86/include/asm/uaccess.h
> >> @@ -42,7 +42,7 @@
> >> ? * Returns 0 if the range is valid, nonzero otherwise.
> >> ? *
> >> ? * This is equivalent to the following test:
> >> - * (u33)addr + (u33)size >= (u33)current->addr_limit.seg (u65 for x86_64)
> >> + * (u33)addr + (u33)size > (u33)current->addr_limit.seg (u65 for x86_64)
> >> ? *
> >> ? * This needs 33-bit (65-bit for x86_64) arithmetic. We have a carry...
> >> ? */
> >> diff --git a/arch/x86/lib/copy_user_64.S b/arch/x86/lib/copy_user_64.S
> >> index 99e4826..a73397f 100644
> >> --- a/arch/x86/lib/copy_user_64.S
> >> +++ b/arch/x86/lib/copy_user_64.S
> >> @@ -72,7 +72,7 @@ ENTRY(_copy_to_user)
> >> ? ? ? addq %rdx,%rcx
> >> ? ? ? jc bad_to_user
> >> ? ? ? cmpq TI_addr_limit(%rax),%rcx
> >> - ? ? jae bad_to_user
> >> + ? ? ja bad_to_user
> >> ? ? ? ALTERNATIVE_JUMP X86_FEATURE_REP_GOOD,copy_user_generic_unrolled,copy_user_generic_string
> >> ? ? ? CFI_ENDPROC
> >> ?ENDPROC(_copy_to_user)
> >> @@ -85,7 +85,7 @@ ENTRY(_copy_from_user)
> >> ? ? ? addq %rdx,%rcx
> >> ? ? ? jc bad_from_user
> >> ? ? ? cmpq TI_addr_limit(%rax),%rcx
> >> - ? ? jae bad_from_user
> >> + ? ? ja bad_from_user
> >> ? ? ? ALTERNATIVE_JUMP X86_FEATURE_REP_GOOD,copy_user_generic_unrolled,copy_user_generic_string
> >> ? ? ? CFI_ENDPROC
> >> ?ENDPROC(_copy_from_user)
> >
> > Hm, something tickles me about this area that we would reintroduce a security
> > hole, that we really wanted to treat the last page of user-space as some sort
> > of guard page but i cannot quite remember it why ...
> >
> > IIRC Linus wrote bits of this so i'm Cc:-ing him just in case he remembers.
> >
> > Thanks,
> >
> > ? ? ? ?Ingo
>
> The guard page is apparently due to an erratum on K8 cpus (#121
> Sequential Execution Across Non-Canonical Boundary
> Causes Processor Hang). However, his test code is using the last
> valid page before the guard page. The bug is that the last byte
> before the guard page can't be read because of the off-by-one error.
Ok, so if you otherwise agree with the change then Jiri please update the
changelog with this information and Brian's Acked-by.
Thanks,
Ingo
On Mon, May 16, 2011 at 4:42 AM, Ingo Molnar <[email protected]> wrote:
>
> Hm, something tickles me about this area that we would reintroduce a security
> hole, that we really wanted to treat the last page of user-space as some sort
> of guard page but i cannot quite remember it why ...
>
> IIRC Linus wrote bits of this so i'm Cc:-ing him just in case he remembers.
No, I suspect the patch is correct, and it's just a bug. I think it
comes from the "get_user_X()" cases that afaik use "jae" because they
add one less than the size (and thus avoid it entirely for the
single-byte case). See "getuser.S" in the same directory.
But right now I think I need to do a 2.6.39 release later today (after
I get some sleep), so doing it as a stable patch (presumably going
back to pretty much the beginning of time) is probably the right
thing.
Linus
* Linus Torvalds <[email protected]> wrote:
> On Mon, May 16, 2011 at 4:42 AM, Ingo Molnar <[email protected]> wrote:
> >
> > Hm, something tickles me about this area that we would reintroduce a security
> > hole, that we really wanted to treat the last page of user-space as some sort
> > of guard page but i cannot quite remember it why ...
> >
> > IIRC Linus wrote bits of this so i'm Cc:-ing him just in case he remembers.
>
> No, I suspect the patch is correct, and it's just a bug. I think it
> comes from the "get_user_X()" cases that afaik use "jae" because they
> add one less than the size (and thus avoid it entirely for the
> single-byte case). See "getuser.S" in the same directory.
>
> But right now I think I need to do a 2.6.39 release later today (after
> I get some sleep), so doing it as a stable patch (presumably going
> back to pretty much the beginning of time) is probably the right
> thing.
Absolutely - we had this for a long time so it's not a regression and there is
very little gain from trying to squeeze this fix in so close to v2.6.39 - and
there are nonzero risks, considering how widely used assembly code this
changes.
I've queued it up for v2.6.40 with your Acked-by.
Thanks,
Ingo
Commit-ID: 26afb7c661080ae3f1f13ddf7f0c58c4f931c22b
Gitweb: http://git.kernel.org/tip/26afb7c661080ae3f1f13ddf7f0c58c4f931c22b
Author: Jiri Olsa <[email protected]>
AuthorDate: Thu, 12 May 2011 16:30:30 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 18 May 2011 12:49:00 +0200
x86, 64-bit: Fix copy_[to/from]_user() checks for the userspace address limit
As reported in BZ #30352:
https://bugzilla.kernel.org/show_bug.cgi?id=30352
there's a kernel bug related to reading the last allowed page on x86_64.
The _copy_to_user() and _copy_from_user() functions use the following
check for address limit:
if (buf + size >= limit)
fail();
while it should be more permissive:
if (buf + size > limit)
fail();
That's because the size represents the number of bytes being
read/write from/to buf address AND including the buf address.
So the copy function will actually never touch the limit
address even if "buf + size == limit".
Following program fails to use the last page as buffer
due to the wrong limit check:
#include <sys/mman.h>
#include <sys/socket.h>
#include <assert.h>
#define PAGE_SIZE (4096)
#define LAST_PAGE ((void*)(0x7fffffffe000))
int main()
{
int fds[2], err;
void * ptr = mmap(LAST_PAGE, PAGE_SIZE, PROT_READ | PROT_WRITE,
MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
assert(ptr == LAST_PAGE);
err = socketpair(AF_LOCAL, SOCK_STREAM, 0, fds);
assert(err == 0);
err = send(fds[0], ptr, PAGE_SIZE, 0);
perror("send");
assert(err == PAGE_SIZE);
err = recv(fds[1], ptr, PAGE_SIZE, MSG_WAITALL);
perror("recv");
assert(err == PAGE_SIZE);
return 0;
}
The other place checking the addr limit is the access_ok() function,
which is working properly. There's just a misleading comment
for the __range_not_ok() macro - which this patch fixes as well.
The last page of the user-space address range is a guard page and
Brian Gerst observed that the guard page itself due to an erratum on K8 cpus
(#121 Sequential Execution Across Non-Canonical Boundary Causes Processor
Hang).
However, the test code is using the last valid page before the guard page.
The bug is that the last byte before the guard page can't be read
because of the off-by-one error. The guard page is left in place.
This bug would normally not show up because the last page is
part of the process stack and never accessed via syscalls.
Signed-off-by: Jiri Olsa <[email protected]>
Acked-by: Brian Gerst <[email protected]>
Acked-by: Linus Torvalds <[email protected]>
Cc: <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/include/asm/uaccess.h | 2 +-
arch/x86/lib/copy_user_64.S | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index abd3e0e..99f0ad7 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -42,7 +42,7 @@
* Returns 0 if the range is valid, nonzero otherwise.
*
* This is equivalent to the following test:
- * (u33)addr + (u33)size >= (u33)current->addr_limit.seg (u65 for x86_64)
+ * (u33)addr + (u33)size > (u33)current->addr_limit.seg (u65 for x86_64)
*
* This needs 33-bit (65-bit for x86_64) arithmetic. We have a carry...
*/
diff --git a/arch/x86/lib/copy_user_64.S b/arch/x86/lib/copy_user_64.S
index d17a117..0248402 100644
--- a/arch/x86/lib/copy_user_64.S
+++ b/arch/x86/lib/copy_user_64.S
@@ -79,7 +79,7 @@ ENTRY(_copy_to_user)
addq %rdx,%rcx
jc bad_to_user
cmpq TI_addr_limit(%rax),%rcx
- jae bad_to_user
+ ja bad_to_user
ALTERNATIVE_JUMP X86_FEATURE_REP_GOOD,X86_FEATURE_ERMS, \
copy_user_generic_unrolled,copy_user_generic_string, \
copy_user_enhanced_fast_string
@@ -94,7 +94,7 @@ ENTRY(_copy_from_user)
addq %rdx,%rcx
jc bad_from_user
cmpq TI_addr_limit(%rax),%rcx
- jae bad_from_user
+ ja bad_from_user
ALTERNATIVE_JUMP X86_FEATURE_REP_GOOD,X86_FEATURE_ERMS, \
copy_user_generic_unrolled,copy_user_generic_string, \
copy_user_enhanced_fast_string
On Wed, May 18, 2011 at 1:43 PM, tip-bot for Jiri Olsa <[email protected]> wrote:
> As reported in BZ #30352:
>
> ?https://bugzilla.kernel.org/show_bug.cgi?id=30352
>
> there's a kernel bug related to reading the last allowed page on x86_64.
Btw, I think this message is very misleading.
It has nothing what-so-ever to do with "the last allowed page".
It is not about pages at all. It's about bytes. It's the very last
byte of the address space. The fact that the *test*program* uses mmap
and then sends a single page is totally irrelevant both for the kernel
and for the patch itself.
Linus
* Linus Torvalds <[email protected]> wrote:
> On Wed, May 18, 2011 at 1:43 PM, tip-bot for Jiri Olsa <[email protected]> wrote:
> > As reported in BZ #30352:
> >
> > ?https://bugzilla.kernel.org/show_bug.cgi?id=30352
> >
> > there's a kernel bug related to reading the last allowed page on x86_64.
>
> Btw, I think this message is very misleading.
>
> It has nothing what-so-ever to do with "the last allowed page".
Yeah, i noticed that and this is why i added this bit from Brian's explanation
to the end of the commit:
The bug is that the last byte before the guard page can't be read
because of the off-by-one error. The guard page is left in place.
I should have fixed the original changelog instead of trying to clarify it via
appending it ...
Thanks,
Ingo