copy_from_user

copy_from_user

[Paul Mackerras]

Some time ago (in the 2.1 series IIRC) we added code to copy_from_user
to zero the remainder of the destination buffer if we faulted on the
source.  The motive was to eliminate some potential security holes
that could arise if callers didn't check the return value from
copy_from_user and continued on to pass the contents of the
destination buffer back to userspace in one way or another.

However, I notice that copy_from_user on i386 in 2.5 doesn't clear the
destination if the access_ok() check fails, or if the size is 1, 2 or
4.  Have all the callers of copy_from_user been checked?  Is the
zeroing of the destination no longer necessary?

Thanks,
Paul.

Re: copy_from_user

[Andrew Morton]

Paul Mackerras <paulus@samba.org> wrote:
>
> Some time ago (in the 2.1 series IIRC) we added code to copy_from_user
>  to zero the remainder of the destination buffer if we faulted on the
>  source.  The motive was to eliminate some potential security holes
>  that could arise if callers didn't check the return value from
>  copy_from_user and continued on to pass the contents of the
>  destination buffer back to userspace in one way or another.
> 
>  However, I notice that copy_from_user on i386 in 2.5 doesn't clear the
>  destination if the access_ok() check fails,

This was not deliberate - the memset simply got lost.

It is simple enough to fix.  Do we remember the details of the
security hole?

> or if the size is 1, 2 or 4.

This one is OK - __get_user_asm() does the zeroing in the fixup code.

Re: copy_from_user

[Paul Mackerras]

Andrew Morton writes:

> This was not deliberate - the memset simply got lost.
> 
> It is simple enough to fix.  Do we remember the details of the
> security hole?

My memory is a little hazy since it is several years ago, but as I
remember it, when you did a write on a pipe, the code would do
copy_from_user and not check the return value.  So, if you did a write
from an unmapped address, and then a read from the pipe, you would get
whatever was in the page that the kernel had allocated as the pipe
buffer.  Tridge had a program that would read out the contents of most
of kernel memory by doing this (I think he had a loop that created a
pipe, did a 4k write from a bad address and then a 4k read, then close
the pipe).  IIRC it relied on the kernel usually using a different
page for each pipe.

The primary fix was of course to make the pipe code check the return
value from copy_from_user and return an EFAULT error.  But the
question was then, how many other places were there that did the same
thing?  So the zeroing of the destination was added as a backup to
make sure that we wouldn't leak the contents of kernel memory as a
result of not checking the result from copy_from_user.  It's a
belt-and-braces kind of thing really.

> > or if the size is 1, 2 or 4.
> 
> This one is OK - __get_user_asm() does the zeroing in the fixup code.

Ah, good.  Maybe ppc should do that too. :)

Paul.