Re: kern/48135: Bad locking for umount

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

The following reply was made to PR kern/48135; it has been noted by GNATS.

From: Michael van Elst <mlelstv%serpens.de@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: kern-bug-people%NetBSD.org@localhost, gnats-admin%NetBSD.org@localhost, 
netbsd-bugs%NetBSD.org@localhost
Subject: Re: kern/48135: Bad locking for umount
Date: Wed, 21 Aug 2013 23:15:02 +0200

 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."