| Message ID | 2d3096dd4b1b6e758287e4062e3147c57c007eaa.1702411083.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | scsi: myrb: Fix a potential string truncation | expand |
On 12/12/23 10:09, Christophe JAILLET wrote: > "physical device - not rebuilding\n" is 34 bytes long. When written in > 'buf' with a limit of 32 bytes, it is truncated. > > When building with W=1, it leads to: > drivers/scsi/myrb.c: In function ‘rebuild_show’: > drivers/scsi/myrb.c:1906:24: error: ‘physical device - not rebuil...’ directive output truncated writing 33 bytes into a region of size 32 [-Werror=format-truncation=] > 1906 | return snprintf(buf, 32, "physical device - not rebuilding\n"); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/scsi/myrb.c:1906:24: note: ‘snprintf’ output 34 bytes into a destination of size 32 > > Change the allowed size to 64 to fix the issue. > > Fixes: 081ff398c56c ("scsi: myrb: Add Mylex RAID controller (block interface)") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > drivers/scsi/myrb.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c > index ca2e932dd9b7..ca2380d2d6d3 100644 > --- a/drivers/scsi/myrb.c > +++ b/drivers/scsi/myrb.c > @@ -1903,15 +1903,15 @@ static ssize_t rebuild_show(struct device *dev, > unsigned char status; > > if (sdev->channel < myrb_logical_channel(sdev->host)) > - return snprintf(buf, 32, "physical device - not rebuilding\n"); > + return snprintf(buf, 64, "physical device - not rebuilding\n"); > > status = myrb_get_rbld_progress(cb, &rbld_buf); > > if (rbld_buf.ldev_num != sdev->id || > status != MYRB_STATUS_SUCCESS) > - return snprintf(buf, 32, "not rebuilding\n"); > + return snprintf(buf, 64, "not rebuilding\n"); > > - return snprintf(buf, 32, "rebuilding block %u of %u\n", > + return snprintf(buf, 64, "rebuilding block %u of %u\n", > rbld_buf.ldev_size - rbld_buf.blocks_left, > rbld_buf.ldev_size); > } Anyone who sees the resulting code without having seen the above patch will wonder where the magic number '64' comes from. Please use sysfs_emit() instead of snprintf(buf, 64, ...). Thanks, Bart.
Le 12/12/2023 à 21:14, Bart Van Assche a écrit : > On 12/12/23 10:09, Christophe JAILLET wrote: >> "physical device - not rebuilding\n" is 34 bytes long. When written in >> 'buf' with a limit of 32 bytes, it is truncated. >> >> When building with W=1, it leads to: >> drivers/scsi/myrb.c: In function ‘rebuild_show’: >> drivers/scsi/myrb.c:1906:24: error: ‘physical device - not >> rebuil...’ directive output truncated writing 33 bytes into a region >> of size 32 [-Werror=format-truncation=] >> 1906 | return snprintf(buf, 32, "physical device >> - not rebuilding\n"); >> | >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/scsi/myrb.c:1906:24: note: ‘snprintf’ output 34 bytes into >> a destination of size 32 >> >> Change the allowed size to 64 to fix the issue. >> >> Fixes: 081ff398c56c ("scsi: myrb: Add Mylex RAID controller (block >> interface)") >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >> --- >> drivers/scsi/myrb.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c >> index ca2e932dd9b7..ca2380d2d6d3 100644 >> --- a/drivers/scsi/myrb.c >> +++ b/drivers/scsi/myrb.c >> @@ -1903,15 +1903,15 @@ static ssize_t rebuild_show(struct device *dev, >> unsigned char status; >> if (sdev->channel < myrb_logical_channel(sdev->host)) >> - return snprintf(buf, 32, "physical device - not rebuilding\n"); >> + return snprintf(buf, 64, "physical device - not rebuilding\n"); >> status = myrb_get_rbld_progress(cb, &rbld_buf); >> if (rbld_buf.ldev_num != sdev->id || >> status != MYRB_STATUS_SUCCESS) >> - return snprintf(buf, 32, "not rebuilding\n"); >> + return snprintf(buf, 64, "not rebuilding\n"); >> - return snprintf(buf, 32, "rebuilding block %u of %u\n", >> + return snprintf(buf, 64, "rebuilding block %u of %u\n", >> rbld_buf.ldev_size - rbld_buf.blocks_left, >> rbld_buf.ldev_size); >> } > > Anyone who sees the resulting code without having seen the above patch will > wonder where the magic number '64' comes from. Please use sysfs_emit() > instead > of snprintf(buf, 64, ...). Ok. In this case, do you still prefer 2 patches (one to fix rebuild_show() and one for all the other _show function) or only 1 with everything in it? CJ > > Thanks, > > Bart. >
On 12/12/23 10:20, Christophe JAILLET wrote: > In this case, do you still prefer 2 patches (one to fix > rebuild_show() and one for all the other _show function) or only 1 > with everything in it? One patch with all the changes is probably better. Thanks, Bart.
diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c index ca2e932dd9b7..ca2380d2d6d3 100644 --- a/drivers/scsi/myrb.c +++ b/drivers/scsi/myrb.c @@ -1903,15 +1903,15 @@ static ssize_t rebuild_show(struct device *dev, unsigned char status; if (sdev->channel < myrb_logical_channel(sdev->host)) - return snprintf(buf, 32, "physical device - not rebuilding\n"); + return snprintf(buf, 64, "physical device - not rebuilding\n"); status = myrb_get_rbld_progress(cb, &rbld_buf); if (rbld_buf.ldev_num != sdev->id || status != MYRB_STATUS_SUCCESS) - return snprintf(buf, 32, "not rebuilding\n"); + return snprintf(buf, 64, "not rebuilding\n"); - return snprintf(buf, 32, "rebuilding block %u of %u\n", + return snprintf(buf, 64, "rebuilding block %u of %u\n", rbld_buf.ldev_size - rbld_buf.blocks_left, rbld_buf.ldev_size); }
"physical device - not rebuilding\n" is 34 bytes long. When written in 'buf' with a limit of 32 bytes, it is truncated. When building with W=1, it leads to: drivers/scsi/myrb.c: In function ‘rebuild_show’: drivers/scsi/myrb.c:1906:24: error: ‘physical device - not rebuil...’ directive output truncated writing 33 bytes into a region of size 32 [-Werror=format-truncation=] 1906 | return snprintf(buf, 32, "physical device - not rebuilding\n"); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/scsi/myrb.c:1906:24: note: ‘snprintf’ output 34 bytes into a destination of size 32 Change the allowed size to 64 to fix the issue. Fixes: 081ff398c56c ("scsi: myrb: Add Mylex RAID controller (block interface)") Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- drivers/scsi/myrb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)