On 3/13/19 12:53 PM, James Smart wrote: > On 3/13/2019 10:55 AM, Christoph Hellwig wrote: >> From: James Smart <[email protected]> >> >> There are two changes: >> >> 1) The logic in the __nvmet_fc_free_assoc() routine is bad. It uses >> "safe" routines assuming pointers will come back valid. However, the >> intervening next structure being linked can be removed from the list and >> the resulting safe pointers are bad, resulting in NULL ptrs being hit. >> >> Correct by scheduling a work element to perform the association delete, >> which can be done while under lock. >> >> 2) Prior patch that added the work element scheduling left a possible >> reference on the object if the work element couldn't be scheduled. >> >> Correct by doing the put on a failing schedule_work() call. >> >> Signed-off-by: Nigel Kirkland <[email protected]> >> Signed-off-by: James Smart <[email protected]> >> Reviewed-by: Ewan D. Milne <[email protected]> >> Signed-off-by: Christoph Hellwig <[email protected]> >> --- >> drivers/nvme/target/fc.c | 9 ++++----- >> 1 file changed, 4 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c >> index 1e9654f04c60..8d34aa573d5b 100644 >> --- a/drivers/nvme/target/fc.c >> +++ b/drivers/nvme/target/fc.c >> @@ -1143,10 +1143,8 @@ __nvmet_fc_free_assocs(struct nvmet_fc_tgtport >> *tgtport) >> &tgtport->assoc_list, a_list) { >> if (!nvmet_fc_tgt_a_get(assoc)) >> continue; >> - spin_unlock_irqrestore(&tgtport->lock, flags); >> - nvmet_fc_delete_target_assoc(assoc); >> - nvmet_fc_tgt_a_put(assoc); >> - spin_lock_irqsave(&tgtport->lock, flags); >> + if (!schedule_work(&assoc->del_work)) >> + nvmet_fc_tgt_a_put(assoc); >> } >> spin_unlock_irqrestore(&tgtport->lock, flags); >> } >> @@ -1185,7 +1183,8 @@ nvmet_fc_delete_ctrl(struct nvmet_ctrl *ctrl) >> nvmet_fc_tgtport_put(tgtport); >> >> if (found_ctrl) { >> - schedule_work(&assoc->del_work); >> + if (schedule_work(&assoc->del_work)) >> + nvmet_fc_tgt_a_put(assoc); >> return; >> } >> > V1 was checked in > (http://lists.infradead.org/pipermail/linux-nvme/2019-February/022193.html > V2 was skipped > (http://lists.infradead.org/pipermail/linux-nvme/2019-February/022540.html) > > V2 changes the last > + if (schedule_work(&assoc->del_work)) > to > + if (!schedule_work(&assoc->del_work)) > > > Jens - can you do this manual fixup ?
Fixed up. -- Jens Axboe
