linux-kernel - Re: [PATCH V5 3/4] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a1gqtEB76nZTNKpBTko-j7aPx-XHwsZFKzW7EzCbBcX6A@mail.gmail.com>
Date:   Mon, 13 Feb 2017 21:16:18 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     David Laight <David.Laight@...lab.com>
Cc:     Scott Bauer <scott.bauer@...el.com>,
        "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
        "axboe@...com" <axboe@...com>,
        "keith.busch@...el.com" <keith.busch@...el.com>,
        "jonathan.derrick@...el.com" <jonathan.derrick@...el.com>,
        "hch@...radead.org" <hch@...radead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>
Subject: Re: [PATCH V5 3/4] Move stack parameters for sed_ioctl to prevent
 oversized stack with CONFIG_KASAN

On Mon, Feb 13, 2017 at 6:07 PM, David Laight <David.Laight@...lab.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