diff mbox

[v3] mmc: sdio: check the buffer address for sdio API

Message ID 1486368670-150702-1-git-send-email-shawn.lin@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shawn Lin Feb. 6, 2017, 8:11 a.m. UTC 
It's fine if the host driver use PIO mode to transfer the
vmalloc area buffer but not for DMA mode. The sdio APIs haven't
provide the capability to tell the caller whether it will use DMA
to finish the IO transfer or not, so don't give the randomly
insmoded sdio function driver the possibility to break the kernel.
Also the APIs shouldn't take the liberty to do a copy for these
cases and just kick out these requests should be enough.

This issue is observed by doing insmod a downloaded wifi module
driver and the kernel panic right away. Unfortunately we don't have
the source code but adding this patch that it proves that the module
driver was passing on a vmalloc area buffer for sdio APIs.

It could very easy to be reproduced by writing a simple function
module driver and insmod it, and panic log looks like:

unsigned u8 __aligned(32) buf[PAGE_SIZE];

static int wifi_probe(struct sdio_func *func, const struct
sdio_device_id *id)
{
	sdio_claim_host(func);
	sdio_enable_func(func);
	sdio_memcpy_toio(func, 0x0, buf, 0x200);
}

[  236.748210] wifi_probe: buf = 0xffffff8000a40b80
[  236.748258] swiotlb_tbl_map_single: orig_addr = 0xfffffffff8c40b80,
tlb_addr = 0xf7eae000 //I added log here
[  236.748276] Unable to handle kernel paging request at virtual address
fffffffff8c40b80
[  236.776486] pgd = ffffffc0b3417000
[  236.776789] [fffffffff8c40b80] *pgd=00000000b3427003,
*pud=00000000b3427003, *pmd=0000000000000000
[  236.777601] Internal error: Oops: 96000005 [#1] PREEMPT SMP
[  236.778093] Modules linked in: drvtst(O+)
[  236.778463] CPU: 0 PID: 1918 Comm: insmod Tainted: G           O
4.4.36 #25
[  236.779096] Hardware name: Rockchip RK3399 Evaluation Board v3 edp
(Android) (DT)
[  236.779753] task: ffffffc0e3db0c40 ti: ffffffc0b342c000 task.ti:
ffffffc0b342c000
[  236.780418] PC is at __memcpy+0x100/0x180
[  236.780777] LR is at swiotlb_tbl_map_single+0x254/0x274
[  236.781237] pc : [<ffffff80083579c0>] lr : [<ffffff800837cf70>]
...

[  236.941392] f460: 3d20726464615f67 6666666666783020
[  236.941826] [<ffffff80083579c0>] __memcpy+0x100/0x180
[  236.942274] [<ffffff800837dc18>] swiotlb_map_sg_attrs+0xa8/0x170
[  236.942810] [<ffffff800809359c>] __swiotlb_map_sg_attrs+0x24/0x8c
[  236.943353] [<ffffff800871113c>]
dw_mci_pre_dma_transfer.isra.16+0xf0/0x11c
[  236.943967] [<ffffff80087129cc>] __dw_mci_start_request+0x17c/0x4d0
[  236.944520] [<ffffff80087132e0>] dw_mci_request+0xb8/0xf0
[  236.945002] [<ffffff80086f8b44>] __mmc_start_request+0x9c/0xc0
[  236.945520] [<ffffff80087ae0e8>]
mmc_start_request.part.17+0x100/0x11c
[  236.946097] [<ffffff80086f9c74>] mmc_wait_for_req+0x78/0x1a8
[  236.946600] [<ffffff800870421c>] mmc_io_rw_extended+0x27c/0x2fc
[  236.947124] [<ffffff8008705800>] sdio_io_rw_ext_helper+0x1e4/0x238
[  236.947670] [<ffffff8008705954>] sdio_memcpy_toio+0x24/0x2c
[  236.948169] [<ffffff8000a40108>] wifi_probe+0xa8/0x198 [drvtst]
[  236.948708] [<ffffff8008704734>] sdio_bus_probe+0xb0/0x140
[  236.949195] [<ffffff80084bfd6c>] driver_probe_device+0x118/0x2b0
[  236.949724] [<ffffff80084bff68>] __driver_attach+0x64/0x90
[  236.950212] [<ffffff80084bee18>] bus_for_each_dev+0x80/0xb0
[  236.950706] [<ffffff80084bf8ac>] driver_attach+0x20/0x28
[  236.951176] [<ffffff80084bf45c>] bus_add_driver+0xe8/0x1ec
[  236.951661] [<ffffff80084c0afc>] driver_register+0x98/0xe4
[  236.952147] [<ffffff8008704580>] sdio_register_driver+0x24/0x2c
[  236.952675] [<ffffff8000a40228>] wifi_sdio_init+0x30/0x68 [drvtst]
[  236.953241] [<ffffff8000a4400c>] wifi_drv_init+0xc/0x38 [drvtst]
[  236.953789] [<ffffff8008082ba4>] do_one_initcall+0x17c/0x198
[  236.954293] [<ffffff800815e564>] do_init_module+0x60/0x1b8
[  236.954779] [<ffffff80081150f0>] load_module+0x1660/0x1a50
[  236.955264] [<ffffff800811562c>] SyS_init_module+0x14c/0x180
[  236.955765] [<ffffff80080826f0>] el0_svc_naked+0x24/0x28

So the kernel crash since the vmalloc area buffer, on-stack buffer or
the on-module buffer aren't physical continuous and swiotlb couldn't find
the correct mapping address in fact. Anyway, don't give the chance to panic
kernel by using sdio APIs.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

Changes in v3:
- fix build issue reported by Kbuild Robot

Changes in v2:
- add new function to check the addr of vmalloc/module/stack

 drivers/mmc/core/sdio_io.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

kernel test robot Feb. 6, 2017, 9 a.m. UTC | #1 
Hi Shawn,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.10-rc7 next-20170203]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Shawn-Lin/mmc-sdio-check-the-buffer-address-for-sdio-API/20170206-163619
config: x86_64-randconfig-x008-201706 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/mmc/core/sdio_io.c: In function 'sdio_io_addr_sanity_check':
>> drivers/mmc/core/sdio_io.c:313:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^

