Re: AUDIT: copy_from_user is a deathtrap.

Arnaldo Carvalho de Melo
2002-05-21T06:22:29Z

Em Tue, May 21, 2002 at 08:57:28AM -0200, Denis Vlasenko escreveu:
> On 20 May 2002 13:22, you wrote:
> > > Can you tell me what's wrong with copy_from_user? How did you used it
> > > wrongly?
> >
> > Denis, I agree with the essense of Rusty's argument, which is that
> > copy_to_user is easy to misuse in the following way:
> >
> > xxx_ioctl (..., cmd, arg) {
> > return copy_to_user(....);
> > }
> >
> > Since copy_to_user returns a number of residue bytes instead of
> > -EINVAL, such statement confuses the caller.
> > Rusty found something about 54 instances of this.
>
> Oh. Do you think a pair of
>
> copy_to_user_or_EINVAL(...)
> copy_to_user_return_residue(...)
>
> will help avoid such bugs?
> It is possible to audit kernel once, move it to new functions
> and deprecate/delete old one.

As Linus and others pointed out, copy_{to_from}_user has its uses and will
stay, but something like:

#define copyin(...) (copy_from_user(...) ? -EFAULT : 0)
#define copyout(...) (copy_to_user(...) ? -EFAULT : 0)

Like several drivers already have (with these names or with other names)
would be something interesting, that way we could clean up the ones that
use this construct and all the others that use the longer
'copy_{to,from}_user(...) ? -EFAULT : 0' construct. If the powers that be
accept this, I'd do the work 8)

Is it *BSD that have copyin/copyout with this semantic? If so it'd even
have an extra bonus to make porting a little bit faster... 8)

- Arnaldo