2022-08-09 07:59:36

by Ajay Singh

[permalink] [raw]
Subject: [PATCH v3] wifi: wilc1000: fix DMA on stack objects

Sometimes 'wilc_sdio_cmd53' is called with addresses pointing to an
object on the stack. Use dynamically allocated memory for cmd53 instead
of stack address which is not DMA'able.

Fixes: 5625f965d764 ("wilc1000: move wilc driver out of staging")
Reported-by: Michael Walle <[email protected]>
Suggested-by: Michael Walle <[email protected]>
Signed-off-by: Ajay Singh <[email protected]>
---
This patch is created based on [1] and changes are done as discussed in
the same thread.

[1]. https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

changes since v2:
- use 'bool' datatype instead of 'u8' for 'use_global_buf'
- add size fit check before copying to 'cmd53_buf' buffer in
wilc_sdio_cmd53()

changes since v1:
- add 'use_global_buf' variable to know when to use bounce buffer
- remove unnecessary goto label
- dynamically allocate 'vmm_table'

.../net/wireless/microchip/wilc1000/netdev.h | 1 +
.../net/wireless/microchip/wilc1000/sdio.c | 39 ++++++++++++++++---
.../net/wireless/microchip/wilc1000/wlan.c | 15 ++++++-
3 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.h b/drivers/net/wireless/microchip/wilc1000/netdev.h
index 43c085c74b7a..bb1a315a7b7e 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.h
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.h
@@ -245,6 +245,7 @@ struct wilc {
u8 *rx_buffer;
u32 rx_buffer_offset;
u8 *tx_buffer;
+ u32 *vmm_table;

struct txq_handle txq[NQUEUES];
int txq_entries;
diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c b/drivers/net/wireless/microchip/wilc1000/sdio.c
index 600cc57e9da2..7390f94cd4ca 100644
--- a/drivers/net/wireless/microchip/wilc1000/sdio.c
+++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
@@ -28,6 +28,7 @@ struct wilc_sdio {
u32 block_size;
bool isinit;
int has_thrpt_enh3;
+ u8 *cmd53_buf;
};

struct sdio_cmd52 {
@@ -47,6 +48,7 @@ struct sdio_cmd53 {
u32 count: 9;
u8 *buffer;
u32 block_size;
+ bool use_global_buf;
};

static const struct wilc_hif_func wilc_hif_sdio;
@@ -91,6 +93,8 @@ static int wilc_sdio_cmd53(struct wilc *wilc, struct sdio_cmd53 *cmd)
{
struct sdio_func *func = container_of(wilc->dev, struct sdio_func, dev);
int size, ret;
+ struct wilc_sdio *sdio_priv = wilc->bus_data;
+ u8 *buf = cmd->buffer;

sdio_claim_host(func);

@@ -101,12 +105,23 @@ static int wilc_sdio_cmd53(struct wilc *wilc, struct sdio_cmd53 *cmd)
else
size = cmd->count;

+ if (cmd->use_global_buf) {
+ if (size > sizeof(u32))
+ return -EINVAL;
+
+ buf = sdio_priv->cmd53_buf;
+ }
+
if (cmd->read_write) { /* write */
- ret = sdio_memcpy_toio(func, cmd->address,
- (void *)cmd->buffer, size);
+ if (cmd->use_global_buf)
+ memcpy(buf, cmd->buffer, size);
+
+ ret = sdio_memcpy_toio(func, cmd->address, buf, size);
} else { /* read */
- ret = sdio_memcpy_fromio(func, (void *)cmd->buffer,
- cmd->address, size);
+ ret = sdio_memcpy_fromio(func, buf, cmd->address, size);
+
+ if (cmd->use_global_buf)
+ memcpy(cmd->buffer, buf, size);
}

sdio_release_host(func);
@@ -128,6 +143,12 @@ static int wilc_sdio_probe(struct sdio_func *func,
if (!sdio_priv)
return -ENOMEM;

+ sdio_priv->cmd53_buf = kzalloc(sizeof(u32), GFP_KERNEL);
+ if (!sdio_priv->cmd53_buf) {
+ ret = -ENOMEM;
+ goto free;
+ }
+
ret = wilc_cfg80211_init(&wilc, &func->dev, WILC_HIF_SDIO,
&wilc_hif_sdio);
if (ret)
@@ -161,6 +182,7 @@ static int wilc_sdio_probe(struct sdio_func *func,
irq_dispose_mapping(wilc->dev_irq_num);
wilc_netdev_cleanup(wilc);
free:
+ kfree(sdio_priv->cmd53_buf);
kfree(sdio_priv);
return ret;
}
@@ -172,6 +194,7 @@ static void wilc_sdio_remove(struct sdio_func *func)

clk_disable_unprepare(wilc->rtc_clk);
wilc_netdev_cleanup(wilc);
+ kfree(sdio_priv->cmd53_buf);
kfree(sdio_priv);
}

@@ -375,8 +398,9 @@ static int wilc_sdio_write_reg(struct wilc *wilc, u32 addr, u32 data)
cmd.address = WILC_SDIO_FBR_DATA_REG;
cmd.block_mode = 0;
cmd.increment = 1;
- cmd.count = 4;
+ cmd.count = sizeof(u32);
cmd.buffer = (u8 *)&data;
+ cmd.use_global_buf = true;
cmd.block_size = sdio_priv->block_size;
ret = wilc_sdio_cmd53(wilc, &cmd);
if (ret)
@@ -414,6 +438,7 @@ static int wilc_sdio_write(struct wilc *wilc, u32 addr, u8 *buf, u32 size)
nblk = size / block_size;
nleft = size % block_size;

+ cmd.use_global_buf = false;
if (nblk > 0) {
cmd.block_mode = 1;
cmd.increment = 1;
@@ -492,8 +517,9 @@ static int wilc_sdio_read_reg(struct wilc *wilc, u32 addr, u32 *data)
cmd.address = WILC_SDIO_FBR_DATA_REG;
cmd.block_mode = 0;
cmd.increment = 1;
- cmd.count = 4;
+ cmd.count = sizeof(u32);
cmd.buffer = (u8 *)data;
+ cmd.use_global_buf = true;

cmd.block_size = sdio_priv->block_size;
ret = wilc_sdio_cmd53(wilc, &cmd);
@@ -535,6 +561,7 @@ static int wilc_sdio_read(struct wilc *wilc, u32 addr, u8 *buf, u32 size)
nblk = size / block_size;
nleft = size % block_size;

+ cmd.use_global_buf = false;
if (nblk > 0) {
cmd.block_mode = 1;
cmd.increment = 1;
diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.c b/drivers/net/wireless/microchip/wilc1000/wlan.c
index 947d9a0a494e..58bbf50081e4 100644
--- a/drivers/net/wireless/microchip/wilc1000/wlan.c
+++ b/drivers/net/wireless/microchip/wilc1000/wlan.c
@@ -714,7 +714,7 @@ int wilc_wlan_handle_txq(struct wilc *wilc, u32 *txq_count)
int ret = 0;
int counter;
int timeout;
- u32 vmm_table[WILC_VMM_TBL_SIZE];
+ u32 *vmm_table = wilc->vmm_table;
u8 ac_pkt_num_to_chip[NQUEUES] = {0, 0, 0, 0};
const struct wilc_hif_func *func;
int srcu_idx;
@@ -1252,6 +1252,8 @@ void wilc_wlan_cleanup(struct net_device *dev)
while ((rqe = wilc_wlan_rxq_remove(wilc)))
kfree(rqe);

+ kfree(wilc->vmm_table);
+ wilc->vmm_table = NULL;
kfree(wilc->rx_buffer);
wilc->rx_buffer = NULL;
kfree(wilc->tx_buffer);
@@ -1489,6 +1491,14 @@ int wilc_wlan_init(struct net_device *dev)
goto fail;
}

+ if (!wilc->vmm_table)
+ wilc->vmm_table = kzalloc(WILC_VMM_TBL_SIZE, GFP_KERNEL);
+
+ if (!wilc->vmm_table) {
+ ret = -ENOBUFS;
+ goto fail;
+ }
+
if (!wilc->tx_buffer)
wilc->tx_buffer = kmalloc(WILC_TX_BUFF_SIZE, GFP_KERNEL);

@@ -1513,7 +1523,8 @@ int wilc_wlan_init(struct net_device *dev)
return 0;

fail:
-
+ kfree(wilc->vmm_table);
+ wilc->vmm_table = NULL;
kfree(wilc->rx_buffer);
wilc->rx_buffer = NULL;
kfree(wilc->tx_buffer);
--
2.34.1


2022-08-10 12:16:05

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v3] wifi: wilc1000: fix DMA on stack objects

Am 2022-08-09 09:57, schrieb [email protected]:
> Sometimes 'wilc_sdio_cmd53' is called with addresses pointing to an
> object on the stack. Use dynamically allocated memory for cmd53 instead
> of stack address which is not DMA'able.
>
> Fixes: 5625f965d764 ("wilc1000: move wilc driver out of staging")
> Reported-by: Michael Walle <[email protected]>
> Suggested-by: Michael Walle <[email protected]>
> Signed-off-by: Ajay Singh <[email protected]>

Thanks!

Reviewed-by: Michael Walle <[email protected]>
Tested-by: Michael Walle <[email protected]>

-michael

2022-08-11 04:56:06

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v3] wifi: wilc1000: fix DMA on stack objects

Michael Walle <[email protected]> writes:

> Am 2022-08-09 09:57, schrieb [email protected]:
>> Sometimes 'wilc_sdio_cmd53' is called with addresses pointing to an
>> object on the stack. Use dynamically allocated memory for cmd53 instead
>> of stack address which is not DMA'able.
>>
>> Fixes: 5625f965d764 ("wilc1000: move wilc driver out of staging")
>> Reported-by: Michael Walle <[email protected]>
>> Suggested-by: Michael Walle <[email protected]>
>> Signed-off-by: Ajay Singh <[email protected]>
>
> Thanks!
>
> Reviewed-by: Michael Walle <[email protected]>
> Tested-by: Michael Walle <[email protected]>

Thanks Michael. My plan is to take this to the wireless tree, seems
important enough fix.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2022-08-22 07:28:56

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v3] wifi: wilc1000: fix DMA on stack objects

Hi Kalle,

Am 2022-08-11 06:55, schrieb Kalle Valo:
> Michael Walle <[email protected]> writes:
>
>> Am 2022-08-09 09:57, schrieb [email protected]:
>>> Sometimes 'wilc_sdio_cmd53' is called with addresses pointing to an
>>> object on the stack. Use dynamically allocated memory for cmd53
>>> instead
>>> of stack address which is not DMA'able.
>>>
>>> Fixes: 5625f965d764 ("wilc1000: move wilc driver out of staging")
>>> Reported-by: Michael Walle <[email protected]>
>>> Suggested-by: Michael Walle <[email protected]>
>>> Signed-off-by: Ajay Singh <[email protected]>
>>
>> Thanks!
>>
>> Reviewed-by: Michael Walle <[email protected]>
>> Tested-by: Michael Walle <[email protected]>
>
> Thanks Michael. My plan is to take this to the wireless tree, seems
> important enough fix.

Do you already have any plan, when you will pick this?

-michael

2022-08-29 16:15:56

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v3] wifi: wilc1000: fix DMA on stack objects

