All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
@ 2017-03-01 18:59 Chris Wilson
  2017-03-01 20:17 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Chris Wilson @ 2017-03-01 18:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Takashi Iwai

[31908.547136] BUG: KASAN: use-after-free in intel_lpe_audio_teardown+0x78/0xb0 [i915] at addr ffff8801f7788358
[31908.547297] Read of size 8 by task drv_selftest/3781
[31908.547405] CPU: 0 PID: 3781 Comm: drv_selftest Tainted: G    BU  W       4.10.0+ #451
[31908.547553] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[31908.547682] Call Trace:
[31908.547772]  dump_stack+0x68/0x9f
[31908.547857]  kasan_object_err+0x1c/0x70
[31908.547947]  kasan_report_error+0x1f1/0x4f0
[31908.548038]  ? kfree+0xaa/0x170
[31908.548121]  kasan_report+0x34/0x40
[31908.548211]  ? klist_children_get+0x20/0x30
[31908.548472]  ? intel_lpe_audio_teardown+0x78/0xb0 [i915]
[31908.548567]  __asan_load8+0x5e/0x70
[31908.548824]  intel_lpe_audio_teardown+0x78/0xb0 [i915]
[31908.549080]  intel_audio_deinit+0x28/0x80 [i915]
[31908.549315]  i915_driver_unload+0xe4/0x360 [i915]
[31908.549551]  ? i915_driver_load+0x1d70/0x1d70 [i915]
[31908.549651]  ? trace_hardirqs_on+0xd/0x10
[31908.549885]  i915_pci_remove+0x23/0x30 [i915]
[31908.549978]  pci_device_remove+0x5c/0x100
[31908.550069]  device_release_driver_internal+0x1db/0x2e0
[31908.550165]  driver_detach+0x68/0xc0
[31908.550256]  bus_remove_driver+0x8b/0x150
[31908.550346]  driver_unregister+0x3e/0x60
[31908.550439]  pci_unregister_driver+0x1d/0x110
[31908.550531]  ? find_module_all+0x7a/0xa0
[31908.550791]  i915_exit+0x1a/0x87 [i915]
[31908.550881]  SyS_delete_module+0x264/0x2c0
[31908.550971]  ? free_module+0x430/0x430
[31908.551064]  ? trace_hardirqs_off_caller+0x16/0x110
[31908.551159]  ? trace_hardirqs_on_caller+0x16/0x280
[31908.551256]  ? trace_hardirqs_on_thunk+0x1a/0x1c
[31908.551350]  entry_SYSCALL_64_fastpath+0x1c/0xb1
[31908.551440] RIP: 0033:0x7f1d67312ec7
[31908.551520] RSP: 002b:00007ffebe34e888 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
[31908.551650] RAX: ffffffffffffffda RBX: ffffffff811123f6 RCX: 00007f1d67312ec7
[31908.551743] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000560d0af476b8
[31908.551837] RBP: ffff880233d87f98 R08: 0000000000000000 R09: 00007ffebe34e8b8
[31908.551930] R10: 00007f1d68adf8c0 R11: 0000000000000206 R12: 0000000000000000
[31908.552023] R13: 0000560d0af46440 R14: 0000000000000034 R15: 00007ffebe34d860
[31908.552121]  ? trace_hardirqs_off_caller+0x16/0x110
[31908.552217] Object at ffff8801f7788000, in cache kmalloc-2048 size: 2048
[31908.552306] Allocated:
[31908.552377] PID = 3781
[31908.552456]  save_stack_trace+0x16/0x20
[31908.552539]  kasan_kmalloc+0xee/0x190
[31908.552627]  __kmalloc+0xdb/0x1b0
[31908.552713]  platform_device_alloc+0x27/0x90
[31908.552804]  platform_device_register_full+0x36/0x220
[31908.553066]  intel_lpe_audio_init+0x41e/0x570 [i915]
[31908.553320]  intel_audio_init+0xd/0x40 [i915]
[31908.553552]  i915_driver_load+0x13f5/0x1d70 [i915]
[31908.553788]  i915_pci_probe+0x65/0xe0 [i915]
[31908.553881]  pci_device_probe+0xda/0x140
[31908.553969]  driver_probe_device+0x400/0x660
[31908.554058]  __driver_attach+0x11c/0x120
[31908.554147]  bus_for_each_dev+0xe6/0x150
[31908.554237]  driver_attach+0x26/0x30
[31908.554325]  bus_add_driver+0x26b/0x3b0
[31908.554412]  driver_register+0xce/0x190
[31908.554502]  __pci_register_driver+0xaf/0xc0
[31908.554589]  0xffffffffa0550063
[31908.554675]  do_one_initcall+0x8b/0x1e0
[31908.554764]  do_init_module+0x102/0x325
[31908.554852]  load_module+0x3aad/0x45e0
[31908.554944]  SyS_finit_module+0x169/0x1a0
[31908.555033]  entry_SYSCALL_64_fastpath+0x1c/0xb1
[31908.555119] Freed:
[31908.555188] PID = 3781
[31908.555266]  save_stack_trace+0x16/0x20
[31908.555349]  kasan_slab_free+0xb0/0x180
[31908.555436]  kfree+0xaa/0x170
[31908.555520]  platform_device_release+0x76/0x80
[31908.555610]  device_release+0x45/0xe0
[31908.555698]  kobject_put+0x11f/0x260
[31908.555785]  put_device+0x12/0x20
[31908.555871]  platform_device_unregister+0x1b/0x20
[31908.556135]  intel_lpe_audio_teardown+0x5c/0xb0 [i915]
[31908.556390]  intel_audio_deinit+0x28/0x80 [i915]
[31908.556622]  i915_driver_unload+0xe4/0x360 [i915]
[31908.556858]  i915_pci_remove+0x23/0x30 [i915]
[31908.556948]  pci_device_remove+0x5c/0x100
[31908.557037]  device_release_driver_internal+0x1db/0x2e0
[31908.557129]  driver_detach+0x68/0xc0
[31908.557217]  bus_remove_driver+0x8b/0x150
[31908.557304]  driver_unregister+0x3e/0x60
[31908.557394]  pci_unregister_driver+0x1d/0x110
[31908.557653]  i915_exit+0x1a/0x87 [i915]
[31908.557741]  SyS_delete_module+0x264/0x2c0
[31908.557834]  entry_SYSCALL_64_fastpath+0x1c/0xb1
[31908.557919] Memory state around the buggy address:
[31908.558005]  ffff8801f7788200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[31908.558127]  ffff8801f7788280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[31908.558255] >ffff8801f7788300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[31908.558374]                                                     ^
[31908.558467]  ffff8801f7788380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[31908.558595]  ffff8801f7788400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb

