svn commit: r184641 - in stable/7/sys: . kern

[Kostik Belousov]

On Tue, Nov 04, 2008 at 09:03:32AM -0700, Scott Long wrote:
> In stable branches, and especially during release cycles, would it be 
> possible to annotate whether changes like this fix known panics or 
> user-visible bugs?
I thought that description of the change made it obvious.
Access to the partially initialized structure is sure reason for a bad
behaviour, panic in this particular case.

It is slightly more involved in this case, because other thread was
able to overwrite pointer to fully initialized structure put by current
thread. This is what prevented by vnode interlock region.

> 
> Scott
> 
> 
> Konstantin Belousov wrote:
> >Author: kib
> >Date: Tue Nov  4 15:56:44 2008
> >New Revision: 184641
> >URL: http://svn.freebsd.org/changeset/base/184641
> >
> >Log:
> >  MFC r184409:
> >  Protect check for v_pollinfo == NULL and assignment of the newly 
> >  allocated
> >  vpollinfo with vnode interlock. Fully initialize vpollinfo before putting
> >  pointer to it into vp->v_pollinfo.
> >  
> >  Approved by:	re (kensmith)
> >
> >Modified:
> >  stable/7/sys/   (props changed)
> >  stable/7/sys/kern/vfs_subr.c
> >
> >Modified: stable/7/sys/kern/vfs_subr.c
> >==============================================================================
> >--- stable/7/sys/kern/vfs_subr.c	Tue Nov  4 15:47:06 2008 (r184640)
> >+++ stable/7/sys/kern/vfs_subr.c	Tue Nov  4 15:56:44 2008 (r184641)
> >@@ -109,7 +109,7 @@ static void	vgonel(struct vnode *);
> > static void	vfs_knllock(void *arg);
> > static void	vfs_knlunlock(void *arg);
> > static int	vfs_knllocked(void *arg);
> >-
> >+static void	destroy_vpollinfo(struct vpollinfo *vi);
> > 
> > /*
> >  * Enable Giant pushdown based on whether or not the vm is mpsafe in this
> >@@ -815,11 +815,8 @@ vdestroy(struct vnode *vp)
> > #ifdef MAC
> > 	mac_destroy_vnode(vp);
> > #endif
> >-	if (vp->v_pollinfo != NULL) {
> >-		knlist_destroy(&vp->v_pollinfo->vpi_selinfo.si_note);
> >-		mtx_destroy(&vp->v_pollinfo->vpi_lock);
> >-		uma_zfree(vnodepoll_zone, vp->v_pollinfo);
> >-	}
> >+	if (vp->v_pollinfo != NULL)
> >+		destroy_vpollinfo(vp->v_pollinfo);
> > #ifdef INVARIANTS
> > 	/* XXX Elsewhere we can detect an already freed vnode via NULL v_op. 
> > 	*/
> > 	vp->v_op = NULL;
> >@@ -3050,6 +3047,14 @@ vbusy(struct vnode *vp)
> > 	mtx_unlock(&vnode_free_list_mtx);
> > }
> > 
> >+static void
> >+destroy_vpollinfo(struct vpollinfo *vi)
> >+{
> >+	knlist_destroy(&vi->vpi_selinfo.si_note);
> >+	mtx_destroy(&vi->vpi_lock);
> >+	uma_zfree(vnodepoll_zone, vi);
> >+}
> >+
> > /*
> >  * Initalize per-vnode helper structure to hold poll-related state.
> >  */
> >@@ -3058,15 +3063,20 @@ v_addpollinfo(struct vnode *vp)
> > {
> > 	struct vpollinfo *vi;
> > 
> >+	if (vp->v_pollinfo != NULL)
> >+		return;
> > 	vi = uma_zalloc(vnodepoll_zone, M_WAITOK);
> >+	mtx_init(&vi->vpi_lock, "vnode pollinfo", NULL, MTX_DEF);
> >+	knlist_init(&vi->vpi_selinfo.si_note, vp, vfs_knllock,
> >+	    vfs_knlunlock, vfs_knllocked);
> >+	VI_LOCK(vp);
> > 	if (vp->v_pollinfo != NULL) {
> >-		uma_zfree(vnodepoll_zone, vi);
> >+		VI_UNLOCK(vp);
> >+		destroy_vpollinfo(vi);
> > 		return;
> > 	}
> > 	vp->v_pollinfo = vi;
> >-	mtx_init(&vp->v_pollinfo->vpi_lock, "vnode pollinfo", NULL, MTX_DEF);
> >-	knlist_init(&vp->v_pollinfo->vpi_selinfo.si_note, vp, vfs_knllock,
> >-	    vfs_knlunlock, vfs_knllocked);
> >+	VI_UNLOCK(vp);
> > }
> > 
> > /*
> >@@ -3081,8 +3091,7 @@ int
> > vn_pollrecord(struct vnode *vp, struct thread *td, int events)
> > {
> > 
> >-	if (vp->v_pollinfo == NULL)
> >-		v_addpollinfo(vp);
> >+	v_addpollinfo(vp);
> > 	mtx_lock(&vp->v_pollinfo->vpi_lock);
> > 	if (vp->v_pollinfo->vpi_revents & events) {
> > 		/*
> >@@ -3917,8 +3926,7 @@ vfs_kqfilter(struct vop_kqfilter_args *a
> > 
> > 	kn->kn_hook = (caddr_t)vp;
> > 
> >-	if (vp->v_pollinfo == NULL)
> >-		v_addpollinfo(vp);
> >+	v_addpollinfo(vp);
> > 	if (vp->v_pollinfo == NULL)
> > 		return (ENOMEM);
> > 	knl = &vp->v_pollinfo->vpi_selinfo.si_note;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-stable/attachments/20081104/df2c71ab/attachment.pgp