| Message ID | 20220809075749.62752-1-ajay.kathat@microchip.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] wifi: wilc1000: fix DMA on stack objects | expand |
Am 2022-08-09 09:57, schrieb Ajay.Kathat@microchip.com: > 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 <mwalle@kernel.org> > Suggested-by: Michael Walle <mwalle@kernel.org> > Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> Thanks! Reviewed-by: Michael Walle <mwalle@kernel.org> Tested-by: Michael Walle <mwalle@kernel.org> -michael
Michael Walle <michael@walle.cc> writes: > Am 2022-08-09 09:57, schrieb Ajay.Kathat@microchip.com: >> 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 <mwalle@kernel.org> >> Suggested-by: Michael Walle <mwalle@kernel.org> >> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> > > Thanks! > > Reviewed-by: Michael Walle <mwalle@kernel.org> > Tested-by: Michael Walle <mwalle@kernel.org> Thanks Michael. My plan is to take this to the wireless tree, seems important enough fix.
Hi Kalle, Am 2022-08-11 06:55, schrieb Kalle Valo: > Michael Walle <michael@walle.cc> writes: > >> Am 2022-08-09 09:57, schrieb Ajay.Kathat@microchip.com: >>> 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 <mwalle@kernel.org> >>> Suggested-by: Michael Walle <mwalle@kernel.org> >>> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> >> >> Thanks! >> >> Reviewed-by: Michael Walle <mwalle@kernel.org> >> Tested-by: Michael Walle <mwalle@kernel.org> > > 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
Michael Walle <michael@walle.cc> writes: > Am 2022-08-11 06:55, schrieb Kalle Valo: >> Michael Walle <michael@walle.cc> writes: >> >>> Am 2022-08-09 09:57, schrieb Ajay.Kathat@microchip.com: >>>> 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 <mwalle@kernel.org> >>>> Suggested-by: Michael Walle <mwalle@kernel.org> >>>> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> >>> >>> Thanks! >>> >>> Reviewed-by: Michael Walle <mwalle@kernel.org> >>> Tested-by: Michael Walle <mwalle@kernel.org> >> >> 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.
<Ajay.Kathat@microchip.com> 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 <mwalle@kernel.org> > Suggested-by: Michael Walle <mwalle@kernel.org> > Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> > Reviewed-by: Michael Walle <mwalle@kernel.org> > Tested-by: Michael Walle <mwalle@kernel.org> Patch applied to wireless.git, thanks. 40b717bfcefa wifi: wilc1000: fix DMA on stack objects
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);
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 <mwalle@kernel.org> Suggested-by: Michael Walle <mwalle@kernel.org> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> --- 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/20220728152037.386543-1-michael@walle.cc/ 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(-)