Re: [PATCH] mmc: mmc_spi: fix snprintf() output buffer size

[Bartosz Golaszewski]

On Mon, Oct 7, 2024 at 8:09 PM Christophe JAILLET
<christophe.jaillet@xxxxxxxxxx> wrote:
>
> Le 07/10/2024 à 13:45, Bartosz Golaszewski a écrit :
> > From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> >
> > GCC 13 complains about the truncated output of snprintf():
> >
> > drivers/mmc/host/mmc_spi.c: In function ‘mmc_spi_response_get’:
> > drivers/mmc/host/mmc_spi.c:227:64: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=]
> >    227 |         snprintf(tag, sizeof(tag), "  ... CMD%d response SPI_%s",
> >        |                                                                ^
> > drivers/mmc/host/mmc_spi.c:227:9: note: ‘snprintf’ output between 26 and 43 bytes into a destination of size 32
> >    227 |         snprintf(tag, sizeof(tag), "  ... CMD%d response SPI_%s",
> >        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    228 |                 cmd->opcode, maptype(cmd));
> >
> > Increase the size of the target buffer.
> >
> > Fixes: 15a0580ced08 ("mmc_spi host driver")
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> > ---
> >   drivers/mmc/host/mmc_spi.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
> > index 8fee7052f2ef..fa1d1a1b3142 100644
> > --- a/drivers/mmc/host/mmc_spi.c
> > +++ b/drivers/mmc/host/mmc_spi.c
> > @@ -222,7 +222,7 @@ static int mmc_spi_response_get(struct mmc_spi_host *host,
> >       u8      leftover = 0;
> >       unsigned short rotator;
> >       int     i;
> > -     char    tag[32];
> > +     char    tag[43];
> >
> >       snprintf(tag, sizeof(tag), "  ... CMD%d response SPI_%s",
> >               cmd->opcode, maptype(cmd));
>
> 'tag' is only used in a dev_dbg() at the end of the function.
>
> Maybe "  ... CMD%d response SPI_%s" could me moved directly within the
> dev_dbg(). This would save a few bytes on the stack, save a snprintf()
> in the normal path and fix the warning without the need of magic number.
>
> just my 2c.
>
> CJ

I would be hesitant to change this logic here. The cmd struct is
manipulated pretty extensively later in the function which leads me to
believe that this snprintf() here was done on purpose.

Bart