stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kalle Valo <kvalo@codeaurora.org>
To: Brian Norris <briannorris@chromium.org>
Cc: Ganapathi Bhat <gbhat@marvell.com>,
	Nishant Sarmukadam <nishants@marvell.com>,
	<linux-kernel@vger.kernel.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	linux-wireless@vger.kernel.org,
	Brian Norris <briannorris@chromium.org>,
	stable@vger.kernel.org
Subject: Re: [1/4] mwifiex: pcie: fix cmd_buf use-after-free in remove/reset
Date: Thu, 20 Apr 2017 07:22:10 +0000 (UTC)	[thread overview]
Message-ID: <20170420072210.0454B6114F@smtp.codeaurora.org> (raw)
In-Reply-To: <20170414215120.145038-1-briannorris@chromium.org>

Brian Norris <briannorris@chromium.org> wrote:
> Command buffers (skb's) are allocated by the main driver, and freed upon
> the last use. That last use is often in mwifiex_free_cmd_buffer(). In
> the meantime, if the command buffer gets used by the PCI driver, we map
> it as DMA-able, and store the mapping information in the 'cb' memory.
> 
> However, if a command was in-flight when resetting the device (and
> therefore was still mapped), we don't get a chance to unmap this memory
> until after the core has cleaned up its command handling.
> 
> Let's keep a refcount within the PCI driver, so we ensure the memory
> only gets freed after we've finished unmapping it.
> 
> Noticed by KASAN when forcing a reset via:
> 
>   echo 1 > /sys/bus/pci/.../reset
> 
> The same code path can presumably be exercised in remove() and
> shutdown().
> 
> [  205.390377] mwifiex_pcie 0000:01:00.0: info: shutdown mwifiex...
> [  205.400393] ==================================================================
> [  205.407719] BUG: KASAN: use-after-free in mwifiex_unmap_pci_memory.isra.14+0x4c/0x100 [mwifiex_pcie] at addr ffffffc0ad471b28
> [  205.419040] Read of size 16 by task bash/1913
> [  205.423421] =============================================================================
> [  205.431625] BUG skbuff_head_cache (Tainted: G    B          ): kasan: bad access detected
> [  205.439815] -----------------------------------------------------------------------------
> [  205.439815]
> [  205.449534] INFO: Allocated in __build_skb+0x48/0x114 age=1311 cpu=4 pid=1913
> [  205.456709] 	alloc_debug_processing+0x124/0x178
> [  205.461282] 	___slab_alloc.constprop.58+0x528/0x608
> [  205.466196] 	__slab_alloc.isra.54.constprop.57+0x44/0x54
> [  205.471542] 	kmem_cache_alloc+0xcc/0x278
> [  205.475497] 	__build_skb+0x48/0x114
> [  205.479019] 	__netdev_alloc_skb+0xe0/0x170
> [  205.483244] 	mwifiex_alloc_cmd_buffer+0x68/0xdc [mwifiex]
> [  205.488759] 	mwifiex_init_fw+0x40/0x6cc [mwifiex]
> [  205.493584] 	_mwifiex_fw_dpc+0x158/0x520 [mwifiex]
> [  205.498491] 	mwifiex_reinit_sw+0x2c4/0x398 [mwifiex]
> [  205.503510] 	mwifiex_pcie_reset_notify+0x114/0x15c [mwifiex_pcie]
> [  205.509643] 	pci_reset_notify+0x5c/0x6c
> [  205.513519] 	pci_reset_function+0x6c/0x7c
> [  205.517567] 	reset_store+0x68/0x98
> [  205.521003] 	dev_attr_store+0x54/0x60
> [  205.524705] 	sysfs_kf_write+0x9c/0xb0
> [  205.528413] INFO: Freed in __kfree_skb+0xb0/0xbc age=131 cpu=4 pid=1913
> [  205.535064] 	free_debug_processing+0x264/0x370
> [  205.539550] 	__slab_free+0x84/0x40c
> [  205.543075] 	kmem_cache_free+0x1c8/0x2a0
> [  205.547030] 	__kfree_skb+0xb0/0xbc
> [  205.550465] 	consume_skb+0x164/0x178
> [  205.554079] 	__dev_kfree_skb_any+0x58/0x64
> [  205.558304] 	mwifiex_free_cmd_buffer+0xa0/0x158 [mwifiex]
> [  205.563817] 	mwifiex_shutdown_drv+0x578/0x5c4 [mwifiex]
> [  205.569164] 	mwifiex_shutdown_sw+0x178/0x310 [mwifiex]
> [  205.574353] 	mwifiex_pcie_reset_notify+0xd4/0x15c [mwifiex_pcie]
> [  205.580398] 	pci_reset_notify+0x5c/0x6c
> [  205.584274] 	pci_dev_save_and_disable+0x24/0x6c
> [  205.588837] 	pci_reset_function+0x30/0x7c
> [  205.592885] 	reset_store+0x68/0x98
> [  205.596324] 	dev_attr_store+0x54/0x60
> [  205.600017] 	sysfs_kf_write+0x9c/0xb0
> ...
> [  205.800488] Call trace:
> [  205.802980] [<ffffffc00020a69c>] dump_backtrace+0x0/0x190
> [  205.808415] [<ffffffc00020a96c>] show_stack+0x20/0x28
> [  205.813506] [<ffffffc0005d020c>] dump_stack+0xa4/0xcc
> [  205.818598] [<ffffffc0003be44c>] print_trailer+0x158/0x168
> [  205.824120] [<ffffffc0003be5f0>] object_err+0x4c/0x5c
> [  205.829210] [<ffffffc0003c45bc>] kasan_report+0x334/0x500
> [  205.834641] [<ffffffc0003c3994>] check_memory_region+0x20/0x14c
> [  205.840593] [<ffffffc0003c3b14>] __asan_loadN+0x14/0x1c
> [  205.845879] [<ffffffbffc46171c>] mwifiex_unmap_pci_memory.isra.14+0x4c/0x100 [mwifiex_pcie]
> [  205.854282] [<ffffffbffc461864>] mwifiex_pcie_delete_cmdrsp_buf+0x94/0xa8 [mwifiex_pcie]
> [  205.862421] [<ffffffbffc462028>] mwifiex_pcie_free_buffers+0x11c/0x158 [mwifiex_pcie]
> [  205.870302] [<ffffffbffc4620d4>] mwifiex_pcie_down_dev+0x70/0x80 [mwifiex_pcie]
> [  205.877736] [<ffffffbffc1397a8>] mwifiex_shutdown_sw+0x190/0x310 [mwifiex]
> [  205.884658] [<ffffffbffc4606b4>] mwifiex_pcie_reset_notify+0xd4/0x15c [mwifiex_pcie]
> [  205.892446] [<ffffffc000635f54>] pci_reset_notify+0x5c/0x6c
> [  205.898048] [<ffffffc00063a044>] pci_dev_save_and_disable+0x24/0x6c
> [  205.904350] [<ffffffc00063cf0c>] pci_reset_function+0x30/0x7c
> [  205.910134] [<ffffffc000641118>] reset_store+0x68/0x98
> [  205.915312] [<ffffffc000771588>] dev_attr_store+0x54/0x60
> [  205.920750] [<ffffffc00046f53c>] sysfs_kf_write+0x9c/0xb0
> [  205.926182] [<ffffffc00046dfb0>] kernfs_fop_write+0x184/0x1f8
> [  205.931963] [<ffffffc0003d64f4>] __vfs_write+0x6c/0x17c
> [  205.937221] [<ffffffc0003d7164>] vfs_write+0xf0/0x1c4
> [  205.942310] [<ffffffc0003d7da0>] SyS_write+0x78/0xd8
> [  205.947312] [<ffffffc000204634>] el0_svc_naked+0x24/0x28
> ...
> [  205.998268] ==================================================================
> 
> This bug has been around in different forms for a while. It was sort of
> noticed in commit 955ab095c51a ("mwifiex: Do not kfree cmd buf while
> unregistering PCIe"), but it just fixed the double-free, without
> acknowledging the potential for use-after-free.
> 
> Fixes: fc3314609047 ("mwifiex: use pci_alloc/free_consistent APIs for PCIe")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Brian Norris <briannorris@chromium.org>

4 patches applied to wireless-drivers-next.git, thanks.

3c8cb9ad032d mwifiex: pcie: fix cmd_buf use-after-free in remove/reset
9ae3fbd109d9 mwifiex: reset timeout flag when resetting device
35e67d3d58b9 mwifiex: pcie: clear outstanding work when resetting
fb9e67bee3ab mwifiex: don't leak 'chan_stats' on reset

-- 
https://patchwork.kernel.org/patch/9681823/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

      reply	other threads:[~2017-04-20  7:22 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-14 21:51 [PATCH 1/4] mwifiex: pcie: fix cmd_buf use-after-free in remove/reset Brian Norris
2017-04-20  7:22 ` Kalle Valo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170420072210.0454B6114F@smtp.codeaurora.org \
    --to=kvalo@codeaurora.org \
    --cc=briannorris@chromium.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=gbhat@marvell.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=nishants@marvell.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).