linux-kernel - Re: [PATCH] aio: prevent the final fput() in the middle of vfs_poll() (Re: KASAN: use-after-free Read in unix_dgram

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Sun, 3 Mar 2019 12:13:33 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Al Viro <viro@...iv.linux.org.uk>,
        Eric Dumazet <eric.dumazet@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        Jason Baron <jbaron@...mai.com>, kgraul@...ux.ibm.com,
        ktkhai@...tuozzo.com, kyeongdon.kim@....com,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        Netdev <netdev@...r.kernel.org>, pabeni@...hat.com,
        syzkaller-bugs@...glegroups.com, xiyou.wangcong@...il.com,
        Christoph Hellwig <hch@....de>
Subject: Re: [PATCH] aio: prevent the final fput() in the middle of vfs_poll()
 (Re: KASAN: use-after-free Read in unix_dgram_poll)

On Sun, Mar 3, 2019 at 11:44 AM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> But doesn't it look nice to see
>
>  2 files changed, 41 insertions(+), 50 deletions(-)
>
> with actual code reduction, and a fundamental simplification in
> handling of the file pointer?

A coupl,e of the changes are "useless", and do the same thing as not
having them at all:

-               struct inode *inode = file_inode(kiocb->ki_filp);
+               struct inode *inode = file_inode(iocb->ki_filp);

-               file_end_write(kiocb->ki_filp);
+               file_end_write(iocb->ki_filp);

because the "ki_filp" ends up existing in both kiocb and iocb. At one
point of editing that file I decided to try to just remove it from the
sub-structs entirely and only keep it in the top-level structure, but
it needs to be inside the 'struct kiocb' anyway for all the other
users outside of fs/aio.c.

Anyway, I don't think the patch is wrong (although I haven't actually
_tested_ it) but I wanted to point out that those two one-liner
changes are just "noise" that doesn't matter for the working of the
patch.

In the above, we have 'kiocb' being the embedded 'struct kiocb', and
'iocb' is the 'struct aio_kiocb' that contains it. 'ki_filp' is the
exact same field in both cases.

                 Linus

               Linus