Subject: Re: RFC small patch for vfs_subr.c:vfs_mountedon()
To: None <tech-kern@netbsd.org>
From: Chuck Silvers <chuq@chuq.com>
List: tech-kern
Date: 01/18/2006 06:46:33
does vp->v_type being anything other than VBLK mean that there's bug somewhere,
or can it happen if the user just types the wrong thing?
if the former, then it'd be better to have a KASSERT() than the check
you have below.

-Chuck


On Wed, Jan 18, 2006 at 03:36:03AM +0100, Reinoud Zandijk wrote:
> Dear folks,
> 
> after bughunting for a while, i stumbled on a for me unknown feature that 
> vfs_mountedon() can only and only handle special device nodes and will 
> otherwise generate an UVM trap or will access random memory. This is due to 
> the dereference of vp->v_specmountpoint that is really 
> vp->v_specinfo->si_mountpoint. The field v_specinfo is multiplexed with 
> other fields in struct vnode like struct socket.
> 
> Before i looked at the implementation of vfs_mountedon() i thought it was 
> using the circleq of mountpoints to check... but no, it uses a specfs 
> specific structure.
> 
> Just as a sanity check for accessing the specinfo i'd like to add the 
> simple patch:
> 
> Index: vfs_subr.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/vfs_subr.c,v
> retrieving revision 1.259
> diff -u -p -r1.259 vfs_subr.c
> --- vfs_subr.c  31 Dec 2005 14:05:01 -0000      1.259
> +++ vfs_subr.c  18 Jan 2006 02:18:25 -0000
> @@ -2065,6 +2065,8 @@ vfs_mountedon(struct vnode *vp)
>         struct vnode *vq;
>         int error = 0;
>  
> +       if (vp->v_type != VBLK && vp->v_type != VCHR)
> +               return (ENXIO);
>         if (vp->v_specmountpoint != NULL)
>                 return (EBUSY);
>         if (vp->v_flag & VALIASED) {
> ---------------
> 
> All miscfs's don't use the function other than specfs itself to check for 
> double mounts and tmpfs/nfs and friends don't have a device to mount so 
> they don't call it.
> 
> Thoughts?
> 
> Reinoud
> p.s. patch not tested yet