linux-kernel - RE: [PATCH] ASoC: renesas: rz-ssi: Add a check for negative sample

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]

Message-ID:
 <TY3PR01MB113463ACD4D18075A0DCD2F6D86122@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Date: Wed, 8 Jan 2025 11:08:24 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>, Dan Carpenter
	<dan.carpenter@...aro.org>
CC: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>, Liam
 Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, Jaroslav
 Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
	"linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>
Subject: RE: [PATCH] ASoC: renesas: rz-ssi: Add a check for negative
 sample_space

Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@...ux-m68k.org>
> Sent: 08 January 2025 10:58
> Subject: Re: [PATCH] ASoC: renesas: rz-ssi: Add a check for negative sample_space
> 
> Hi Dan,
> 
> On Wed, Jan 8, 2025 at 10:29 AM Dan Carpenter <dan.carpenter@...aro.org> wrote:
> > My static checker rule complains about this code.  The concern is that
> > if "sample_space" is negative then the "sample_space >= runtime->channels"
> > condition will not work as intended because it will be type promoted
> > to a high unsigned int value.
> >
> > strm->fifo_sample_size is SSI_FIFO_DEPTH (32).  The SSIFSR_TDC_MASK is
> > 0x3f.  Without any further context it does seem like a reasonable
> > warning and it can't hurt to add a check for negatives.
> >
> > Cc: stable@...r.kernel.org
> > Fixes: 03e786bd4341 ("ASoC: sh: Add RZ/G2L SSIF-2 driver")
> > Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>
> 
> Thanks for your patch!
> 
> > --- a/sound/soc/renesas/rz-ssi.c
> > +++ b/sound/soc/renesas/rz-ssi.c
> > @@ -521,6 +521,8 @@ static int rz_ssi_pio_send(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm)
> >         sample_space = strm->fifo_sample_size;
> >         ssifsr = rz_ssi_reg_readl(ssi, SSIFSR);
> >         sample_space -= (ssifsr >> SSIFSR_TDC_SHIFT) &
> > SSIFSR_TDC_MASK;
> > +       if (sample_space < 0)
> > +               return -EINVAL;
> >
> >         /* Only add full frames at a time */
> >         while (frames_left && (sample_space >= runtime->channels)) {
> 
> In practice this cannot happen, as the maximum value of the field is 0x20 (= SSI_FIFO_DEPTH), but
> better safe than sorry, so
> Reviewed-by: Geert Uytterhoeven <geert+renesas@...der.be>
> 
> Biju/Prabhakar: The documentation for the TDC bits in the FIFO Status Register seems to be incorrect
> (in all of the RZ/G2L, RZ/G2UL, RZ/G3S, and RZ/A2M documentation):
> 
>     TDC[5:0] Bits
>     These bits indicate the number of valid data that are stored in
>     the transmit FIFO data register (SSIFTDR). With this flag as H’0,
>     there is no data to be transmitted. With H’10, there is no space to
>     write data.
> 
> As the FIFO size is 4 bytes x 32 stages for both the transmit and receive FIFOs, the above should be
> H'20 instead of H'10.

Thanks for pointing it out. Will check this with HW documentation team about this issue.

Cheers,
Biju