| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150306105108.GA3318@x1>
Date: Fri, 6 Mar 2015 10:51:08 +0000
From: Lee Jones <lee.jones@...aro.org>
To: Roger Tseng <rogerable@...ltek.com>
Cc: Samuel Ortiz <sameo@...ux.intel.com>, linux-kernel@...r.kernel.org,
Josh Boyer <jwboyer@...oraproject.org>,
Wei Wang <wei_wang@...lsil.com.cn>,
Micky Ching <micky_ching@...lsil.com.cn>
Subject: Re: [PATCH] mfd: rtsx_usb: prevent DMA from stack
On Fri, 06 Mar 2015, Roger Tseng wrote:
> Functions rtsx_usb_ep0_read_register() and rtsx_usb_get_card_status()
> both use arbitrary buffer addresses from arguments directly for DMA and
> the buffers could be located in stack. This was caught by DMA-API debug
> check.
>
> Fixes this by using double-buffers via kzalloc in both functions to
> guarantee the validity of DMA buffer.
>
> WARNING: CPU: 1 PID: 25 at lib/dma-debug.c:1166 check_for_stack+0x96/0xe0()
> ehci-pci 0000:00:1a.0: DMA-API: device driver maps memory from stack
> [addr=ffff8801199e3cef]
> Modules linked in: rtsx_usb_ms arc4 memstick intel_rapl iosf_mbi
> rtl8192ce snd_hda_codec_hdmi snd_hda_codec_realtek
> snd_hda_codec_generic snd_hda_intel rtl_pci rtl8192c_common
> snd_hda_controller x86_pkg_temp_thermal snd_hda_codec rtlwifi mac80211
> coretemp kvm_intel kvm iTCO_wdt snd_hwdep snd_seq snd_seq_device
> crct10dif_pclmul iTCO_vendor_support sparse_keymap cfg80211
> crc32_pclmul snd_pcm crc32c_intel ghash_clmulni_intel rfkill i2c_i801
> snd_timer shpchp snd serio_raw mei_me lpc_ich soundcore mei tpm_tis
> tpm wmi nfsd auth_rpcgss nfs_acl lockd grace sunrpc i915
> rtsx_usb_sdmmc mmc_core 8021q uas garp stp i2c_algo_bit llc mrp
> drm_kms_helper usb_storage drm rtsx_usb mfd_core r8169 mii video
> CPU: 1 PID: 25 Comm: kworker/1:2 Not tainted 3.20.0-0.rc0.git7.3.fc22.x86_64 #1
> Hardware name: WB WB-B06211/WB-B0621, BIOS EB062IWB V1.0 12/12/2013
> Workqueue: events rtsx_usb_ms_handle_req [rtsx_usb_ms]
> 0000000000000000 000000003d188e66 ffff8801199e3808 ffffffff8187642b
> 0000000000000000 ffff8801199e3860 ffff8801199e3848 ffffffff810ab39a
> ffff8801199e3864 ffff8801199e3cef ffff880119b57098 ffff880119b37320
> Call Trace:
> [<ffffffff8187642b>] dump_stack+0x4c/0x65
> [<ffffffff810ab39a>] warn_slowpath_common+0x8a/0xc0
> [<ffffffff810ab425>] warn_slowpath_fmt+0x55/0x70
> [<ffffffff8187efe6>] ? _raw_spin_unlock_irqrestore+0x36/0x70
> [<ffffffff81453156>] check_for_stack+0x96/0xe0
> [<ffffffff81453934>] debug_dma_map_page+0x104/0x150
> [<ffffffff81613b86>] usb_hcd_map_urb_for_dma+0x646/0x790
> [<ffffffff81614165>] usb_hcd_submit_urb+0x1d5/0xa90
> [<ffffffff81106f8f>] ? mark_held_locks+0x7f/0xc0
> [<ffffffff81106f8f>] ? mark_held_locks+0x7f/0xc0
> [<ffffffff81103a15>] ? lockdep_init_map+0x65/0x5d0
> [<ffffffff81615d7e>] usb_submit_urb+0x42e/0x5f0
> [<ffffffff81616787>] usb_start_wait_urb+0x77/0x190
> [<ffffffff8124f035>] ? __kmalloc+0x205/0x2d0
> [<ffffffff8161697c>] usb_control_msg+0xdc/0x130
> [<ffffffffa0031669>] rtsx_usb_ep0_read_register+0x59/0x70 [rtsx_usb]
> [<ffffffffa00310c1>] ? rtsx_usb_get_rsp+0x41/0x50 [rtsx_usb]
> [<ffffffffa071da4e>] rtsx_usb_ms_handle_req+0x7ce/0x9c5 [rtsx_usb_ms]
>
> Reported-by: Josh Boyer <jwboyer@...oraproject.org>
> Signed-off-by: Roger Tseng <rogerable@...ltek.com>
> ---
> drivers/mfd/rtsx_usb.c | 30 ++++++++++++++++++++++++------
> 1 file changed, 24 insertions(+), 6 deletions(-)
Applied, thanks.
> diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
> index ede50244f265..dbd907d7170e 100644
> --- a/drivers/mfd/rtsx_usb.c
> +++ b/drivers/mfd/rtsx_usb.c
> @@ -196,18 +196,27 @@ EXPORT_SYMBOL_GPL(rtsx_usb_ep0_write_register);
> int rtsx_usb_ep0_read_register(struct rtsx_ucr *ucr, u16 addr, u8 *data)
> {
> u16 value;
> + u8 *buf;
> + int ret;
>
> if (!data)
> return -EINVAL;
> - *data = 0;
> +
> + buf = kzalloc(sizeof(u8), GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
>
> addr |= EP0_READ_REG_CMD << EP0_OP_SHIFT;
> value = swab16(addr);
>
> - return usb_control_msg(ucr->pusb_dev,
> + ret = usb_control_msg(ucr->pusb_dev,
> usb_rcvctrlpipe(ucr->pusb_dev, 0), RTSX_USB_REQ_REG_OP,
> USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> - value, 0, data, 1, 100);
> + value, 0, buf, 1, 100);
> + *data = *buf;
> +
> + kfree(buf);
> + return ret;
> }
> EXPORT_SYMBOL_GPL(rtsx_usb_ep0_read_register);
>
> @@ -288,18 +297,27 @@ static int rtsx_usb_get_status_with_bulk(struct rtsx_ucr *ucr, u16 *status)
> int rtsx_usb_get_card_status(struct rtsx_ucr *ucr, u16 *status)
> {
> int ret;
> + u16 *buf;
>
> if (!status)
> return -EINVAL;
>
> - if (polling_pipe == 0)
> + if (polling_pipe == 0) {
> + buf = kzalloc(sizeof(u16), GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> ret = usb_control_msg(ucr->pusb_dev,
> usb_rcvctrlpipe(ucr->pusb_dev, 0),
> RTSX_USB_REQ_POLL,
> USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> - 0, 0, status, 2, 100);
> - else
> + 0, 0, buf, 2, 100);
> + *status = *buf;
> +
> + kfree(buf);
> + } else {
> ret = rtsx_usb_get_status_with_bulk(ucr, status);
> + }
>
> /* usb_control_msg may return positive when success */
> if (ret < 0)
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists