Page MenuHomeFreeBSD

Prevent detaching driver if the attach is not finished

Authored by on Feb 27 2019, 7:55 AM.



When device is in attaching state, detach should return EBUSY instead of success. In other case, there could be race between attach and detach during driver unloading. If driver goes sleep and releases GIANT lock during attaching, unloading module could start. In such case when attach continues after module unload, page fault "supervisor read instruction, page not present" occurred.

Test Plan

Running multiple (~100) kldload; kldunload on driver that sleeps in attach (for example ENA on AWS instance).

Diff Detail

rS FreeBSD src repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

imp accepted this revision.Feb 27 2019, 2:29 PM

It's an OK workaround, but the real problem is that we're not properly interlocked elsewhere.

3005 ↗(On Diff #54453)

Please add a comment that this is a horrific hack designed to cope with busses that aren't properly locked that allow entry into both attach and detach.

Sadly, most busses are like this in the tree, and it's one of the big issues with my work trying to properly lock newbus.

This revision is now accepted and ready to land.Feb 27 2019, 2:29 PM
emaste added inline comments.Feb 27 2019, 3:15 PM
3005 ↗(On Diff #54453)

Warner is it worth going a little further than a comment, and emitting a warning upon DS_ATTACHING?

mw added inline comments.Feb 28 2019, 2:35 PM
3005 ↗(On Diff #54453)

How about following then:

if (dev->state == DS_BUSY)
    return (EBUSY);
if (dev->state == DS_ATTACHING) {
    device_printf(dev, "device in attaching state! Deferring detach.\n");
    return (EBUSY);
emaste added inline comments.Feb 28 2019, 2:50 PM
3005 ↗(On Diff #54453)

That LGTM but I'll defer to @imp

imp added inline comments.Feb 28 2019, 10:29 PM
3005 ↗(On Diff #54453)

This makes me happier since we warn about the problem, even if the root cause is well beyond the scope of this review to fix.

This revision was automatically updated to reflect the committed changes.
mw added inline comments.Mar 1 2019, 1:19 AM
3005 ↗(On Diff #54453)

I also added mentioning the work-around character of the patch in the commit log (r344676.)