Patchwork [2/2] UBUNTU: SAUCE: drm -- stop early access to drm devices

login
register
mail settings
Submitter Andy Whitcroft
Date July 29, 2010, 3:48 p.m.
Message ID <1280418501-9219-3-git-send-email-apw@canonical.com>
Download mbox | patch
Permalink /patch/60273/
State Accepted
Delegated to: Leann Ogasawara
Headers show

Comments

Andy Whitcroft - July 29, 2010, 3:48 p.m.
When a drm driver is initialised we first allocate and initialise the
drm minor numbers including creating the sysfs files, then we trigger
the driver load method.  The act of creating the sysfs files triggers the
uevent.  This means udev may start programs which open /dev/dri/card0 and
other interfaces, this can occur before the load method has even started
and thus before the driver has fully initialised its data structures.
In the case of plymouthd this leads to it opening and closing (in disgust)
the interface, which in turn leads to a kernel panic as the mutexes are
yet to be initialised.

This patch delays the linking up of the drm devices minor numbers until
the driver is fully initialised.  As it is possible for consumers of
these interfaces to reach them before they are fully initialised we
arrange for opens of these devices to return EAGAIN until the device is
fully initialised.

Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
 drivers/gpu/drm/drm_fops.c |    8 ++++++--
 drivers/gpu/drm/drm_stub.c |    6 +++++-
 2 files changed, 11 insertions(+), 3 deletions(-)
Stefan Bader - July 29, 2010, 3:57 p.m.
Seems ok.

Acked-by: Stefan Bader <stefan.bader@canonical.com>

On 07/29/2010 05:48 PM, Andy Whitcroft wrote:
> When a drm driver is initialised we first allocate and initialise the
> drm minor numbers including creating the sysfs files, then we trigger
> the driver load method.  The act of creating the sysfs files triggers the
> uevent.  This means udev may start programs which open /dev/dri/card0 and
> other interfaces, this can occur before the load method has even started
> and thus before the driver has fully initialised its data structures.
> In the case of plymouthd this leads to it opening and closing (in disgust)
> the interface, which in turn leads to a kernel panic as the mutexes are
> yet to be initialised.
> 
> This patch delays the linking up of the drm devices minor numbers until
> the driver is fully initialised.  As it is possible for consumers of
> these interfaces to reach them before they are fully initialised we
> arrange for opens of these devices to return EAGAIN until the device is
> fully initialised.
> 
> Signed-off-by: Andy Whitcroft <apw@canonical.com>
> ---
>  drivers/gpu/drm/drm_fops.c |    8 ++++++--
>  drivers/gpu/drm/drm_stub.c |    6 +++++-
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index e7aace2..3bd5552 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -125,7 +125,8 @@ int drm_open(struct inode *inode, struct file *filp)
>  	minor = idr_find(&drm_minors_idr, minor_id);
>  	if (!minor)
>  		return -ENODEV;
> -
> +	if (IS_ERR(minor))
> +		return PTR_ERR(minor);
>  	if (!(dev = minor->dev))
>  		return -ENODEV;
>  
> @@ -180,7 +181,10 @@ int drm_stub_open(struct inode *inode, struct file *filp)
>  	minor = idr_find(&drm_minors_idr, minor_id);
>  	if (!minor)
>  		goto out;
> -
> +	if (IS_ERR(minor)) {
> +		err = PTR_ERR(minor);
> +		goto out;
> +	}
>  	if (!(dev = minor->dev))
>  		goto out;
>  
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index a0c365f..1110c37 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -345,7 +345,7 @@ static int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int t
>  	new_minor->index = minor_id;
>  	INIT_LIST_HEAD(&new_minor->master_list);
>  
> -	idr_replace(&drm_minors_idr, new_minor, minor_id);
> +	idr_replace(&drm_minors_idr, ERR_PTR(-EAGAIN), minor_id);
>  
>  	if (type == DRM_MINOR_LEGACY) {
>  		ret = drm_proc_init(new_minor, minor_id, drm_proc_root);
> @@ -445,6 +445,10 @@ int drm_get_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
>  
>  	list_add_tail(&dev->driver_item, &driver->device_list);
>  
> +	if (drm_core_check_feature(dev, DRIVER_MODESET))
> +		idr_replace(&drm_minors_idr, dev->control, dev->control->index);
> +	idr_replace(&drm_minors_idr, dev->primary, dev->primary->index);
> +
>  	DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
>  		 driver->name, driver->major, driver->minor, driver->patchlevel,
>  		 driver->date, pci_name(pdev), dev->primary->index);

Patch

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index e7aace2..3bd5552 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -125,7 +125,8 @@  int drm_open(struct inode *inode, struct file *filp)
 	minor = idr_find(&drm_minors_idr, minor_id);
 	if (!minor)
 		return -ENODEV;
-
+	if (IS_ERR(minor))
+		return PTR_ERR(minor);
 	if (!(dev = minor->dev))
 		return -ENODEV;
 
@@ -180,7 +181,10 @@  int drm_stub_open(struct inode *inode, struct file *filp)
 	minor = idr_find(&drm_minors_idr, minor_id);
 	if (!minor)
 		goto out;
-
+	if (IS_ERR(minor)) {
+		err = PTR_ERR(minor);
+		goto out;
+	}
 	if (!(dev = minor->dev))
 		goto out;
 
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index a0c365f..1110c37 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -345,7 +345,7 @@  static int drm_get_minor(struct drm_device *dev, struct drm_minor **minor, int t
 	new_minor->index = minor_id;
 	INIT_LIST_HEAD(&new_minor->master_list);
 
-	idr_replace(&drm_minors_idr, new_minor, minor_id);
+	idr_replace(&drm_minors_idr, ERR_PTR(-EAGAIN), minor_id);
 
 	if (type == DRM_MINOR_LEGACY) {
 		ret = drm_proc_init(new_minor, minor_id, drm_proc_root);
@@ -445,6 +445,10 @@  int drm_get_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 
 	list_add_tail(&dev->driver_item, &driver->device_list);
 
+	if (drm_core_check_feature(dev, DRIVER_MODESET))
+		idr_replace(&drm_minors_idr, dev->control, dev->control->index);
+	idr_replace(&drm_minors_idr, dev->primary, dev->primary->index);
+
 	DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
 		 driver->name, driver->major, driver->minor, driver->patchlevel,
 		 driver->date, pci_name(pdev), dev->primary->index);