From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2992438AbbCFKvU (ORCPT ); Fri, 6 Mar 2015 05:51:20 -0500 Received: from mail-wg0-f44.google.com ([74.125.82.44]:37157 "EHLO mail-wg0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933408AbbCFKvN (ORCPT ); Fri, 6 Mar 2015 05:51:13 -0500 Date: Fri, 6 Mar 2015 10:51:08 +0000 From: Lee Jones To: Roger Tseng Cc: Samuel Ortiz , linux-kernel@vger.kernel.org, Josh Boyer , Wei Wang , Micky Ching Subject: Re: [PATCH] mfd: rtsx_usb: prevent DMA from stack Message-ID: <20150306105108.GA3318@x1> References: <1425612966-22494-1-git-send-email-rogerable@realtek.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1425612966-22494-1-git-send-email-rogerable@realtek.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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: > [] dump_stack+0x4c/0x65 > [] warn_slowpath_common+0x8a/0xc0 > [] warn_slowpath_fmt+0x55/0x70 > [] ? _raw_spin_unlock_irqrestore+0x36/0x70 > [] check_for_stack+0x96/0xe0 > [] debug_dma_map_page+0x104/0x150 > [] usb_hcd_map_urb_for_dma+0x646/0x790 > [] usb_hcd_submit_urb+0x1d5/0xa90 > [] ? mark_held_locks+0x7f/0xc0 > [] ? mark_held_locks+0x7f/0xc0 > [] ? lockdep_init_map+0x65/0x5d0 > [] usb_submit_urb+0x42e/0x5f0 > [] usb_start_wait_urb+0x77/0x190 > [] ? __kmalloc+0x205/0x2d0 > [] usb_control_msg+0xdc/0x130 > [] rtsx_usb_ep0_read_register+0x59/0x70 [rtsx_usb] > [] ? rtsx_usb_get_rsp+0x41/0x50 [rtsx_usb] > [] rtsx_usb_ms_handle_req+0x7ce/0x9c5 [rtsx_usb_ms] > > Reported-by: Josh Boyer > Signed-off-by: Roger Tseng > --- > 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