[Intel-gfx] [PATCH] drm/i915/guc: Fix null pointer dereference when GuC FW is not available

[Michal Wajdeczko]

On Thu, 22 Mar 2018 17:18:55 +0100, Piotr Piórkowski  
<piotr.piorkowski at intel.com> wrote:

> If GuC firmware is not available on the system and we load i915 with  
> enable
> GuC, then we hit this null pointer dereference issue:
>
> [   71.098873] BUG: unable to handle kernel NULL pointer dereference at  
> 0000000000000008
> [   71.098938] IP: intel_uc_fw_upload+0x1f/0x360 [i915]
> [   71.098947] PGD 0 P4D 0
> [   71.098956] Oops: 0000 [#1] PREEMPT SMP PTI
> [   71.098965] Modules linked in: i915(O+) netconsole  
> x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul  
> crc32_pclmul ghash_clmulni_intel mei_me i2c_i801 prime_numbers mei [last  
> unloaded: i915]
> [   71.099005] CPU: 2 PID: 1167 Comm: insmod Tainted: G     U  W  O      
> 4.16.0-rc1+ #337
> [   71.099018] Hardware name: /NUC6i5SYB, BIOS  
> SYSKLi35.86A.0065.2018.0103.1000 01/03/2018
> [   71.099077] RIP: 0010:intel_uc_fw_upload+0x1f/0x360 [i915]
> [   71.099087] RSP: 0018:ffffc90000417aa0 EFLAGS: 00010282
> [   71.099097] RAX: 0000000000000000 RBX: ffff88084cad12f8 RCX:  
> ffffffffa03e9357
> [   71.099108] RDX: 0000000000000002 RSI: ffffffffa034dba0 RDI:  
> ffff88084cad12f8
> [   71.099118] RBP: 0000000000000002 R08: ffff88085344ca90 R09:  
> 0000000000000001
> [   71.099128] R10: 0000000000000000 R11: 0000000000000000 R12:  
> ffff88084cad0000
> [   71.099139] R13: ffffffffa034dba0 R14: 00000000fffffff5 R15:  
> ffff88084cad12b0
> [   71.099151] FS:  00007f7f24ae2740(0000) GS:ffff88085e200000(0000)  
> knlGS:0000000000000000
> [   71.099162] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   71.099171] CR2: 0000000000000008 CR3: 0000000855f48001 CR4:  
> 00000000003606e0
> [   71.099182] Call Trace:
> [   71.099246]  intel_uc_init_hw+0xc8/0x520 [i915]
> [   71.099303]  i915_gem_init_hw+0x11f/0x2d0 [i915]
> [   71.099364]  i915_gem_init+0x2b9/0x640 [i915]
> [   71.099413]  i915_driver_load+0xb74/0x1110 [i915]
> [   71.099462]  i915_pci_probe+0x2e/0x90 [i915]
> [   71.099476]  pci_device_probe+0xa1/0x130
> [   71.099488]  driver_probe_device+0x302/0x470
> [   71.099502]  __driver_attach+0xb9/0xe0
> [   71.099513]  ? driver_probe_device+0x470/0x470
> [   71.099525]  ? driver_probe_device+0x470/0x470
> [   71.099538]  bus_for_each_dev+0x64/0x90
> [   71.099550]  bus_add_driver+0x164/0x260
> [   71.099561]  ? 0xffffffffa04d6000
> [   71.099572]  driver_register+0x57/0xc0
> [   71.099582]  ? 0xffffffffa04d6000
> [   71.099593]  do_one_initcall+0x3b/0x160
> [   71.099606]  ? kmem_cache_alloc_trace+0x1c3/0x2a0
> [   71.099621]  do_init_module+0x5b/0x1f9
> [   71.099635]  load_module+0x2467/0x2a70
> [   71.099654]  ? SyS_finit_module+0xbd/0xe0
> [   71.099668]  SyS_finit_module+0xbd/0xe0
> [   71.099682]  do_syscall_64+0x73/0x1c0
> [   71.099694]  entry_SYSCALL_64_after_hwframe+0x26/0x9b
> [   71.099706] RIP: 0033:0x7f7f23fb40d9
> [   71.099717] RSP: 002b:00007ffda7d67ed8 EFLAGS: 00000246 ORIG_RAX:  
> 0000000000000139
> [   71.099734] RAX: ffffffffffffffda RBX: 000055f96e2a8870 RCX:  
> 00007f7f23fb40d9
> [   71.099748] RDX: 0000000000000000 RSI: 000055f96e2a8260 RDI:  
> 0000000000000003
> [   71.099763] RBP: 000055f96e2a8260 R08: 0000000000000000 R09:  
> 00007ffda7d68088
> [   71.099777] R10: 0000000000000003 R11: 0000000000000246 R12:  
> 0000000000000000
> [   71.099791] R13: 000055f96e2a8830 R14: 0000000000000000 R15:  
> 000055f96e2a8260
> [   71.099810] Code: 00 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00  
> 41 55 41 54 49 89 f5 55 53 48 c7 c1 57 93 3e a0 48 8b 47 10 48 89 fb 4c  
> 8b 07 <48> 8b 68 08 8b 47 28 85 c0 74 15 83 f8 01 48 c7 c1 5b 93 3e a0
> [   71.100004] RIP: intel_uc_fw_upload+0x1f/0x360 [i915] RSP:  
> ffffc90000417aa0
> [   71.100020] CR2: 0000000000000008
> [   71.100031] ---[ end trace d8ac93c30ceff5b2 ]--
>
> Fixes: 6b0478fb722a ("drm/i915: Implement dynamic GuC WOPCM offset and  
> size calculation")
>
> Signed-off-by: Piotr Piórkowski <piotr.piorkowski at intel.com>
> Cc: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Jackie Li <yaodong.li at intel.com>
> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uc_fw.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c  
> b/drivers/gpu/drm/i915/intel_uc_fw.c
> index 30c73243f54d..f143c2437c99 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.c
> @@ -199,7 +199,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
>  		       int (*xfer)(struct intel_uc_fw *uc_fw,
>  				   struct i915_vma *vma))
>  {
> -	struct drm_i915_private *i915 = to_i915(uc_fw->obj->base.dev);
> +	struct intel_guc *guc = container_of(uc_fw, struct intel_guc, fw);

You don't know if this uc_fw is referring to guc->fw or huc->fw

>  	struct i915_vma *vma;
>  	int err;
> @@ -224,7 +224,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
> 	vma = i915_gem_object_ggtt_pin(uc_fw->obj, NULL, 0, 0,
>  				       PIN_OFFSET_BIAS |
> -				       i915->guc.ggtt_pin_bias);
> +				       guc->ggtt_pin_bias);

as we need guc only here, maybe just here:

		to_i915(uc_fw->obj->base.dev)->guc.ggtt_pin_bias);

>  	if (IS_ERR(vma)) {
>  		err = PTR_ERR(vma);
>  		DRM_DEBUG_DRIVER("%s fw ggtt-pin err=%d\n",