On Thu, Jun 14, 2018 at 10:01:08AM +0300, Leon Romanovsky wrote: > On Wed, Jun 13, 2018 at 11:21:54PM -0700, Cong Wang wrote: > > On Wed, Jun 13, 2018 at 10:34 PM, Leon Romanovsky <leon@xxxxxxxxxx> wrote: > > > > > > Hi Cong, > > > > > > If the compiler optimizes the first line (mutex_lock) as you wrote, > > > it will reuse "f" for the second line (mutex_unlock) too. > > > > Nope, check the assembly if you don't trust me, at least > > my compiler always fetches ctx->file without this patch. > > > > I can show you the assembly code tomorrow (too late to > > access my dev machine now). > > I trust you, so don't need to check it however wanted to emphasize > that your solution is compiler specific and not universally true. > > > > > > > > > > > You need to ensure that ucma_modify_id() doesn't run in parallel to > > > anything that uses "ctx->file" directly and indirectly. > > > > > > > Talk is easy, show me the code. :) I knew there is probably > > some other race with this code even after my patch, possibly with > > ->close() for example, but for this specific unlock warning, this patch > > is sufficient. I can't solve all the races in one patch. > > We do prefer complete solution once the problem is fully understood. > > It looks like you are one step away from final patch. It will be conversion > of mutex to be rwlock and separating between read (almost in all places) > and write (in ucma_migrate_id) paths. This was my brief reaction too, this code path almost certainly has a use-after-free, and we should fix the concurrency between the two places in some correct way.. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
