svn commit: r239303 - in head: share/man/man9 sys/cddl/contrib/opensolaris/uts/common/dtrace sys/dev/drm sys/dev/drm2 sys/dev/ksyms sys/fs/devfs sys/ofed/include/linux

Hans Petter Selasky hselasky at FreeBSD.org
Wed Aug 15 16:19:40 UTC 2012


Author: hselasky
Date: Wed Aug 15 16:19:39 2012
New Revision: 239303
URL: http://svn.freebsd.org/changeset/base/239303

Log:
  Streamline use of cdevpriv and correct some corner cases.
  
  1) It is not useful to call "devfs_clear_cdevpriv()" from
  "d_close" callbacks, hence for example read, write, ioctl and
  so on might be sleeping at the time of "d_close" being called
  and then then freed private data can still be accessed.
  Examples: dtrace, linux_compat, ksyms (all fixed by this patch)
  
  2) In sys/dev/drm* there are some cases in which memory will
  be freed twice, if open fails, first by code in the open
  routine, secondly by the cdevpriv destructor. Move registration
  of the cdevpriv to the end of the drm open routines.
  
  3) devfs_clear_cdevpriv() is not called if the "d_open" callback
  registered cdevpriv data and the "d_open" callback function
  returned an error. Fix this.
  
  Discussed with:	phk
  MFC after:	2 weeks

Modified:
  head/share/man/man9/devfs_set_cdevpriv.9
  head/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
  head/sys/dev/drm/drm_fops.c
  head/sys/dev/drm2/drm_fops.c
  head/sys/dev/ksyms/ksyms.c
  head/sys/fs/devfs/devfs_vnops.c
  head/sys/ofed/include/linux/linux_compat.c

Modified: head/share/man/man9/devfs_set_cdevpriv.9
==============================================================================
--- head/share/man/man9/devfs_set_cdevpriv.9	Wed Aug 15 16:01:45 2012	(r239302)
+++ head/share/man/man9/devfs_set_cdevpriv.9	Wed Aug 15 16:19:39 2012	(r239303)
@@ -24,7 +24,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd September 8, 2008
+.Dd August 15, 2012
 .Dt DEVFS_CDEVPRIV 9
 .Os
 .Sh NAME
@@ -79,6 +79,10 @@ finished operating, the
 callback is called, with private data supplied
 .Va data
 argument.
+The
+.Fn devfs_clear_cdevpriv
+function will be also be called if the open callback
+function returns an error code.
 .Pp
 On the last filedescriptor close, system automatically arranges
 .Fn devfs_clear_cdevpriv

Modified: head/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c	Wed Aug 15 16:01:45 2012	(r239302)
+++ head/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c	Wed Aug 15 16:19:39 2012	(r239303)
@@ -15517,8 +15517,6 @@ dtrace_close(struct cdev *dev, int flags
 		kmem_free(state, 0);
 #if __FreeBSD_version < 800039
 		dev->si_drv1 = NULL;
-#else
-		devfs_clear_cdevpriv();
 #endif
 #endif
 	}

Modified: head/sys/dev/drm/drm_fops.c
==============================================================================
--- head/sys/dev/drm/drm_fops.c	Wed Aug 15 16:01:45 2012	(r239302)
+++ head/sys/dev/drm/drm_fops.c	Wed Aug 15 16:19:39 2012	(r239303)
@@ -57,12 +57,6 @@ int drm_open_helper(struct cdev *kdev, i
 		return ENOMEM;
 	}
 
-	retcode = devfs_set_cdevpriv(priv, drm_close);
-	if (retcode != 0) {
-		free(priv, DRM_MEM_FILES);
-		return retcode;
-	}
-
 	DRM_LOCK();
 	priv->dev		= dev;
 	priv->uid		= p->td_ucred->cr_svuid;
