lkml.org 
[lkml]   [2022]   [Jul]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH] wilc1000: fix DMA on stack objects
    Date
    From: Michael Walle
    > Sent: 28 July 2022 16:21
    >
    > From: Michael Walle <mwalle@kernel.org>
    >
    > Sometimes wilc_sdio_cmd53() is called with addresses pointing to an
    > object on the stack. E.g. wilc_sdio_write_reg() will call it with an
    > address pointing to one of its arguments. Detect whether the buffer
    > address is not DMA-able in which case a bounce buffer is used. The bounce
    > buffer itself is protected from parallel accesses by sdio_claim_host().
    >
    > Fixes: 5625f965d764 ("wilc1000: move wilc driver out of staging")
    > Signed-off-by: Michael Walle <mwalle@kernel.org>
    > ---
    > The bug itself probably goes back way more, but I don't know if it makes
    > any sense to use an older commit for the Fixes tag. If so, please suggest
    > one.
    >
    > The bug leads to an actual error on an imx8mn SoC with 1GiB of RAM. But the
    > error will also be catched by CONFIG_DEBUG_VIRTUAL:
    > [ 9.817512] virt_to_phys used for non-linear address: (____ptrval____) (0xffff80000a94bc9c)
    >
    > .../net/wireless/microchip/wilc1000/sdio.c | 28 ++++++++++++++++---
    > 1 file changed, 24 insertions(+), 4 deletions(-)
    >
    > diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c
    > b/drivers/net/wireless/microchip/wilc1000/sdio.c
    > index 7962c11cfe84..e988bede880c 100644
    > --- a/drivers/net/wireless/microchip/wilc1000/sdio.c
    > +++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
    > @@ -27,6 +27,7 @@ struct wilc_sdio {
    > bool irq_gpio;
    > u32 block_size;
    > int has_thrpt_enh3;
    > + u8 *dma_buffer;
    > };
    >
    > struct sdio_cmd52 {
    > @@ -89,6 +90,9 @@ static int wilc_sdio_cmd52(struct wilc *wilc, struct sdio_cmd52 *cmd)
    > static int wilc_sdio_cmd53(struct wilc *wilc, struct sdio_cmd53 *cmd)
    > {
    > struct sdio_func *func = container_of(wilc->dev, struct sdio_func, dev);
    > + struct wilc_sdio *sdio_priv = wilc->bus_data;
    > + bool need_bounce_buf = false;
    > + u8 *buf = cmd->buffer;
    > int size, ret;
    >
    > sdio_claim_host(func);
    > @@ -100,12 +104,20 @@ static int wilc_sdio_cmd53(struct wilc *wilc, struct sdio_cmd53 *cmd)
    > else
    > size = cmd->count;
    >
    > + if ((!virt_addr_valid(buf) || object_is_on_stack(buf)) &&

    How cheap are the above tests?
    It might just be worth always doing the 'bounce'?

    > + !WARN_ON_ONCE(size > WILC_SDIO_BLOCK_SIZE)) {

    That WARN() ought to be an error return?
    Or just assume that large buffers will dma-capable?

    David

    > + need_bounce_buf = true;
    > + buf = sdio_priv->dma_buffer;
    > + }
    > +
    > if (cmd->read_write) { /* write */
    > - ret = sdio_memcpy_toio(func, cmd->address,
    > - (void *)cmd->buffer, size);
    > + if (need_bounce_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 (need_bounce_buf)
    > + memcpy(cmd->buffer, buf, size);
    > }
    >
    > sdio_release_host(func);
    > @@ -127,6 +139,12 @@ static int wilc_sdio_probe(struct sdio_func *func,
    > if (!sdio_priv)
    > return -ENOMEM;
    >
    > + sdio_priv->dma_buffer = kzalloc(WILC_SDIO_BLOCK_SIZE, GFP_KERNEL);
    > + if (!sdio_priv->dma_buffer) {
    > + ret = -ENOMEM;
    > + goto free;
    > + }
    > +
    > ret = wilc_cfg80211_init(&wilc, &func->dev, WILC_HIF_SDIO,
    > &wilc_hif_sdio);
    > if (ret)
    > @@ -160,6 +178,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->dma_buffer);
    > kfree(sdio_priv);
    > return ret;
    > }
    > @@ -171,6 +190,7 @@ static void wilc_sdio_remove(struct sdio_func *func)
    >
    > clk_disable_unprepare(wilc->rtc_clk);
    > wilc_netdev_cleanup(wilc);
    > + kfree(sdio_priv->dma_buffer);
    > kfree(sdio_priv);
    > }
    >
    > --
    > 2.30.2

    -
    Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
    Registration No: 1397386 (Wales)

    \
     
     \ /
      Last update: 2022-07-29 11:52    [W:4.896 / U:0.080 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site