Re: [PATCH v2] wifi: wilc1000: fix DMA on stack objects

[<Ajay.Kathat@microchip.com>]

Hi Michael,

On 05/08/22 13:36, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> the content is safe
>
> Hi,
>
> Am 2022-08-04 20:13, schrieb Ajay.Kathat@microchip.com:
>> From: Ajay Singh <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>
>> ---
>> 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 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    | 35 +++++++++++++++----
>>  .../net/wireless/microchip/wilc1000/wlan.c    | 15 ++++++--
>>  3 files changed, 43 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..b12f411aec06 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;
>> +     u8 use_global_buf;
>
> bool
>
Ok.
>>  };
>>
>>  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,19 @@ static int wilc_sdio_cmd53(struct wilc *wilc,
>> struct sdio_cmd53 *cmd)
>>       else
>>               size = cmd->count;
>>
>> +     if (cmd->use_global_buf)
>> +             buf = sdio_priv->cmd53_buf;
>
> There is no check if the size fits into the buffer. So maybe:
>
> if (size > sizeof(u32))
>   return -EINVAL;
>
Sure,  I will make the changes and send the updated patch.

Regards,
Ajay