@@ -76,7 +70,6 @@ int drm_open_helper(struct cdev *kdev, i
 		/* shared code returns -errno */
 		retcode = -dev->driver->open(dev, priv);
 		if (retcode != 0) {
-			devfs_clear_cdevpriv();
 			free(priv, DRM_MEM_FILES);
 			DRM_UNLOCK();
 			return retcode;
@@ -89,7 +82,12 @@ int drm_open_helper(struct cdev *kdev, i
 	TAILQ_INSERT_TAIL(&dev->files, priv, link);
 	DRM_UNLOCK();
 	kdev->si_drv1 = dev;
-	return 0;
+
+	retcode = devfs_set_cdevpriv(priv, drm_close);
+	if (retcode != 0)
+		drm_close(priv);
+
+	return (retcode);
 }
 
 

Modified: head/sys/dev/drm2/drm_fops.c
==============================================================================
--- head/sys/dev/drm2/drm_fops.c	Wed Aug 15 16:01:45 2012	(r239302)
+++ head/sys/dev/drm2/drm_fops.c	Wed Aug 15 16:19:39 2012	(r239303)
@@ -57,12 +57,6 @@ int drm_open_helper(struct cdev *kdev, i
 		return ENOMEM;
 	}
 
-	retcode = devfs_set_cdevpriv(priv, drm_close);
-	if (retcode != 0) {
-		free(priv, DRM_MEM_FILES);
-		return retcode;
-	}
-
 	DRM_LOCK(dev);
 	priv->dev		= dev;
 	priv->uid		= p->td_ucred->cr_svuid;
@@ -83,7 +77,6 @@ int drm_open_helper(struct cdev *kdev, i
 		/* shared code returns -errno */
 		retcode = -dev->driver->open(dev, priv);
 		if (retcode != 0) {
-			devfs_clear_cdevpriv();
 			free(priv, DRM_MEM_FILES);
 			DRM_UNLOCK(dev);
 			return retcode;
@@ -96,7 +89,12 @@ int drm_open_helper(struct cdev *kdev, i
 	TAILQ_INSERT_TAIL(&dev->files, priv, link);
 	DRM_UNLOCK(dev);
 	kdev->si_drv1 = dev;
-	return 0;
+
+	retcode = devfs_set_cdevpriv(priv, drm_close);
+	if (retcode != 0)
+		drm_close(priv);
+
+	return (retcode);
 }
 
 static bool

Modified: head/sys/dev/ksyms/ksyms.c
==============================================================================
--- head/sys/dev/ksyms/ksyms.c	Wed Aug 15 16:01:45 2012	(r239302)
+++ head/sys/dev/ksyms/ksyms.c	Wed Aug 15 16:19:39 2012	(r239303)
@@ -579,8 +579,6 @@ ksyms_close(struct cdev *dev, int flags 
 	/* Unmap the buffer from the process address space. */
 	error = copyout_unmap(td, sc->sc_uaddr, sc->sc_usize);
 
-	devfs_clear_cdevpriv();
-
 	return (error);
 }
 

Modified: head/sys/fs/devfs/devfs_vnops.c
==============================================================================
--- head/sys/fs/devfs/devfs_vnops.c	Wed Aug 15 16:01:45 2012	(r239302)
+++ head/sys/fs/devfs/devfs_vnops.c	Wed Aug 15 16:19:39 2012	(r239303)
@@ -1081,6 +1081,9 @@ devfs_open(struct vop_open_args *ap)
 		error = dsw->d_fdopen(dev, ap->a_mode, td, fp);
 	else
 		error = dsw->d_open(dev, ap->a_mode, S_IFCHR, td);
+	/* cleanup any cdevpriv upon error */
+	if (error != 0)
+		devfs_clear_cdevpriv();
 	td->td_fpop = fpop;
 
 	vn_lock(vp, vlocked | LK_RETRY);

Modified: head/sys/ofed/include/linux/linux_compat.c
==============================================================================
--- head/sys/ofed/include/linux/linux_compat.c	Wed Aug 15 16:01:45 2012	(r239302)
+++ head/sys/ofed/include/linux/linux_compat.c	Wed Aug 15 16:19:39 2012	(r239303)
@@ -263,7 +263,6 @@ linux_dev_close(struct cdev *dev, int ff
 	if ((error = devfs_get_cdevpriv((void **)&filp)) != 0)
 		return (error);
 	filp->f_flags = file->f_flag;
-	devfs_clear_cdevpriv();
 
 	return (0);
 }


More information about the svn-src-all mailing list