[PATCH] drm/nouveau: avoid null deref on bad arguments to nouveau_vma_getmap

[Maarten Lankhorst]

Op 22-08-13 02:10, Ilia Mirkin schreef:
> The code expects non-VRAM mem nodes to have a pages list. If that's not
> set, it will do a null deref down the line. Warn on that condition and
> return an error.
>
> See https://bugs.freedesktop.org/show_bug.cgi?id=64774
>
> Reported-by: Pasi K?rkk?inen <pasik at iki.fi>
> Tested-by: Pasi K?rkk?inen <pasik at iki.fi>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: <stable at vger.kernel.org> # 3.8+
> ---
>
> I don't exactly understand what's going on, but this is just a
> straightforward way to avoid a null deref that you see happens in the
> bug. I haven't figured out the root cause of this, but it's getting
> well into the "I have no idea how TTM works" space. However this seems
> like a bit of defensive programming -- nouveau_vm_map_sg will pass
> node->pages as a list down, which will be dereferenced by
> nvc0_vm_map_sg. Perhaps the other arguments should make that
> dereferencing not happen, but it definitely was happening here, as you
> can see in the bug.
>
> Ben/Maarten, I'll let you judge whether this check is appropriate,
> since like I hope I was able to convey above, I'm just not really sure :)
Not it really isn't appropriate..

You'd have to call call nouveau_vm_map_sg_table instead, the only place that 
doesn't handle that correctly
is where it's not expected to be called.

Here, have a completely untested patch to fix things...

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
b/drivers/gpu/drm/nouveau/nouveau_display.c
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -138,17 +143,26 @@ nouveau_user_framebuffer_create(struct drm_device *dev,
 {
        struct nouveau_framebuffer *nouveau_fb;
        struct drm_gem_object *gem;
+       struct nouveau_bo *nvbo;
        int ret = -ENOMEM;

        gem = drm_gem_object_lookup(dev, file_priv, mode_cmd->handles[0]);
        if (!gem)
                return ERR_PTR(-ENOENT);

+       nvbo = nouveau_gem_object(gem);
+       if (!(nvbo->valid_domains & NOUVEAU_GEM_DOMAIN_VRAM)) {
+               nv_warn(nouveau_drm(dev), "Trying to create a fb in vram with"
+                       " valid_domains=%08x\n", nvbo->valid_domains);
+               ret = -EINVAL;
+               goto err_unref;
+       }
+
        nouveau_fb = kzalloc(sizeof(struct nouveau_framebuffer), GFP_KERNEL);
        if (!nouveau_fb)
                goto err_unref;

-       ret = nouveau_framebuffer_init(dev, nouveau_fb, mode_cmd, 
nouveau_gem_object(gem));
+       ret = nouveau_framebuffer_init(dev, nouveau_fb, mode_cmd, nvbo);
        if (ret)
                goto err;