[RFC PATCH] cw1200: use kmalloc() allocation instead of stack

Subsystems: cw1200 wlan driver, the rest

STALE1870d

10 messages, 4 authors, 2021-06-30 · open the first message on its own page

[RFC PATCH] cw1200: use kmalloc() allocation instead of stack

From: Jernej Skrabec <jernej.skrabec@gmail.com>
Date: 2021-06-22 20:23:55 

It turns out that if CONFIG_VMAP_STACK is enabled and src or dst is
memory allocated on stack, SDIO operations fail due to invalid memory
address conversion:

cw1200_wlan_sdio: Probe called
sunxi-mmc 4021000.mmc: DMA addr 0x0000800051eab954+4 overflow (mask ffffffff, bus limit 0).
WARNING: CPU: 2 PID: 152 at kernel/dma/direct.h:97 dma_direct_map_sg+0x26c/0x28c
CPU: 2 PID: 152 Comm: kworker/2:2 Not tainted 5.13.0-rc1-00026-g84114ef026b9-dirty #85
Hardware name: X96 Mate (DT)
Workqueue: events_freezable mmc_rescan
pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
pc : dma_direct_map_sg+0x26c/0x28c
lr : dma_direct_map_sg+0x26c/0x28c
sp : ffff800011eab540
x29: ffff800011eab540 x28: ffff800011eab738 x27: 0000000000000000
x26: ffff000001daf010 x25: 0000000000000000 x24: 0000000000000000
x23: 0000000000000002 x22: fffffc0000000000 x21: ffff8000113b0ab0
x20: ffff80001181abb0 x19: 0000000000000001 x18: ffffffffffffffff
x17: 00000000fa97f83f x16: 00000000d2e01bf8 x15: ffff8000117ffb1d
x14: ffffffffffffffff x13: ffff8000117ffb18 x12: fffffffffffc593f
x11: ffff800011676ad0 x10: fffffffffffe0000 x9 : ffff800011eab540
x8 : 206b73616d282077 x7 : 000000000000000f x6 : 000000000000000c
x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00000283b800
Call trace:
 dma_direct_map_sg+0x26c/0x28c
 dma_map_sg_attrs+0x2c/0x60
 sunxi_mmc_request+0x70/0x420
 __mmc_start_request+0x68/0x134
 mmc_start_request+0x84/0xac
 mmc_wait_for_req+0x70/0x100
 mmc_io_rw_extended+0x1cc/0x2c0
 sdio_io_rw_ext_helper+0x194/0x240
 sdio_memcpy_fromio+0x20/0x2c
 cw1200_sdio_memcpy_fromio+0x20/0x2c
 __cw1200_reg_read+0x34/0x60
 cw1200_reg_read+0x48/0x70
 cw1200_load_firmware+0x38/0x5d0
 cw1200_core_probe+0x794/0x970
 cw1200_sdio_probe+0x124/0x22c
 sdio_bus_probe+0xe8/0x1d0
 really_probe+0xe4/0x504
 driver_probe_device+0x64/0xcc
 __device_attach_driver+0xd0/0x14c
 bus_for_each_drv+0x78/0xd0
 __device_attach+0xdc/0x184
 device_initial_probe+0x14/0x20
 bus_probe_device+0x9c/0xa4
 device_add+0x350/0x83c
 sdio_add_func+0x6c/0x90
 mmc_attach_sdio+0x1b0/0x430
 mmc_rescan+0x254/0x2e0
 process_one_work+0x1d0/0x34c
 worker_thread+0x13c/0x470
 kthread+0x154/0x160
 ret_from_fork+0x10/0x34
sunxi-mmc 4021000.mmc: dma_map_sg failed
sunxi-mmc 4021000.mmc: map DMA failed
Can't read config register.

Fix that by using kmalloc() allocated memory for read/write 16/32
funtions.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/net/wireless/st/cw1200/hwio.c | 52 +++++++++++++++++++++------
 drivers/net/wireless/st/cw1200/hwio.h | 51 ++++++++++++++++++++------
 2 files changed, 83 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/st/cw1200/hwio.c b/drivers/net/wireless/st/cw1200/hwio.c
