Bogus buffer length check in linux-2.6.11 read()

Bogus buffer length check in linux-2.6.11 read()

[linux-os]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 719 bytes --]


The attached file shows that the kernel thinks it's doing
something helpful by checking the length of the input
buffer for a read(). It will return "Bad Address" until
the length is 1632 bytes.  Apparently the kernel thinks
1632 is a good length!

Did anybody consider the overhead necessary to do this
and the fact that the kernel has no way of knowing if
the pointer to the buffer is valid until it actually
does the write. What was wrong with copy_to_user()?
Why is there the additional bogus check?

Cheers,
Dick Johnson
Penguin : Linux version 2.6.11 on an i686 machine (5537.79 BogoMips).
  Notice : All mail here is now cached for review by Dictator Bush.
                  98.36% of all statistics are fiction.

[-- Attachment #2: Type: TEXT/PLAIN, Size: 426 bytes --]


#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int main()
{
     char buf[0x100];
     size_t i;
     int val;
     for(i=0x1000; i > 0; i--)
     {
         if((val = read(STDIN_FILENO, buf, i)) == -1)
             perror("read");
         else
         {
             printf("Apparently the kernel thinks %u is a good length!\n", i);
             break;
         }
     }
    return 0;
}

Re: Bogus buffer length check in linux-2.6.11 read()

[Tom Felker]

On Tuesday 15 March 2005 11:59 am, linux-os wrote:
> The attached file shows that the kernel thinks it's doing
> something helpful by checking the length of the input
> buffer for a read(). It will return "Bad Address" until
> the length is 1632 bytes.  Apparently the kernel thinks
> 1632 is a good length!
>
> Did anybody consider the overhead necessary to do this
> and the fact that the kernel has no way of knowing if
> the pointer to the buffer is valid until it actually
> does the write. What was wrong with copy_to_user()?
> Why is there the additional bogus check?

I don't think that's what's happening.  The kernel is perfectly happy to read 
data into any virtual address range that your process can legally write to - 
this includes any part of the heap and any part of the stack.  The kernel 
can't check whether writing to the given address would clobber the stack or 
heap - it's your memory, you manage it.  The kernel's notion of an "invalid 
address" is very simple, and doesn't include every address that you would 
consider invalid from a C perspective.

So what's probably happening is that your stack is (1632+256) bytes tall, 
including the buffer you allocated.  (Stack grows downward on i386.)  So 
ideally you read less than 256 bytes.  If you read more than 256 but less 
than 1888 bytes, the read would damage other elements on the stack, but it is 
OK as far as the kernel is concerned.  But if you read more than that, you're 
asking the kernel to write to an address that is higher than the highest 
address of the stack (the address of the bottom element), and this address 
isn't mapped into your process, so you get EINVAL.

If you were to type more than 256 (but less than 1888) characters before 
pressing enter, the read would silently overflow the buffer, thus clobbering 
the stack, including the return address of main().  So when main tried to 
return, you'd get a segfault.  Somebody with assembly skills could probably 
craft a string which, when your program reads it, would take control of the 
program.

-- 
Tom Felker, <tcfelker@mtco.com>
<http://vlevel.sourceforge.net> - Stop fiddling with the volume knob.

No army can withstand the strength of an idea whose time has come.

Re: Bogus buffer length check in linux-2.6.11 read()

[linux-os]

On Tue, 15 Mar 2005, Tom Felker wrote:

> On Tuesday 15 March 2005 11:59 am, linux-os wrote:
>> The attached file shows that the kernel thinks it's doing
>> something helpful by checking the length of the input
>> buffer for a read(). It will return "Bad Address" until
>> the length is 1632 bytes.  Apparently the kernel thinks
>> 1632 is a good length!
>>
>> Did anybody consider the overhead necessary to do this
>> and the fact that the kernel has no way of knowing if
>> the pointer to the buffer is valid until it actually
>> does the write. What was wrong with copy_to_user()?
>> Why is there the additional bogus check?
>

Again. Assume NOTHING. Execute the code. There is NO data
being obtained from standard input. The blocking read from
standard input returns immediately without anybody hitting
any keys whatsover. No data are generated or read.

This is because somebody wrongly added code that they
wrongly thought would prevent writing beyond a user's
allocated space.

This means that the read() is no longer perfectly happy
to corrupt all of the user's memory which is the defacto
correct response for a bad buffer as shown. Instead, some
added "check in software" claims to prevent this, but
is wrong anyway because it can't possibly know how much
data area is available.

> I don't think that's what's happening.  The kernel is perfectly happy to read
> data into any virtual address range that your process can legally write to -
> this includes any part of the heap and any part of the stack.  The kernel
> can't check whether writing to the given address would clobber the stack or
> heap - it's your memory, you manage it.  The kernel's notion of an "invalid
> address" is very simple, and doesn't include every address that you would
> consider invalid from a C perspective.
>
> So what's probably happening is that your stack is (1632+256) bytes tall,
> including the buffer you allocated.  (Stack grows downward on i386.)  So
> ideally you read less than 256 bytes.  If you read more than 256 but less
> than 1888 bytes, the read would damage other elements on the stack, but it is
> OK as far as the kernel is concerned.  But if you read more than that, you're
> asking the kernel to write to an address that is higher than the highest
> address of the stack (the address of the bottom element), and this address
> isn't mapped into your process, so you get EINVAL.
>
> If you were to type more than 256 (but less than 1888) characters before
> pressing enter, the read would silently overflow the buffer, thus clobbering
> the stack, including the return address of main().  So when main tried to
> return, you'd get a segfault.  Somebody with assembly skills could probably
> craft a string which, when your program reads it, would take control of the
> program.
>
> -- 
> Tom Felker, <tcfelker@mtco.com>
> <http://vlevel.sourceforge.net> - Stop fiddling with the volume knob.
>
> No army can withstand the strength of an idea whose time has come.
>

Cheers,
Dick Johnson
Penguin : Linux version 2.6.11 on an i686 machine (5537.79 BogoMips).
  Notice : All mail here is now cached for review by Dictator Bush.
                  98.36% of all statistics are fiction.

Re: Bogus buffer length check in linux-2.6.11 read()

[Ian Campbell]

On Wed, 2005-03-16 at 07:29 -0500, linux-os wrote:

> This means that the read() is no longer perfectly happy
> to corrupt all of the user's memory which is the defacto
> correct response for a bad buffer as shown. Instead, some
> added "check in software" claims to prevent this, but
> is wrong anyway because it can't possibly know how much
> data area is available.

The manpage for read(2) that I've got says

       EFAULT buf is outside your accessible address space.

which is exactly what it would appear
        if (unlikely(!access_ok(VERIFY_WRITE, buf, count)))
                return -EFAULT;
checks for. Assuming this is the check you are bitching about -- you
could be a little more precise if you are going to complain about stuff.

Ian.

-- 
Ian Campbell

flannister, n.:
	The plastic yoke that holds a six-pack of beer together.
		-- "Sniglets", Rich Hall & Friends

Re: Bogus buffer length check in linux-2.6.11 read()

[linux-os]

On Wed, 16 Mar 2005, Ian Campbell wrote:

>
> On Wed, 2005-03-16 at 07:29 -0500, linux-os wrote:
>
>> This means that the read() is no longer perfectly happy
>> to corrupt all of the user's memory which is the defacto
>> correct response for a bad buffer as shown. Instead, some
>> added "check in software" claims to prevent this, but
>> is wrong anyway because it can't possibly know how much
>> data area is available.
>
> The manpage for read(2) that I've got says
>
>       EFAULT buf is outside your accessible address space.
>
> which is exactly what it would appear
>        if (unlikely(!access_ok(VERIFY_WRITE, buf, count)))
>                return -EFAULT;
> checks for. Assuming this is the check you are bitching about -- you
> could be a little more precise if you are going to complain about stuff.
>
> Ian.


I don't know how much more precise I could have been. I show the
code that will cause the observed condition. I explain that this
condition is new, that it doesn't correspond to the previous
behavior.

Never before was some buffer checked for length before some data
was written to it. The EFAULT is supposed to occur IFF a write
attempt occurs outside the caller's accessible address space.
This used to be done by hardware during the write to user-space.
This had zero impact upon performance. Now there is some
software added that adds CPU cycles, subtracts performance,
and cannot possibly do anything useful.

Also, the code was written to show the problem. The code
is not designed to be an example of good coding practice.

The actual problem observed with the new kernel was
when some legacy code used gets() instead of fgets().
The call returned immediately with an EFAULT because
the 'C' runtime library put some value that the kernel
didn't 'like' (4096 bytes) in the subsequent read.

This is code for which there are no sources available
and it is required to be used, cannot be replaced,
cannot be thrown away and costs about US$ 10,000
from a company that is no longer in business.

Somebody's arbitrary and capricious addition of spook
code destroyed an application's functionality.

Cheers,
Dick Johnson
Penguin : Linux version 2.6.11 on an i686 machine (5537.79 BogoMips).
  Notice : All mail here is now cached for review by Dictator Bush.
                  98.36% of all statistics are fiction.

Re: Bogus buffer length check in linux-2.6.11 read()

[Eric Dumazet]

linux-os wrote:

> 
> 
> I don't know how much more precise I could have been. I show the
> code that will cause the observed condition. I explain that this
> condition is new, that it doesn't correspond to the previous
> behavior.
> 
> Never before was some buffer checked for length before some data
> was written to it. The EFAULT is supposed to occur IFF a write
> attempt occurs outside the caller's accessible address space.
> This used to be done by hardware during the write to user-space.
> This had zero impact upon performance. Now there is some
> software added that adds CPU cycles, subtracts performance,
> and cannot possibly do anything useful.
> 
> Also, the code was written to show the problem. The code
> is not designed to be an example of good coding practice.
> 
> The actual problem observed with the new kernel was
> when some legacy code used gets() instead of fgets().
> The call returned immediately with an EFAULT because
> the 'C' runtime library put some value that the kernel
> didn't 'like' (4096 bytes) in the subsequent read.


If you use a buggy program, that had a hidden bug now exposed because 
of different kernel checks, dont complain, and use your brain.

If you do

$ export VAR1=" A very very very very long chain just to be sure my 
environnement (which is placed at the top of the stack at exec() time) 
will use at least 4 Kb  : then my litle buggy program will run if I 
type few chars but destroy my stack if I type a long string or if I 
use : cat longfile | ./xxx : So I wont complain again on lkml that I 
am sooooo lazy. Oh what could I type now, I'm tired, maybe I can copy 
this string to others variables. Yes... sure...."
$ export VAR2=$VAR1
$ export VAR3=$VAR1
$ export VAR4=$VAR1
$ export VAR5=$VAR1
Then check your env size is large enough
$ env|wc -c
    4508
$ ./xxx
./xxx 2>/dev/null

Apparently the kernel thinks 4096 is a good length!

So what ? Your program works well now, on a linux-2.6.11 typical 
machine. Ready to buffer overflow again.

Maybe you can pay me $1000 :)

