diff mbox series

[next] drm/i915/gem: fix null pointer dereference on vm

Message ID 20200123151406.51679-1-colin.king@canonical.com (mailing list archive)
State New, archived
Headers show
Series [next] drm/i915/gem: fix null pointer dereference on vm | expand

Commit Message

Colin King Jan. 23, 2020, 3:14 p.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

Currently if the call to function context_get_vm_rcu returns
a null pointer for vm then the error exit path via label err_put
will call i915_vm_put on the null vm, causing a null pointer
dereference.  Fix this by adding a null check on vm and returning
without calling the i915_vm_put.

Fixes: 5dbd2b7be61e ("drm/i915/gem: Convert vm idr to xarray")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Chris Wilson Jan. 23, 2020, 3:24 p.m. UTC | #1 
Quoting Colin King (2020-01-23 15:14:06)
> From: Colin Ian King <colin.king@canonical.com>
> 
> Currently if the call to function context_get_vm_rcu returns
> a null pointer for vm then the error exit path via label err_put
> will call i915_vm_put on the null vm, causing a null pointer
> dereference.  Fix this by adding a null check on vm and returning
> without calling the i915_vm_put.
> 
> Fixes: 5dbd2b7be61e ("drm/i915/gem: Convert vm idr to xarray")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Hmm. Actually, we can drop the rcu_read_lock as soon as we've acquire
the local ref to ctx->vm. So something like,

        if (!rcu_access_pointer(ctx->vm))
                return -ENODEV;

-       err = -ENODEV;
        rcu_read_lock();
        vm = context_get_vm_rcu(ctx);
-       if (vm)
-               err = xa_alloc(&file_priv->vm_xa, &id, vm,
-                              xa_limit_32b, GFP_KERNEL);
        rcu_read_unlock();
+       if (!vm)
+               return -ENODEV;
+
+       err = xa_alloc(&file_priv->vm_xa, &id, vm,
+                      xa_limit_32b, GFP_KERNEL);
        if (err)
                goto err_put;

would work.
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 5d4157e1ccf7..3e6e34ec9fa8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1005,9 +1005,12 @@  static int get_ppgtt(struct drm_i915_file_private *file_priv,
 	err = -ENODEV;
 	rcu_read_lock();
 	vm = context_get_vm_rcu(ctx);
-	if (vm)
-		err = xa_alloc(&file_priv->vm_xa, &id, vm,
-			       xa_limit_32b, GFP_KERNEL);
+	if (!vm) {
+		rcu_read_unlock();
+		return err;
+	}
+	err = xa_alloc(&file_priv->vm_xa, &id, vm,
+		       xa_limit_32b, GFP_KERNEL);
 	rcu_read_unlock();
 	if (err)
 		goto err_put;