index 3ba462de8e91..5521cb7f2233 100644
--- a/drivers/net/wireless/st/cw1200/hwio.c
+++ b/drivers/net/wireless/st/cw1200/hwio.c
@@ -66,33 +66,65 @@ static int __cw1200_reg_write(struct cw1200_common *priv, u16 addr,
 static inline int __cw1200_reg_read_32(struct cw1200_common *priv,
 					u16 addr, u32 *val)
 {
-	__le32 tmp;
-	int i = __cw1200_reg_read(priv, addr, &tmp, sizeof(tmp), 0);
-	*val = le32_to_cpu(tmp);
+	__le32 *tmp;
+	int i;
+
+	tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	i = __cw1200_reg_read(priv, addr, tmp, sizeof(*tmp), 0);
+	*val = le32_to_cpu(*tmp);
+	kfree(tmp);
 	return i;
 }
 
 static inline int __cw1200_reg_write_32(struct cw1200_common *priv,
 					u16 addr, u32 val)
 {
-	__le32 tmp = cpu_to_le32(val);
-	return __cw1200_reg_write(priv, addr, &tmp, sizeof(tmp), 0);
+	__le32 *tmp;
+	int i;
+
+	tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	*tmp = cpu_to_le32(val);
+	i = __cw1200_reg_write(priv, addr, tmp, sizeof(*tmp), 0);
+	kfree(tmp);
+	return i;
 }
 
 static inline int __cw1200_reg_read_16(struct cw1200_common *priv,
 					u16 addr, u16 *val)
 {
-	__le16 tmp;
-	int i = __cw1200_reg_read(priv, addr, &tmp, sizeof(tmp), 0);
-	*val = le16_to_cpu(tmp);
+	__le16 *tmp;
+	int i;
+
+	tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	i = __cw1200_reg_read(priv, addr, tmp, sizeof(*tmp), 0);
+	*val = le16_to_cpu(*tmp);
+	kfree(tmp);
 	return i;
 }
 
 static inline int __cw1200_reg_write_16(struct cw1200_common *priv,
 					u16 addr, u16 val)
 {
-	__le16 tmp = cpu_to_le16(val);
-	return __cw1200_reg_write(priv, addr, &tmp, sizeof(tmp), 0);
+	__le16 *tmp;
+	int i;
+
+	tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	*tmp = cpu_to_le16(val);
+	i = __cw1200_reg_write(priv, addr, tmp, sizeof(*tmp), 0);
+	kfree(tmp);
+	return i;
 }
 
 int cw1200_reg_read(struct cw1200_common *priv, u16 addr, void *buf,
diff --git a/drivers/net/wireless/st/cw1200/hwio.h b/drivers/net/wireless/st/cw1200/hwio.h
index d1e629a566c2..088d2a1bacc0 100644
--- a/drivers/net/wireless/st/cw1200/hwio.h
+++ b/drivers/net/wireless/st/cw1200/hwio.h
@@ -166,34 +166,65 @@ int cw1200_reg_write(struct cw1200_common *priv, u16 addr,
 static inline int cw1200_reg_read_16(struct cw1200_common *priv,
 				     u16 addr, u16 *val)
 {
-	__le32 tmp;
+	__le32 *tmp;
 	int i;
-	i = cw1200_reg_read(priv, addr, &tmp, sizeof(tmp));
-	*val = le32_to_cpu(tmp) & 0xfffff;
+
+	tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	i = cw1200_reg_read(priv, addr, tmp, sizeof(*tmp));
+	*val = le32_to_cpu(*tmp) & 0xfffff;
+	kfree(tmp);
 	return i;
 }
 
 static inline int cw1200_reg_write_16(struct cw1200_common *priv,
 				      u16 addr, u16 val)
 {
-	__le32 tmp = cpu_to_le32((u32)val);
-	return cw1200_reg_write(priv, addr, &tmp, sizeof(tmp));
+	__le32 *tmp;
+	int i;
+
+	tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	*tmp = cpu_to_le32((u32)val);
+	i = cw1200_reg_write(priv, addr, tmp, sizeof(*tmp));
+	kfree(tmp);
+	return i;
 }
 
 static inline int cw1200_reg_read_32(struct cw1200_common *priv,
 				     u16 addr, u32 *val)
 {
-	__le32 tmp;
-	int i = cw1200_reg_read(priv, addr, &tmp, sizeof(tmp));
-	*val = le32_to_cpu(tmp);
+	__le32 *tmp;
+	int i;
+
+	tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	i = cw1200_reg_read(priv, addr, tmp, sizeof(*tmp));
+	*val = le32_to_cpu(*tmp);
+	kfree(tmp);
 	return i;
 }
 
 static inline int cw1200_reg_write_32(struct cw1200_common *priv,
 				      u16 addr, u32 val)
 {
-	__le32 tmp = cpu_to_le32(val);
-	return cw1200_reg_write(priv, addr, &tmp, sizeof(val));
+	__le32 *tmp;
+	int i;
+
+	tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	*tmp = cpu_to_le32(val);
+	i = cw1200_reg_write(priv, addr, tmp, sizeof(val));
+	kfree(tmp);
+	return i;
 }
 
 int cw1200_indirect_read(struct cw1200_common *priv, u32 addr, void *buf,
-- 
2.32.0

Re: [RFC PATCH] cw1200: use kmalloc() allocation instead of stack

From: Arnd Bergmann <arnd@arndb.de>
Date: 2021-06-22 20:33:22 

On Tue, Jun 22, 2021 at 10:24 PM Jernej Skrabec
[off-list ref] wrote:
It turns out that if CONFIG_VMAP_STACK is enabled and src or dst is
memory allocated on stack, SDIO operations fail due to invalid memory
address conversion:
Thank you for sending this!

It's worth pointing out that even without CONFIG_VMAP_STACK, using
dma_map_sg() on a stack variable is broken, though it will appear to
work most of the time but rarely cause a stack data corruption when
the cache management goes wrong.

This clearly needs to be fixed somewhere, if not with your patch, then
a similar one.
quoted hunk
diff --git a/drivers/net/wireless/st/cw1200/hwio.c b/drivers/net/wireless/st/cw1200/hwio.c
index 3ba462de8e91..5521cb7f2233 100644
--- a/drivers/net/wireless/st/cw1200/hwio.c
+++ b/drivers/net/wireless/st/cw1200/hwio.c
@@ -66,33 +66,65 @@ static int __cw1200_reg_write(struct cw1200_common *priv, u16 addr,
 static inline int __cw1200_reg_read_32(struct cw1200_common *priv,
                                        u16 addr, u32 *val)
 {
-       __le32 tmp;
-       int i = __cw1200_reg_read(priv, addr, &tmp, sizeof(tmp), 0);
-       *val = le32_to_cpu(tmp);
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       i = __cw1200_reg_read(priv, addr, tmp, sizeof(*tmp), 0);
+       *val = le32_to_cpu(*tmp);
+       kfree(tmp);
        return i;
 }
There is a possible problem here when the function gets called from
atomic context, so it might need to use GFP_ATOMIC instead of
GFP_KERNEL. If it's never called from atomic context, then this patch
looks correct to me.

The alternative would be to add a bounce buffer check based on
is_vmalloc_or_module_addr() in sdio_io_rw_ext_helper(), which would
add a small bit of complexity there but solve the problem for
all drivers at once. In this case, it would probably have to use
GFP_ATOMIC regardless of whether __cw1200_reg_read_32()
is allowed to sleep, since other callers might not.

      Arnd

Re: [RFC PATCH] cw1200: use kmalloc() allocation instead of stack

From: Ulf Hansson <hidden>
Date: 2021-06-30 09:56:25 

On Tue, 22 Jun 2021 at 22:33, Arnd Bergmann [off-list ref] wrote:
On Tue, Jun 22, 2021 at 10:24 PM Jernej Skrabec
[off-list ref] wrote:
quoted
It turns out that if CONFIG_VMAP_STACK is enabled and src or dst is
memory allocated on stack, SDIO operations fail due to invalid memory
address conversion:
Thank you for sending this!

It's worth pointing out that even without CONFIG_VMAP_STACK, using
dma_map_sg() on a stack variable is broken, though it will appear to
work most of the time but rarely cause a stack data corruption when
the cache management goes wrong.

This clearly needs to be fixed somewhere, if not with your patch, then
a similar one.
quoted
diff --git a/drivers/net/wireless/st/cw1200/hwio.c b/drivers/net/wireless/st/cw1200/hwio.c
index 3ba462de8e91..5521cb7f2233 100644
--- a/drivers/net/wireless/st/cw1200/hwio.c
+++ b/drivers/net/wireless/st/cw1200/hwio.c
@@ -66,33 +66,65 @@ static int __cw1200_reg_write(struct cw1200_common *priv, u16 addr,
 static inline int __cw1200_reg_read_32(struct cw1200_common *priv,
                                        u16 addr, u32 *val)
 {
-       __le32 tmp;
-       int i = __cw1200_reg_read(priv, addr, &tmp, sizeof(tmp), 0);
-       *val = le32_to_cpu(tmp);
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       i = __cw1200_reg_read(priv, addr, tmp, sizeof(*tmp), 0);
+       *val = le32_to_cpu(*tmp);
+       kfree(tmp);
        return i;
 }
There is a possible problem here when the function gets called from
atomic context, so it might need to use GFP_ATOMIC instead of
GFP_KERNEL. If it's never called from atomic context, then this patch
looks correct to me.
I would be surprised if this is called from atomic context (when IRQs
are turned off), because in most cases, to complete the read/write
request the mmc controller driver relies on IRQs being delivered.

The alternative would be to add a bounce buffer check based on
is_vmalloc_or_module_addr() in sdio_io_rw_ext_helper(), which would
add a small bit of complexity there but solve the problem for
all drivers at once. In this case, it would probably have to use
GFP_ATOMIC regardless of whether __cw1200_reg_read_32()
is allowed to sleep, since other callers might not.
I like the idea, but...

I don't think we should see this as an alternative, but rather as a
complement which would have performance issues. A warning should be
printed, if the buffer isn't properly allocated.

Additionally, I don't think GFT_ATOMIC should be needed.

Kind regards
Uffe

Re: [RFC PATCH] cw1200: use kmalloc() allocation instead of stack

From: Ulf Hansson <hidden>
Date: 2021-06-30 10:04:02 

On Tue, 22 Jun 2021 at 22:23, Jernej Skrabec [off-list ref] wrote:
It turns out that if CONFIG_VMAP_STACK is enabled and src or dst is
memory allocated on stack, SDIO operations fail due to invalid memory
address conversion:

cw1200_wlan_sdio: Probe called
sunxi-mmc 4021000.mmc: DMA addr 0x0000800051eab954+4 overflow (mask ffffffff, bus limit 0).
WARNING: CPU: 2 PID: 152 at kernel/dma/direct.h:97 dma_direct_map_sg+0x26c/0x28c
CPU: 2 PID: 152 Comm: kworker/2:2 Not tainted 5.13.0-rc1-00026-g84114ef026b9-dirty #85
Hardware name: X96 Mate (DT)
Workqueue: events_freezable mmc_rescan
pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
pc : dma_direct_map_sg+0x26c/0x28c
lr : dma_direct_map_sg+0x26c/0x28c
sp : ffff800011eab540
x29: ffff800011eab540 x28: ffff800011eab738 x27: 0000000000000000
x26: ffff000001daf010 x25: 0000000000000000 x24: 0000000000000000
x23: 0000000000000002 x22: fffffc0000000000 x21: ffff8000113b0ab0
x20: ffff80001181abb0 x19: 0000000000000001 x18: ffffffffffffffff
x17: 00000000fa97f83f x16: 00000000d2e01bf8 x15: ffff8000117ffb1d
x14: ffffffffffffffff x13: ffff8000117ffb18 x12: fffffffffffc593f
x11: ffff800011676ad0 x10: fffffffffffe0000 x9 : ffff800011eab540
x8 : 206b73616d282077 x7 : 000000000000000f x6 : 000000000000000c
x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00000283b800
Call trace:
 dma_direct_map_sg+0x26c/0x28c
 dma_map_sg_attrs+0x2c/0x60
 sunxi_mmc_request+0x70/0x420
 __mmc_start_request+0x68/0x134
 mmc_start_request+0x84/0xac
 mmc_wait_for_req+0x70/0x100
 mmc_io_rw_extended+0x1cc/0x2c0
 sdio_io_rw_ext_helper+0x194/0x240
 sdio_memcpy_fromio+0x20/0x2c
 cw1200_sdio_memcpy_fromio+0x20/0x2c
 __cw1200_reg_read+0x34/0x60
 cw1200_reg_read+0x48/0x70
 cw1200_load_firmware+0x38/0x5d0
 cw1200_core_probe+0x794/0x970
 cw1200_sdio_probe+0x124/0x22c
 sdio_bus_probe+0xe8/0x1d0
 really_probe+0xe4/0x504
 driver_probe_device+0x64/0xcc
 __device_attach_driver+0xd0/0x14c
 bus_for_each_drv+0x78/0xd0
 __device_attach+0xdc/0x184
 device_initial_probe+0x14/0x20
 bus_probe_device+0x9c/0xa4
 device_add+0x350/0x83c
 sdio_add_func+0x6c/0x90
 mmc_attach_sdio+0x1b0/0x430
 mmc_rescan+0x254/0x2e0
 process_one_work+0x1d0/0x34c
 worker_thread+0x13c/0x470
 kthread+0x154/0x160
 ret_from_fork+0x10/0x34
sunxi-mmc 4021000.mmc: dma_map_sg failed
sunxi-mmc 4021000.mmc: map DMA failed
Can't read config register.

Fix that by using kmalloc() allocated memory for read/write 16/32
funtions.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Reviewed-by: Ulf Hansson <redacted>

Kind regards
Uffe
quoted hunk
---
 drivers/net/wireless/st/cw1200/hwio.c | 52 +++++++++++++++++++++------
 drivers/net/wireless/st/cw1200/hwio.h | 51 ++++++++++++++++++++------
 2 files changed, 83 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/st/cw1200/hwio.c b/drivers/net/wireless/st/cw1200/hwio.c
index 3ba462de8e91..5521cb7f2233 100644
--- a/drivers/net/wireless/st/cw1200/hwio.c
+++ b/drivers/net/wireless/st/cw1200/hwio.c
@@ -66,33 +66,65 @@ static int __cw1200_reg_write(struct cw1200_common *priv, u16 addr,
 static inline int __cw1200_reg_read_32(struct cw1200_common *priv,
                                        u16 addr, u32 *val)
 {
-       __le32 tmp;
-       int i = __cw1200_reg_read(priv, addr, &tmp, sizeof(tmp), 0);
-       *val = le32_to_cpu(tmp);
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       i = __cw1200_reg_read(priv, addr, tmp, sizeof(*tmp), 0);
+       *val = le32_to_cpu(*tmp);
+       kfree(tmp);
        return i;
 }

 static inline int __cw1200_reg_write_32(struct cw1200_common *priv,
                                        u16 addr, u32 val)
 {
-       __le32 tmp = cpu_to_le32(val);
-       return __cw1200_reg_write(priv, addr, &tmp, sizeof(tmp), 0);
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       *tmp = cpu_to_le32(val);
+       i = __cw1200_reg_write(priv, addr, tmp, sizeof(*tmp), 0);
+       kfree(tmp);
+       return i;
 }

 static inline int __cw1200_reg_read_16(struct cw1200_common *priv,
                                        u16 addr, u16 *val)
 {
-       __le16 tmp;
-       int i = __cw1200_reg_read(priv, addr, &tmp, sizeof(tmp), 0);
-       *val = le16_to_cpu(tmp);
+       __le16 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       i = __cw1200_reg_read(priv, addr, tmp, sizeof(*tmp), 0);
+       *val = le16_to_cpu(*tmp);
+       kfree(tmp);
        return i;
 }

 static inline int __cw1200_reg_write_16(struct cw1200_common *priv,
                                        u16 addr, u16 val)
 {
-       __le16 tmp = cpu_to_le16(val);
-       return __cw1200_reg_write(priv, addr, &tmp, sizeof(tmp), 0);
+       __le16 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       *tmp = cpu_to_le16(val);
+       i = __cw1200_reg_write(priv, addr, tmp, sizeof(*tmp), 0);
+       kfree(tmp);
+       return i;
 }

 int cw1200_reg_read(struct cw1200_common *priv, u16 addr, void *buf,
diff --git a/drivers/net/wireless/st/cw1200/hwio.h b/drivers/net/wireless/st/cw1200/hwio.h
index d1e629a566c2..088d2a1bacc0 100644
--- a/drivers/net/wireless/st/cw1200/hwio.h
+++ b/drivers/net/wireless/st/cw1200/hwio.h
@@ -166,34 +166,65 @@ int cw1200_reg_write(struct cw1200_common *priv, u16 addr,
 static inline int cw1200_reg_read_16(struct cw1200_common *priv,
                                     u16 addr, u16 *val)
 {
-       __le32 tmp;
+       __le32 *tmp;
        int i;
-       i = cw1200_reg_read(priv, addr, &tmp, sizeof(tmp));
-       *val = le32_to_cpu(tmp) & 0xfffff;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       i = cw1200_reg_read(priv, addr, tmp, sizeof(*tmp));
+       *val = le32_to_cpu(*tmp) & 0xfffff;
+       kfree(tmp);
        return i;
 }

 static inline int cw1200_reg_write_16(struct cw1200_common *priv,
                                      u16 addr, u16 val)
 {
-       __le32 tmp = cpu_to_le32((u32)val);
-       return cw1200_reg_write(priv, addr, &tmp, sizeof(tmp));
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       *tmp = cpu_to_le32((u32)val);
+       i = cw1200_reg_write(priv, addr, tmp, sizeof(*tmp));
+       kfree(tmp);
+       return i;
 }

 static inline int cw1200_reg_read_32(struct cw1200_common *priv,
                                     u16 addr, u32 *val)
 {
-       __le32 tmp;
-       int i = cw1200_reg_read(priv, addr, &tmp, sizeof(tmp));
-       *val = le32_to_cpu(tmp);
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       i = cw1200_reg_read(priv, addr, tmp, sizeof(*tmp));
+       *val = le32_to_cpu(*tmp);
+       kfree(tmp);
        return i;
 }

 static inline int cw1200_reg_write_32(struct cw1200_common *priv,
                                      u16 addr, u32 val)
 {
-       __le32 tmp = cpu_to_le32(val);
-       return cw1200_reg_write(priv, addr, &tmp, sizeof(val));
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       *tmp = cpu_to_le32(val);
+       i = cw1200_reg_write(priv, addr, tmp, sizeof(val));
+       kfree(tmp);
+       return i;
 }

 int cw1200_indirect_read(struct cw1200_common *priv, u32 addr, void *buf,
--
2.32.0

Re: [RFC PATCH] cw1200: use kmalloc() allocation instead of stack

From: Jernej Škrabec <jernej.skrabec@gmail.com>
Date: 2021-06-30 10:09:42 

Hi Ulf!

Dne sreda, 30. junij 2021 ob 12:03:13 CEST je Ulf Hansson napisal(a):
On Tue, 22 Jun 2021 at 22:23, Jernej Skrabec [off-list ref] 
wrote:
quoted
It turns out that if CONFIG_VMAP_STACK is enabled and src or dst is
memory allocated on stack, SDIO operations fail due to invalid memory
address conversion:

cw1200_wlan_sdio: Probe called
sunxi-mmc 4021000.mmc: DMA addr 0x0000800051eab954+4 overflow (mask
ffffffff, bus limit 0). WARNING: CPU: 2 PID: 152 at
kernel/dma/direct.h:97 dma_direct_map_sg+0x26c/0x28c CPU: 2 PID: 152
Comm: kworker/2:2 Not tainted 5.13.0-rc1-00026-g84114ef026b9-dirty #85
Hardware name: X96 Mate (DT)
Workqueue: events_freezable mmc_rescan
pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
pc : dma_direct_map_sg+0x26c/0x28c
lr : dma_direct_map_sg+0x26c/0x28c
sp : ffff800011eab540
x29: ffff800011eab540 x28: ffff800011eab738 x27: 0000000000000000
x26: ffff000001daf010 x25: 0000000000000000 x24: 0000000000000000
x23: 0000000000000002 x22: fffffc0000000000 x21: ffff8000113b0ab0
x20: ffff80001181abb0 x19: 0000000000000001 x18: ffffffffffffffff
x17: 00000000fa97f83f x16: 00000000d2e01bf8 x15: ffff8000117ffb1d
x14: ffffffffffffffff x13: ffff8000117ffb18 x12: fffffffffffc593f
x11: ffff800011676ad0 x10: fffffffffffe0000 x9 : ffff800011eab540
x8 : 206b73616d282077 x7 : 000000000000000f x6 : 000000000000000c
x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00000283b800

Call trace:
 dma_direct_map_sg+0x26c/0x28c
 dma_map_sg_attrs+0x2c/0x60
 sunxi_mmc_request+0x70/0x420
 __mmc_start_request+0x68/0x134
 mmc_start_request+0x84/0xac
 mmc_wait_for_req+0x70/0x100
 mmc_io_rw_extended+0x1cc/0x2c0
 sdio_io_rw_ext_helper+0x194/0x240
 sdio_memcpy_fromio+0x20/0x2c
 cw1200_sdio_memcpy_fromio+0x20/0x2c
 __cw1200_reg_read+0x34/0x60
 cw1200_reg_read+0x48/0x70
 cw1200_load_firmware+0x38/0x5d0
 cw1200_core_probe+0x794/0x970
 cw1200_sdio_probe+0x124/0x22c
 sdio_bus_probe+0xe8/0x1d0
 really_probe+0xe4/0x504
 driver_probe_device+0x64/0xcc
 __device_attach_driver+0xd0/0x14c
 bus_for_each_drv+0x78/0xd0
 __device_attach+0xdc/0x184
 device_initial_probe+0x14/0x20
 bus_probe_device+0x9c/0xa4
 device_add+0x350/0x83c
 sdio_add_func+0x6c/0x90
 mmc_attach_sdio+0x1b0/0x430
 mmc_rescan+0x254/0x2e0
 process_one_work+0x1d0/0x34c
 worker_thread+0x13c/0x470
 kthread+0x154/0x160
 ret_from_fork+0x10/0x34

sunxi-mmc 4021000.mmc: dma_map_sg failed
sunxi-mmc 4021000.mmc: map DMA failed
Can't read config register.

Fix that by using kmalloc() allocated memory for read/write 16/32
funtions.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Reviewed-by: Ulf Hansson <redacted>
Thanks! But I found few more places which need this kind of fix:
https://github.com/jernejsk/linux-1/commit/
1cba9a7764c7d5bbdeb4ddeaa91ff20a0339f6ff

I guess I can keep R-b tag?

Best regards,
Jernej

Kind regards
Uffe
quoted
---

 drivers/net/wireless/st/cw1200/hwio.c | 52 +++++++++++++++++++++------
 drivers/net/wireless/st/cw1200/hwio.h | 51 ++++++++++++++++++++------
 2 files changed, 83 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/st/cw1200/hwio.c
b/drivers/net/wireless/st/cw1200/hwio.c index 3ba462de8e91..5521cb7f2233
100644
--- a/drivers/net/wireless/st/cw1200/hwio.c
+++ b/drivers/net/wireless/st/cw1200/hwio.c
@@ -66,33 +66,65 @@ static int __cw1200_reg_write(struct cw1200_common
*priv, u16 addr,> 
 static inline int __cw1200_reg_read_32(struct cw1200_common *priv,
 
                                        u16 addr, u32 *val)
 
 {

-       __le32 tmp;
-       int i = __cw1200_reg_read(priv, addr, &tmp, sizeof(tmp), 0);
-       *val = le32_to_cpu(tmp);
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       i = __cw1200_reg_read(priv, addr, tmp, sizeof(*tmp), 0);
+       *val = le32_to_cpu(*tmp);
+       kfree(tmp);

        return i;
 
 }
 
 static inline int __cw1200_reg_write_32(struct cw1200_common *priv,
 
                                        u16 addr, u32 val)
 
 {

-       __le32 tmp = cpu_to_le32(val);
-       return __cw1200_reg_write(priv, addr, &tmp, sizeof(tmp), 0);
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       *tmp = cpu_to_le32(val);
+       i = __cw1200_reg_write(priv, addr, tmp, sizeof(*tmp), 0);
+       kfree(tmp);
+       return i;

 }
 
 static inline int __cw1200_reg_read_16(struct cw1200_common *priv,
 
                                        u16 addr, u16 *val)
 
 {

-       __le16 tmp;
-       int i = __cw1200_reg_read(priv, addr, &tmp, sizeof(tmp), 0);
-       *val = le16_to_cpu(tmp);
+       __le16 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       i = __cw1200_reg_read(priv, addr, tmp, sizeof(*tmp), 0);
+       *val = le16_to_cpu(*tmp);
+       kfree(tmp);

        return i;
 
 }
 
 static inline int __cw1200_reg_write_16(struct cw1200_common *priv,
 
                                        u16 addr, u16 val)
 
 {

-       __le16 tmp = cpu_to_le16(val);
-       return __cw1200_reg_write(priv, addr, &tmp, sizeof(tmp), 0);
+       __le16 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       *tmp = cpu_to_le16(val);
+       i = __cw1200_reg_write(priv, addr, tmp, sizeof(*tmp), 0);
+       kfree(tmp);
+       return i;

 }
 
 int cw1200_reg_read(struct cw1200_common *priv, u16 addr, void *buf,

diff --git a/drivers/net/wireless/st/cw1200/hwio.h
b/drivers/net/wireless/st/cw1200/hwio.h index d1e629a566c2..088d2a1bacc0
100644
--- a/drivers/net/wireless/st/cw1200/hwio.h
+++ b/drivers/net/wireless/st/cw1200/hwio.h
@@ -166,34 +166,65 @@ int cw1200_reg_write(struct cw1200_common *priv, u16
addr,> 
 static inline int cw1200_reg_read_16(struct cw1200_common *priv,
 
                                     u16 addr, u16 *val)
 
 {

-       __le32 tmp;
+       __le32 *tmp;

        int i;

-       i = cw1200_reg_read(priv, addr, &tmp, sizeof(tmp));
-       *val = le32_to_cpu(tmp) & 0xfffff;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       i = cw1200_reg_read(priv, addr, tmp, sizeof(*tmp));
+       *val = le32_to_cpu(*tmp) & 0xfffff;
+       kfree(tmp);

        return i;
 
 }
 
 static inline int cw1200_reg_write_16(struct cw1200_common *priv,
 
                                      u16 addr, u16 val)
 
 {

-       __le32 tmp = cpu_to_le32((u32)val);
-       return cw1200_reg_write(priv, addr, &tmp, sizeof(tmp));
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       *tmp = cpu_to_le32((u32)val);
+       i = cw1200_reg_write(priv, addr, tmp, sizeof(*tmp));
+       kfree(tmp);
+       return i;

 }
 
 static inline int cw1200_reg_read_32(struct cw1200_common *priv,
 
                                     u16 addr, u32 *val)
 
 {

-       __le32 tmp;
-       int i = cw1200_reg_read(priv, addr, &tmp, sizeof(tmp));
-       *val = le32_to_cpu(tmp);
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       i = cw1200_reg_read(priv, addr, tmp, sizeof(*tmp));
+       *val = le32_to_cpu(*tmp);
+       kfree(tmp);

        return i;
 
 }
 
 static inline int cw1200_reg_write_32(struct cw1200_common *priv,
 
                                      u16 addr, u32 val)
 
 {

-       __le32 tmp = cpu_to_le32(val);
-       return cw1200_reg_write(priv, addr, &tmp, sizeof(val));
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       *tmp = cpu_to_le32(val);
+       i = cw1200_reg_write(priv, addr, tmp, sizeof(val));
+       kfree(tmp);
+       return i;

 }
 
 int cw1200_indirect_read(struct cw1200_common *priv, u32 addr, void *buf,

--
2.32.0

Re: [RFC PATCH] cw1200: use kmalloc() allocation instead of stack

From: Arnd Bergmann <arnd@arndb.de>
Date: 2021-06-30 11:30:57 

On Wed, Jun 30, 2021 at 11:56 AM Ulf Hansson [off-list ref] wrote:
On Tue, 22 Jun 2021 at 22:33, Arnd Bergmann [off-list ref] wrote:
quoted
On Tue, Jun 22, 2021 at 10:24 PM Jernej Skrabec
[off-list ref] wrote:
quoted
It turns out that if CONFIG_VMAP_STACK is enabled and src or dst is
memory allocated on stack, SDIO operations fail due to invalid memory
address conversion:
Thank you for sending this!

It's worth pointing out that even without CONFIG_VMAP_STACK, using
dma_map_sg() on a stack variable is broken, though it will appear to
work most of the time but rarely cause a stack data corruption when
the cache management goes wrong.

This clearly needs to be fixed somewhere, if not with your patch, then
a similar one.
quoted
diff --git a/drivers/net/wireless/st/cw1200/hwio.c b/drivers/net/wireless/st/cw1200/hwio.c
index 3ba462de8e91..5521cb7f2233 100644
--- a/drivers/net/wireless/st/cw1200/hwio.c
+++ b/drivers/net/wireless/st/cw1200/hwio.c
@@ -66,33 +66,65 @@ static int __cw1200_reg_write(struct cw1200_common *priv, u16 addr,
 static inline int __cw1200_reg_read_32(struct cw1200_common *priv,
                                        u16 addr, u32 *val)
 {
-       __le32 tmp;
-       int i = __cw1200_reg_read(priv, addr, &tmp, sizeof(tmp), 0);
-       *val = le32_to_cpu(tmp);
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       i = __cw1200_reg_read(priv, addr, tmp, sizeof(*tmp), 0);
+       *val = le32_to_cpu(*tmp);
+       kfree(tmp);
        return i;
 }
There is a possible problem here when the function gets called from
atomic context, so it might need to use GFP_ATOMIC instead of
GFP_KERNEL. If it's never called from atomic context, then this patch
looks correct to me.
I would be surprised if this is called from atomic context (when IRQs
are turned off), because in most cases, to complete the read/write
request the mmc controller driver relies on IRQs being delivered.
I thought I had seen a spinlock in the forked driver, but I don't see
it now, so I probably misremembered that bit.
quoted
The alternative would be to add a bounce buffer check based on
is_vmalloc_or_module_addr() in sdio_io_rw_ext_helper(), which would
add a small bit of complexity there but solve the problem for
all drivers at once. In this case, it would probably have to use
GFP_ATOMIC regardless of whether __cw1200_reg_read_32()
is allowed to sleep, since other callers might not.
I like the idea, but...

I don't think we should see this as an alternative, but rather as a
complement which would have performance issues. A warning should be
printed, if the buffer isn't properly allocated.
Fair enough. I found the function call I was looking for: object_is_on_stack(),
the patch below should print a warning once when a driver passes
a bad buffer, but I did not test that.

There are some possible variations on that: an on-stack buffer by
itself can work as long as the DMA is cache-coherent and stacks
are not vmapped. For the is_vmalloc_or_module_addr() case,
we may decide to just return an error, rather than running into
a kernel oops.

Additionally, I don't think GFT_ATOMIC should be needed.
Ok, I now see the mmc_wait_for_req() in mmc_io_rw_extended()
that probably means it can not be called in atomic context at all,
and that GFP_KERNEL is safe, and that any driver calling it with
a spinlock held is already broken.

       Arnd

8<---
diff --git a/drivers/mmc/core/sdio_ops.c b/drivers/mmc/core/sdio_ops.c
index 4c229dd2b6e5..845f9ca3b200 100644
--- a/drivers/mmc/core/sdio_ops.c
+++ b/drivers/mmc/core/sdio_ops.c
@@ -124,6 +124,7 @@ int mmc_io_rw_extended(struct mmc_card *card, int
write, unsigned fn,
        int err;

        WARN_ON(blksz == 0);
+       WARN_ON_ONCE(is_vmalloc_or_module_addr(buf) || object_is_on_stack(buf));

        /* sanity check */
        if (addr & ~0x1FFFF)

Re: [RFC PATCH] cw1200: use kmalloc() allocation instead of stack

From: Ulf Hansson <hidden>
Date: 2021-06-30 12:00:44 

On Wed, 30 Jun 2021 at 12:09, Jernej Škrabec [off-list ref] wrote:
Hi Ulf!

Dne sreda, 30. junij 2021 ob 12:03:13 CEST je Ulf Hansson napisal(a):
quoted
On Tue, 22 Jun 2021 at 22:23, Jernej Skrabec [off-list ref]
wrote:
quoted
quoted
It turns out that if CONFIG_VMAP_STACK is enabled and src or dst is
memory allocated on stack, SDIO operations fail due to invalid memory
address conversion:

cw1200_wlan_sdio: Probe called
sunxi-mmc 4021000.mmc: DMA addr 0x0000800051eab954+4 overflow (mask
ffffffff, bus limit 0). WARNING: CPU: 2 PID: 152 at
kernel/dma/direct.h:97 dma_direct_map_sg+0x26c/0x28c CPU: 2 PID: 152
Comm: kworker/2:2 Not tainted 5.13.0-rc1-00026-g84114ef026b9-dirty #85
Hardware name: X96 Mate (DT)
Workqueue: events_freezable mmc_rescan
pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
pc : dma_direct_map_sg+0x26c/0x28c
lr : dma_direct_map_sg+0x26c/0x28c
sp : ffff800011eab540
x29: ffff800011eab540 x28: ffff800011eab738 x27: 0000000000000000
x26: ffff000001daf010 x25: 0000000000000000 x24: 0000000000000000
x23: 0000000000000002 x22: fffffc0000000000 x21: ffff8000113b0ab0
x20: ffff80001181abb0 x19: 0000000000000001 x18: ffffffffffffffff
x17: 00000000fa97f83f x16: 00000000d2e01bf8 x15: ffff8000117ffb1d
x14: ffffffffffffffff x13: ffff8000117ffb18 x12: fffffffffffc593f
x11: ffff800011676ad0 x10: fffffffffffe0000 x9 : ffff800011eab540
x8 : 206b73616d282077 x7 : 000000000000000f x6 : 000000000000000c
x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00000283b800

Call trace:
 dma_direct_map_sg+0x26c/0x28c
 dma_map_sg_attrs+0x2c/0x60
 sunxi_mmc_request+0x70/0x420
 __mmc_start_request+0x68/0x134
 mmc_start_request+0x84/0xac
 mmc_wait_for_req+0x70/0x100
 mmc_io_rw_extended+0x1cc/0x2c0
 sdio_io_rw_ext_helper+0x194/0x240
 sdio_memcpy_fromio+0x20/0x2c
 cw1200_sdio_memcpy_fromio+0x20/0x2c
 __cw1200_reg_read+0x34/0x60
 cw1200_reg_read+0x48/0x70
 cw1200_load_firmware+0x38/0x5d0
 cw1200_core_probe+0x794/0x970
 cw1200_sdio_probe+0x124/0x22c
 sdio_bus_probe+0xe8/0x1d0
 really_probe+0xe4/0x504
 driver_probe_device+0x64/0xcc
 __device_attach_driver+0xd0/0x14c
 bus_for_each_drv+0x78/0xd0
 __device_attach+0xdc/0x184
 device_initial_probe+0x14/0x20
 bus_probe_device+0x9c/0xa4
 device_add+0x350/0x83c
 sdio_add_func+0x6c/0x90
 mmc_attach_sdio+0x1b0/0x430
 mmc_rescan+0x254/0x2e0
 process_one_work+0x1d0/0x34c
 worker_thread+0x13c/0x470
 kthread+0x154/0x160
 ret_from_fork+0x10/0x34

sunxi-mmc 4021000.mmc: dma_map_sg failed
sunxi-mmc 4021000.mmc: map DMA failed
Can't read config register.

Fix that by using kmalloc() allocated memory for read/write 16/32
funtions.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Reviewed-by: Ulf Hansson <redacted>
Thanks! But I found few more places which need this kind of fix:
https://github.com/jernejsk/linux-1/commit/
1cba9a7764c7d5bbdeb4ddeaa91ff20a0339f6ff
I couldn't find it.

I guess I can keep R-b tag?
Well, just send a new version and I will respond to it again, no
worries. Or send an additional one on top.

[...]

Kind regards
Uffe

Re: [RFC PATCH] cw1200: use kmalloc() allocation instead of stack

From: Ulf Hansson <hidden>
Date: 2021-06-30 12:04:09 

On Wed, 30 Jun 2021 at 13:30, Arnd Bergmann [off-list ref] wrote:
quoted hunk
On Wed, Jun 30, 2021 at 11:56 AM Ulf Hansson [off-list ref] wrote:
quoted
On Tue, 22 Jun 2021 at 22:33, Arnd Bergmann [off-list ref] wrote:
quoted
On Tue, Jun 22, 2021 at 10:24 PM Jernej Skrabec
[off-list ref] wrote:
quoted
It turns out that if CONFIG_VMAP_STACK is enabled and src or dst is
memory allocated on stack, SDIO operations fail due to invalid memory
address conversion:
Thank you for sending this!

It's worth pointing out that even without CONFIG_VMAP_STACK, using
dma_map_sg() on a stack variable is broken, though it will appear to
work most of the time but rarely cause a stack data corruption when
the cache management goes wrong.

This clearly needs to be fixed somewhere, if not with your patch, then
a similar one.
quoted
diff --git a/drivers/net/wireless/st/cw1200/hwio.c b/drivers/net/wireless/st/cw1200/hwio.c
index 3ba462de8e91..5521cb7f2233 100644
--- a/drivers/net/wireless/st/cw1200/hwio.c
+++ b/drivers/net/wireless/st/cw1200/hwio.c
@@ -66,33 +66,65 @@ static int __cw1200_reg_write(struct cw1200_common *priv, u16 addr,
 static inline int __cw1200_reg_read_32(struct cw1200_common *priv,
                                        u16 addr, u32 *val)
 {
-       __le32 tmp;
-       int i = __cw1200_reg_read(priv, addr, &tmp, sizeof(tmp), 0);
-       *val = le32_to_cpu(tmp);
+       __le32 *tmp;
+       int i;
+
+       tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+       if (!tmp)
+               return -ENOMEM;
+
+       i = __cw1200_reg_read(priv, addr, tmp, sizeof(*tmp), 0);
+       *val = le32_to_cpu(*tmp);
+       kfree(tmp);
        return i;
 }
There is a possible problem here when the function gets called from
atomic context, so it might need to use GFP_ATOMIC instead of
GFP_KERNEL. If it's never called from atomic context, then this patch
looks correct to me.
I would be surprised if this is called from atomic context (when IRQs
are turned off), because in most cases, to complete the read/write
request the mmc controller driver relies on IRQs being delivered.
I thought I had seen a spinlock in the forked driver, but I don't see
it now, so I probably misremembered that bit.
quoted
quoted
The alternative would be to add a bounce buffer check based on
is_vmalloc_or_module_addr() in sdio_io_rw_ext_helper(), which would
add a small bit of complexity there but solve the problem for
all drivers at once. In this case, it would probably have to use
GFP_ATOMIC regardless of whether __cw1200_reg_read_32()
is allowed to sleep, since other callers might not.
I like the idea, but...

I don't think we should see this as an alternative, but rather as a
complement which would have performance issues. A warning should be
printed, if the buffer isn't properly allocated.
Fair enough. I found the function call I was looking for: object_is_on_stack(),
the patch below should print a warning once when a driver passes
a bad buffer, but I did not test that.

There are some possible variations on that: an on-stack buffer by
itself can work as long as the DMA is cache-coherent and stacks
are not vmapped. For the is_vmalloc_or_module_addr() case,
we may decide to just return an error, rather than running into
a kernel oops.
quoted
Additionally, I don't think GFT_ATOMIC should be needed.
Ok, I now see the mmc_wait_for_req() in mmc_io_rw_extended()
that probably means it can not be called in atomic context at all,
and that GFP_KERNEL is safe, and that any driver calling it with
a spinlock held is already broken.

       Arnd

8<---
diff --git a/drivers/mmc/core/sdio_ops.c b/drivers/mmc/core/sdio_ops.c
index 4c229dd2b6e5..845f9ca3b200 100644
--- a/drivers/mmc/core/sdio_ops.c
+++ b/drivers/mmc/core/sdio_ops.c
@@ -124,6 +124,7 @@ int mmc_io_rw_extended(struct mmc_card *card, int
write, unsigned fn,
        int err;

        WARN_ON(blksz == 0);
+       WARN_ON_ONCE(is_vmalloc_or_module_addr(buf) || object_is_on_stack(buf));
Looks reasonable to me, at least we should start giving a warning.
Would you like to send a formal patch that we can test?

Kind regards
Uffe

Re: [RFC PATCH] cw1200: use kmalloc() allocation instead of stack

From: Arnd Bergmann <arnd@arndb.de>
Date: 2021-06-30 12:21:48 

On Wed, Jun 30, 2021 at 2:03 PM Ulf Hansson [off-list ref] wrote:
quoted
diff --git a/drivers/mmc/core/sdio_ops.c b/drivers/mmc/core/sdio_ops.c
index 4c229dd2b6e5..845f9ca3b200 100644
--- a/drivers/mmc/core/sdio_ops.c
+++ b/drivers/mmc/core/sdio_ops.c
@@ -124,6 +124,7 @@ int mmc_io_rw_extended(struct mmc_card *card, int
write, unsigned fn,
        int err;

        WARN_ON(blksz == 0);
+       WARN_ON_ONCE(is_vmalloc_or_module_addr(buf) || object_is_on_stack(buf));
Looks reasonable to me, at least we should start giving a warning.
Would you like to send a formal patch that we can test?
Done.

        Arnd

RE: [RFC PATCH] cw1200: use kmalloc() allocation instead of stack

From: David Laight <hidden>
Date: 2021-06-30 16:08:15 

From: Ulf Hansson
Sent: 30 June 2021 11:03
...
quoted
It turns out that if CONFIG_VMAP_STACK is enabled and src or dst is
memory allocated on stack, SDIO operations fail due to invalid memory
address conversion:
...
quoted
Fix that by using kmalloc() allocated memory for read/write 16/32
funtions.
Could a field be added to 'struct cw1200_common'
that the functions could use as a bounce buffer?

ISTM that is DMA are being done there must be some
serialisation in there somewhere that will stop
concurrent accesses.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help