vim +313 drivers/mmc/core/sdio_io.c

   297		 */
   298		return orig_sz;
   299	}
   300	EXPORT_SYMBOL_GPL(sdio_align_size);
   301	
   302	static int sdio_io_addr_sanity_check(void *buf)
   303	{
   304	#if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
   305		unsigned long addr = (unsigned long)buf;
   306	
   307		if ((addr >= MODULES_VADDR && addr < MODULES_END) ||
   308		    object_is_on_stack(buf))
   309			return 1;
   310	#else
   311		return (is_vmalloc_addr(buf) || object_is_on_stack(buf));
   312	#endif
 > 313	}
   314	
   315	/* Split an arbitrarily sized data transfer into several
   316	 * IO_RW_EXTENDED commands. */
   317	static int sdio_io_rw_ext_helper(struct sdio_func *func, int write,
   318		unsigned addr, int incr_addr, u8 *buf, unsigned size)
   319	{
   320		unsigned remainder = size;
   321		unsigned max_blocks;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/drivers/mmc/core/sdio_io.c b/drivers/mmc/core/sdio_io.c
index 406e5f0..a413a85 100644
--- a/drivers/mmc/core/sdio_io.c
+++ b/drivers/mmc/core/sdio_io.c
@@ -10,6 +10,7 @@ 
  */
 
 #include <linux/export.h>
+#include <linux/mm.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
 #include <linux/mmc/sdio.h>
@@ -298,6 +299,19 @@  unsigned int sdio_align_size(struct sdio_func *func, unsigned int sz)
 }
 EXPORT_SYMBOL_GPL(sdio_align_size);
 
+static int sdio_io_addr_sanity_check(void *buf)
+{
+#if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
+	unsigned long addr = (unsigned long)buf;
+
+	if ((addr >= MODULES_VADDR && addr < MODULES_END) ||
+	    object_is_on_stack(buf))
+		return 1;
+#else
+	return (is_vmalloc_addr(buf) || object_is_on_stack(buf));
+#endif
+}
+
 /* Split an arbitrarily sized data transfer into several
  * IO_RW_EXTENDED commands. */
 static int sdio_io_rw_ext_helper(struct sdio_func *func, int write,
@@ -307,7 +321,7 @@  static int sdio_io_rw_ext_helper(struct sdio_func *func, int write,
 	unsigned max_blocks;
 	int ret;
 
-	if (!func || (func->num > 7))
+	if (!func || (func->num > 7) || sdio_io_addr_sanity_check(buf))
 		return -EINVAL;
 
 	/* Do the bulk of the transfer using block mode (if supported). */