Re: kern/48135: Bad locking for umount

[Michael van Elst <mlelstv%serpens.de@localhost>]

On Wed, Aug 21, 2013 at 10:35:00AM +0000, J. Hannken-Illjes wrote:
> 
>  This fix looks wrong.
>
>  If it is possible for the (last) reference to go away while vfs_busy() is
>  running it is also unsafe to increment the reference counter here as it
>  could be zero and be freed from vfs_destroy().

It is supposed to be a precondition to vfs_busy() that the reference count
is not zero, otherwise it couldn't use the mp pointer for anything.

There is a race condition where the data structure is freed by
vfs_destroy and then used. In the current code it is likely
to happen because vfs_busy will just take the mutex when dounmount
releases it and calls vfs_destroy.


>  Do you have stack traces from the threads running vfs_busy()
>  and vfs_destroy()?

Only from vfs_destroy() which is basically

sys_unmount()
dounmount()
vfs_destroy()
-> LOCKDEBUG cries because a mutex is destroyed while owned.

This probably happens because vfs_busy can take the mutex
between dounmount releasing it and vfs_destroy destroying it.
I'm not sure why VFS_UMOUNT can succeed (otherwise vfs_destroy
wouldn't happen) when another thread is calling vfs_busy.


Greetings,
-- 
                                Michael van Elst
Internet: mlelstv%serpens.de@localhost
                                "A potential Snark may lurk in every tree."