linux-kernel - Re: [PATCH v2] i2c: at91: use dma safe buffers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 5 May 2022 07:51:42 -0700
From:   Nathan Chancellor <nathan@...nel.org>
To:     Michael Walle <michael@...le.cc>
Cc:     Codrin Ciubotariu <codrin.ciubotariu@...rochip.com>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Claudiu Beznea <claudiu.beznea@...rochip.com>,
        linux-i2c@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH v2] i2c: at91: use dma safe buffers

On Thu, Apr 07, 2022 at 05:08:28PM +0200, Michael Walle wrote:
> The supplied buffer might be on the stack and we get the following error
> message:
> [    3.312058] at91_i2c e0070600.i2c: rejecting DMA map of vmalloc memory
> 
> Use i2c_{get,put}_dma_safe_msg_buf() to get a DMA-able memory region if
> necessary.
> 
> Fixes: 60937b2cdbf9 ("i2c: at91: add dma support")
> Signed-off-by: Michael Walle <michael@...le.cc>
> Reviewed-by: Codrin Ciubotariu <codrin.ciubotariu@...rochip.com>
> ---
> changes since v1:
>  - remove extra empty line
>  - add fixes tag as suggested by Codrin
> 
>  drivers/i2c/busses/i2c-at91-master.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
> index b0eae94909f4..5eca3b3bb609 100644
> --- a/drivers/i2c/busses/i2c-at91-master.c
> +++ b/drivers/i2c/busses/i2c-at91-master.c
> @@ -656,6 +656,7 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int num)
>  	unsigned int_addr_flag = 0;
>  	struct i2c_msg *m_start = msg;
>  	bool is_read;
> +	u8 *dma_buf;
>  
>  	dev_dbg(&adap->dev, "at91_xfer: processing %d messages:\n", num);
>  
> @@ -703,7 +704,17 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int num)
>  	dev->msg = m_start;
>  	dev->recv_len_abort = false;
>  
> +	if (dev->use_dma) {
> +		dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
> +		if (!dma_buf) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
> +		dev->buf = dma_buf;
> +	}
> +
>  	ret = at91_do_twi_transfer(dev);
> +	i2c_put_dma_safe_msg_buf(dma_buf, m_start, !ret);
>  
>  	ret = (ret < 0) ? ret : num;
>  out:
> -- 
> 2.30.2
> 
> 

This change as commit 03fbb903c8bf ("i2c: at91: use dma safe buffers")
causes the following clang warning:

drivers/i2c/busses/i2c-at91-master.c:707:6: error: variable 'dma_buf' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
        if (dev->use_dma) {
            ^~~~~~~~~~~~
drivers/i2c/busses/i2c-at91-master.c:717:27: note: uninitialized use occurs here
        i2c_put_dma_safe_msg_buf(dma_buf, m_start, !ret);
                                 ^~~~~~~
drivers/i2c/busses/i2c-at91-master.c:707:2: note: remove the 'if' if its condition is always true
        if (dev->use_dma) {
        ^~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-at91-master.c:659:13: note: initialize the variable 'dma_buf' to silence this warning
        u8 *dma_buf;
                   ^
                    = NULL
1 error generated.

Should this variable be initialized or should the call to
i2c_put_dma_safe_msg_buf() be moved into the if statement?

Cheers,
Nathan