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
prev parent reply other threads:[~2017-04-20 7:22 UTC|newest]
Thread overview: 5+ 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-14 21:51 ` [PATCH 2/4] mwifiex: reset timeout flag when resetting device Brian Norris
2017-04-14 21:51 ` [PATCH 3/4] mwifiex: pcie: clear outstanding work when resetting Brian Norris
2017-04-14 21:51 ` [PATCH 4/4] mwifiex: don't leak 'chan_stats' on 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 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.