Messages in this thread | | | Date | Tue, 5 Sep 2023 17:28:00 +0200 | Subject | Re: [PATCH] ata: sata_mv: Fix incorrect string length computation in mv_dump_mem() | From | Christophe JAILLET <> |
| |
Le 05/09/2023 à 07:04, Damien Le Moal a écrit : > On 9/5/23 04:54, Christophe JAILLET wrote: >> snprintf() returns the "number of characters which *would* be generated for >> the given input", not the size *really* generated. >> >> In order to avoid too large values for 'o' (and potential negative values >> for "sizeof(linebuf) o") use scnprintf() instead of snprintf(). >> >> Note that given the "w < 4" in the for loop, the buffer can NOT >> overflow, but using the *right* function is always better. >> >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > Doesn't this need Fixes and CC stable tags ?
I don't think so. As said in the commit message : Note that given the "w < 4" in the for loop, the buffer can NOT overflow, but using the *right* function is always better.
linebuf is 38 chars. In each iteration, we write 9 bytes + NULL. We write only 4 elements per line (because of w < 4), so 9 * 4 + 1 = 37 bytes are needed. 9 is for %08x<space>
It can't overflow. Moreover, it is really unlikely that the size of linebuf or the number of elements on each line change in a stable kernel.
So, from my POV, this patch is more a clean-up than anything else.
I would even agree that it is maybe not even needed. But should someone cut'n'paste it one day, then using the correct function could maybe help him.
CJ
> >> --- >> drivers/ata/sata_mv.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c >> index d105db5c7d81..45e48d653c60 100644 >> --- a/drivers/ata/sata_mv.c >> +++ b/drivers/ata/sata_mv.c >> @@ -1255,8 +1255,8 @@ static void mv_dump_mem(struct device *dev, void __iomem *start, unsigned bytes) >> >> for (b = 0; b < bytes; ) { >> for (w = 0, o = 0; b < bytes && w < 4; w++) { >> - o += snprintf(linebuf + o, sizeof(linebuf) - o, >> - "%08x ", readl(start + b)); >> + o += scnprintf(linebuf + o, sizeof(linebuf) - o, >> + "%08x ", readl(start + b)); >> b += sizeof(u32); >> } >> dev_dbg(dev, "%s: %p: %s\n", >
| |