[PATCH] drm: Fix use-after-free in the shadow-attache exit code

[PATCH] drm: Fix use-after-free in the shadow-attache exit code

[Daniel Vetter]

This regression has been introduced in

commit b3f2333de8e81b089262b26d52272911523e605f
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Wed Dec 11 11:34:31 2013 +0100

    drm: restrict the device list for shadow attached drivers

Reported-by: Dave Jones <davej@redhat.com>
Cc: Dave Jones <davej@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 5736aaa7e86c..f7af69bcf3f4 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -468,8 +468,8 @@ void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver)
 	} else {
 		list_for_each_entry_safe(dev, tmp, &driver->legacy_dev_list,
 					 legacy_dev_list) {
-			drm_put_dev(dev);
 			list_del(&dev->legacy_dev_list);
+			drm_put_dev(dev);
 		}
 	}
 	DRM_INFO("Module unloaded\n");
-- 
1.8.5.2

Re: [PATCH] drm: Fix use-after-free in the shadow-attache exit code

[David Herrmann]

Hi

On Thu, Jan 30, 2014 at 5:58 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> This regression has been introduced in
>
> commit b3f2333de8e81b089262b26d52272911523e605f
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Wed Dec 11 11:34:31 2013 +0100
>
>     drm: restrict the device list for shadow attached drivers
>
> Reported-by: Dave Jones <davej@redhat.com>
> Cc: Dave Jones <davej@redhat.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: David Herrmann <dh.herrmann@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 5736aaa7e86c..f7af69bcf3f4 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -468,8 +468,8 @@ void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver)
>         } else {
>                 list_for_each_entry_safe(dev, tmp, &driver->legacy_dev_list,
>                                          legacy_dev_list) {
> -                       drm_put_dev(dev);
>                         list_del(&dev->legacy_dev_list);
> +                       drm_put_dev(dev);

This code-path is the only user of legacy_dev_list (besides ->probe)
and both are locked against each other. So removing the device before
destroying it is fine. So no objections from me:

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

>                 }
>         }
>         DRM_INFO("Module unloaded\n");
> --
> 1.8.5.2
>

Re: [PATCH] drm: Fix use-after-free in the shadow-attache exit code

[Dave Jones]

On Thu, Jan 30, 2014 at 05:58:38PM +0100, Daniel Vetter wrote:
 > This regression has been introduced in
 > 
 > commit b3f2333de8e81b089262b26d52272911523e605f
 > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
 > Date:   Wed Dec 11 11:34:31 2013 +0100
 > 
 >     drm: restrict the device list for shadow attached drivers
 > 

btw, I noticed this because it got flagged in the nightly coverity runs.
Of the 18 new issues added yesterday 14 were from drivers/gpu/

If drm developers want to sign up at http://scan.coverity.com
to help out looking over those (and the backlog: stats below)
I can get those accounts approved quickly.

I've been going through trying to clear out as much of the 'noise'
as possible, but it's a huge job.  There's a bunch of cases where
the checker can't figure out if it's a real bug or not because
it doesn't know things like "the hardware will only ever return
these values", but the majority look like actual coding flaws.

	Dave

Currently outstanding issues:

Radeon: 64 
Nouveau: 36
i915: 32
misc drm: 24
gma500: 11
qxl: 7