On Thu, May 23, 2019 at 3:54 AM Nathan Chancellor
[off-list ref] wrote:

On Thu, May 02, 2019 at 08:17:18PM -0700, Nathan Chancellor wrote:

quoted

On Thu, May 02, 2019 at 11:18:01AM -0700, Nick Desaulniers wrote:

quoted

On Thu, May 2, 2019 at 8:16 AM Nathan Chancellor
u8 data [4];

data was __le32 in rsi_reset_chip() before commit f700546682a6 ("rsi:
fix nommu_map_sg overflow kernel panic").

I wonder if this would be okay for this function:

-------------------------------------------------

diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c
index f9c67ed473d1..0330c50ab99c 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c

@@ -927,7 +927,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
 {
        int status;
        u32 addr;
-       u8 *data;
+       u8 data;

        status = rsi_sdio_master_access_msword(adapter, TA_BASE_ADDR);
        if (status < 0) {

@@ -937,7 +937,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
        }

        rsi_dbg(INIT_ZONE, "%s: Bring TA out of reset\n", __func__);
-       put_unaligned_le32(TA_HOLD_THREAD_VALUE, data);
+       put_unaligned_le32(TA_HOLD_THREAD_VALUE, &data);
        addr = TA_HOLD_THREAD_REG | RSI_SD_REQUEST_MASTER;
        status = rsi_sdio_write_register_multiple(adapter, addr,
                                                  (u8 *)&data,

This is clearly not ok, as put_unaligned_le32() stores four bytes, and
the local variable is only one byte!

Also, sdio does use DMA for transfers, so the variable has to be
dynamically allocated. I think your original patch was correct.
The only change I'd possibly make would be to use
RSI_9116_REG_SIZE instead of sizeof(u32).

Did any of the maintainers have any comments on what the correct
solution is here to resolve this warning? It is one of the few left
before we can turn on -Wuninitialized for the whole kernel.

I would argue that this should not stop us from turning it on, as the
warning is for a clear bug in the code that absolutely needs to be
fixed, rather than a false-positive.

       Arnd