Eric Dumazet
> 
> This is code for which there are no sources available
> and it is required to be used, cannot be replaced,
> cannot be thrown away and costs about US$ 10,000
> from a company that is no longer in business.
> 
> Somebody's arbitrary and capricious addition of spook
> code destroyed an application's functionality.
> 
> Cheers,
> Dick Johnson

Re: Bogus buffer length check in linux-2.6.11 read()

[linux-os]

Brilliant! And it even works!
Now if the kernel hadn't screwed up in the first place, then
your expertise wouldn't have been needed.

Thanks.

On Wed, 16 Mar 2005, Eric Dumazet wrote:

> linux-os wrote:
>
>> 
>> 
>> I don't know how much more precise I could have been. I show the
>> code that will cause the observed condition. I explain that this
>> condition is new, that it doesn't correspond to the previous
>> behavior.
>> 
>> Never before was some buffer checked for length before some data
>> was written to it. The EFAULT is supposed to occur IFF a write
>> attempt occurs outside the caller's accessible address space.
>> This used to be done by hardware during the write to user-space.
>> This had zero impact upon performance. Now there is some
>> software added that adds CPU cycles, subtracts performance,
>> and cannot possibly do anything useful.
>> 
>> Also, the code was written to show the problem. The code
>> is not designed to be an example of good coding practice.
>> 
>> The actual problem observed with the new kernel was
>> when some legacy code used gets() instead of fgets().
>> The call returned immediately with an EFAULT because
>> the 'C' runtime library put some value that the kernel
>> didn't 'like' (4096 bytes) in the subsequent read.
>
>
> If you use a buggy program, that had a hidden bug now exposed because of 
> different kernel checks, dont complain, and use your brain.
>
> If you do
>
> $ export VAR1=" A very very very very long chain just to be sure my 
> environnement (which is placed at the top of the stack at exec() time) will 
> use at least 4 Kb  : then my litle buggy program will run if I type few chars 
> but destroy my stack if I type a long string or if I use : cat longfile | 
> ./xxx : So I wont complain again on lkml that I am sooooo lazy. Oh what could 
> I type now, I'm tired, maybe I can copy this string to others variables. 
> Yes... sure...."
> $ export VAR2=$VAR1
> $ export VAR3=$VAR1
> $ export VAR4=$VAR1
> $ export VAR5=$VAR1
> Then check your env size is large enough
> $ env|wc -c
>   4508
> $ ./xxx
> ./xxx 2>/dev/null
>
> Apparently the kernel thinks 4096 is a good length!
>
> So what ? Your program works well now, on a linux-2.6.11 typical machine. 
> Ready to buffer overflow again.
>
> Maybe you can pay me $1000 :)
>
> Eric Dumazet
>> 
>> This is code for which there are no sources available
>> and it is required to be used, cannot be replaced,
>> cannot be thrown away and costs about US$ 10,000
>> from a company that is no longer in business.
>> 
>> Somebody's arbitrary and capricious addition of spook
>> code destroyed an application's functionality.
>> 
>> Cheers,
>> Dick Johnson
>

