From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753430AbdGJHPD (ORCPT ); Mon, 10 Jul 2017 03:15:03 -0400 Received: from mail-wr0-f194.google.com ([209.85.128.194]:33334 "EHLO mail-wr0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753173AbdGJHPC (ORCPT ); Mon, 10 Jul 2017 03:15:02 -0400 Date: Mon, 10 Jul 2017 09:14:53 +0200 From: Alexandru Moise <00moses.alexander00@gmail.com> To: Daniel Vetter Cc: linux-kernel@vger.kernel.org, daniel.vetter@intel.com, dri-devel@lists.freedesktop.org, airlied@linux.ie, seanpaul@chromium.org, jani.nikula@linux.intel.com Subject: Re: [PATCH] drm: inhibit drm drivers register to uninitialized drm core Message-ID: <20170710071453.GA16847@gmail.com> References: <20170708214352.GA27205@gmail.com> <20170710065246.rn4o3bjje37bktww@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170710065246.rn4o3bjje37bktww@phenom.ffwll.local> User-Agent: Mutt/1.8.3 (2017-05-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jul 10, 2017 at 08:52:46AM +0200, Daniel Vetter wrote: > On Sat, Jul 08, 2017 at 11:43:52PM +0200, Alexandru Moise wrote: > > If the DRM core fails to init for whatever reason, ensure that > > no driver ever calls drm_dev_register(). > > > > This is best done at drm_dev_init() as it covers drivers that call > > drm_dev_alloc() as well as drivers that prefer to embed struct > > drm_device into their own device struct and call drm_dev_init() > > themselves. > > > > In my case I had so many dynamic device majors used that the major > > number for DRM (226) was stolen, causing DRM core init to fail after > > failing to register a chrdev, and ultimately calling debugfs_remove() > > on drm_debugfs_root in drm_core_exit(). > > > > After drm core failed to init, VGEM was still calling drm_dev_register(), > > ultimately leading to drm_debugfs_init(), with drm_debugfs_root passed > > as the root for the new debugfs dir at debugfs_create_dir(). > > > > This led to a kernel panic once we were either derefencing root->d_inode > > while it was NULL or calling root->d_inode->i_op->lookup() while it was > > NULL in debugfs at inode_lock() or lookup_*(). > > > > Signed-off-by: Alexandru Moise <00moses.alexander00@gmail.com> > > --- > > drivers/gpu/drm/drm_drv.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > > index 37b8ad3e30d8..2ed2d919beae 100644 > > --- a/drivers/gpu/drm/drm_drv.c > > +++ b/drivers/gpu/drm/drm_drv.c > > @@ -63,6 +63,15 @@ module_param_named(debug, drm_debug, int, 0600); > > static DEFINE_SPINLOCK(drm_minor_lock); > > static struct idr drm_minors_idr; > > > > +/* > > + * If the drm core fails to init for whatever reason, > > + * we should prevent any drivers from registering with it. > > + * It's best to check this at drm_dev_init(), as some drivers > > + * prefer to embed struct drm_device into their own device > > + * structure and call drm_dev_init() themselves. > > + */ > > +static bool drm_core_init_complete = false; > > + > > static struct dentry *drm_debugfs_root; > > > > #define DRM_PRINTK_FMT "[" DRM_NAME ":%s]%s %pV" > > @@ -484,6 +493,11 @@ int drm_dev_init(struct drm_device *dev, > > { > > int ret; > > > > + if (!drm_core_init_complete) { > > + DRM_ERROR("DRM core is not initialized\n"); > > + return -ENODEV; > > + } > > + > > kref_init(&dev->ref); > > dev->dev = parent; > > dev->driver = driver; > > @@ -966,6 +980,8 @@ static int __init drm_core_init(void) > > if (ret < 0) > > goto error; > > > > + drm_core_init_complete = true; > > + > > DRM_DEBUG("Initialized\n"); > > return 0; > > Isn't the correct fix to pass down the error value, which iirc should > make the kmod stuff unload the module again? Or does this not work' > -Daniel What if everything is built in? ../Alex > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch