| Message ID | 20241007114549.51213-1-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: mmc_spi: fix snprintf() output buffer size | expand |
Le 07/10/2024 à 13:45, Bartosz Golaszewski a écrit : > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > 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@linaro.org> > --- > 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
On Mon, Oct 7, 2024 at 8:09 PM Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > > Le 07/10/2024 à 13:45, Bartosz Golaszewski a écrit : > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > 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@linaro.org> > > --- > > 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
On Tue, 8 Oct 2024 at 09:13, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > On Mon, Oct 7, 2024 at 8:09 PM Christophe JAILLET > <christophe.jaillet@wanadoo.fr> wrote: > > > > Le 07/10/2024 à 13:45, Bartosz Golaszewski a écrit : > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > 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@linaro.org> > > > --- > > > 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. The cmd->opcode and cmd->flags aren't really something a host driver should need to change. It's the core that sets them to inform the host driver about the command. I looked closer and it seems to be correct in this case too. Kind regards Uffe
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));