On 2/4/20 12:51 AM, Christoph Hellwig wrote:

On Mon, Feb 03, 2020 at 01:07:48PM -0800, Bijan Mottahedeh wrote:

quoted

My concern is with the code below for the single bio async case:

                           qc = submit_bio(bio);

                           if (polled)
                                   WRITE_ONCE(iocb->ki_cookie, qc);

The bio/dio can be freed before the the cookie is written which is what I'm
seeing, and I thought this may lead to a scenario where that iocb request
could be completed, freed, reallocated, and resubmitted in io_uring layer;
i.e., I thought the cookie could be written into the wrong iocb.

I think we do have a potential use after free of the iocb here.
But taking a bio reference isn't going to help with that, as the iocb
and bio/dio life times are unrelated.

I vaguely remember having that discussion with Jens a while ago, and
tried to pass a pointer to the qc to submit_bio so that we can set
it at submission time, but he came up with a reason why that might not
be required.  I'd have to dig out all notes unless Jens remembers
better.

Don't remember that either, so I'd have to dig out emails! But looking
at it now, for the async case with io_uring, the iocb is embedded in the
io_kiocb from io_uring. We hold two references to the io_kiocb, one for
submit and one for completion. Hence even if the bio completes
immediately and someone else finds the completion before the application
doing this submit, we still hold the submission reference to the
io_kiocb. Hence I don't really see how we can end up with a
use-after-free situation here.

IIRC, Bijan had traces showing this can happen, KASAN complaining about
it. Which makes me think that I'm missing a case here, though I don't
immediately see what it is.

Bijan, could post your trace again, I can't seem to find it?

-- 
Jens Axboe