> -----Original Message----- > From: Jason Gunthorpe > Sent: Thursday, March 22, 2018 4:22 PM > To: Parav Pandit <parav@xxxxxxxxxxxx> > Cc: linux-rdma@xxxxxxxxxxxxxxx; Leon Romanovsky <leonro@xxxxxxxxxxxx>; > Mark Bloch <markb@xxxxxxxxxxxx>; Dmitry Vyukov <dvyukov@xxxxxxxxxx>; > syzbot <syzbot+3b4acab09b6463472d0a@xxxxxxxxxxxxxxxxxxxxxxxxx>; Daniel > Jurgens <danielj@xxxxxxxxxxxx>; dledford@xxxxxxxxxx; Johannes Berg > <johannes.berg@xxxxxxxxx>; syzkaller-bugs@xxxxxxxxxxxxxxxx > Subject: Re: [PATCH rdma-rc] RDMA/rdma_cm: Fix use after free race with > process_one_req > > On Thu, Mar 22, 2018 at 02:59:24PM -0600, Parav Pandit wrote: > > > Thanks for looking into it. > > Though I didn't ack on the ML, I had debugged it similar report few > > days back, fixed and verified it differently by rdma_addr_cancel() > > doing cancel_delayed_work_sync() and completing the request after > > that. > > > And if already cancelled than process_one_req to skip processing it. > > I wasn't sure if work item itself can cancel itself reliably while it > > is running. Do you know? > > Yes, the work pointer can be free'd within the worker, the workqueue code does > not touch the pointer after the function runs to allow for this. > > > cancel_delayed_work_sync() appears more robust to me following similar > other users. > > This also avoid canceling it on every execution in process_one_req(). > > I like yours better because it ensures the callback will not be called after > rdma_addr_cancel() which is overall much saner behavior, and I think possibly > another bug? > > But let just do that with flush_workqueue() ? > flush_workqueue() will force to execute all the work items for all pending entries, all must have to completed. Those pending delayed entries are unrelated to this work item/request in progress, and if they are large number of entries having 1 sec timeout, flush_workqueue() might take long. So one rdma_destroy_id will wait for other requests to be completed, which I think we should avoid. > > Assuming cancel_delayed_work can cancel itself based on kdoc comment > > that it can be called from any context, patch you posted should work > > too. Minor modified hunk to reuse code in process_one_req() and > > process_req() on top of your patch below. > > Yes. > > 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
