From owner-svn-src-all@FreeBSD.ORG Wed Aug 15 16:19:40 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 9AF6C106566C; Wed, 15 Aug 2012 16:19:40 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 82AF68FC12; Wed, 15 Aug 2012 16:19:40 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id q7FGJexe078475; Wed, 15 Aug 2012 16:19:40 GMT (envelope-from hselasky@svn.freebsd.org) Received: (from hselasky@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id q7FGJe8j078467; Wed, 15 Aug 2012 16:19:40 GMT (envelope-from hselasky@svn.freebsd.org) Message-Id: <201208151619.q7FGJe8j078467@svn.freebsd.org> From: Hans Petter Selasky Date: Wed, 15 Aug 2012 16:19:40 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: 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 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Aug 2012 16:19:40 -0000 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); }