[Bug 212337] scsi_debug: race at module load and module unload

[bugzilla-daemon@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=212337

--- Comment #14 from Luis Chamberlain (mcgrof@kernel.org) ---
(In reply to d gilbert from comment #8)
> On 2021-03-18 3:14 p.m., bugzilla-daemon@bugzilla.kernel.org wrote:
> > https://bugzilla.kernel.org/show_bug.cgi?id=212337
> > 
> > --- Comment #4 from Luis Chamberlain (mcgrof@kernel.org) ---
> > I'm afraid scsi_debug is filled with bug bombs bound to happen, because it
> > was
> > written without certain consideration of races now coming up as tangible
> with
> > syfs / driver load. Namely, if you hold a lock at init and also use it on
> > sysfs
> > attributes you can easily deadlock. I discovered this issue first with the
> > zram
> > driver, and fixed the issue with try_module_get()'s on each driver sysfs
> > attribute, I posted patches for that, for discussion on that see the post
> [0]
> > [1], although discussion is mostly on the first patch, the patch you want
> to
> > look at is the second one [1].
> > 
> > [0] https://lkml.kernel.org/r/20210306022035.11266-2-mcgrof@kernel.org
> > [1] https://lkml.kernel.org/r/20210306022035.11266-3-mcgrof@kernel.org
> > 
> > I considered fixing scsi_debug in light of this, but given that module
> > initialization is *also* calling helpers used by syfs attributes, *and*
> this
> > is
> > also true at module removal, I'm afraid much more care is needed here. In
> my
> > patch to zram for the sysfs issue I mention ways to trigger the deadlock,
> if
> > you're up for the task to fix that, it would be wonderful. But hey, these
> are
> > separate issues. just figured you should be aware.
> 
> In the hack proposed above, not much would appear in sysfs until that thread
> finished adding at least adding its first host by which time
> scsi_debug_init()
> should have long since finished.

The sysfs races are a bit more complex than what I think is being conveyed. I
will Cc you on some fixes for zram which demo what the issues are, which are
generic, which I also think we need to fix in this driver. There would be two
issues:

 * the syfs deadlock
 * races against the private data which is type specific

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.