Re: [PATCH 06/12] gadgetfs: use-after-free in ->aio_read()

[Michal Nazarewicz <mina86@mina86.com>]

On Sun, Mar 08 2015, Ming Lei <tom.leiming@gmail.com> wrote:
> On Tue, Feb 24, 2015 at 2:00 AM, Christoph Hellwig <hch@lst.de> wrote:
>> From: Al Viro <viro@zeniv.linux.org.uk>
>>
>> AIO_PREAD requests call ->aio_read() with iovec on caller's stack, so if
>> we are going to access it asynchronously, we'd better get ourselves
>> a copy - the one on kernel stack of aio_run_iocb() won't be there
>> anymore.  function/f_fs.c take care of doing that, legacy/inode.c
>> doesn't...
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

but at the same time:

>> @@ -649,7 +650,14 @@ fail:
>>         }
>>         iocb->private = priv;
>>         priv->iocb = iocb;
>> -       priv->iv = iv;
>> +       if (iv) {
>> +               priv->iv = kmemdup(iv, nr_segs * sizeof(struct iovec),
>> +                                  GFP_KERNEL);
>> +               if (!priv->iv) {
>> +                       kfree(priv);
>> +                       goto fail;
>> +               }
>> +       }
>
> It should be simpler and more efficient to allocate 'iv' piggyback
> inside 'priv'.

+1

	priv = kmalloc(sizeof *priv + (iv ? nr_segs * sizeof *iv : 0),
		       GFP_KERNEL);
	…
	priv->iv = iv ? (void*)(priv + 1) : NULL;

>>         priv->nr_segs = nr_segs;
>>         INIT_WORK(&priv->work, ep_user_copy_worker);
>>

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--