linux-kernel - Re: [PATCH] aio: fix a user triggered use after free (and fix freeze protection of aio writes)

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:   Sat, 29 Oct 2016 10:47:58 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Christoph Hellwig <hch@....de>
Cc:     Al Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>,
        Dmitry Monakhov <dmonakhov@...nvz.org>,
        Jeff Moyer <jmoyer@...hat.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-aio@...ck.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>
Subject: Re: [PATCH] aio: fix a user triggered use after free (and fix freeze
 protection of aio writes)

On Sat, Oct 29, 2016 at 8:20 AM, Christoph Hellwig <hch@....de> wrote:
>
> We can't as that would not fix the use after free (at least for the lockdep
> case - otherwise the call is a no-op).  Once iter_op returns aio_complete
> might have dropped our reference to the file, and another thread might
> have closed the fd so that the fput from aio_complete was the last one.

I don't concpetually mind the patch per se, but the repeated

             if (rw == WRITE) {
                   ..
             }

             if (rw == WRITE) {
                   ..
             }

is just insane and makes the code less legible than it should be.

Also, honestly, make it use a helper: "aio_file_start_write()" and
"aio_file_end_write()" that has the comments and the lockdep games.

Because that patch is just too effing ugly.

Does something like the attached work for you guys?

                Linus

View attachment "patch.diff" of type "text/plain" (2113 bytes)