Re: AUDIT: copy_from_user is a deathtrap.

Denis Vlasenko
2002-05-17T13:43:15Z

On 17 May 2002 10:26, Rusty Russell wrote:
> >
> > Sorry I wasn't clear: I'm saying *replace*, not add,
> >
> > I don't understand what you are proposing then. There are some
> > instances that do want to know how many bytes did make it before
> > the -EFAULT event.
>
> Yes. There are 52 places which care. Most of these are unneccessary
> attempts to return eg. number of bytes written in read call before we
> hit the fault, instead of -EFAULT.
>
> The one case I found which obviously needed it was the mount options
> code, and I proposed a simple (slow) gradually_copy_from_user for this
> case:
>
> static inline unsigned long
> gradual_copy_from_user(void *to, const void *from, unsigned long n)
> {
> unsigned long i;
>
> for (i = 0; i < n; i++, to++, from++) {
> if (copy_from_user(from, to, 1) != 0)
> break;
> }
> return n - i;
> }

It looks simple to my uncomplicated mind to make
copy_{to,from}_user() return number of bytes copied.
If they != bytes requested, there was -EFAULT.

This can be easily wrapped by inline which returns only
0 on success or -EFAULT on failure.

Then use appropriate version for each case.
--
vda