Re: [PATCH] infiniband: fix a subtle race condition

Jason Gunthorpe
2018-06-15T19:08:55Z

On Fri, Jun 15, 2018 at 10:57:49AM -0700, Cong Wang wrote:

> > No, it *is* the point - you've proposed a solution, one of many, and
> > we need to see an actual sensible design for how the locking around
> > ctx->file should work correctly.
>
> I proposed to a solution for imbalance unlock, you ask a design
> for use-after-free, which is *irrelevant*. So why it is the point?

The point is, I don't care about the imbalance report.

I care about the actual bug, which you have identified as
ucma_migrate_id() running concurrently with ucma_event_handler(). That
seems like a great analysis, BTW.

Stop that from happening and the lock imbalance warning will naturally go
away. So will the use after free.

I gave you some general ideas on how to do that, obviously they are
not easy to do eg somehow solving the dealock with mut would be
tricky. But maybe there is still some kind of simple solution..

Another option might be to just fail ucma_migrate_id() when
ucma_event_handler() is outstanding.. That *might* be OK.. We've
talked about doing things like this for other ucma syzkaller
bugs. Also a bit complicated.

Anyhow I'm NAK'ing this patch, since it just doesn't move things
forward, and removes a warning that is pointing at a bunch of
different bugs.

Jason