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  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

Powered by blists - more mailing lists