From patchwork Wed Apr 12 11:41:05 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 9677253 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 708BC601C3 for ; Wed, 12 Apr 2017 11:41:11 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 61D8F28636 for ; Wed, 12 Apr 2017 11:41:11 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5695928640; Wed, 12 Apr 2017 11:41:11 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 81B6928636 for ; Wed, 12 Apr 2017 11:41:09 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2DAFA6E6D0; Wed, 12 Apr 2017 11:41:09 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5A61A6E6D0 for ; Wed, 12 Apr 2017 11:41:07 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id B815CABEB; Wed, 12 Apr 2017 11:41:05 +0000 (UTC) Date: Wed, 12 Apr 2017 13:41:05 +0200 Message-ID: From: Takashi Iwai To: Ville =?UTF-8?B?U3lyasOkbMOk?= In-Reply-To: <20170412085254.GP30290@intel.com> References: <20170301185918.9114-1-chris@chris-wilson.co.uk> <20170412083139.10649-1-chris@chris-wilson.co.uk> <20170412085254.GP30290@intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.1 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Cc: Jani Nikula , intel-gfx@lists.freedesktop.org, Takashi Iwai Subject: Re: [Intel-gfx] [PATCH v3] drm/i915: Fix use after free in lpe_audio_platdev_destroy() X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP 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 > > Cc: Pierre-Louis Bossart > > Cc: Jerome Anand > > Cc: Jani Nikula > > Cc: Takashi Iwai > > Cc: Ville Syrjälä > > Signed-off-by: Chris Wilson > > --- > > 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);