Cheers,
Dick Johnson
Penguin : Linux version 2.6.11 on an i686 machine (5537.79 BogoMips).
  Notice : All mail here is now cached for review by Dictator Bush.
                  98.36% of all statistics are fiction.

Re: Bogus buffer length check in linux-2.6.11 read()

[Robert Hancock]

linux-os wrote:
> 
> The attached file shows that the kernel thinks it's doing
> something helpful by checking the length of the input
> buffer for a read(). It will return "Bad Address" until
> the length is 1632 bytes.  Apparently the kernel thinks
> 1632 is a good length!

Likely because only 1632 bytes of memory is accessible after the start 
of the buf buffer, and trying to read in more than that results in 
copy_to_user failing to write some data.

> 
> Did anybody consider the overhead necessary to do this
> and the fact that the kernel has no way of knowing if
> the pointer to the buffer is valid until it actually
> does the write. What was wrong with copy_to_user()?
> Why is there the additional bogus check?

What additional check?

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: Bogus buffer length check in linux-2.6.11 read()

[linux-os]

On Tue, 15 Mar 2005, Robert Hancock wrote:

> linux-os wrote:
>> 
>> The attached file shows that the kernel thinks it's doing
>> something helpful by checking the length of the input
>> buffer for a read(). It will return "Bad Address" until
>> the length is 1632 bytes.  Apparently the kernel thinks
>> 1632 is a good length!
>
> Likely because only 1632 bytes of memory is accessible after the start of the 
> buf buffer, and trying to read in more than that results in copy_to_user 
> failing to write some data.
>

