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
This commit is contained in:
Hans Petter Selasky 2012-08-15 16:19:39 +00:00
parent b80225cd7b
commit 07da61a6cc
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=239303
7 changed files with 20 additions and 22 deletions

View File

@ -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

View File

@ -15517,8 +15517,6 @@ dtrace_close(struct cdev *dev, int flags, int fmt __unused, struct thread *td)
kmem_free(state, 0);
#if __FreeBSD_version < 800039
dev->si_drv1 = NULL;
#else
devfs_clear_cdevpriv();
#endif
#endif
}

View File

@ -57,12 +57,6 @@ int drm_open_helper(struct cdev *kdev, int flags, int fmt, DRM_STRUCTPROC *p,
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, int flags, int fmt, DRM_STRUCTPROC *p,
/* 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, int flags, int fmt, DRM_STRUCTPROC *p,
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);
}

View File

@ -57,12 +57,6 @@ int drm_open_helper(struct cdev *kdev, int flags, int fmt, DRM_STRUCTPROC *p,
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, int flags, int fmt, DRM_STRUCTPROC *p,
/* 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, int flags, int fmt, DRM_STRUCTPROC *p,
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

View File

@ -579,8 +579,6 @@ ksyms_close(struct cdev *dev, int flags __unused, int fmt __unused,
/* Unmap the buffer from the process address space. */
error = copyout_unmap(td, sc->sc_uaddr, sc->sc_usize);
devfs_clear_cdevpriv();
return (error);
}

View File

@ -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);

View File

@ -263,7 +263,6 @@ linux_dev_close(struct cdev *dev, int fflag, int devtype, struct thread *td)
if ((error = devfs_get_cdevpriv((void **)&filp)) != 0)
return (error);
filp->f_flags = file->f_flag;
devfs_clear_cdevpriv();
return (0);
}