Re: CVS commit: src/sys/kern

[Andrew Doran <ad%netbsd.org@localhost>]

On Mon, Apr 20, 2009 at 10:09:55PM +0000, Elad Efrat wrote:

> Module Name:  src
> Committed By: elad
> Date:         Mon Apr 20 22:09:55 UTC 2009
> 
> Modified Files:
>       src/sys/kern: kern_verifiedexec.c
> 
> Log Message:
> PR/41251: YAMAMOTO Takashi: veriexec locking seems broken
> 
> Part 1: Take the mountlist_lock before traversing the mount list.

Thanks for looking into this. However, there are some problems with your
change:

- mountlist_lock is a `leaf'. That is to say, it is never held for long and
  is never held for heavyweight operations. It also doesn't get intermixed
  with other locks.

- For similar reasons, I suspect your change may introduce a lock ordering
  between mountlist_lock and other locks, which is undesirable.

- The mountpoints you are looking at can be unmounted while you're examining
  them. The 'struct mount' won't disappear since you hold mountlist_lock,
  but see above.

The solution to this is vfs_busy+vfs_unbusy. Have a grep in kern/ for uses
of mountlist_lock. I think there are a few loops where all three are used
together. Basically, they give you this:

- vfs_busy() drops mountlist lock, but holds a reference to the mount for
  you, so it will not go away while you are inspecting it.

- vfs_unbusy() reacquires mountlist_lock, and lets you continue traversing
  the list from where you left off. It also drpos the reference.

- vfs_busy() locks the mount to prevent unmount().