[Intel-gfx] [PATCH] drm/i915: return DRIVER_NAME for the fence driver name

Tue Jun 22 21:32:59 UTC 2021

This fixes the NULL pointer dereference in i915_fence_get_driver_name(), 
but now it crashes in i915_fence_get_timeline with another NULL pointer
dereference. It attempts to use i915_gem_context.name, which apparently
also hasn't been initialized

static const char *i915_fence_get_timeline_name(struct dma_fence *fence)
{
    const struct i915_gem_context *ctx;

    /*
     * The timeline struct (as part of the ppgtt underneath a context)
     * may be freed when the request is no longer in use by the GPU.
     * We could extend the life of a context to beyond that of all
     * fences, possibly keeping the hw resource around indefinitely,
     * or we just give them a false name. Since
     * dma_fence_ops.get_timeline_name is a debug feature, the occasional
     * lie seems justifiable.
     */
    if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
        return "signaled";

    ctx = i915_request_gem_context(to_request(fence));
    if (!ctx)
        return "[" DRIVER_NAME "]";

    return ctx->name;
}

<1>[  414.327761] BUG: kernel NULL pointer dereference, address: 0000000000000020
<1>[  414.327766] #PF: supervisor read access in kernel mode
<1>[  414.327768] #PF: error_code(0x0000) - not-present page
<6>[  414.327769] PGD 0 P4D 0
<4>[  414.327772] Oops: 0000 [#1] PREEMPT SMP NOPTI
<4>[  414.327774] CPU: 3 PID: 1866 Comm: chrome Tainted: G     U            5.4.125 #2
<4>[  414.327776] Hardware name: Google Voxel/Voxel, BIOS Google_Voxel.13913.0.0 04/12/2021
<4>[  414.327781] RIP: 0010:i915_fence_get_timeline_name+0x1d/0x37
<4>[  414.327783] Code: 55 48 89 e5 48 c7 c0 ee d1 72 bd 5d c3 0f 1f 44 00 00 55 48 89 e5 48 8b 4f 30 48 c7 c0 b4 49 72 bd f6 c1 01 75 1c 48 8b 47 60 <48> 8b 40 20 48 85 c0 74 08 48 05 30 01 00 00 eb 07 48 c7 c0 79 7b
<4>[  414.327785] RSP: 0018:ffffa4a300d177b8 EFLAGS: 00010246
<4>[  414.327787] RAX: 0000000000000000 RBX: ffffffffbd7a8680 RCX: 0000000000000000
<4>[  414.327788] RDX: ffff9c27b3b0f4c0 RSI: ffff9c27b3b0f480 RDI: ffff9c27b3b0f480
<4>[  414.327789] RBP: ffffa4a300d177b8 R08: 0000000000000000 R09: 0000000000000004
<4>[  414.327791] R10: 0000000000000001 R11: ffffffffbca45536 R12: 0000000000000005
<4>[  414.327792] R13: 0000000000000004 R14: ffff9c27b6c79bb0 R15: ffff9c27b3b0f480
<4>[  414.327794] FS:  00007898a2965e00(0000) GS:ffff9c27b7f80000(0000) knlGS:0000000000000000
<4>[  414.327795] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[  414.327797] CR2: 0000000000000020 CR3: 000000022ded2006 CR4: 0000000000762ee0
<4>[  414.327798] PKRU: 55555554
<4>[  414.327799] Call Trace:
<4>[  414.327804]  trace_event_raw_event_dma_fence+0xc9/0x1e5
<4>[  414.327807]  dma_fence_init+0xa6/0xca
<4>[  414.327809]  __i915_request_ctor+0x7f/0xa9
<4>[  414.327812]  setup_object+0x88/0x8a
<4>[  414.327815]  new_slab+0x22e/0x429
<4>[  414.327817]  ___slab_alloc+0x2c8/0x42e
<4>[  414.327820]  ? __i915_request_create+0x68/0x238
<4>[  414.327822]  ? __i915_request_create+0x68/0x238
<4>[  414.327824]  __slab_alloc+0x3c/0x5f
<4>[  414.327826]  kmem_cache_alloc+0x19b/0x201
<4>[  414.327828]  __i915_request_create+0x68/0x238
<4>[  414.327830]  i915_request_create+0x8a/0xca
<4>[  414.327833]  i915_gem_do_execbuffer+0x12f9/0x1830
<4>[  414.327837]  i915_gem_execbuffer2_ioctl+0x157/0x398
<4>[  414.327839]  ? i915_gem_do_execbuffer+0x1830/0x1830
<4>[  414.327840]  drm_ioctl_kernel+0x94/0xf6
<4>[  414.327842]  drm_ioctl+0x276/0x39b
<4>[  414.327845]  ? i915_gem_do_execbuffer+0x1830/0x1830
<4>[  414.327847]  do_vfs_ioctl+0x4f4/0x771
<4>[  414.327849]  ksys_ioctl+0x58/0x83
<4>[  414.327851]  __x64_sys_ioctl+0x1a/0x1e
<4>[  414.327853]  do_syscall_64+0x54/0x7e
<4>[  414.327856]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
<4>[  414.327859] RIP: 0033:0x7898a2f88f07

> -----Original Message-----
> From: Auld, Matthew <matthew.auld at intel.com>
> Sent: Wednesday, June 16, 2021 5:29 AM
> To: intel-gfx at lists.freedesktop.org
> Cc: dri-devel at lists.freedesktop.org; Mason, Michael W
> <michael.w.mason at intel.com>; Daniel Vetter <daniel at ffwll.ch>
> Subject: [PATCH] drm/i915: return DRIVER_NAME for the fence driver name
> 
> The first tracepoint for a request is trace_dma_fence_init which is called in
> the ctor before we have properly setup the request->engine. So if it's a non-
> recycled request the rq->engine might be NULL, or some garbage value,
> which leads to a crash.
> 
> Since we are not permitted to use kmem_cache_zalloc() here with
> SLAB_TYPESAFE_BY_RCU, one approach is simply to return DRIVER_NAME.
> We can then revisit this later if we decide to get rid of
> SLAB_TYPESAFE_BY_RCU.
> 
> Fixes: 855e39e65cfc ("drm/i915: Initialise basic fence before acquiring
> seqno")
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Michael Mason <michael.w.mason at intel.com>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_request.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_request.c
> b/drivers/gpu/drm/i915/i915_request.c
> index 1014c71cf7f5..55fa94bde22e 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -61,7 +61,7 @@ static struct i915_global_request {
> 
>  static const char *i915_fence_get_driver_name(struct dma_fence *fence)  {
> -	return dev_name(to_request(fence)->engine->i915->drm.dev);
> +	return DRIVER_NAME;
>  }
> 
>  static const char *i915_fence_get_timeline_name(struct dma_fence
> *fence)
> --
> 2.26.3