Re: [PATCH V5 3/4] 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 V5 3/4] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN
Date	2017-02-13 21:20 +0100
Message-ID	<tavgm-8uf-7@gated-at.bofh.it> (permalink)
References	(1 earlier) <tarFN-67l-49@gated-at.bofh.it> <tarPr-6aX-5@gated-at.bofh.it> <tarPs-6aX-39@gated-at.bofh.it> <tasiu-6AY-23@gated-at.bofh.it> <tasiu-6AY-29@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Mon, Feb 13, 2017 at 6:07 PM, David Laight <David.Laight@aculab.com> wrote:
> From: Arnd Bergmann
>> Sent: 13 February 2017 17:02
> ...
>> >> > +   ioctl_ptr = memdup_user(arg,  _IOC_SIZE(cmd));
>> >> > +   if (IS_ERR_OR_NULL(ioctl_ptr)) {
>> >> > +           ret = PTR_ERR(ioctl_ptr);
>> >> > +           goto out;
>> >> ...
>> >> > + out:
>> >> > +   kfree(ioctl_ptr);
>> >> > +   return ret;
> ...
>> >> That error path doesn't look quite right to me.
> ...
>> > good god, this is a mess this morning. Thanks for the catch, I'll review these
>> > more aggressively before sending out.
>>
>> memdup_user() never returns NULL, and generally speaking any use of
>> IS_ERR_OR_NULL() indicates that there is something wrong with the
>> interface, so aside from passing the right pointer (or NULL) into kfree()
>> I think using IS_ERR() is the correct solution.
>
> You missed the problem entirely.
> Code needs to be:
>         if (IS_ERR(ioctl_ptr))
>                 return PTR_ERR(ioctl_ptr);

Sorry if that wasn't clear, I expected the part about the kfree(errptr) to be
obvious but was trying to avoid having Scott go through another revision
for removing the IS_ERR_OR_NULL() after fixing the first problem.

    Arnd

Thread

SED Opal Fixups Scott Bauer <scott.bauer@intel.com> - 2017-02-13 17:30 +0100
  [PATCH V5 4/4] Maintainers: Modify SED list from nvme to block Scott Bauer <scott.bauer@intel.com> - 2017-02-13 17:30 +0100
    Re: [PATCH V5 4/4] Maintainers: Modify SED list from nvme to block Christoph Hellwig <hch@infradead.org> - 2017-02-14 09:20 +0100
  Re: [PATCH V5 1/4] block: sed-opal: change ioctl to take user  pointer instead of unsinged long Scott Bauer <scott.bauer@intel.com> - 2017-02-13 17:30 +0100
    Re: [PATCH V5 1/4] block: sed-opal: change ioctl to take user  pointer instead of unsinged long Christoph Hellwig <hch@infradead.org> - 2017-02-14 09:20 +0100
  [PATCH V5 1/4] block: sed-opal: change ioctl to take user pointer instead of unsinged long Scott Bauer <scott.bauer@intel.com> - 2017-02-13 17:30 +0100
  [PATCH V5 3/4] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN Scott Bauer <scott.bauer@intel.com> - 2017-02-13 17:30 +0100
    RE: [PATCH V5 3/4] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN David Laight <David.Laight@ACULAB.COM> - 2017-02-13 17:40 +0100
      Re: [PATCH V5 3/4] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN Scott Bauer <scott.bauer@intel.com> - 2017-02-13 17:40 +0100
        Re: [PATCH V5 3/4] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN Arnd Bergmann <arnd@arndb.de> - 2017-02-13 18:10 +0100
          RE: [PATCH V5 3/4] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN David Laight <David.Laight@ACULAB.COM> - 2017-02-13 18:10 +0100
            Re: [PATCH V5 3/4] Move stack parameters for sed_ioctl to prevent  oversized stack with CONFIG_KASAN Arnd Bergmann <arnd@arndb.de> - 2017-02-13 21:20 +0100
  [PATCH V5 2/4] uapi: sed-opal fix IOW for activate lsp to use correct struct Scott Bauer <scott.bauer@intel.com> - 2017-02-13 17:30 +0100
    Re: [PATCH V5 2/4] uapi: sed-opal fix IOW for activate lsp to use  correct struct Christoph Hellwig <hch@infradead.org> - 2017-02-14 09:20 +0100

csiph-web