Fixes: eef57324d926 ("drm/i915: setup bridge for HDMI LPE audio driver")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Jerome Anand <jerome.anand@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lpe_audio.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 7a5b41b1c024..32000902a204 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -131,8 +131,12 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
 
 static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
 {
-	platform_device_unregister(dev_priv->lpe_audio.platdev);
-	kfree(dev_priv->lpe_audio.platdev->dev.dma_mask);
+	struct platform_device *platdev = dev_priv->lpe_audio.platdev;
+
+	kfree(platdev->dev.dma_mask);
+	platdev->dev.dma_mask = NULL;
+
+	platform_device_unregister(platdev);
 }
 
 static void lpe_audio_irq_unmask(struct irq_data *d)
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-03-01 18:59 [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy() Chris Wilson
@ 2017-03-01 20:17 ` Patchwork
  2017-04-11 20:41 ` [PATCH] " Chris Wilson
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-03-01 20:17 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Fix use after free in lpe_audio_platdev_destroy()
URL   : https://patchwork.freedesktop.org/series/20476/
State : success

== Summary ==

Series 20476v1 drm/i915: Fix use after free in lpe_audio_platdev_destroy()
https://patchwork.freedesktop.org/api/1.0/series/20476/revisions/1/mbox/

Test gem_exec_reloc:
        Subgroup basic-gtt-cpu-active:
                incomplete -> PASS       (fi-byt-j1900)

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20 
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:278  pass:192  dwarn:36  dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:278  pass:216  dwarn:44  dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29 

07e9f4ad3a2ca599975099c1fe49189a24be1884 drm-tip: 2017y-03m-01d-18h-00m-03s UTC integration manifest
6df3ff3 drm/i915: Fix use after free in lpe_audio_platdev_destroy()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4022/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-03-01 18:59 [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy() Chris Wilson
  2017-03-01 20:17 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-04-11 20:41 ` Chris Wilson
  2017-04-11 21:01   ` Takashi Iwai
  2017-04-12  8:02 ` [PATCH v2] " Chris Wilson
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-04-11 20:41 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Takashi Iwai

On Wed, Mar 01, 2017 at 06:59:18PM +0000, Chris Wilson wrote:
> [31908.547136] BUG: KASAN: use-after-free in intel_lpe_audio_teardown+0x78/0xb0 [i915] at addr ffff8801f7788358
> [31908.547297] Read of size 8 by task drv_selftest/3781
> [31908.547405] CPU: 0 PID: 3781 Comm: drv_selftest Tainted: G    BU  W       4.10.0+ #451
> [31908.547553] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> [31908.547682] Call Trace:
> [31908.547772]  dump_stack+0x68/0x9f
> [31908.547857]  kasan_object_err+0x1c/0x70
> [31908.547947]  kasan_report_error+0x1f1/0x4f0
> [31908.548038]  ? kfree+0xaa/0x170
> [31908.548121]  kasan_report+0x34/0x40
> [31908.548211]  ? klist_children_get+0x20/0x30
> [31908.548472]  ? intel_lpe_audio_teardown+0x78/0xb0 [i915]
> [31908.548567]  __asan_load8+0x5e/0x70
> [31908.548824]  intel_lpe_audio_teardown+0x78/0xb0 [i915]
> [31908.549080]  intel_audio_deinit+0x28/0x80 [i915]
> [31908.549315]  i915_driver_unload+0xe4/0x360 [i915]
> [31908.549551]  ? i915_driver_load+0x1d70/0x1d70 [i915]
> [31908.549651]  ? trace_hardirqs_on+0xd/0x10
> [31908.549885]  i915_pci_remove+0x23/0x30 [i915]
> [31908.549978]  pci_device_remove+0x5c/0x100
> [31908.550069]  device_release_driver_internal+0x1db/0x2e0
> [31908.550165]  driver_detach+0x68/0xc0
> [31908.550256]  bus_remove_driver+0x8b/0x150
> [31908.550346]  driver_unregister+0x3e/0x60
> [31908.550439]  pci_unregister_driver+0x1d/0x110
> [31908.550531]  ? find_module_all+0x7a/0xa0
> [31908.550791]  i915_exit+0x1a/0x87 [i915]
> [31908.550881]  SyS_delete_module+0x264/0x2c0
> [31908.550971]  ? free_module+0x430/0x430
> [31908.551064]  ? trace_hardirqs_off_caller+0x16/0x110
> [31908.551159]  ? trace_hardirqs_on_caller+0x16/0x280
> [31908.551256]  ? trace_hardirqs_on_thunk+0x1a/0x1c
> [31908.551350]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> [31908.551440] RIP: 0033:0x7f1d67312ec7
> [31908.551520] RSP: 002b:00007ffebe34e888 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
> [31908.551650] RAX: ffffffffffffffda RBX: ffffffff811123f6 RCX: 00007f1d67312ec7
> [31908.551743] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000560d0af476b8
> [31908.551837] RBP: ffff880233d87f98 R08: 0000000000000000 R09: 00007ffebe34e8b8
> [31908.551930] R10: 00007f1d68adf8c0 R11: 0000000000000206 R12: 0000000000000000
> [31908.552023] R13: 0000560d0af46440 R14: 0000000000000034 R15: 00007ffebe34d860
> [31908.552121]  ? trace_hardirqs_off_caller+0x16/0x110
> [31908.552217] Object at ffff8801f7788000, in cache kmalloc-2048 size: 2048
> [31908.552306] Allocated:
> [31908.552377] PID = 3781
> [31908.552456]  save_stack_trace+0x16/0x20
> [31908.552539]  kasan_kmalloc+0xee/0x190
> [31908.552627]  __kmalloc+0xdb/0x1b0
> [31908.552713]  platform_device_alloc+0x27/0x90
> [31908.552804]  platform_device_register_full+0x36/0x220
> [31908.553066]  intel_lpe_audio_init+0x41e/0x570 [i915]
> [31908.553320]  intel_audio_init+0xd/0x40 [i915]
> [31908.553552]  i915_driver_load+0x13f5/0x1d70 [i915]
> [31908.553788]  i915_pci_probe+0x65/0xe0 [i915]
> [31908.553881]  pci_device_probe+0xda/0x140
> [31908.553969]  driver_probe_device+0x400/0x660
> [31908.554058]  __driver_attach+0x11c/0x120
> [31908.554147]  bus_for_each_dev+0xe6/0x150
> [31908.554237]  driver_attach+0x26/0x30
> [31908.554325]  bus_add_driver+0x26b/0x3b0
> [31908.554412]  driver_register+0xce/0x190
> [31908.554502]  __pci_register_driver+0xaf/0xc0
> [31908.554589]  0xffffffffa0550063
> [31908.554675]  do_one_initcall+0x8b/0x1e0
> [31908.554764]  do_init_module+0x102/0x325
> [31908.554852]  load_module+0x3aad/0x45e0
> [31908.554944]  SyS_finit_module+0x169/0x1a0
> [31908.555033]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> [31908.555119] Freed:
> [31908.555188] PID = 3781
> [31908.555266]  save_stack_trace+0x16/0x20
> [31908.555349]  kasan_slab_free+0xb0/0x180
> [31908.555436]  kfree+0xaa/0x170
> [31908.555520]  platform_device_release+0x76/0x80
> [31908.555610]  device_release+0x45/0xe0
> [31908.555698]  kobject_put+0x11f/0x260
> [31908.555785]  put_device+0x12/0x20
> [31908.555871]  platform_device_unregister+0x1b/0x20
> [31908.556135]  intel_lpe_audio_teardown+0x5c/0xb0 [i915]
> [31908.556390]  intel_audio_deinit+0x28/0x80 [i915]
> [31908.556622]  i915_driver_unload+0xe4/0x360 [i915]
> [31908.556858]  i915_pci_remove+0x23/0x30 [i915]
> [31908.556948]  pci_device_remove+0x5c/0x100
> [31908.557037]  device_release_driver_internal+0x1db/0x2e0
> [31908.557129]  driver_detach+0x68/0xc0
> [31908.557217]  bus_remove_driver+0x8b/0x150
> [31908.557304]  driver_unregister+0x3e/0x60
> [31908.557394]  pci_unregister_driver+0x1d/0x110
> [31908.557653]  i915_exit+0x1a/0x87 [i915]
> [31908.557741]  SyS_delete_module+0x264/0x2c0
> [31908.557834]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> [31908.557919] Memory state around the buggy address:
> [31908.558005]  ffff8801f7788200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [31908.558127]  ffff8801f7788280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [31908.558255] >ffff8801f7788300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [31908.558374]                                                     ^
> [31908.558467]  ffff8801f7788380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [31908.558595]  ffff8801f7788400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> 
> Fixes: eef57324d926 ("drm/i915: setup bridge for HDMI LPE audio driver")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Cc: Jerome Anand <jerome.anand@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Ping?

> ---
>  drivers/gpu/drm/i915/intel_lpe_audio.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> index 7a5b41b1c024..32000902a204 100644
> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> @@ -131,8 +131,12 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
>  
>  static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
>  {
> -	platform_device_unregister(dev_priv->lpe_audio.platdev);
> -	kfree(dev_priv->lpe_audio.platdev->dev.dma_mask);
> +	struct platform_device *platdev = dev_priv->lpe_audio.platdev;
> +
> +	kfree(platdev->dev.dma_mask);
> +	platdev->dev.dma_mask = NULL;
> +
> +	platform_device_unregister(platdev);
>  }
>  
>  static void lpe_audio_irq_unmask(struct irq_data *d)
> -- 
> 2.11.0
> 

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-04-11 20:41 ` [PATCH] " Chris Wilson
@ 2017-04-11 21:01   ` Takashi Iwai
  2017-04-11 21:20     ` Chris Wilson
  0 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2017-04-11 21:01 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Jani Nikula, Takashi Iwai, intel-gfx

On Tue, 11 Apr 2017 22:41:12 +0200,
Chris Wilson wrote:
> 
> On Wed, Mar 01, 2017 at 06:59:18PM +0000, Chris Wilson wrote:
> > [31908.547136] BUG: KASAN: use-after-free in intel_lpe_audio_teardown+0x78/0xb0 [i915] at addr ffff8801f7788358
> > [31908.547297] Read of size 8 by task drv_selftest/3781
> > [31908.547405] CPU: 0 PID: 3781 Comm: drv_selftest Tainted: G    BU  W       4.10.0+ #451
> > [31908.547553] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> > [31908.547682] Call Trace:
> > [31908.547772]  dump_stack+0x68/0x9f
> > [31908.547857]  kasan_object_err+0x1c/0x70
> > [31908.547947]  kasan_report_error+0x1f1/0x4f0
> > [31908.548038]  ? kfree+0xaa/0x170
> > [31908.548121]  kasan_report+0x34/0x40
> > [31908.548211]  ? klist_children_get+0x20/0x30
> > [31908.548472]  ? intel_lpe_audio_teardown+0x78/0xb0 [i915]
> > [31908.548567]  __asan_load8+0x5e/0x70
> > [31908.548824]  intel_lpe_audio_teardown+0x78/0xb0 [i915]
> > [31908.549080]  intel_audio_deinit+0x28/0x80 [i915]
> > [31908.549315]  i915_driver_unload+0xe4/0x360 [i915]
> > [31908.549551]  ? i915_driver_load+0x1d70/0x1d70 [i915]
> > [31908.549651]  ? trace_hardirqs_on+0xd/0x10
> > [31908.549885]  i915_pci_remove+0x23/0x30 [i915]
> > [31908.549978]  pci_device_remove+0x5c/0x100
> > [31908.550069]  device_release_driver_internal+0x1db/0x2e0
> > [31908.550165]  driver_detach+0x68/0xc0
> > [31908.550256]  bus_remove_driver+0x8b/0x150
> > [31908.550346]  driver_unregister+0x3e/0x60
> > [31908.550439]  pci_unregister_driver+0x1d/0x110
> > [31908.550531]  ? find_module_all+0x7a/0xa0
> > [31908.550791]  i915_exit+0x1a/0x87 [i915]
> > [31908.550881]  SyS_delete_module+0x264/0x2c0
> > [31908.550971]  ? free_module+0x430/0x430
> > [31908.551064]  ? trace_hardirqs_off_caller+0x16/0x110
> > [31908.551159]  ? trace_hardirqs_on_caller+0x16/0x280
> > [31908.551256]  ? trace_hardirqs_on_thunk+0x1a/0x1c
> > [31908.551350]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > [31908.551440] RIP: 0033:0x7f1d67312ec7
> > [31908.551520] RSP: 002b:00007ffebe34e888 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
> > [31908.551650] RAX: ffffffffffffffda RBX: ffffffff811123f6 RCX: 00007f1d67312ec7
> > [31908.551743] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000560d0af476b8
> > [31908.551837] RBP: ffff880233d87f98 R08: 0000000000000000 R09: 00007ffebe34e8b8
> > [31908.551930] R10: 00007f1d68adf8c0 R11: 0000000000000206 R12: 0000000000000000
> > [31908.552023] R13: 0000560d0af46440 R14: 0000000000000034 R15: 00007ffebe34d860
> > [31908.552121]  ? trace_hardirqs_off_caller+0x16/0x110
> > [31908.552217] Object at ffff8801f7788000, in cache kmalloc-2048 size: 2048
> > [31908.552306] Allocated:
> > [31908.552377] PID = 3781
> > [31908.552456]  save_stack_trace+0x16/0x20
> > [31908.552539]  kasan_kmalloc+0xee/0x190
> > [31908.552627]  __kmalloc+0xdb/0x1b0
> > [31908.552713]  platform_device_alloc+0x27/0x90
> > [31908.552804]  platform_device_register_full+0x36/0x220
> > [31908.553066]  intel_lpe_audio_init+0x41e/0x570 [i915]
> > [31908.553320]  intel_audio_init+0xd/0x40 [i915]
> > [31908.553552]  i915_driver_load+0x13f5/0x1d70 [i915]
> > [31908.553788]  i915_pci_probe+0x65/0xe0 [i915]
> > [31908.553881]  pci_device_probe+0xda/0x140
> > [31908.553969]  driver_probe_device+0x400/0x660
> > [31908.554058]  __driver_attach+0x11c/0x120
> > [31908.554147]  bus_for_each_dev+0xe6/0x150
> > [31908.554237]  driver_attach+0x26/0x30
> > [31908.554325]  bus_add_driver+0x26b/0x3b0
> > [31908.554412]  driver_register+0xce/0x190
> > [31908.554502]  __pci_register_driver+0xaf/0xc0
> > [31908.554589]  0xffffffffa0550063
> > [31908.554675]  do_one_initcall+0x8b/0x1e0
> > [31908.554764]  do_init_module+0x102/0x325
> > [31908.554852]  load_module+0x3aad/0x45e0
> > [31908.554944]  SyS_finit_module+0x169/0x1a0
> > [31908.555033]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > [31908.555119] Freed:
> > [31908.555188] PID = 3781
> > [31908.555266]  save_stack_trace+0x16/0x20
> > [31908.555349]  kasan_slab_free+0xb0/0x180
> > [31908.555436]  kfree+0xaa/0x170
> > [31908.555520]  platform_device_release+0x76/0x80
> > [31908.555610]  device_release+0x45/0xe0
> > [31908.555698]  kobject_put+0x11f/0x260
> > [31908.555785]  put_device+0x12/0x20
> > [31908.555871]  platform_device_unregister+0x1b/0x20
> > [31908.556135]  intel_lpe_audio_teardown+0x5c/0xb0 [i915]
> > [31908.556390]  intel_audio_deinit+0x28/0x80 [i915]
> > [31908.556622]  i915_driver_unload+0xe4/0x360 [i915]
> > [31908.556858]  i915_pci_remove+0x23/0x30 [i915]
> > [31908.556948]  pci_device_remove+0x5c/0x100
> > [31908.557037]  device_release_driver_internal+0x1db/0x2e0
> > [31908.557129]  driver_detach+0x68/0xc0
> > [31908.557217]  bus_remove_driver+0x8b/0x150
> > [31908.557304]  driver_unregister+0x3e/0x60
> > [31908.557394]  pci_unregister_driver+0x1d/0x110
> > [31908.557653]  i915_exit+0x1a/0x87 [i915]
> > [31908.557741]  SyS_delete_module+0x264/0x2c0
> > [31908.557834]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > [31908.557919] Memory state around the buggy address:
> > [31908.558005]  ffff8801f7788200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [31908.558127]  ffff8801f7788280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [31908.558255] >ffff8801f7788300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [31908.558374]                                                     ^
> > [31908.558467]  ffff8801f7788380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [31908.558595]  ffff8801f7788400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > 
> > Fixes: eef57324d926 ("drm/i915: setup bridge for HDMI LPE audio driver")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > Cc: Jerome Anand <jerome.anand@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Ping?

Oh, this fell into a crack as it was sent just before my vacation.

About the change:

> > ---
> >  drivers/gpu/drm/i915/intel_lpe_audio.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > index 7a5b41b1c024..32000902a204 100644
> > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > @@ -131,8 +131,12 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> >  
> >  static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
> >  {
> > -	platform_device_unregister(dev_priv->lpe_audio.platdev);
> > -	kfree(dev_priv->lpe_audio.platdev->dev.dma_mask);
> > +	struct platform_device *platdev = dev_priv->lpe_audio.platdev;
> > +
> > +	kfree(platdev->dev.dma_mask);
> > +	platdev->dev.dma_mask = NULL;
> > +
> > +	platform_device_unregister(platdev);

I'm not sure whether it's good idea to fiddle dma_mask bits before the
unregister call.  Interestingly, this is the only driver that calls
kfree() for pdev's dma_mask.  Either we do something wrong, or
everyone forgot this?


thanks,

Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-04-11 21:01   ` Takashi Iwai
@ 2017-04-11 21:20     ` Chris Wilson
  2017-04-11 21:27       ` Chris Wilson
  0 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-04-11 21:20 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Jani Nikula, intel-gfx

On Tue, Apr 11, 2017 at 11:01:57PM +0200, Takashi Iwai wrote:
> On Tue, 11 Apr 2017 22:41:12 +0200,
> Chris Wilson wrote:
> > 
> > On Wed, Mar 01, 2017 at 06:59:18PM +0000, Chris Wilson wrote:
> > > [31908.547136] BUG: KASAN: use-after-free in intel_lpe_audio_teardown+0x78/0xb0 [i915] at addr ffff8801f7788358
> > > [31908.547297] Read of size 8 by task drv_selftest/3781
> > > [31908.547405] CPU: 0 PID: 3781 Comm: drv_selftest Tainted: G    BU  W       4.10.0+ #451
> > > [31908.547553] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> > > [31908.547682] Call Trace:
> > > [31908.547772]  dump_stack+0x68/0x9f
> > > [31908.547857]  kasan_object_err+0x1c/0x70
> > > [31908.547947]  kasan_report_error+0x1f1/0x4f0
> > > [31908.548038]  ? kfree+0xaa/0x170
> > > [31908.548121]  kasan_report+0x34/0x40
> > > [31908.548211]  ? klist_children_get+0x20/0x30
> > > [31908.548472]  ? intel_lpe_audio_teardown+0x78/0xb0 [i915]
> > > [31908.548567]  __asan_load8+0x5e/0x70
> > > [31908.548824]  intel_lpe_audio_teardown+0x78/0xb0 [i915]
> > > [31908.549080]  intel_audio_deinit+0x28/0x80 [i915]
> > > [31908.549315]  i915_driver_unload+0xe4/0x360 [i915]
> > > [31908.549551]  ? i915_driver_load+0x1d70/0x1d70 [i915]
> > > [31908.549651]  ? trace_hardirqs_on+0xd/0x10
> > > [31908.549885]  i915_pci_remove+0x23/0x30 [i915]
> > > [31908.549978]  pci_device_remove+0x5c/0x100
> > > [31908.550069]  device_release_driver_internal+0x1db/0x2e0
> > > [31908.550165]  driver_detach+0x68/0xc0
> > > [31908.550256]  bus_remove_driver+0x8b/0x150
> > > [31908.550346]  driver_unregister+0x3e/0x60
> > > [31908.550439]  pci_unregister_driver+0x1d/0x110
> > > [31908.550531]  ? find_module_all+0x7a/0xa0
> > > [31908.550791]  i915_exit+0x1a/0x87 [i915]
> > > [31908.550881]  SyS_delete_module+0x264/0x2c0
> > > [31908.550971]  ? free_module+0x430/0x430
> > > [31908.551064]  ? trace_hardirqs_off_caller+0x16/0x110
> > > [31908.551159]  ? trace_hardirqs_on_caller+0x16/0x280
> > > [31908.551256]  ? trace_hardirqs_on_thunk+0x1a/0x1c
> > > [31908.551350]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > > [31908.551440] RIP: 0033:0x7f1d67312ec7
> > > [31908.551520] RSP: 002b:00007ffebe34e888 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
> > > [31908.551650] RAX: ffffffffffffffda RBX: ffffffff811123f6 RCX: 00007f1d67312ec7
> > > [31908.551743] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000560d0af476b8
> > > [31908.551837] RBP: ffff880233d87f98 R08: 0000000000000000 R09: 00007ffebe34e8b8
> > > [31908.551930] R10: 00007f1d68adf8c0 R11: 0000000000000206 R12: 0000000000000000
> > > [31908.552023] R13: 0000560d0af46440 R14: 0000000000000034 R15: 00007ffebe34d860
> > > [31908.552121]  ? trace_hardirqs_off_caller+0x16/0x110
> > > [31908.552217] Object at ffff8801f7788000, in cache kmalloc-2048 size: 2048
> > > [31908.552306] Allocated:
> > > [31908.552377] PID = 3781
> > > [31908.552456]  save_stack_trace+0x16/0x20
> > > [31908.552539]  kasan_kmalloc+0xee/0x190
> > > [31908.552627]  __kmalloc+0xdb/0x1b0
> > > [31908.552713]  platform_device_alloc+0x27/0x90
> > > [31908.552804]  platform_device_register_full+0x36/0x220
> > > [31908.553066]  intel_lpe_audio_init+0x41e/0x570 [i915]
> > > [31908.553320]  intel_audio_init+0xd/0x40 [i915]
> > > [31908.553552]  i915_driver_load+0x13f5/0x1d70 [i915]
> > > [31908.553788]  i915_pci_probe+0x65/0xe0 [i915]
> > > [31908.553881]  pci_device_probe+0xda/0x140
> > > [31908.553969]  driver_probe_device+0x400/0x660
> > > [31908.554058]  __driver_attach+0x11c/0x120
> > > [31908.554147]  bus_for_each_dev+0xe6/0x150
> > > [31908.554237]  driver_attach+0x26/0x30
> > > [31908.554325]  bus_add_driver+0x26b/0x3b0
> > > [31908.554412]  driver_register+0xce/0x190
> > > [31908.554502]  __pci_register_driver+0xaf/0xc0
> > > [31908.554589]  0xffffffffa0550063
> > > [31908.554675]  do_one_initcall+0x8b/0x1e0
> > > [31908.554764]  do_init_module+0x102/0x325
> > > [31908.554852]  load_module+0x3aad/0x45e0
> > > [31908.554944]  SyS_finit_module+0x169/0x1a0
> > > [31908.555033]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > > [31908.555119] Freed:
> > > [31908.555188] PID = 3781
> > > [31908.555266]  save_stack_trace+0x16/0x20
> > > [31908.555349]  kasan_slab_free+0xb0/0x180
> > > [31908.555436]  kfree+0xaa/0x170
> > > [31908.555520]  platform_device_release+0x76/0x80
> > > [31908.555610]  device_release+0x45/0xe0
> > > [31908.555698]  kobject_put+0x11f/0x260
> > > [31908.555785]  put_device+0x12/0x20
> > > [31908.555871]  platform_device_unregister+0x1b/0x20
> > > [31908.556135]  intel_lpe_audio_teardown+0x5c/0xb0 [i915]
> > > [31908.556390]  intel_audio_deinit+0x28/0x80 [i915]
> > > [31908.556622]  i915_driver_unload+0xe4/0x360 [i915]
> > > [31908.556858]  i915_pci_remove+0x23/0x30 [i915]
> > > [31908.556948]  pci_device_remove+0x5c/0x100
> > > [31908.557037]  device_release_driver_internal+0x1db/0x2e0
> > > [31908.557129]  driver_detach+0x68/0xc0
> > > [31908.557217]  bus_remove_driver+0x8b/0x150
> > > [31908.557304]  driver_unregister+0x3e/0x60
> > > [31908.557394]  pci_unregister_driver+0x1d/0x110
> > > [31908.557653]  i915_exit+0x1a/0x87 [i915]
> > > [31908.557741]  SyS_delete_module+0x264/0x2c0
> > > [31908.557834]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > > [31908.557919] Memory state around the buggy address:
> > > [31908.558005]  ffff8801f7788200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > > [31908.558127]  ffff8801f7788280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > > [31908.558255] >ffff8801f7788300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > > [31908.558374]                                                     ^
> > > [31908.558467]  ffff8801f7788380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > > [31908.558595]  ffff8801f7788400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > > 
> > > Fixes: eef57324d926 ("drm/i915: setup bridge for HDMI LPE audio driver")
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > Cc: Jerome Anand <jerome.anand@intel.com>
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Cc: Takashi Iwai <tiwai@suse.de>
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > Ping?
> 
> Oh, this fell into a crack as it was sent just before my vacation.
> 
> About the change:
> 
> > > ---
> > >  drivers/gpu/drm/i915/intel_lpe_audio.c | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > index 7a5b41b1c024..32000902a204 100644
> > > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > @@ -131,8 +131,12 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> > >  
> > >  static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
> > >  {
> > > -	platform_device_unregister(dev_priv->lpe_audio.platdev);
> > > -	kfree(dev_priv->lpe_audio.platdev->dev.dma_mask);
> > > +	struct platform_device *platdev = dev_priv->lpe_audio.platdev;
> > > +
> > > +	kfree(platdev->dev.dma_mask);
> > > +	platdev->dev.dma_mask = NULL;
> > > +
> > > +	platform_device_unregister(platdev);
> 
> I'm not sure whether it's good idea to fiddle dma_mask bits before the
> unregister call.  Interestingly, this is the only driver that calls
> kfree() for pdev's dma_mask.  Either we do something wrong, or
> everyone forgot this?

Afaict, everyone else chose the blissful ignorance strategy and we are
the only fools to try and free the dma_mask.

Would you feel more comfortable with:

	void *mask = platdev->dev.dma_mask;

	platform_device_unregister(platdev);

	/* XXX see platform_device_register_full():
	 * "This memory isn't freed when the device is put."
	 * It's not clear why it hasn't been fixed in a decade...
	 */
	kfree(mask);

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-04-11 21:20     ` Chris Wilson
@ 2017-04-11 21:27       ` Chris Wilson
  2017-04-12  4:59         ` Takashi Iwai
  0 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-04-11 21:27 UTC (permalink / raw)
  To: Takashi Iwai, intel-gfx, Pierre-Louis Bossart, Jerome Anand, Jani Nikula

On Tue, Apr 11, 2017 at 10:20:56PM +0100, Chris Wilson wrote:
> On Tue, Apr 11, 2017 at 11:01:57PM +0200, Takashi Iwai wrote:
> > On Tue, 11 Apr 2017 22:41:12 +0200,
> > Chris Wilson wrote:
> > Oh, this fell into a crack as it was sent just before my vacation.
> > 
> > About the change:
> > 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_lpe_audio.c | 8 ++++++--
> > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > > index 7a5b41b1c024..32000902a204 100644
> > > > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > > @@ -131,8 +131,12 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> > > >  
> > > >  static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
> > > >  {
> > > > -	platform_device_unregister(dev_priv->lpe_audio.platdev);
> > > > -	kfree(dev_priv->lpe_audio.platdev->dev.dma_mask);
> > > > +	struct platform_device *platdev = dev_priv->lpe_audio.platdev;
> > > > +
> > > > +	kfree(platdev->dev.dma_mask);
> > > > +	platdev->dev.dma_mask = NULL;
> > > > +
> > > > +	platform_device_unregister(platdev);
> > 
> > I'm not sure whether it's good idea to fiddle dma_mask bits before the
> > unregister call.  Interestingly, this is the only driver that calls
> > kfree() for pdev's dma_mask.  Either we do something wrong, or
> > everyone forgot this?
> 
> Afaict, everyone else chose the blissful ignorance strategy and we are
> the only fools to try and free the dma_mask.
> 
> Would you feel more comfortable with:
> 
> 	void *mask = platdev->dev.dma_mask;
> 
> 	platform_device_unregister(platdev);
> 
> 	/* XXX see platform_device_register_full():
> 	 * "This memory isn't freed when the device is put."
> 	 * It's not clear why it hasn't been fixed in a decade...
> 	 */
> 	kfree(mask);

Still has the issue that unregister may not the final put, it should but
still...

I think we just leak the memory. We are not the owner and so shouldn't
be fiddling around trying to free it.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-04-11 21:27       ` Chris Wilson
@ 2017-04-12  4:59         ` Takashi Iwai
  2017-04-12  7:46           ` Ville Syrjälä
  0 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2017-04-12  4:59 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Takashi Iwai, Jani Nikula, intel-gfx

On Tue, 11 Apr 2017 23:27:07 +0200,
Chris Wilson wrote:
> 
> On Tue, Apr 11, 2017 at 10:20:56PM +0100, Chris Wilson wrote:
> > On Tue, Apr 11, 2017 at 11:01:57PM +0200, Takashi Iwai wrote:
> > > On Tue, 11 Apr 2017 22:41:12 +0200,
> > > Chris Wilson wrote:
> > > Oh, this fell into a crack as it was sent just before my vacation.
> > > 
> > > About the change:
> > > 
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_lpe_audio.c | 8 ++++++--
> > > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > > > index 7a5b41b1c024..32000902a204 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > > > @@ -131,8 +131,12 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> > > > >  
> > > > >  static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
> > > > >  {
> > > > > -	platform_device_unregister(dev_priv->lpe_audio.platdev);
> > > > > -	kfree(dev_priv->lpe_audio.platdev->dev.dma_mask);
> > > > > +	struct platform_device *platdev = dev_priv->lpe_audio.platdev;
> > > > > +
> > > > > +	kfree(platdev->dev.dma_mask);
> > > > > +	platdev->dev.dma_mask = NULL;
> > > > > +
> > > > > +	platform_device_unregister(platdev);
> > > 
> > > I'm not sure whether it's good idea to fiddle dma_mask bits before the
> > > unregister call.  Interestingly, this is the only driver that calls
> > > kfree() for pdev's dma_mask.  Either we do something wrong, or
> > > everyone forgot this?
> > 
> > Afaict, everyone else chose the blissful ignorance strategy and we are
> > the only fools to try and free the dma_mask.
> > 
> > Would you feel more comfortable with:
> > 
> > 	void *mask = platdev->dev.dma_mask;
> > 
> > 	platform_device_unregister(platdev);
> > 
> > 	/* XXX see platform_device_register_full():
> > 	 * "This memory isn't freed when the device is put."
> > 	 * It's not clear why it hasn't been fixed in a decade...
> > 	 */
> > 	kfree(mask);
> 
> Still has the issue that unregister may not the final put, it should but
> still...
> 
> I think we just leak the memory. We are not the owner and so shouldn't
> be fiddling around trying to free it.

Agreed, IMO, we should handle better in the platform device side.


thanks,

Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-04-12  4:59         ` Takashi Iwai
@ 2017-04-12  7:46           ` Ville Syrjälä
  2017-04-12  8:25             ` Chris Wilson
  0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2017-04-12  7:46 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Jani Nikula, intel-gfx

On Wed, Apr 12, 2017 at 06:59:55AM +0200, Takashi Iwai wrote:
> On Tue, 11 Apr 2017 23:27:07 +0200,
> Chris Wilson wrote:
> > 
> > On Tue, Apr 11, 2017 at 10:20:56PM +0100, Chris Wilson wrote:
> > > On Tue, Apr 11, 2017 at 11:01:57PM +0200, Takashi Iwai wrote:
> > > > On Tue, 11 Apr 2017 22:41:12 +0200,
> > > > Chris Wilson wrote:
> > > > Oh, this fell into a crack as it was sent just before my vacation.
> > > > 
> > > > About the change:
> > > > 
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/intel_lpe_audio.c | 8 ++++++--
> > > > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > > > > index 7a5b41b1c024..32000902a204 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > > > > @@ -131,8 +131,12 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> > > > > >  
> > > > > >  static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
> > > > > >  {
> > > > > > -	platform_device_unregister(dev_priv->lpe_audio.platdev);
> > > > > > -	kfree(dev_priv->lpe_audio.platdev->dev.dma_mask);
> > > > > > +	struct platform_device *platdev = dev_priv->lpe_audio.platdev;
> > > > > > +
> > > > > > +	kfree(platdev->dev.dma_mask);
> > > > > > +	platdev->dev.dma_mask = NULL;
> > > > > > +
> > > > > > +	platform_device_unregister(platdev);
> > > > 
> > > > I'm not sure whether it's good idea to fiddle dma_mask bits before the
> > > > unregister call.  Interestingly, this is the only driver that calls
> > > > kfree() for pdev's dma_mask.  Either we do something wrong, or
> > > > everyone forgot this?
> > > 
> > > Afaict, everyone else chose the blissful ignorance strategy and we are
> > > the only fools to try and free the dma_mask.
> > > 
> > > Would you feel more comfortable with:
> > > 
> > > 	void *mask = platdev->dev.dma_mask;
> > > 
> > > 	platform_device_unregister(platdev);
> > > 
> > > 	/* XXX see platform_device_register_full():
> > > 	 * "This memory isn't freed when the device is put."
> > > 	 * It's not clear why it hasn't been fixed in a decade...
> > > 	 */
> > > 	kfree(mask);
> > 
> > Still has the issue that unregister may not the final put, it should but
> > still...
> > 
> > I think we just leak the memory. We are not the owner and so shouldn't
> > be fiddling around trying to free it.
> 
> Agreed, IMO, we should handle better in the platform device side.

Maybe just use dma_coerce_mask_and_coherent() and avoid the stupid
kmalloc()?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* [PATCH v2] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-03-01 18:59 [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy() Chris Wilson
  2017-03-01 20:17 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-04-11 20:41 ` [PATCH] " Chris Wilson
@ 2017-04-12  8:02 ` Chris Wilson
  2017-04-12 11:42   ` Takashi Iwai
  2017-04-12  8:22 ` ✓ Fi.CI.BAT: success for drm/i915: Fix use after free in lpe_audio_platdev_destroy() (rev2) Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-04-12  8:02 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Takashi Iwai

[31908.547136] BUG: KASAN: use-after-free in intel_lpe_audio_teardown+0x78/0xb0 [i915] at addr ffff8801f7788358
[31908.547297] Read of size 8 by task drv_selftest/3781
[31908.547405] CPU: 0 PID: 3781 Comm: drv_selftest Tainted: G    BU  W       4.10.0+ #451
[31908.547553] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[31908.547682] Call Trace:
[31908.547772]  dump_stack+0x68/0x9f
[31908.547857]  kasan_object_err+0x1c/0x70
[31908.547947]  kasan_report_error+0x1f1/0x4f0
[31908.548038]  ? kfree+0xaa/0x170
[31908.548121]  kasan_report+0x34/0x40
[31908.548211]  ? klist_children_get+0x20/0x30
[31908.548472]  ? intel_lpe_audio_teardown+0x78/0xb0 [i915]
[31908.548567]  __asan_load8+0x5e/0x70
[31908.548824]  intel_lpe_audio_teardown+0x78/0xb0 [i915]
[31908.549080]  intel_audio_deinit+0x28/0x80 [i915]
[31908.549315]  i915_driver_unload+0xe4/0x360 [i915]
[31908.549551]  ? i915_driver_load+0x1d70/0x1d70 [i915]
[31908.549651]  ? trace_hardirqs_on+0xd/0x10
[31908.549885]  i915_pci_remove+0x23/0x30 [i915]
[31908.549978]  pci_device_remove+0x5c/0x100
[31908.550069]  device_release_driver_internal+0x1db/0x2e0
[31908.550165]  driver_detach+0x68/0xc0
[31908.550256]  bus_remove_driver+0x8b/0x150
[31908.550346]  driver_unregister+0x3e/0x60
[31908.550439]  pci_unregister_driver+0x1d/0x110
[31908.550531]  ? find_module_all+0x7a/0xa0
[31908.550791]  i915_exit+0x1a/0x87 [i915]
[31908.550881]  SyS_delete_module+0x264/0x2c0
[31908.550971]  ? free_module+0x430/0x430
[31908.551064]  ? trace_hardirqs_off_caller+0x16/0x110
[31908.551159]  ? trace_hardirqs_on_caller+0x16/0x280
[31908.551256]  ? trace_hardirqs_on_thunk+0x1a/0x1c
[31908.551350]  entry_SYSCALL_64_fastpath+0x1c/0xb1
[31908.551440] RIP: 0033:0x7f1d67312ec7
[31908.551520] RSP: 002b:00007ffebe34e888 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
[31908.551650] RAX: ffffffffffffffda RBX: ffffffff811123f6 RCX: 00007f1d67312ec7
[31908.551743] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000560d0af476b8
[31908.551837] RBP: ffff880233d87f98 R08: 0000000000000000 R09: 00007ffebe34e8b8
[31908.551930] R10: 00007f1d68adf8c0 R11: 0000000000000206 R12: 0000000000000000
[31908.552023] R13: 0000560d0af46440 R14: 0000000000000034 R15: 00007ffebe34d860
[31908.552121]  ? trace_hardirqs_off_caller+0x16/0x110
[31908.552217] Object at ffff8801f7788000, in cache kmalloc-2048 size: 2048
[31908.552306] Allocated:
[31908.552377] PID = 3781
[31908.552456]  save_stack_trace+0x16/0x20
[31908.552539]  kasan_kmalloc+0xee/0x190
[31908.552627]  __kmalloc+0xdb/0x1b0
[31908.552713]  platform_device_alloc+0x27/0x90
[31908.552804]  platform_device_register_full+0x36/0x220
[31908.553066]  intel_lpe_audio_init+0x41e/0x570 [i915]
[31908.553320]  intel_audio_init+0xd/0x40 [i915]
[31908.553552]  i915_driver_load+0x13f5/0x1d70 [i915]
[31908.553788]  i915_pci_probe+0x65/0xe0 [i915]
[31908.553881]  pci_device_probe+0xda/0x140
[31908.553969]  driver_probe_device+0x400/0x660
[31908.554058]  __driver_attach+0x11c/0x120
[31908.554147]  bus_for_each_dev+0xe6/0x150
[31908.554237]  driver_attach+0x26/0x30
[31908.554325]  bus_add_driver+0x26b/0x3b0
[31908.554412]  driver_register+0xce/0x190
[31908.554502]  __pci_register_driver+0xaf/0xc0
[31908.554589]  0xffffffffa0550063
[31908.554675]  do_one_initcall+0x8b/0x1e0
[31908.554764]  do_init_module+0x102/0x325
[31908.554852]  load_module+0x3aad/0x45e0
[31908.554944]  SyS_finit_module+0x169/0x1a0
[31908.555033]  entry_SYSCALL_64_fastpath+0x1c/0xb1
[31908.555119] Freed:
[31908.555188] PID = 3781
[31908.555266]  save_stack_trace+0x16/0x20
[31908.555349]  kasan_slab_free+0xb0/0x180
[31908.555436]  kfree+0xaa/0x170
[31908.555520]  platform_device_release+0x76/0x80
[31908.555610]  device_release+0x45/0xe0
[31908.555698]  kobject_put+0x11f/0x260
[31908.555785]  put_device+0x12/0x20
[31908.555871]  platform_device_unregister+0x1b/0x20
[31908.556135]  intel_lpe_audio_teardown+0x5c/0xb0 [i915]
[31908.556390]  intel_audio_deinit+0x28/0x80 [i915]
[31908.556622]  i915_driver_unload+0xe4/0x360 [i915]
[31908.556858]  i915_pci_remove+0x23/0x30 [i915]
[31908.556948]  pci_device_remove+0x5c/0x100
[31908.557037]  device_release_driver_internal+0x1db/0x2e0
[31908.557129]  driver_detach+0x68/0xc0
[31908.557217]  bus_remove_driver+0x8b/0x150
[31908.557304]  driver_unregister+0x3e/0x60
[31908.557394]  pci_unregister_driver+0x1d/0x110
[31908.557653]  i915_exit+0x1a/0x87 [i915]
[31908.557741]  SyS_delete_module+0x264/0x2c0
[31908.557834]  entry_SYSCALL_64_fastpath+0x1c/0xb1
[31908.557919] Memory state around the buggy address:
[31908.558005]  ffff8801f7788200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[31908.558127]  ffff8801f7788280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[31908.558255] >ffff8801f7788300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[31908.558374]                                                     ^
[31908.558467]  ffff8801f7788380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[31908.558595]  ffff8801f7788400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb

v2: Just leak the memory (8 bytes) as freeing it ourselves is not safe,
and we need to coordinate a proper fix in platform_device itself.

Fixes: eef57324d926 ("drm/i915: setup bridge for HDMI LPE audio driver")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Jerome Anand <jerome.anand@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lpe_audio.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index d8ca187ae001..d19053353a2b 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -131,8 +131,15 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
 
 static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
 {
+	/* XXX Note that platform_device_register_full() allocates a dma_mask
+	 * and never frees it. We can't free it here as we cannot guarrantee
+	 * this is the last reference (i.e. that the dma_mask will not be
+	 * used after our unregister). We choose to leak the sizeof(u64)
+	 * allocation - it should be fixed in the platform_device rather
+	 * than us fiddle with its internals.
+	 */
+
 	platform_device_unregister(dev_priv->lpe_audio.platdev);
-	kfree(dev_priv->lpe_audio.platdev->dev.dma_mask);
 }
 
 static void lpe_audio_irq_unmask(struct irq_data *d)
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Fix use after free in lpe_audio_platdev_destroy() (rev2)
  2017-03-01 18:59 [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy() Chris Wilson
                   ` (2 preceding siblings ...)
  2017-04-12  8:02 ` [PATCH v2] " Chris Wilson
@ 2017-04-12  8:22 ` Patchwork
  2017-04-12  8:31 ` [PATCH v3] drm/i915: Fix use after free in lpe_audio_platdev_destroy() Chris Wilson
  2017-04-12  8:51 ` ✓ Fi.CI.BAT: success for drm/i915: Fix use after free in lpe_audio_platdev_destroy() (rev4) Patchwork
  5 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-04-12  8:22 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Fix use after free in lpe_audio_platdev_destroy() (rev2)
URL   : https://patchwork.freedesktop.org/series/20476/
State : success

== Summary ==

Series 20476v2 drm/i915: Fix use after free in lpe_audio_platdev_destroy()
https://patchwork.freedesktop.org/api/1.0/series/20476/revisions/2/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100125
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-WARN (fi-bsw-n3050) fdo#100651

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100651 https://bugs.freedesktop.org/show_bug.cgi?id=100651

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:432s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:424s
fi-bsw-n3050     total:278  pass:241  dwarn:1   dfail:0   fail:0   skip:36  time:577s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:512s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:483s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:482s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:414s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:403s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:417s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:493s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:472s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:454s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:559s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:449s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:569s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:455s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:498s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:429s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:534s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:409s

f27f076a9b505b7d16bc743af16c3b7b142fcdc3 drm-tip: 2017y-04m-11d-19h-50m-43s UTC integration manifest
f713693 drm/i915: Fix use after free in lpe_audio_platdev_destroy()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4483/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-04-12  7:46           ` Ville Syrjälä
@ 2017-04-12  8:25             ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-04-12  8:25 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Takashi Iwai, Jani Nikula, intel-gfx

On Wed, Apr 12, 2017 at 10:46:30AM +0300, Ville Syrjälä wrote:
> On Wed, Apr 12, 2017 at 06:59:55AM +0200, Takashi Iwai wrote:
> > On Tue, 11 Apr 2017 23:27:07 +0200,
> > Chris Wilson wrote:
> > > 
> > > On Tue, Apr 11, 2017 at 10:20:56PM +0100, Chris Wilson wrote:
> > > > On Tue, Apr 11, 2017 at 11:01:57PM +0200, Takashi Iwai wrote:
> > > > > On Tue, 11 Apr 2017 22:41:12 +0200,
> > > > > Chris Wilson wrote:
> > > > > Oh, this fell into a crack as it was sent just before my vacation.
> > > > > 
> > > > > About the change:
> > > > > 
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/i915/intel_lpe_audio.c | 8 ++++++--
> > > > > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > > > > > index 7a5b41b1c024..32000902a204 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > > > > > @@ -131,8 +131,12 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> > > > > > >  
> > > > > > >  static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
> > > > > > >  {
> > > > > > > -	platform_device_unregister(dev_priv->lpe_audio.platdev);
> > > > > > > -	kfree(dev_priv->lpe_audio.platdev->dev.dma_mask);
> > > > > > > +	struct platform_device *platdev = dev_priv->lpe_audio.platdev;
> > > > > > > +
> > > > > > > +	kfree(platdev->dev.dma_mask);
> > > > > > > +	platdev->dev.dma_mask = NULL;
> > > > > > > +
> > > > > > > +	platform_device_unregister(platdev);
> > > > > 
> > > > > I'm not sure whether it's good idea to fiddle dma_mask bits before the
> > > > > unregister call.  Interestingly, this is the only driver that calls
> > > > > kfree() for pdev's dma_mask.  Either we do something wrong, or
> > > > > everyone forgot this?
> > > > 
> > > > Afaict, everyone else chose the blissful ignorance strategy and we are
> > > > the only fools to try and free the dma_mask.
> > > > 
> > > > Would you feel more comfortable with:
> > > > 
> > > > 	void *mask = platdev->dev.dma_mask;
> > > > 
> > > > 	platform_device_unregister(platdev);
> > > > 
> > > > 	/* XXX see platform_device_register_full():
> > > > 	 * "This memory isn't freed when the device is put."
> > > > 	 * It's not clear why it hasn't been fixed in a decade...
> > > > 	 */
> > > > 	kfree(mask);
> > > 
> > > Still has the issue that unregister may not the final put, it should but
> > > still...
> > > 
> > > I think we just leak the memory. We are not the owner and so shouldn't
> > > be fiddling around trying to free it.
> > 
> > Agreed, IMO, we should handle better in the platform device side.
> 
> Maybe just use dma_coerce_mask_and_coherent() and avoid the stupid
> kmalloc()?

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index d19053353a2b..a4c20490c48b 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -108,7 +108,6 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
        pinfo.num_res = 2;
        pinfo.data = pdata;
        pinfo.size_data = sizeof(*pdata);
-       pinfo.dma_mask = DMA_BIT_MASK(32);
 
        spin_lock_init(&pdata->lpe_audio_slock);
 
@@ -119,6 +118,8 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
                goto err;
        }
 
+       dma_coerce_mask_and_coherent(&platdev->dev, DMA_BIT_MASK(32));
+
        kfree(rsc);
 
        return platdev;

I think.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v3] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-03-01 18:59 [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy() Chris Wilson
                   ` (3 preceding siblings ...)
  2017-04-12  8:22 ` ✓ Fi.CI.BAT: success for drm/i915: Fix use after free in lpe_audio_platdev_destroy() (rev2) Patchwork
@ 2017-04-12  8:31 ` Chris Wilson
  2017-04-12  8:52   ` Ville Syrjälä
  2017-04-12  8:51 ` ✓ Fi.CI.BAT: success for drm/i915: Fix use after free in lpe_audio_platdev_destroy() (rev4) Patchwork
  5 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2017-04-12  8:31 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Takashi Iwai

[31908.547136] BUG: KASAN: use-after-free in intel_lpe_audio_teardown+0x78/0xb0 [i915] at addr ffff8801f7788358
[31908.547297] Read of size 8 by task drv_selftest/3781
[31908.547405] CPU: 0 PID: 3781 Comm: drv_selftest Tainted: G    BU  W       4.10.0+ #451
[31908.547553] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[31908.547682] Call Trace:
[31908.547772]  dump_stack+0x68/0x9f
[31908.547857]  kasan_object_err+0x1c/0x70
[31908.547947]  kasan_report_error+0x1f1/0x4f0
[31908.548038]  ? kfree+0xaa/0x170
[31908.548121]  kasan_report+0x34/0x40
[31908.548211]  ? klist_children_get+0x20/0x30
[31908.548472]  ? intel_lpe_audio_teardown+0x78/0xb0 [i915]
[31908.548567]  __asan_load8+0x5e/0x70
[31908.548824]  intel_lpe_audio_teardown+0x78/0xb0 [i915]
[31908.549080]  intel_audio_deinit+0x28/0x80 [i915]
[31908.549315]  i915_driver_unload+0xe4/0x360 [i915]
[31908.549551]  ? i915_driver_load+0x1d70/0x1d70 [i915]
[31908.549651]  ? trace_hardirqs_on+0xd/0x10
[31908.549885]  i915_pci_remove+0x23/0x30 [i915]
[31908.549978]  pci_device_remove+0x5c/0x100
[31908.550069]  device_release_driver_internal+0x1db/0x2e0
[31908.550165]  driver_detach+0x68/0xc0
[31908.550256]  bus_remove_driver+0x8b/0x150
[31908.550346]  driver_unregister+0x3e/0x60
[31908.550439]  pci_unregister_driver+0x1d/0x110
[31908.550531]  ? find_module_all+0x7a/0xa0
[31908.550791]  i915_exit+0x1a/0x87 [i915]
[31908.550881]  SyS_delete_module+0x264/0x2c0
[31908.550971]  ? free_module+0x430/0x430
[31908.551064]  ? trace_hardirqs_off_caller+0x16/0x110
[31908.551159]  ? trace_hardirqs_on_caller+0x16/0x280
[31908.551256]  ? trace_hardirqs_on_thunk+0x1a/0x1c
[31908.551350]  entry_SYSCALL_64_fastpath+0x1c/0xb1
[31908.551440] RIP: 0033:0x7f1d67312ec7
[31908.551520] RSP: 002b:00007ffebe34e888 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
[31908.551650] RAX: ffffffffffffffda RBX: ffffffff811123f6 RCX: 00007f1d67312ec7
[31908.551743] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000560d0af476b8
[31908.551837] RBP: ffff880233d87f98 R08: 0000000000000000 R09: 00007ffebe34e8b8
[31908.551930] R10: 00007f1d68adf8c0 R11: 0000000000000206 R12: 0000000000000000
[31908.552023] R13: 0000560d0af46440 R14: 0000000000000034 R15: 00007ffebe34d860
[31908.552121]  ? trace_hardirqs_off_caller+0x16/0x110
[31908.552217] Object at ffff8801f7788000, in cache kmalloc-2048 size: 2048
[31908.552306] Allocated:
[31908.552377] PID = 3781
[31908.552456]  save_stack_trace+0x16/0x20
[31908.552539]  kasan_kmalloc+0xee/0x190
[31908.552627]  __kmalloc+0xdb/0x1b0
[31908.552713]  platform_device_alloc+0x27/0x90
[31908.552804]  platform_device_register_full+0x36/0x220
[31908.553066]  intel_lpe_audio_init+0x41e/0x570 [i915]
[31908.553320]  intel_audio_init+0xd/0x40 [i915]
[31908.553552]  i915_driver_load+0x13f5/0x1d70 [i915]
[31908.553788]  i915_pci_probe+0x65/0xe0 [i915]
[31908.553881]  pci_device_probe+0xda/0x140
[31908.553969]  driver_probe_device+0x400/0x660
[31908.554058]  __driver_attach+0x11c/0x120
[31908.554147]  bus_for_each_dev+0xe6/0x150
[31908.554237]  driver_attach+0x26/0x30
[31908.554325]  bus_add_driver+0x26b/0x3b0
[31908.554412]  driver_register+0xce/0x190
[31908.554502]  __pci_register_driver+0xaf/0xc0
[31908.554589]  0xffffffffa0550063
[31908.554675]  do_one_initcall+0x8b/0x1e0
[31908.554764]  do_init_module+0x102/0x325
[31908.554852]  load_module+0x3aad/0x45e0
[31908.554944]  SyS_finit_module+0x169/0x1a0
[31908.555033]  entry_SYSCALL_64_fastpath+0x1c/0xb1
[31908.555119] Freed:
[31908.555188] PID = 3781
[31908.555266]  save_stack_trace+0x16/0x20
[31908.555349]  kasan_slab_free+0xb0/0x180
[31908.555436]  kfree+0xaa/0x170
[31908.555520]  platform_device_release+0x76/0x80
[31908.555610]  device_release+0x45/0xe0
[31908.555698]  kobject_put+0x11f/0x260
[31908.555785]  put_device+0x12/0x20
[31908.555871]  platform_device_unregister+0x1b/0x20
[31908.556135]  intel_lpe_audio_teardown+0x5c/0xb0 [i915]
[31908.556390]  intel_audio_deinit+0x28/0x80 [i915]
[31908.556622]  i915_driver_unload+0xe4/0x360 [i915]
[31908.556858]  i915_pci_remove+0x23/0x30 [i915]
[31908.556948]  pci_device_remove+0x5c/0x100
[31908.557037]  device_release_driver_internal+0x1db/0x2e0
[31908.557129]  driver_detach+0x68/0xc0
[31908.557217]  bus_remove_driver+0x8b/0x150
[31908.557304]  driver_unregister+0x3e/0x60
[31908.557394]  pci_unregister_driver+0x1d/0x110
[31908.557653]  i915_exit+0x1a/0x87 [i915]
[31908.557741]  SyS_delete_module+0x264/0x2c0
[31908.557834]  entry_SYSCALL_64_fastpath+0x1c/0xb1
[31908.557919] Memory state around the buggy address:
[31908.558005]  ffff8801f7788200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[31908.558127]  ffff8801f7788280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[31908.558255] >ffff8801f7788300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[31908.558374]                                                     ^
[31908.558467]  ffff8801f7788380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[31908.558595]  ffff8801f7788400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb

v2: Just leak the memory (8 bytes) as freeing it ourselves is not safe,
and we need to coordinate a proper fix in platform_device itself.
v3: Use dma_coerce_mask_and_coherent() to skip the kmalloc

Fixes: eef57324d926 ("drm/i915: setup bridge for HDMI LPE audio driver")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Jerome Anand <jerome.anand@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lpe_audio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index d8ca187ae001..dace2fb3154f 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -108,7 +108,6 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
 	pinfo.num_res = 2;
 	pinfo.data = pdata;
 	pinfo.size_data = sizeof(*pdata);
-	pinfo.dma_mask = DMA_BIT_MASK(32);
 
 	spin_lock_init(&pdata->lpe_audio_slock);
 
@@ -119,6 +118,8 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
 		goto err;
 	}
 
+	dma_coerce_mask_and_coherent(&platdev->dev, DMA_BIT_MASK(32));
+
 	kfree(rsc);
 
 	return platdev;
@@ -132,7 +133,6 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
 static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
 {
 	platform_device_unregister(dev_priv->lpe_audio.platdev);
-	kfree(dev_priv->lpe_audio.platdev->dev.dma_mask);
 }
 
 static void lpe_audio_irq_unmask(struct irq_data *d)
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Fix use after free in lpe_audio_platdev_destroy() (rev4)
  2017-03-01 18:59 [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy() Chris Wilson
                   ` (4 preceding siblings ...)
  2017-04-12  8:31 ` [PATCH v3] drm/i915: Fix use after free in lpe_audio_platdev_destroy() Chris Wilson
@ 2017-04-12  8:51 ` Patchwork
  5 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-04-12  8:51 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Fix use after free in lpe_audio_platdev_destroy() (rev4)
URL   : https://patchwork.freedesktop.org/series/20476/
State : success

== Summary ==

Series 20476v4 drm/i915: Fix use after free in lpe_audio_platdev_destroy()
https://patchwork.freedesktop.org/api/1.0/series/20476/revisions/4/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100125

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:433s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:429s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:572s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:504s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:484s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:479s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:412s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:412s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:414s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:496s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:471s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:457s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:569s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:451s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:567s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:459s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:487s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:429s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:533s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:401s

f27f076a9b505b7d16bc743af16c3b7b142fcdc3 drm-tip: 2017y-04m-11d-19h-50m-43s UTC integration manifest
0fbc753 drm/i915: Fix use after free in lpe_audio_platdev_destroy()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4484/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v3] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-04-12  8:31 ` [PATCH v3] drm/i915: Fix use after free in lpe_audio_platdev_destroy() Chris Wilson
@ 2017-04-12  8:52   ` Ville Syrjälä
  2017-04-12  9:03     ` Chris Wilson
  2017-04-12 11:41     ` Takashi Iwai
  0 siblings, 2 replies; 20+ messages in thread
From: Ville Syrjälä @ 2017-04-12  8:52 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Jani Nikula, Takashi Iwai, intel-gfx

On Wed, Apr 12, 2017 at 09:31:39AM +0100, Chris Wilson wrote:
> [31908.547136] BUG: KASAN: use-after-free in intel_lpe_audio_teardown+0x78/0xb0 [i915] at addr ffff8801f7788358
> [31908.547297] Read of size 8 by task drv_selftest/3781
> [31908.547405] CPU: 0 PID: 3781 Comm: drv_selftest Tainted: G    BU  W       4.10.0+ #451
> [31908.547553] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> [31908.547682] Call Trace:
> [31908.547772]  dump_stack+0x68/0x9f
> [31908.547857]  kasan_object_err+0x1c/0x70
> [31908.547947]  kasan_report_error+0x1f1/0x4f0
> [31908.548038]  ? kfree+0xaa/0x170
> [31908.548121]  kasan_report+0x34/0x40
> [31908.548211]  ? klist_children_get+0x20/0x30
> [31908.548472]  ? intel_lpe_audio_teardown+0x78/0xb0 [i915]
> [31908.548567]  __asan_load8+0x5e/0x70
> [31908.548824]  intel_lpe_audio_teardown+0x78/0xb0 [i915]
> [31908.549080]  intel_audio_deinit+0x28/0x80 [i915]
> [31908.549315]  i915_driver_unload+0xe4/0x360 [i915]
> [31908.549551]  ? i915_driver_load+0x1d70/0x1d70 [i915]
> [31908.549651]  ? trace_hardirqs_on+0xd/0x10
> [31908.549885]  i915_pci_remove+0x23/0x30 [i915]
> [31908.549978]  pci_device_remove+0x5c/0x100
> [31908.550069]  device_release_driver_internal+0x1db/0x2e0
> [31908.550165]  driver_detach+0x68/0xc0
> [31908.550256]  bus_remove_driver+0x8b/0x150
> [31908.550346]  driver_unregister+0x3e/0x60
> [31908.550439]  pci_unregister_driver+0x1d/0x110
> [31908.550531]  ? find_module_all+0x7a/0xa0
> [31908.550791]  i915_exit+0x1a/0x87 [i915]
> [31908.550881]  SyS_delete_module+0x264/0x2c0
> [31908.550971]  ? free_module+0x430/0x430
> [31908.551064]  ? trace_hardirqs_off_caller+0x16/0x110
> [31908.551159]  ? trace_hardirqs_on_caller+0x16/0x280
> [31908.551256]  ? trace_hardirqs_on_thunk+0x1a/0x1c
> [31908.551350]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> [31908.551440] RIP: 0033:0x7f1d67312ec7
> [31908.551520] RSP: 002b:00007ffebe34e888 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
> [31908.551650] RAX: ffffffffffffffda RBX: ffffffff811123f6 RCX: 00007f1d67312ec7
> [31908.551743] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000560d0af476b8
> [31908.551837] RBP: ffff880233d87f98 R08: 0000000000000000 R09: 00007ffebe34e8b8
> [31908.551930] R10: 00007f1d68adf8c0 R11: 0000000000000206 R12: 0000000000000000
> [31908.552023] R13: 0000560d0af46440 R14: 0000000000000034 R15: 00007ffebe34d860
> [31908.552121]  ? trace_hardirqs_off_caller+0x16/0x110
> [31908.552217] Object at ffff8801f7788000, in cache kmalloc-2048 size: 2048
> [31908.552306] Allocated:
> [31908.552377] PID = 3781
> [31908.552456]  save_stack_trace+0x16/0x20
> [31908.552539]  kasan_kmalloc+0xee/0x190
> [31908.552627]  __kmalloc+0xdb/0x1b0
> [31908.552713]  platform_device_alloc+0x27/0x90
> [31908.552804]  platform_device_register_full+0x36/0x220
> [31908.553066]  intel_lpe_audio_init+0x41e/0x570 [i915]
> [31908.553320]  intel_audio_init+0xd/0x40 [i915]
> [31908.553552]  i915_driver_load+0x13f5/0x1d70 [i915]
> [31908.553788]  i915_pci_probe+0x65/0xe0 [i915]
> [31908.553881]  pci_device_probe+0xda/0x140
> [31908.553969]  driver_probe_device+0x400/0x660
> [31908.554058]  __driver_attach+0x11c/0x120
> [31908.554147]  bus_for_each_dev+0xe6/0x150
> [31908.554237]  driver_attach+0x26/0x30
> [31908.554325]  bus_add_driver+0x26b/0x3b0
> [31908.554412]  driver_register+0xce/0x190
> [31908.554502]  __pci_register_driver+0xaf/0xc0
> [31908.554589]  0xffffffffa0550063
> [31908.554675]  do_one_initcall+0x8b/0x1e0
> [31908.554764]  do_init_module+0x102/0x325
> [31908.554852]  load_module+0x3aad/0x45e0
> [31908.554944]  SyS_finit_module+0x169/0x1a0
> [31908.555033]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> [31908.555119] Freed:
> [31908.555188] PID = 3781
> [31908.555266]  save_stack_trace+0x16/0x20
> [31908.555349]  kasan_slab_free+0xb0/0x180
> [31908.555436]  kfree+0xaa/0x170
> [31908.555520]  platform_device_release+0x76/0x80
> [31908.555610]  device_release+0x45/0xe0
> [31908.555698]  kobject_put+0x11f/0x260
> [31908.555785]  put_device+0x12/0x20
> [31908.555871]  platform_device_unregister+0x1b/0x20
> [31908.556135]  intel_lpe_audio_teardown+0x5c/0xb0 [i915]
> [31908.556390]  intel_audio_deinit+0x28/0x80 [i915]
> [31908.556622]  i915_driver_unload+0xe4/0x360 [i915]
> [31908.556858]  i915_pci_remove+0x23/0x30 [i915]
> [31908.556948]  pci_device_remove+0x5c/0x100
> [31908.557037]  device_release_driver_internal+0x1db/0x2e0
> [31908.557129]  driver_detach+0x68/0xc0
> [31908.557217]  bus_remove_driver+0x8b/0x150
> [31908.557304]  driver_unregister+0x3e/0x60
> [31908.557394]  pci_unregister_driver+0x1d/0x110
> [31908.557653]  i915_exit+0x1a/0x87 [i915]
> [31908.557741]  SyS_delete_module+0x264/0x2c0
> [31908.557834]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> [31908.557919] Memory state around the buggy address:
> [31908.558005]  ffff8801f7788200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [31908.558127]  ffff8801f7788280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [31908.558255] >ffff8801f7788300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [31908.558374]                                                     ^
> [31908.558467]  ffff8801f7788380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [31908.558595]  ffff8801f7788400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> 
> v2: Just leak the memory (8 bytes) as freeing it ourselves is not safe,
> and we need to coordinate a proper fix in platform_device itself.
> v3: Use dma_coerce_mask_and_coherent() to skip the kmalloc
> 
> Fixes: eef57324d926 ("drm/i915: setup bridge for HDMI LPE audio driver")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Cc: Jerome Anand <jerome.anand@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_lpe_audio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> index d8ca187ae001..dace2fb3154f 100644
> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> @@ -108,7 +108,6 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
>  	pinfo.num_res = 2;
>  	pinfo.data = pdata;
>  	pinfo.size_data = sizeof(*pdata);
> -	pinfo.dma_mask = DMA_BIT_MASK(32);
>  
>  	spin_lock_init(&pdata->lpe_audio_slock);
>  
> @@ -119,6 +118,8 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
>  		goto err;
>  	}
>  
> +	dma_coerce_mask_and_coherent(&platdev->dev, DMA_BIT_MASK(32));
> +

Not sure how racy that is since we've already registered the platdev at
that point. The whole platform_register_full() API looks misdesigned to
me since you can't do stuff between alloc and register.

We could shovel the dma_coerce_mask_and_coherent() call into
platform_register_full() itself I suppose. Or we just stop using the
register_full() stuff and do each step ourselves, but that looks a bit
tedious.

>  	kfree(rsc);
>  
>  	return platdev;
> @@ -132,7 +133,6 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
>  static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
>  {
>  	platform_device_unregister(dev_priv->lpe_audio.platdev);
> -	kfree(dev_priv->lpe_audio.platdev->dev.dma_mask);
>  }
>  
>  static void lpe_audio_irq_unmask(struct irq_data *d)
> -- 
> 2.11.0

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v3] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-04-12  8:52   ` Ville Syrjälä
@ 2017-04-12  9:03     ` Chris Wilson
  2017-04-12 11:41     ` Takashi Iwai
  1 sibling, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-04-12  9:03 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Jani Nikula, Takashi Iwai, intel-gfx

On Wed, Apr 12, 2017 at 11:52:54AM +0300, Ville Syrjälä wrote:
> On Wed, Apr 12, 2017 at 09:31:39AM +0100, Chris Wilson wrote:
> >  drivers/gpu/drm/i915/intel_lpe_audio.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > index d8ca187ae001..dace2fb3154f 100644
> > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > @@ -108,7 +108,6 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> >  	pinfo.num_res = 2;
> >  	pinfo.data = pdata;
> >  	pinfo.size_data = sizeof(*pdata);
> > -	pinfo.dma_mask = DMA_BIT_MASK(32);
> >  
> >  	spin_lock_init(&pdata->lpe_audio_slock);
> >  
> > @@ -119,6 +118,8 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> >  		goto err;
> >  	}
> >  
> > +	dma_coerce_mask_and_coherent(&platdev->dev, DMA_BIT_MASK(32));
> > +
> 
> Not sure how racy that is since we've already registered the platdev at
> that point. The whole platform_register_full() API looks misdesigned to
> me since you can't do stuff between alloc and register.

I had the same sinking feeling looking over
platform_device_register_full().
 
> We could shovel the dma_coerce_mask_and_coherent() call into
> platform_register_full() itself I suppose. Or we just stop using the
> register_full() stuff and do each step ourselves, but that looks a bit
> tedious.

I'm quite happy to use v2 and ask CI to file all bug reports to GregKH.

Let's be democratic, the version with the most r-b wins.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v3] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-04-12  8:52   ` Ville Syrjälä
  2017-04-12  9:03     ` Chris Wilson
@ 2017-04-12 11:41     ` Takashi Iwai
  2017-04-12 19:17       ` Ville Syrjälä
  1 sibling, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2017-04-12 11:41 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Jani Nikula, intel-gfx, Takashi Iwai

On Wed, 12 Apr 2017 10:52:54 +0200,
Ville Syrjälä wrote:
> 
> On Wed, Apr 12, 2017 at 09:31:39AM +0100, Chris Wilson wrote:
> > [31908.547136] BUG: KASAN: use-after-free in intel_lpe_audio_teardown+0x78/0xb0 [i915] at addr ffff8801f7788358
> > [31908.547297] Read of size 8 by task drv_selftest/3781
> > [31908.547405] CPU: 0 PID: 3781 Comm: drv_selftest Tainted: G    BU  W       4.10.0+ #451
> > [31908.547553] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> > [31908.547682] Call Trace:
> > [31908.547772]  dump_stack+0x68/0x9f
> > [31908.547857]  kasan_object_err+0x1c/0x70
> > [31908.547947]  kasan_report_error+0x1f1/0x4f0
> > [31908.548038]  ? kfree+0xaa/0x170
> > [31908.548121]  kasan_report+0x34/0x40
> > [31908.548211]  ? klist_children_get+0x20/0x30
> > [31908.548472]  ? intel_lpe_audio_teardown+0x78/0xb0 [i915]
> > [31908.548567]  __asan_load8+0x5e/0x70
> > [31908.548824]  intel_lpe_audio_teardown+0x78/0xb0 [i915]
> > [31908.549080]  intel_audio_deinit+0x28/0x80 [i915]
> > [31908.549315]  i915_driver_unload+0xe4/0x360 [i915]
> > [31908.549551]  ? i915_driver_load+0x1d70/0x1d70 [i915]
> > [31908.549651]  ? trace_hardirqs_on+0xd/0x10
> > [31908.549885]  i915_pci_remove+0x23/0x30 [i915]
> > [31908.549978]  pci_device_remove+0x5c/0x100
> > [31908.550069]  device_release_driver_internal+0x1db/0x2e0
> > [31908.550165]  driver_detach+0x68/0xc0
> > [31908.550256]  bus_remove_driver+0x8b/0x150
> > [31908.550346]  driver_unregister+0x3e/0x60
> > [31908.550439]  pci_unregister_driver+0x1d/0x110
> > [31908.550531]  ? find_module_all+0x7a/0xa0
> > [31908.550791]  i915_exit+0x1a/0x87 [i915]
> > [31908.550881]  SyS_delete_module+0x264/0x2c0
> > [31908.550971]  ? free_module+0x430/0x430
> > [31908.551064]  ? trace_hardirqs_off_caller+0x16/0x110
> > [31908.551159]  ? trace_hardirqs_on_caller+0x16/0x280
> > [31908.551256]  ? trace_hardirqs_on_thunk+0x1a/0x1c
> > [31908.551350]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > [31908.551440] RIP: 0033:0x7f1d67312ec7
> > [31908.551520] RSP: 002b:00007ffebe34e888 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
> > [31908.551650] RAX: ffffffffffffffda RBX: ffffffff811123f6 RCX: 00007f1d67312ec7
> > [31908.551743] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000560d0af476b8
> > [31908.551837] RBP: ffff880233d87f98 R08: 0000000000000000 R09: 00007ffebe34e8b8
> > [31908.551930] R10: 00007f1d68adf8c0 R11: 0000000000000206 R12: 0000000000000000
> > [31908.552023] R13: 0000560d0af46440 R14: 0000000000000034 R15: 00007ffebe34d860
> > [31908.552121]  ? trace_hardirqs_off_caller+0x16/0x110
> > [31908.552217] Object at ffff8801f7788000, in cache kmalloc-2048 size: 2048
> > [31908.552306] Allocated:
> > [31908.552377] PID = 3781
> > [31908.552456]  save_stack_trace+0x16/0x20
> > [31908.552539]  kasan_kmalloc+0xee/0x190
> > [31908.552627]  __kmalloc+0xdb/0x1b0
> > [31908.552713]  platform_device_alloc+0x27/0x90
> > [31908.552804]  platform_device_register_full+0x36/0x220
> > [31908.553066]  intel_lpe_audio_init+0x41e/0x570 [i915]
> > [31908.553320]  intel_audio_init+0xd/0x40 [i915]
> > [31908.553552]  i915_driver_load+0x13f5/0x1d70 [i915]
> > [31908.553788]  i915_pci_probe+0x65/0xe0 [i915]
> > [31908.553881]  pci_device_probe+0xda/0x140
> > [31908.553969]  driver_probe_device+0x400/0x660
> > [31908.554058]  __driver_attach+0x11c/0x120
> > [31908.554147]  bus_for_each_dev+0xe6/0x150
> > [31908.554237]  driver_attach+0x26/0x30
> > [31908.554325]  bus_add_driver+0x26b/0x3b0
> > [31908.554412]  driver_register+0xce/0x190
> > [31908.554502]  __pci_register_driver+0xaf/0xc0
> > [31908.554589]  0xffffffffa0550063
> > [31908.554675]  do_one_initcall+0x8b/0x1e0
> > [31908.554764]  do_init_module+0x102/0x325
> > [31908.554852]  load_module+0x3aad/0x45e0
> > [31908.554944]  SyS_finit_module+0x169/0x1a0
> > [31908.555033]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > [31908.555119] Freed:
> > [31908.555188] PID = 3781
> > [31908.555266]  save_stack_trace+0x16/0x20
> > [31908.555349]  kasan_slab_free+0xb0/0x180
> > [31908.555436]  kfree+0xaa/0x170
> > [31908.555520]  platform_device_release+0x76/0x80
> > [31908.555610]  device_release+0x45/0xe0
> > [31908.555698]  kobject_put+0x11f/0x260
> > [31908.555785]  put_device+0x12/0x20
> > [31908.555871]  platform_device_unregister+0x1b/0x20
> > [31908.556135]  intel_lpe_audio_teardown+0x5c/0xb0 [i915]
> > [31908.556390]  intel_audio_deinit+0x28/0x80 [i915]
> > [31908.556622]  i915_driver_unload+0xe4/0x360 [i915]
> > [31908.556858]  i915_pci_remove+0x23/0x30 [i915]
> > [31908.556948]  pci_device_remove+0x5c/0x100
> > [31908.557037]  device_release_driver_internal+0x1db/0x2e0
> > [31908.557129]  driver_detach+0x68/0xc0
> > [31908.557217]  bus_remove_driver+0x8b/0x150
> > [31908.557304]  driver_unregister+0x3e/0x60
> > [31908.557394]  pci_unregister_driver+0x1d/0x110
> > [31908.557653]  i915_exit+0x1a/0x87 [i915]
> > [31908.557741]  SyS_delete_module+0x264/0x2c0
> > [31908.557834]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > [31908.557919] Memory state around the buggy address:
> > [31908.558005]  ffff8801f7788200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [31908.558127]  ffff8801f7788280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [31908.558255] >ffff8801f7788300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [31908.558374]                                                     ^
> > [31908.558467]  ffff8801f7788380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [31908.558595]  ffff8801f7788400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > 
> > v2: Just leak the memory (8 bytes) as freeing it ourselves is not safe,
> > and we need to coordinate a proper fix in platform_device itself.
> > v3: Use dma_coerce_mask_and_coherent() to skip the kmalloc
> > 
> > Fixes: eef57324d926 ("drm/i915: setup bridge for HDMI LPE audio driver")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > Cc: Jerome Anand <jerome.anand@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/intel_lpe_audio.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > index d8ca187ae001..dace2fb3154f 100644
> > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > @@ -108,7 +108,6 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> >  	pinfo.num_res = 2;
> >  	pinfo.data = pdata;
> >  	pinfo.size_data = sizeof(*pdata);
> > -	pinfo.dma_mask = DMA_BIT_MASK(32);
> >  
> >  	spin_lock_init(&pdata->lpe_audio_slock);
> >  
> > @@ -119,6 +118,8 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> >  		goto err;
> >  	}
> >  
> > +	dma_coerce_mask_and_coherent(&platdev->dev, DMA_BIT_MASK(32));
> > +
> 
> Not sure how racy that is since we've already registered the platdev at
> that point. The whole platform_register_full() API looks misdesigned to
> me since you can't do stuff between alloc and register.
> 
> We could shovel the dma_coerce_mask_and_coherent() call into
> platform_register_full() itself I suppose. Or we just stop using the
> register_full() stuff and do each step ourselves, but that looks a bit
> tedious.

Agreed.  Through a quick glance, I couldn't find any caller that uses
dev_mask and coherent_mask individually.  We can clean up the whole
mess like below.

The platform_device_register_full() doesn't necessarily support all
use-cases.  If anyone really needs the separated dev_mask from the
coherent_dma_mask, they should do manually.


thanks,

Takashi

---
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index c4af00385502..d7e160b212b2 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -504,21 +504,8 @@ struct platform_device *platform_device_register_full(
 	pdev->dev.parent = pdevinfo->parent;
 	pdev->dev.fwnode = pdevinfo->fwnode;
 
-	if (pdevinfo->dma_mask) {
-		/*
-		 * This memory isn't freed when the device is put,
-		 * I don't have a nice idea for that though.  Conceptually
-		 * dma_mask in struct device should not be a pointer.
-		 * See http://thread.gmane.org/gmane.linux.kernel.pci/9081
-		 */
-		pdev->dev.dma_mask =
-			kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
-		if (!pdev->dev.dma_mask)
-			goto err;
-
-		*pdev->dev.dma_mask = pdevinfo->dma_mask;
-		pdev->dev.coherent_dma_mask = pdevinfo->dma_mask;
-	}
+	if (pdevinfo->dma_mask)
+		dma_coerce_mask_and_coherent(&pdev->dev, pdevinfo->dma_mask);
 
 	ret = platform_device_add_resources(pdev,
 			pdevinfo->res, pdevinfo->num_res);
@@ -541,7 +528,6 @@ struct platform_device *platform_device_register_full(
 	if (ret) {
 err:
 		ACPI_COMPANION_SET(&pdev->dev, NULL);
-		kfree(pdev->dev.dma_mask);
 
 err_alloc:
 		platform_device_put(pdev);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-04-12  8:02 ` [PATCH v2] " Chris Wilson
@ 2017-04-12 11:42   ` Takashi Iwai
  2017-04-12 19:19     ` Ville Syrjälä
  0 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2017-04-12 11:42 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Jani Nikula, Takashi Iwai, intel-gfx

On Wed, 12 Apr 2017 10:02:51 +0200,
Chris Wilson wrote:
> 
> [31908.547136] BUG: KASAN: use-after-free in intel_lpe_audio_teardown+0x78/0xb0 [i915] at addr ffff8801f7788358
> [31908.547297] Read of size 8 by task drv_selftest/3781
> [31908.547405] CPU: 0 PID: 3781 Comm: drv_selftest Tainted: G    BU  W       4.10.0+ #451
> [31908.547553] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> [31908.547682] Call Trace:
> [31908.547772]  dump_stack+0x68/0x9f
> [31908.547857]  kasan_object_err+0x1c/0x70
> [31908.547947]  kasan_report_error+0x1f1/0x4f0
> [31908.548038]  ? kfree+0xaa/0x170
> [31908.548121]  kasan_report+0x34/0x40
> [31908.548211]  ? klist_children_get+0x20/0x30
> [31908.548472]  ? intel_lpe_audio_teardown+0x78/0xb0 [i915]
> [31908.548567]  __asan_load8+0x5e/0x70
> [31908.548824]  intel_lpe_audio_teardown+0x78/0xb0 [i915]
> [31908.549080]  intel_audio_deinit+0x28/0x80 [i915]
> [31908.549315]  i915_driver_unload+0xe4/0x360 [i915]
> [31908.549551]  ? i915_driver_load+0x1d70/0x1d70 [i915]
> [31908.549651]  ? trace_hardirqs_on+0xd/0x10
> [31908.549885]  i915_pci_remove+0x23/0x30 [i915]
> [31908.549978]  pci_device_remove+0x5c/0x100
> [31908.550069]  device_release_driver_internal+0x1db/0x2e0
> [31908.550165]  driver_detach+0x68/0xc0
> [31908.550256]  bus_remove_driver+0x8b/0x150
> [31908.550346]  driver_unregister+0x3e/0x60
> [31908.550439]  pci_unregister_driver+0x1d/0x110
> [31908.550531]  ? find_module_all+0x7a/0xa0
> [31908.550791]  i915_exit+0x1a/0x87 [i915]
> [31908.550881]  SyS_delete_module+0x264/0x2c0
> [31908.550971]  ? free_module+0x430/0x430
> [31908.551064]  ? trace_hardirqs_off_caller+0x16/0x110
> [31908.551159]  ? trace_hardirqs_on_caller+0x16/0x280
> [31908.551256]  ? trace_hardirqs_on_thunk+0x1a/0x1c
> [31908.551350]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> [31908.551440] RIP: 0033:0x7f1d67312ec7
> [31908.551520] RSP: 002b:00007ffebe34e888 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
> [31908.551650] RAX: ffffffffffffffda RBX: ffffffff811123f6 RCX: 00007f1d67312ec7
> [31908.551743] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000560d0af476b8
> [31908.551837] RBP: ffff880233d87f98 R08: 0000000000000000 R09: 00007ffebe34e8b8
> [31908.551930] R10: 00007f1d68adf8c0 R11: 0000000000000206 R12: 0000000000000000
> [31908.552023] R13: 0000560d0af46440 R14: 0000000000000034 R15: 00007ffebe34d860
> [31908.552121]  ? trace_hardirqs_off_caller+0x16/0x110
> [31908.552217] Object at ffff8801f7788000, in cache kmalloc-2048 size: 2048
> [31908.552306] Allocated:
> [31908.552377] PID = 3781
> [31908.552456]  save_stack_trace+0x16/0x20
> [31908.552539]  kasan_kmalloc+0xee/0x190
> [31908.552627]  __kmalloc+0xdb/0x1b0
> [31908.552713]  platform_device_alloc+0x27/0x90
> [31908.552804]  platform_device_register_full+0x36/0x220
> [31908.553066]  intel_lpe_audio_init+0x41e/0x570 [i915]
> [31908.553320]  intel_audio_init+0xd/0x40 [i915]
> [31908.553552]  i915_driver_load+0x13f5/0x1d70 [i915]
> [31908.553788]  i915_pci_probe+0x65/0xe0 [i915]
> [31908.553881]  pci_device_probe+0xda/0x140
> [31908.553969]  driver_probe_device+0x400/0x660
> [31908.554058]  __driver_attach+0x11c/0x120
> [31908.554147]  bus_for_each_dev+0xe6/0x150
> [31908.554237]  driver_attach+0x26/0x30
> [31908.554325]  bus_add_driver+0x26b/0x3b0
> [31908.554412]  driver_register+0xce/0x190
> [31908.554502]  __pci_register_driver+0xaf/0xc0
> [31908.554589]  0xffffffffa0550063
> [31908.554675]  do_one_initcall+0x8b/0x1e0
> [31908.554764]  do_init_module+0x102/0x325
> [31908.554852]  load_module+0x3aad/0x45e0
> [31908.554944]  SyS_finit_module+0x169/0x1a0
> [31908.555033]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> [31908.555119] Freed:
> [31908.555188] PID = 3781
> [31908.555266]  save_stack_trace+0x16/0x20
> [31908.555349]  kasan_slab_free+0xb0/0x180
> [31908.555436]  kfree+0xaa/0x170
> [31908.555520]  platform_device_release+0x76/0x80
> [31908.555610]  device_release+0x45/0xe0
> [31908.555698]  kobject_put+0x11f/0x260
> [31908.555785]  put_device+0x12/0x20
> [31908.555871]  platform_device_unregister+0x1b/0x20
> [31908.556135]  intel_lpe_audio_teardown+0x5c/0xb0 [i915]
> [31908.556390]  intel_audio_deinit+0x28/0x80 [i915]
> [31908.556622]  i915_driver_unload+0xe4/0x360 [i915]
> [31908.556858]  i915_pci_remove+0x23/0x30 [i915]
> [31908.556948]  pci_device_remove+0x5c/0x100
> [31908.557037]  device_release_driver_internal+0x1db/0x2e0
> [31908.557129]  driver_detach+0x68/0xc0
> [31908.557217]  bus_remove_driver+0x8b/0x150
> [31908.557304]  driver_unregister+0x3e/0x60
> [31908.557394]  pci_unregister_driver+0x1d/0x110
> [31908.557653]  i915_exit+0x1a/0x87 [i915]
> [31908.557741]  SyS_delete_module+0x264/0x2c0
> [31908.557834]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> [31908.557919] Memory state around the buggy address:
> [31908.558005]  ffff8801f7788200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [31908.558127]  ffff8801f7788280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [31908.558255] >ffff8801f7788300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [31908.558374]                                                     ^
> [31908.558467]  ffff8801f7788380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [31908.558595]  ffff8801f7788400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> 
> v2: Just leak the memory (8 bytes) as freeing it ourselves is not safe,
> and we need to coordinate a proper fix in platform_device itself.
> 
> Fixes: eef57324d926 ("drm/i915: setup bridge for HDMI LPE audio driver")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Cc: Jerome Anand <jerome.anand@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

I'm for v2.
  Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi


> ---
>  drivers/gpu/drm/i915/intel_lpe_audio.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> index d8ca187ae001..d19053353a2b 100644
> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> @@ -131,8 +131,15 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
>  
>  static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
>  {
> +	/* XXX Note that platform_device_register_full() allocates a dma_mask
> +	 * and never frees it. We can't free it here as we cannot guarrantee
> +	 * this is the last reference (i.e. that the dma_mask will not be
> +	 * used after our unregister). We choose to leak the sizeof(u64)
> +	 * allocation - it should be fixed in the platform_device rather
> +	 * than us fiddle with its internals.
> +	 */
> +
>  	platform_device_unregister(dev_priv->lpe_audio.platdev);
> -	kfree(dev_priv->lpe_audio.platdev->dev.dma_mask);
>  }
>  
>  static void lpe_audio_irq_unmask(struct irq_data *d)
> -- 
> 2.11.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v3] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-04-12 11:41     ` Takashi Iwai
@ 2017-04-12 19:17       ` Ville Syrjälä
  0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2017-04-12 19:17 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Jani Nikula, intel-gfx

On Wed, Apr 12, 2017 at 01:41:05PM +0200, Takashi Iwai wrote:
> On Wed, 12 Apr 2017 10:52:54 +0200,
> Ville Syrjälä wrote:
> > 
> > On Wed, Apr 12, 2017 at 09:31:39AM +0100, Chris Wilson wrote:
> > > [31908.547136] BUG: KASAN: use-after-free in intel_lpe_audio_teardown+0x78/0xb0 [i915] at addr ffff8801f7788358
> > > [31908.547297] Read of size 8 by task drv_selftest/3781
> > > [31908.547405] CPU: 0 PID: 3781 Comm: drv_selftest Tainted: G    BU  W       4.10.0+ #451
> > > [31908.547553] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> > > [31908.547682] Call Trace:
> > > [31908.547772]  dump_stack+0x68/0x9f
> > > [31908.547857]  kasan_object_err+0x1c/0x70
> > > [31908.547947]  kasan_report_error+0x1f1/0x4f0
> > > [31908.548038]  ? kfree+0xaa/0x170
> > > [31908.548121]  kasan_report+0x34/0x40
> > > [31908.548211]  ? klist_children_get+0x20/0x30
> > > [31908.548472]  ? intel_lpe_audio_teardown+0x78/0xb0 [i915]
> > > [31908.548567]  __asan_load8+0x5e/0x70
> > > [31908.548824]  intel_lpe_audio_teardown+0x78/0xb0 [i915]
> > > [31908.549080]  intel_audio_deinit+0x28/0x80 [i915]
> > > [31908.549315]  i915_driver_unload+0xe4/0x360 [i915]
> > > [31908.549551]  ? i915_driver_load+0x1d70/0x1d70 [i915]
> > > [31908.549651]  ? trace_hardirqs_on+0xd/0x10
> > > [31908.549885]  i915_pci_remove+0x23/0x30 [i915]
> > > [31908.549978]  pci_device_remove+0x5c/0x100
> > > [31908.550069]  device_release_driver_internal+0x1db/0x2e0
> > > [31908.550165]  driver_detach+0x68/0xc0
> > > [31908.550256]  bus_remove_driver+0x8b/0x150
> > > [31908.550346]  driver_unregister+0x3e/0x60
> > > [31908.550439]  pci_unregister_driver+0x1d/0x110
> > > [31908.550531]  ? find_module_all+0x7a/0xa0
> > > [31908.550791]  i915_exit+0x1a/0x87 [i915]
> > > [31908.550881]  SyS_delete_module+0x264/0x2c0
> > > [31908.550971]  ? free_module+0x430/0x430
> > > [31908.551064]  ? trace_hardirqs_off_caller+0x16/0x110
> > > [31908.551159]  ? trace_hardirqs_on_caller+0x16/0x280
> > > [31908.551256]  ? trace_hardirqs_on_thunk+0x1a/0x1c
> > > [31908.551350]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > > [31908.551440] RIP: 0033:0x7f1d67312ec7
> > > [31908.551520] RSP: 002b:00007ffebe34e888 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
> > > [31908.551650] RAX: ffffffffffffffda RBX: ffffffff811123f6 RCX: 00007f1d67312ec7
> > > [31908.551743] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000560d0af476b8
> > > [31908.551837] RBP: ffff880233d87f98 R08: 0000000000000000 R09: 00007ffebe34e8b8
> > > [31908.551930] R10: 00007f1d68adf8c0 R11: 0000000000000206 R12: 0000000000000000
> > > [31908.552023] R13: 0000560d0af46440 R14: 0000000000000034 R15: 00007ffebe34d860
> > > [31908.552121]  ? trace_hardirqs_off_caller+0x16/0x110
> > > [31908.552217] Object at ffff8801f7788000, in cache kmalloc-2048 size: 2048
> > > [31908.552306] Allocated:
> > > [31908.552377] PID = 3781
> > > [31908.552456]  save_stack_trace+0x16/0x20
> > > [31908.552539]  kasan_kmalloc+0xee/0x190
> > > [31908.552627]  __kmalloc+0xdb/0x1b0
> > > [31908.552713]  platform_device_alloc+0x27/0x90
> > > [31908.552804]  platform_device_register_full+0x36/0x220
> > > [31908.553066]  intel_lpe_audio_init+0x41e/0x570 [i915]
> > > [31908.553320]  intel_audio_init+0xd/0x40 [i915]
> > > [31908.553552]  i915_driver_load+0x13f5/0x1d70 [i915]
> > > [31908.553788]  i915_pci_probe+0x65/0xe0 [i915]
> > > [31908.553881]  pci_device_probe+0xda/0x140
> > > [31908.553969]  driver_probe_device+0x400/0x660
> > > [31908.554058]  __driver_attach+0x11c/0x120
> > > [31908.554147]  bus_for_each_dev+0xe6/0x150
> > > [31908.554237]  driver_attach+0x26/0x30
> > > [31908.554325]  bus_add_driver+0x26b/0x3b0
> > > [31908.554412]  driver_register+0xce/0x190
> > > [31908.554502]  __pci_register_driver+0xaf/0xc0
> > > [31908.554589]  0xffffffffa0550063
> > > [31908.554675]  do_one_initcall+0x8b/0x1e0
> > > [31908.554764]  do_init_module+0x102/0x325
> > > [31908.554852]  load_module+0x3aad/0x45e0
> > > [31908.554944]  SyS_finit_module+0x169/0x1a0
> > > [31908.555033]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > > [31908.555119] Freed:
> > > [31908.555188] PID = 3781
> > > [31908.555266]  save_stack_trace+0x16/0x20
> > > [31908.555349]  kasan_slab_free+0xb0/0x180
> > > [31908.555436]  kfree+0xaa/0x170
> > > [31908.555520]  platform_device_release+0x76/0x80
> > > [31908.555610]  device_release+0x45/0xe0
> > > [31908.555698]  kobject_put+0x11f/0x260
> > > [31908.555785]  put_device+0x12/0x20
> > > [31908.555871]  platform_device_unregister+0x1b/0x20
> > > [31908.556135]  intel_lpe_audio_teardown+0x5c/0xb0 [i915]
> > > [31908.556390]  intel_audio_deinit+0x28/0x80 [i915]
> > > [31908.556622]  i915_driver_unload+0xe4/0x360 [i915]
> > > [31908.556858]  i915_pci_remove+0x23/0x30 [i915]
> > > [31908.556948]  pci_device_remove+0x5c/0x100
> > > [31908.557037]  device_release_driver_internal+0x1db/0x2e0
> > > [31908.557129]  driver_detach+0x68/0xc0
> > > [31908.557217]  bus_remove_driver+0x8b/0x150
> > > [31908.557304]  driver_unregister+0x3e/0x60
> > > [31908.557394]  pci_unregister_driver+0x1d/0x110
> > > [31908.557653]  i915_exit+0x1a/0x87 [i915]
> > > [31908.557741]  SyS_delete_module+0x264/0x2c0
> > > [31908.557834]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > > [31908.557919] Memory state around the buggy address:
> > > [31908.558005]  ffff8801f7788200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > > [31908.558127]  ffff8801f7788280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > > [31908.558255] >ffff8801f7788300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > > [31908.558374]                                                     ^
> > > [31908.558467]  ffff8801f7788380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > > [31908.558595]  ffff8801f7788400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > > 
> > > v2: Just leak the memory (8 bytes) as freeing it ourselves is not safe,
> > > and we need to coordinate a proper fix in platform_device itself.
> > > v3: Use dma_coerce_mask_and_coherent() to skip the kmalloc
> > > 
> > > Fixes: eef57324d926 ("drm/i915: setup bridge for HDMI LPE audio driver")
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > Cc: Jerome Anand <jerome.anand@intel.com>
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Cc: Takashi Iwai <tiwai@suse.de>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > >  drivers/gpu/drm/i915/intel_lpe_audio.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > index d8ca187ae001..dace2fb3154f 100644
> > > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > > @@ -108,7 +108,6 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> > >  	pinfo.num_res = 2;
> > >  	pinfo.data = pdata;
> > >  	pinfo.size_data = sizeof(*pdata);
> > > -	pinfo.dma_mask = DMA_BIT_MASK(32);
> > >  
> > >  	spin_lock_init(&pdata->lpe_audio_slock);
> > >  
> > > @@ -119,6 +118,8 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> > >  		goto err;
> > >  	}
> > >  
> > > +	dma_coerce_mask_and_coherent(&platdev->dev, DMA_BIT_MASK(32));
> > > +
> > 
> > Not sure how racy that is since we've already registered the platdev at
> > that point. The whole platform_register_full() API looks misdesigned to
> > me since you can't do stuff between alloc and register.
> > 
> > We could shovel the dma_coerce_mask_and_coherent() call into
> > platform_register_full() itself I suppose. Or we just stop using the
> > register_full() stuff and do each step ourselves, but that looks a bit
> > tedious.
> 
> Agreed.  Through a quick glance, I couldn't find any caller that uses
> dev_mask and coherent_mask individually.  We can clean up the whole
> mess like below.
> 
> The platform_device_register_full() doesn't necessarily support all
> use-cases.  If anyone really needs the separated dev_mask from the
> coherent_dma_mask, they should do manually.
> 
> 
> thanks,
> 
> Takashi
> 
> ---
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index c4af00385502..d7e160b212b2 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -504,21 +504,8 @@ struct platform_device *platform_device_register_full(
>  	pdev->dev.parent = pdevinfo->parent;
>  	pdev->dev.fwnode = pdevinfo->fwnode;
>  
> -	if (pdevinfo->dma_mask) {
> -		/*
> -		 * This memory isn't freed when the device is put,
> -		 * I don't have a nice idea for that though.  Conceptually
> -		 * dma_mask in struct device should not be a pointer.
> -		 * See http://thread.gmane.org/gmane.linux.kernel.pci/9081
> -		 */
> -		pdev->dev.dma_mask =
> -			kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
> -		if (!pdev->dev.dma_mask)
> -			goto err;
> -
> -		*pdev->dev.dma_mask = pdevinfo->dma_mask;
> -		pdev->dev.coherent_dma_mask = pdevinfo->dma_mask;
> -	}
> +	if (pdevinfo->dma_mask)
> +		dma_coerce_mask_and_coherent(&pdev->dev, pdevinfo->dma_mask);

A further question is whether this should actually check for the error
and propagate it to the caller. The old code silently assumed the DMA
mask was acceptable, but the new code will actually check for that.

>  
>  	ret = platform_device_add_resources(pdev,
>  			pdevinfo->res, pdevinfo->num_res);
> @@ -541,7 +528,6 @@ struct platform_device *platform_device_register_full(
>  	if (ret) {
>  err:
>  		ACPI_COMPANION_SET(&pdev->dev, NULL);

Unrelated, but I also have to wonder what this guys is doing here.
Some kind of misplaced thing, or a layering violation methinks.
The code never itself set this so I don't see why it would have
to clear it either.

> -		kfree(pdev->dev.dma_mask);
>  
>  err_alloc:
>  		platform_device_put(pdev);

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-04-12 11:42   ` Takashi Iwai
@ 2017-04-12 19:19     ` Ville Syrjälä
  2017-04-12 21:55       ` Chris Wilson
  0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2017-04-12 19:19 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Jani Nikula, intel-gfx

On Wed, Apr 12, 2017 at 01:42:36PM +0200, Takashi Iwai wrote:
> On Wed, 12 Apr 2017 10:02:51 +0200,
> Chris Wilson wrote:
> > 
> > [31908.547136] BUG: KASAN: use-after-free in intel_lpe_audio_teardown+0x78/0xb0 [i915] at addr ffff8801f7788358
> > [31908.547297] Read of size 8 by task drv_selftest/3781
> > [31908.547405] CPU: 0 PID: 3781 Comm: drv_selftest Tainted: G    BU  W       4.10.0+ #451
> > [31908.547553] Hardware name:                  /        , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> > [31908.547682] Call Trace:
> > [31908.547772]  dump_stack+0x68/0x9f
> > [31908.547857]  kasan_object_err+0x1c/0x70
> > [31908.547947]  kasan_report_error+0x1f1/0x4f0
> > [31908.548038]  ? kfree+0xaa/0x170
> > [31908.548121]  kasan_report+0x34/0x40
> > [31908.548211]  ? klist_children_get+0x20/0x30
> > [31908.548472]  ? intel_lpe_audio_teardown+0x78/0xb0 [i915]
> > [31908.548567]  __asan_load8+0x5e/0x70
> > [31908.548824]  intel_lpe_audio_teardown+0x78/0xb0 [i915]
> > [31908.549080]  intel_audio_deinit+0x28/0x80 [i915]
> > [31908.549315]  i915_driver_unload+0xe4/0x360 [i915]
> > [31908.549551]  ? i915_driver_load+0x1d70/0x1d70 [i915]
> > [31908.549651]  ? trace_hardirqs_on+0xd/0x10
> > [31908.549885]  i915_pci_remove+0x23/0x30 [i915]
> > [31908.549978]  pci_device_remove+0x5c/0x100
> > [31908.550069]  device_release_driver_internal+0x1db/0x2e0
> > [31908.550165]  driver_detach+0x68/0xc0
> > [31908.550256]  bus_remove_driver+0x8b/0x150
> > [31908.550346]  driver_unregister+0x3e/0x60
> > [31908.550439]  pci_unregister_driver+0x1d/0x110
> > [31908.550531]  ? find_module_all+0x7a/0xa0
> > [31908.550791]  i915_exit+0x1a/0x87 [i915]
> > [31908.550881]  SyS_delete_module+0x264/0x2c0
> > [31908.550971]  ? free_module+0x430/0x430
> > [31908.551064]  ? trace_hardirqs_off_caller+0x16/0x110
> > [31908.551159]  ? trace_hardirqs_on_caller+0x16/0x280
> > [31908.551256]  ? trace_hardirqs_on_thunk+0x1a/0x1c
> > [31908.551350]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > [31908.551440] RIP: 0033:0x7f1d67312ec7
> > [31908.551520] RSP: 002b:00007ffebe34e888 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
> > [31908.551650] RAX: ffffffffffffffda RBX: ffffffff811123f6 RCX: 00007f1d67312ec7
> > [31908.551743] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000560d0af476b8
> > [31908.551837] RBP: ffff880233d87f98 R08: 0000000000000000 R09: 00007ffebe34e8b8
> > [31908.551930] R10: 00007f1d68adf8c0 R11: 0000000000000206 R12: 0000000000000000
> > [31908.552023] R13: 0000560d0af46440 R14: 0000000000000034 R15: 00007ffebe34d860
> > [31908.552121]  ? trace_hardirqs_off_caller+0x16/0x110
> > [31908.552217] Object at ffff8801f7788000, in cache kmalloc-2048 size: 2048
> > [31908.552306] Allocated:
> > [31908.552377] PID = 3781
> > [31908.552456]  save_stack_trace+0x16/0x20
> > [31908.552539]  kasan_kmalloc+0xee/0x190
> > [31908.552627]  __kmalloc+0xdb/0x1b0
> > [31908.552713]  platform_device_alloc+0x27/0x90
> > [31908.552804]  platform_device_register_full+0x36/0x220
> > [31908.553066]  intel_lpe_audio_init+0x41e/0x570 [i915]
> > [31908.553320]  intel_audio_init+0xd/0x40 [i915]
> > [31908.553552]  i915_driver_load+0x13f5/0x1d70 [i915]
> > [31908.553788]  i915_pci_probe+0x65/0xe0 [i915]
> > [31908.553881]  pci_device_probe+0xda/0x140
> > [31908.553969]  driver_probe_device+0x400/0x660
> > [31908.554058]  __driver_attach+0x11c/0x120
> > [31908.554147]  bus_for_each_dev+0xe6/0x150
> > [31908.554237]  driver_attach+0x26/0x30
> > [31908.554325]  bus_add_driver+0x26b/0x3b0
> > [31908.554412]  driver_register+0xce/0x190
> > [31908.554502]  __pci_register_driver+0xaf/0xc0
> > [31908.554589]  0xffffffffa0550063
> > [31908.554675]  do_one_initcall+0x8b/0x1e0
> > [31908.554764]  do_init_module+0x102/0x325
> > [31908.554852]  load_module+0x3aad/0x45e0
> > [31908.554944]  SyS_finit_module+0x169/0x1a0
> > [31908.555033]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > [31908.555119] Freed:
> > [31908.555188] PID = 3781
> > [31908.555266]  save_stack_trace+0x16/0x20
> > [31908.555349]  kasan_slab_free+0xb0/0x180
> > [31908.555436]  kfree+0xaa/0x170
> > [31908.555520]  platform_device_release+0x76/0x80
> > [31908.555610]  device_release+0x45/0xe0
> > [31908.555698]  kobject_put+0x11f/0x260
> > [31908.555785]  put_device+0x12/0x20
> > [31908.555871]  platform_device_unregister+0x1b/0x20
> > [31908.556135]  intel_lpe_audio_teardown+0x5c/0xb0 [i915]
> > [31908.556390]  intel_audio_deinit+0x28/0x80 [i915]
> > [31908.556622]  i915_driver_unload+0xe4/0x360 [i915]
> > [31908.556858]  i915_pci_remove+0x23/0x30 [i915]
> > [31908.556948]  pci_device_remove+0x5c/0x100
> > [31908.557037]  device_release_driver_internal+0x1db/0x2e0
> > [31908.557129]  driver_detach+0x68/0xc0
> > [31908.557217]  bus_remove_driver+0x8b/0x150
> > [31908.557304]  driver_unregister+0x3e/0x60
> > [31908.557394]  pci_unregister_driver+0x1d/0x110
> > [31908.557653]  i915_exit+0x1a/0x87 [i915]
> > [31908.557741]  SyS_delete_module+0x264/0x2c0
> > [31908.557834]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > [31908.557919] Memory state around the buggy address:
> > [31908.558005]  ffff8801f7788200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [31908.558127]  ffff8801f7788280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [31908.558255] >ffff8801f7788300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [31908.558374]                                                     ^
> > [31908.558467]  ffff8801f7788380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [31908.558595]  ffff8801f7788400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > 
> > v2: Just leak the memory (8 bytes) as freeing it ourselves is not safe,
> > and we need to coordinate a proper fix in platform_device itself.
> > 
> > Fixes: eef57324d926 ("drm/i915: setup bridge for HDMI LPE audio driver")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > Cc: Jerome Anand <jerome.anand@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> I'm for v2.
>   Reviewed-by: Takashi Iwai <tiwai@suse.de>

I concur
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Dropping the comment is easy if/when the platdev code gets fixed.

> 
> 
> thanks,
> 
> Takashi
> 
> 
> > ---
> >  drivers/gpu/drm/i915/intel_lpe_audio.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > index d8ca187ae001..d19053353a2b 100644
> > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > @@ -131,8 +131,15 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv)
> >  
> >  static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
> >  {
> > +	/* XXX Note that platform_device_register_full() allocates a dma_mask
> > +	 * and never frees it. We can't free it here as we cannot guarrantee
> > +	 * this is the last reference (i.e. that the dma_mask will not be
> > +	 * used after our unregister). We choose to leak the sizeof(u64)
> > +	 * allocation - it should be fixed in the platform_device rather
> > +	 * than us fiddle with its internals.
> > +	 */
> > +
> >  	platform_device_unregister(dev_priv->lpe_audio.platdev);
> > -	kfree(dev_priv->lpe_audio.platdev->dev.dma_mask);
> >  }
> >  
> >  static void lpe_audio_irq_unmask(struct irq_data *d)
> > -- 
> > 2.11.0
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: Fix use after free in lpe_audio_platdev_destroy()
  2017-04-12 19:19     ` Ville Syrjälä
@ 2017-04-12 21:55       ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2017-04-12 21:55 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Takashi Iwai, Jani Nikula, intel-gfx

On Wed, Apr 12, 2017 at 10:19:01PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 12, 2017 at 01:42:36PM +0200, Takashi Iwai wrote:
> > On Wed, 12 Apr 2017 10:02:51 +0200,
> > Chris Wilson wrote:
> > > v2: Just leak the memory (8 bytes) as freeing it ourselves is not safe,
> > > and we need to coordinate a proper fix in platform_device itself.
> > > 
> > > Fixes: eef57324d926 ("drm/i915: setup bridge for HDMI LPE audio driver")
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > Cc: Jerome Anand <jerome.anand@intel.com>
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Cc: Takashi Iwai <tiwai@suse.de>
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > I'm for v2.
> >   Reviewed-by: Takashi Iwai <tiwai@suse.de>
> 
> I concur
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

v2 is the clear winner, pushed. Leaks for everyone, yay!
 
> Dropping the comment is easy if/when the platdev code gets fixed.

Hopefully sometime before we enable kasan in BAT... Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-04-12 21:55 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-01 18:59 [PATCH] drm/i915: Fix use after free in lpe_audio_platdev_destroy() Chris Wilson
2017-03-01 20:17 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-04-11 20:41 ` [PATCH] " Chris Wilson
2017-04-11 21:01   ` Takashi Iwai
2017-04-11 21:20     ` Chris Wilson
2017-04-11 21:27       ` Chris Wilson
2017-04-12  4:59         ` Takashi Iwai
2017-04-12  7:46           ` Ville Syrjälä
2017-04-12  8:25             ` Chris Wilson
2017-04-12  8:02 ` [PATCH v2] " Chris Wilson
2017-04-12 11:42   ` Takashi Iwai
2017-04-12 19:19     ` Ville Syrjälä
2017-04-12 21:55       ` Chris Wilson
2017-04-12  8:22 ` ✓ Fi.CI.BAT: success for drm/i915: Fix use after free in lpe_audio_platdev_destroy() (rev2) Patchwork
2017-04-12  8:31 ` [PATCH v3] drm/i915: Fix use after free in lpe_audio_platdev_destroy() Chris Wilson
2017-04-12  8:52   ` Ville Syrjälä
2017-04-12  9:03     ` Chris Wilson
2017-04-12 11:41     ` Takashi Iwai
2017-04-12 19:17       ` Ville Syrjälä
2017-04-12  8:51 ` ✓ Fi.CI.BAT: success for drm/i915: Fix use after free in lpe_audio_platdev_destroy() (rev4) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.