There was NO DATA read or written! The read() call returns immediately
without reading anything. Look at the code, assume nothing. This
is a blocking read from standard-input.

>> 
>> Did anybody consider the overhead necessary to do this
>> and the fact that the kernel has no way of knowing if
>> the pointer to the buffer is valid until it actually
>> does the write. What was wrong with copy_to_user()?
>> Why is there the additional bogus check?
>
> What additional check?
>

Somebody added some very dumb check of the input value
of a read() length that occurs before anything is
actually read.

Previously, a read(), which is a kernel write to user-data
space, would seg-fault if the read exceeded the data-space
that had been mapped. It is done by the CPU, it generates
a trap. The performance cost, when done by the CPU, if
the data doesn't exceed bounds, is zero. Now, there is
a beginning check (a wrong check BTW), in software,
before the data is even obtained from the device.

> -- 
> Robert Hancock      Saskatoon, SK, Canada
> To email, remove "nospam" from hancockr@nospamshaw.ca
> Home Page: http://www.roberthancock.com/
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Cheers,
Dick Johnson
Penguin : Linux version 2.6.11 on an i686 machine (5537.79 BogoMips).
  Notice : All mail here is now cached for review by Dictator Bush.
                  98.36% of all statistics are fiction.

Re: Bogus buffer length check in linux-2.6.11 read()

[Robert Hancock]

linux-os wrote:
> I don't know how much more precise I could have been. I show the
> code that will cause the observed condition. I explain that this
> condition is new, that it doesn't correspond to the previous
> behavior.
> 
> Never before was some buffer checked for length before some data
> was written to it. The EFAULT is supposed to occur IFF a write
> attempt occurs outside the caller's accessible address space.
> This used to be done by hardware during the write to user-space.
> This had zero impact upon performance. Now there is some
> software added that adds CPU cycles, subtracts performance,
> and cannot possibly do anything useful.
> 
> Also, the code was written to show the problem. The code
> is not designed to be an example of good coding practice.
> 
> The actual problem observed with the new kernel was
> when some legacy code used gets() instead of fgets().
> The call returned immediately with an EFAULT because
> the 'C' runtime library put some value that the kernel
> didn't 'like' (4096 bytes) in the subsequent read.
> 
> This is code for which there are no sources available
> and it is required to be used, cannot be replaced,
> cannot be thrown away and costs about US$ 10,000
> from a company that is no longer in business.
> 
> Somebody's arbitrary and capricious addition of spook
> code destroyed an application's functionality.

It appears this was added by the patch shown here:

http://lwn.net/Articles/122581/

The reason given was that if the read or write doesn't use all of the 
available space due to end-of-file, etc. the remaining part of the 
buffer given by the user is not checked for accessibility, thereby 
hiding bugs. It makes little sense for an app to do a read or write with 
a buffer larger than the space that they've actually allocated.

I can see how this might be a problem when using gets, since there is no 
way to know how big the buffer that has been allocated by the 
application is.

Note that access_ok only does a rudimentary check to determine if the 
address might be a valid user-space address, it does not check every 
page to determine if it is accessible or not like verify_area did (and 
copy_to/from_user does).

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/