devfs panic w/INVARIANTS

[Andrew Gallatin]

Kostik Belousov wrote:
> On Thu, Feb 04, 2010 at 03:40:28PM -0500, Andrew Gallatin wrote:
>> I've got a commercial driver that uses device cloning.
>> At unload time, the driver calls clone_cleanup(). When I unload
>> the driver when the kernel is built with INVARIANTS, I'll see a
>> panic in devfs_populate_loop().  This happens in 6-stable,
>> as well as 8-stable.
>>
>> From what I can see the clone has been freed, but it
>> remains on the devfs cdevp_list.   Then the next time
>> devfs_populate_loop() is called, it trips over the bad
>> entry (cdp->cdp_dirents points to 0xdeadc0dedeadc0de)
>> See appended kgdb session.
>>
>> If I trace the code path, it looks like clone_cleanup()
>> calls destroy_devl().  And destroy_devl() will eventually
>> call devfs_free() if the si_refcnt is zero.  But I don't
>> see anything which will get the cdev removed from
>> the cdevp_list prior to it being freed.
>>
>> The only code I see which will get the cdev removed from
>> the cdevp_list() seems to be the "GC any lingering devices"
>> block in devfs_populate_loop
>>
>> What am I missing?
> 
> You did not mentioned it, but my guess is that you create clones from
> the dev_clone event handler. Please note that devfs_lookup() that fires

Yes, I do.

> dev_clone event, consumes a device reference. Thus clone handlers shall
> do dev_ref().
> 
> Due to races with cleanup, you should use MAKEDEV_REF flag for
> make_dev_credv(9) KPI instead of doing make_dev()/dev_ref() pair.

I need to support FreeBSD going all the way back to 6, so that's not an
option in some versions.

But, I'm talking about device removal time.  If I call clone_cleanup()
where the clones have dev->si_refcount==1, then I get the use-after-free
panic.  If I hack things to elevate the reference count (such that
dev->si_refcount==2 when clone_cleanup() is called), then I don't
get the panic.

Are you saying I should have been taking the extra reference
via my dev_clone eventhandler?   Won't having the extra reference
lead to a memory leak?   Or am I just mis-reading the code, and
this will lead to things being freed normally?

> That said, do you really need clones at all ?

I need to support FreeBSD back to 6.x, and I need to support the
linux-like model of opening the "same" /dev/node multiple times
and getting unique handles.  So I think I need clones.

Thanks for the help!
Drew