On Aug 22 Chris Boot wrote:If firewire-sbp2 starts a login to a target that doesn't complete ORBsThe problem is not that sbp2_target_put()'s caller assumes that it is
in a timely manner (and has to retry the login), and the module is
removed before the operation times out, you end up with a null-pointer
dereference and a kernel panic.
This happens because the code in sbp2_remove() just does a
sbp2_target_put(), assuming it will be the last remaining reference. If
there are jobs in the workqueue, this is not the case, and the module is
successfully unloaded while references still exist.
putting the last reference. sbp2_target_put()'s very purpose is to clean
up when, and only when, the last reference is gone.
Signed-off-by: Chris Boot<bootc@xxxxxxxxx>list_for_each_entry() is sufficient here. You are not changing the list
---
drivers/firewire/sbp2.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c
index 41841a3..3867aaa 100644
--- a/drivers/firewire/sbp2.c
+++ b/drivers/firewire/sbp2.c
@@ -1198,6 +1198,11 @@ static int sbp2_remove(struct device *dev)
{
struct fw_unit *unit = fw_unit(dev);
struct sbp2_target *tgt = dev_get_drvdata(&unit->device);
+ struct sbp2_logical_unit *lu, *next;
+
+ list_for_each_entry_safe(lu, next,&tgt->lu_list, link) {
+ cancel_delayed_work_sync(&lu->work);
+ }
sbp2_target_put(tgt);
return 0;
while you iterate over it.
Would you please resend the patch with list_for_each_entry()?
More importantly, I think the whole sbp2_target instance reference
counting can be removed with his work canceling in place. But I have not
analyzed this fully yet, and I don't expect you to do this for me. Though
if you like to do so, that would of course be welcome.