2011-11-21 06:57:47

by Raja Mani

Subject: [PATCH] ath6kl: Use mutex to protect dma buffer in sync read write

Firmware crashes while starting Soft AP in 32 bit x86 platform.
The reason is that the single dma buffer (ar_sdio->dma_buffer)
is used in ath6kl_sdio_read_write_sync() for unaligned buffer
handling and this function is called in the multiple context
at the same time. So, finally hits dma buffer corruption and
firmware crash.

Mutex is used to protect dma buffer to avoid data corruption.
Spin lock can not used to fix this issue since mmc stack
read/write calls may for sleep.

Observed this issue with recently commited patch
"ath6kl: Claim sdio function only at appropriate places"
861dd058f495973c7ad2a44b8f68f3cc05733eab

Signed-off-by: Raja Mani <[email protected]>
---
drivers/net/wireless/ath/ath6kl/sdio.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/sdio.c b/drivers/net/wireless/ath/ath6kl/sdio.c
index c2010bf..36cdb18 100644
--- a/drivers/net/wireless/ath/ath6kl/sdio.c
+++ b/drivers/net/wireless/ath/ath6kl/sdio.c
@@ -54,6 +54,7 @@ struct ath6kl_sdio {
struct work_struct wr_async_work;
struct list_head wr_asyncq;
spinlock_t wr_async_lock;
+ struct mutex rd_wr_sync_mlock;
};

#define CMD53_ARG_READ 0
@@ -396,6 +397,7 @@ static int ath6kl_sdio_read_write_sync(struct ath6kl *ar, u32 addr, u8 *buf,
if (buf_needs_bounce(buf)) {
if (!ar_sdio->dma_buffer)
return -ENOMEM;
+ mutex_lock(&ar_sdio->rd_wr_sync_mlock);
tbuf = ar_sdio->dma_buffer;
memcpy(tbuf, buf, len);
bounced = true;
@@ -406,6 +408,9 @@ static int ath6kl_sdio_read_write_sync(struct ath6kl *ar, u32 addr, u8 *buf,
if ((request & HIF_READ) && bounced)
memcpy(buf, tbuf, len);

+ if (bounced)
+ mutex_unlock(&ar_sdio->rd_wr_sync_mlock);
+
return ret;
}

@@ -1220,6 +1225,7 @@ static int ath6kl_sdio_probe(struct sdio_func *func,
spin_lock_init(&ar_sdio->lock);
spin_lock_init(&ar_sdio->scat_lock);
spin_lock_init(&ar_sdio->wr_async_lock);
+ mutex_init(&ar_sdio->rd_wr_sync_mlock);

INIT_LIST_HEAD(&ar_sdio->scat_req);
INIT_LIST_HEAD(&ar_sdio->bus_req_freeq);
--
1.7.0.4

2011-11-21 17:12:31

by Kalle Valo

Subject: Re: [PATCH] ath6kl: Use mutex to protect dma buffer in sync read write

On 11/21/2011 08:56 AM, Raja Mani wrote:
> Firmware crashes while starting Soft AP in 32 bit x86 platform.
> The reason is that the single dma buffer (ar_sdio->dma_buffer)
> is used in ath6kl_sdio_read_write_sync() for unaligned buffer
> handling and this function is called in the multiple context
> at the same time. So, finally hits dma buffer corruption and
> firmware crash.
>
> Mutex is used to protect dma buffer to avoid data corruption.
> Spin lock can not used to fix this issue since mmc stack
> read/write calls may for sleep.
>
> Observed this issue with recently commited patch
> "ath6kl: Claim sdio function only at appropriate places"
> 861dd058f495973c7ad2a44b8f68f3cc05733eab
>
> Signed-off-by: Raja Mani <[email protected]>

I really would not like to add yet another lock (or mutex) to ath6kl,
but I don't see any other quick fix for this. So I have applied this for
now, but I hope that in the future we get to remove the ugly bounce
buffer altogether.

But I did minor changes in the patch: I added a comment to the mutex
(which I now require for all new locks), renamed the lock to something
easier and moved it after the buffer.

Kalle