* [PATCH] scsi: myrb: fix sprintf buffer overflow warning
@ 2018-11-02 15:34 Arnd Bergmann
2018-11-03 8:46 ` Hannes Reinecke
2018-11-06 3:35 ` Martin K. Petersen
0 siblings, 2 replies; 3+ messages in thread
From: Arnd Bergmann @ 2018-11-02 15:34 UTC (permalink / raw)
To: Hannes Reinecke, James E.J. Bottomley, Martin K. Petersen
Cc: Arnd Bergmann, linux-scsi, linux-kernel
gcc warns that the 12 byte fw_version field might not be long enough to
contain the generated firmware name string:
drivers/scsi/myrb.c: In function 'myrb_get_hba_config':
drivers/scsi/myrb.c:1052:38: error: '%02d' directive writing between 2 and 3 bytes into a region of size between 2 and 5 [-Werror=format-overflow=]
sprintf(cb->fw_version, "%d.%02d-%c-%02d",
^~~~
drivers/scsi/myrb.c:1052:26: note: directive argument in the range [0, 255]
sprintf(cb->fw_version, "%d.%02d-%c-%02d",
^~~~~~~~~~~~~~~~~
drivers/scsi/myrb.c:1052:2: note: 'sprintf' output between 10 and 14 bytes into a destination of size 12
sprintf(cb->fw_version, "%d.%02d-%c-%02d",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
enquiry2->fw.major_version,
~~~~~~~~~~~~~~~~~~~~~~~~~~~
enquiry2->fw.minor_version,
~~~~~~~~~~~~~~~~~~~~~~~~~~~
enquiry2->fw.firmware_type,
~~~~~~~~~~~~~~~~~~~~~~~~~~~
enquiry2->fw.turn_id);
~~~~~~~~~~~~~~~~~~~~~
I have not checked whether there are appropriate range checks before the
sprintf, but there is a range check after it that will bail out in case
of out of range version numbers. This means we can simply use snprintf()
instead of sprintf() to limit the output buffer size, and it will work
correctly.
Fixes: 081ff398c56c ("scsi: myrb: Add Mylex RAID controller (block interface)")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/scsi/myrb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
index aeb282f617c5..0642f2d0a3bb 100644
--- a/drivers/scsi/myrb.c
+++ b/drivers/scsi/myrb.c
@@ -1049,7 +1049,8 @@ static int myrb_get_hba_config(struct myrb_hba *cb)
enquiry2->fw.firmware_type = '0';
enquiry2->fw.turn_id = 0;
}
- sprintf(cb->fw_version, "%d.%02d-%c-%02d",
+ snprintf(cb->fw_version, sizeof(cb->fw_version),
+ "%d.%02d-%c-%02d",
enquiry2->fw.major_version,
enquiry2->fw.minor_version,
enquiry2->fw.firmware_type,
--
2.18.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] scsi: myrb: fix sprintf buffer overflow warning
2018-11-02 15:34 [PATCH] scsi: myrb: fix sprintf buffer overflow warning Arnd Bergmann
@ 2018-11-03 8:46 ` Hannes Reinecke
2018-11-06 3:35 ` Martin K. Petersen
1 sibling, 0 replies; 3+ messages in thread
From: Hannes Reinecke @ 2018-11-03 8:46 UTC (permalink / raw)
To: Arnd Bergmann, Hannes Reinecke, James E.J. Bottomley, Martin K. Petersen
Cc: linux-scsi, linux-kernel
On 11/2/18 4:34 PM, Arnd Bergmann wrote:
> gcc warns that the 12 byte fw_version field might not be long enough to
> contain the generated firmware name string:
>
> drivers/scsi/myrb.c: In function 'myrb_get_hba_config':
> drivers/scsi/myrb.c:1052:38: error: '%02d' directive writing between 2 and 3 bytes into a region of size between 2 and 5 [-Werror=format-overflow=]
> sprintf(cb->fw_version, "%d.%02d-%c-%02d",
> ^~~~
> drivers/scsi/myrb.c:1052:26: note: directive argument in the range [0, 255]
> sprintf(cb->fw_version, "%d.%02d-%c-%02d",
> ^~~~~~~~~~~~~~~~~
> drivers/scsi/myrb.c:1052:2: note: 'sprintf' output between 10 and 14 bytes into a destination of size 12
> sprintf(cb->fw_version, "%d.%02d-%c-%02d",
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> enquiry2->fw.major_version,
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> enquiry2->fw.minor_version,
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> enquiry2->fw.firmware_type,
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> enquiry2->fw.turn_id);
> ~~~~~~~~~~~~~~~~~~~~~
>
> I have not checked whether there are appropriate range checks before the
> sprintf, but there is a range check after it that will bail out in case
> of out of range version numbers. This means we can simply use snprintf()
> instead of sprintf() to limit the output buffer size, and it will work
> correctly.
>
> Fixes: 081ff398c56c ("scsi: myrb: Add Mylex RAID controller (block interface)")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/scsi/myrb.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
> index aeb282f617c5..0642f2d0a3bb 100644
> --- a/drivers/scsi/myrb.c
> +++ b/drivers/scsi/myrb.c
> @@ -1049,7 +1049,8 @@ static int myrb_get_hba_config(struct myrb_hba *cb)
> enquiry2->fw.firmware_type = '0';
> enquiry2->fw.turn_id = 0;
> }
> - sprintf(cb->fw_version, "%d.%02d-%c-%02d",
> + snprintf(cb->fw_version, sizeof(cb->fw_version),
> + "%d.%02d-%c-%02d",
> enquiry2->fw.major_version,
> enquiry2->fw.minor_version,
> enquiry2->fw.firmware_type,
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] scsi: myrb: fix sprintf buffer overflow warning
2018-11-02 15:34 [PATCH] scsi: myrb: fix sprintf buffer overflow warning Arnd Bergmann
2018-11-03 8:46 ` Hannes Reinecke
@ 2018-11-06 3:35 ` Martin K. Petersen
1 sibling, 0 replies; 3+ messages in thread
From: Martin K. Petersen @ 2018-11-06 3:35 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Hannes Reinecke, James E.J. Bottomley, Martin K. Petersen,
linux-scsi, linux-kernel
Arnd,
> gcc warns that the 12 byte fw_version field might not be long enough to
> contain the generated firmware name string:
Applied to 4.20/scsi-fixes, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-06 3:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-02 15:34 [PATCH] scsi: myrb: fix sprintf buffer overflow warning Arnd Bergmann
2018-11-03 8:46 ` Hannes Reinecke
2018-11-06 3:35 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).