Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN

From	Arnd Bergmann <arnd@arndb.de>
Newsgroups	linux.kernel
Subject	Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN
Date	2017-02-10 12:10 +0100
Message-ID	<t9hfr-2El-9@gated-at.bofh.it> (permalink)
References	<t90Rj-BX-7@gated-at.bofh.it> <t90Rj-BX-17@gated-at.bofh.it> <t9e7T-tH-3@gated-at.bofh.it> <t9gW6-2gP-5@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Fri, Feb 10, 2017 at 11:28 AM, David Laight <David.Laight@aculab.com> wrote:

>>
>> > -           if (copy_from_user(&session, arg, sizeof(session)))
>> > -                   return -EFAULT;
>> > -           return opal_erase_locking_range(dev, &session);
>> > +   ioctl_ptr = kzalloc(cmd_size, GFP_KERNEL);
>> > +   if (!ioctl_ptr)
>> > +           return -ENOMEM;
>> > +   if (copy_from_user(ioctl_ptr, arg, cmd_size)) {
>> > +           ret = -EFAULT;
>> > +           goto out;
>> >     }
>>
>> Can't we use memdup_user() instead of kzalloc() + copy_from_user()?
>
> You either want the copy_from_user() or the memzero() not both.
>
> ISTM there could be two 'library' functions, maybe:
> void *get_ioctl_buf(unsigned int cmd, long arg)
> to malloc the buffer, memzero/copy_from_user, returns -EFAULT if copy fails.
> int put_ioctl_buf(int rval, unsigned int cmd, const void *buf)
> does copy_to_user() if rval >= 0 and IOR_READ, then frees buf.
> return value is rval unless the copyout fails.

All the ioctls commands in this driver are IOW, and no data is passed back
to user space, so there is no need for the memzero(): we can either copy
the data from user space or we fail.

    Arnd

Thread

[PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN Scott Bauer <scott.bauer@intel.com> - 2017-02-09 18:40 +0100
  Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN Rafael Antognolli <rafael.antognolli@intel.com> - 2017-02-09 21:00 +0100
  Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN Johannes Thumshirn <jthumshirn@suse.de> - 2017-02-10 08:50 +0100
    RE: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN David Laight <David.Laight@ACULAB.COM> - 2017-02-10 11:50 +0100
      Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN Arnd Bergmann <arnd@arndb.de> - 2017-02-10 12:10 +0100
  Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN Arnd Bergmann <arnd@arndb.de> - 2017-02-10 09:10 +0100
    Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN Scott Bauer <scott.bauer@intel.com> - 2017-02-10 17:10 +0100

csiph-web