Michael Walle <[email protected]> writes:

> Am 2022-08-11 06:55, schrieb Kalle Valo:
>> Michael Walle <[email protected]> writes:
>>
>>> Am 2022-08-09 09:57, schrieb [email protected]:
>>>> Sometimes 'wilc_sdio_cmd53' is called with addresses pointing to an
>>>> object on the stack. Use dynamically allocated memory for cmd53
>>>> instead
>>>> of stack address which is not DMA'able.
>>>>
>>>> Fixes: 5625f965d764 ("wilc1000: move wilc driver out of staging")
>>>> Reported-by: Michael Walle <[email protected]>
>>>> Suggested-by: Michael Walle <[email protected]>
>>>> Signed-off-by: Ajay Singh <[email protected]>
>>>
>>> Thanks!
>>>
>>> Reviewed-by: Michael Walle <[email protected]>
>>> Tested-by: Michael Walle <[email protected]>
>>
>> Thanks Michael. My plan is to take this to the wireless tree, seems
>> important enough fix.
>
> Do you already have any plan, when you will pick this?

Any day now. I was on vacation but now back.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2022-08-30 16:47:00

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v3] wifi: wilc1000: fix DMA on stack objects

<[email protected]> wrote:

> Sometimes 'wilc_sdio_cmd53' is called with addresses pointing to an
> object on the stack. Use dynamically allocated memory for cmd53 instead
> of stack address which is not DMA'able.
>
> Fixes: 5625f965d764 ("wilc1000: move wilc driver out of staging")
> Reported-by: Michael Walle <[email protected]>
> Suggested-by: Michael Walle <[email protected]>
> Signed-off-by: Ajay Singh <[email protected]>
> Reviewed-by: Michael Walle <[email protected]>
> Tested-by: Michael Walle <[email protected]>

Patch applied to wireless.git, thanks.

40b717bfcefa wifi: wilc1000: fix DMA on stack objects

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches