From 525895ba388c949aa906f26e3ec5cb1ab041f56b Mon Sep 17 00:00:00 2001 From: Ben Skeggs Date: Tue, 10 Jan 2012 10:18:28 +1000 Subject: [PATCH] drm/nouveau/gem: fix fence_sync race / oops Due to a race it was possible for a fence to be destroyed while another thread was trying to synchronise with it. If this happened in the fallback non-semaphore path, it lead to the following oops due to fence->channel being NULL. BUG: unable to handle kernel NULL pointer dereference at (null) IP: [] nouveau_fence_update+0xe/0xe0 [nouveau] *pde = a649c067 SMP Modules linked in: fuse nouveau(O) ttm(O) drm_kms_helper(O) drm(O) mxm_wmi video wmi netconsole configfs lockd bnep bluetooth rfkill ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack ip6table_filter ip6_tables snd_hda_codec_realtek snd_hda_intel snd_hda_cobinfmt_misc uinput ata_generic pata_acpi pata_aet2c_algo_bit i2c_core [last unloaded: wmi] Pid: 2255, comm: gnome-shell Tainted: G O 3.2.0-0.rc5.git0.1.fc17.i686 #1 System manufacturer System Product Name/M2A-VM EIP: 0060:[] EFLAGS: 00010296 CPU: 1 EIP is at nouveau_fence_update+0xe/0xe0 [nouveau] EAX: 00000000 EBX: ddfc6dd0 ECX: dd111580 EDX: 00000000 ESI: 00003e80 EDI: dd111580 EBP: dd121d00 ESP: dd121ce8 DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 Process gnome-shell (pid: 2255, ti=dd120000 task=dd111580 task.ti=dd120000) Stack: 7dc86c76 00000000 00003e80 ddfc6dd0 00003e80 dd111580 dd121d0c fa96371f 00000000 dd121d3c fa963773 dd111580 01000246 000ec53d 00000000 ddfc6dd0 00001f40 00000000 ddfc6dd0 00000010 dc7df840 dd121d6c fa9639a0 00000000 Call Trace: [] __nouveau_fence_signalled+0x1f/0x30 [nouveau] [] __nouveau_fence_wait+0x43/0xd0 [nouveau] [] nouveau_fence_sync+0x1a0/0x1c0 [nouveau] [] validate_list+0x176/0x300 [nouveau] [] ? ttm_bo_mem_put+0x30/0x30 [ttm] [] nouveau_gem_ioctl_pushbuf+0x48a/0xfd0 [nouveau] [] ? die+0x31/0x80 [] drm_ioctl+0x388/0x490 [drm] [] ? die+0x31/0x80 [] ? nouveau_gem_ioctl_new+0x150/0x150 [nouveau] [] ? file_has_perm+0xcb/0xe0 [] ? drm_copy_field+0x80/0x80 [drm] [] do_vfs_ioctl+0x86/0x5b0 [] ? die+0x31/0x80 [] ? selinux_file_ioctl+0x62/0x130 [] ? fget_light+0x30/0x340 [] sys_ioctl+0x6f/0x80 [] syscall_call+0x7/0xb [] ? die+0x31/0x80 [] ? die+0x31/0x80 Signed-off-by: Ben Skeggs Cc: stable@vger.kernel.org --- drivers/gpu/drm/nouveau/nouveau_gem.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index 5f0bc57..7ce3fde 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@ -380,6 +380,25 @@ retry: } static int +validate_sync(struct nouveau_channel *chan, struct nouveau_bo *nvbo) +{ + struct nouveau_fence *fence = NULL; + int ret = 0; + + spin_lock(&nvbo->bo.bdev->fence_lock); + if (nvbo->bo.sync_obj) + fence = nouveau_fence_ref(nvbo->bo.sync_obj); + spin_unlock(&nvbo->bo.bdev->fence_lock); + + if (fence) { + ret = nouveau_fence_sync(fence, chan); + nouveau_fence_unref(&fence); + } + + return ret; +} + +static int validate_list(struct nouveau_channel *chan, struct list_head *list, struct drm_nouveau_gem_pushbuf_bo *pbbo, uint64_t user_pbbo_ptr) { @@ -393,7 +412,7 @@ validate_list(struct nouveau_channel *chan, struct list_head *list, list_for_each_entry(nvbo, list, entry) { struct drm_nouveau_gem_pushbuf_bo *b = &pbbo[nvbo->pbbo_index]; - ret = nouveau_fence_sync(nvbo->bo.sync_obj, chan); + ret = validate_sync(chan, nvbo); if (unlikely(ret)) { NV_ERROR(dev, "fail pre-validate sync\n"); return ret; @@ -416,7 +435,7 @@ validate_list(struct nouveau_channel *chan, struct list_head *list, return ret; } - ret = nouveau_fence_sync(nvbo->bo.sync_obj, chan); + ret = validate_sync(chan, nvbo); if (unlikely(ret)) { NV_ERROR(dev, "fail post-validate sync\n"); return ret; -- 2.7.4