Bug#1079642: ITP: mfiutil -- Utility for managing LSI MegaRAID SAS controllers
>
> I built and tested the source on a machine running testing, and it seem
> to work fine. According to the compiler there are some potential buffer
> overflows:
>
> gcc -I include -std=gnu99 -Wall -Wno-unused-value -Os -c -o mfi_drive.o
> mfi_drive.c
> mfi_drive.c: In function ‘mfi_pdstate’:
> mfi_drive.c:160:40: warning: ‘%04x’ directive writing between 4 and 8 bytes
> into a region of size 7 [-Wformat-overflow=]
> 160 | sprintf(buf, "PSTATE 0x%04x", state);
> | ^~~~
> In function ‘mfi_pdstate’,
> inlined from ‘mfi_pdstate’ at mfi_drive.c:136:1:
> mfi_drive.c:160:30: note: directive argument in the range [3, 4294967295]
> 160 | sprintf(buf, "PSTATE 0x%04x", state);
> | ^~~~~~~~~~~~~~~
> mfi_drive.c:160:17: note: ‘sprintf’ output between 14 and 18 bytes into a
> destination of size 16
> 160 | sprintf(buf, "PSTATE 0x%04x", state);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> mfi_drive.c: In function ‘mfi_pd_inq_string’:
> mfi_drive.c:418:57: warning: ‘ ’ directive output may be truncated writing 1
> byte into a region of size between 0 and 62 [-Wformat-truncation=]
> 418 | snprintf(inq_string, sizeof(inq_string), "<%s %s %s
> serial=%s> %s", vendor,
> | ^
> mfi_drive.c:418:9: note: ‘snprintf’ output between 14 and 121 bytes into a
> destination of size 64
> 418 | snprintf(inq_string, sizeof(inq_string), "<%s %s %s
> serial=%s> %s", vendor,
> |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 419 | product, revision, serial, rstr);
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> mfi_drive.c:401:65: warning: ‘ serial=’ directive output may be truncated
> writing 8 bytes into a region of size between 0 and 62
> [-Wformat-truncation=]
> 401 | snprintf(inq_string, sizeof(inq_string), "<%s %s
> serial=%s> SATA",
> |
> ^~~~~~~~
> mfi_drive.c:401:17: note: ‘snprintf’ output between 17 and 98 bytes into a
> destination of size 64
> 401 | snprintf(inq_string, sizeof(inq_string), "<%s %s
> serial=%s> SATA",
> |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 402 | product, revision, serial);
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~
> gcc -I include -std=gnu99 -Wall -Wno-unused-value -Os -c -o mfi_evt.o
> mfi_evt.c
> mfi_evt.c: In function ‘pdrive_location’:
> mfi_evt.c:343:64: warning: ‘snprintf’ output may be truncated before the
> last format character [-Wformat-truncation=]
> 343 | snprintf(buffer, sizeof(buffer), "%02d(e%d/s%d)",
> pd->device_id,
> | ^
> mfi_evt.c:343:17: note: ‘snprintf’ output between 10 and 17 bytes into a
> destination of size 16
> 343 | snprintf(buffer, sizeof(buffer), "%02d(e%d/s%d)",
> pd->device_id,
> |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 344 | pd->enclosure_index, pd->slot_number);
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> [...]
> gcc -I include -std=gnu99 -Wall -Wno-unused-value -Os -c -o mfi_volume.o
> mfi_volume.c
> mfi_volume.c: In function ‘mfi_ldstate’:
> mfi_volume.c:59:40: warning: ‘%02x’ directive writing between 2 and 8 bytes
> into a region of size 7 [-Wformat-overflow=]
> 59 | sprintf(buf, "LSTATE 0x%02x", state);
> | ^~~~
> mfi_volume.c:59:30: note: directive argument in the range [4, 4294967295]
> 59 | sprintf(buf, "LSTATE 0x%02x", state);
> | ^~~~~~~~~~~~~~~
> mfi_volume.c:59:17: note: ‘sprintf’ output between 12 and 18 bytes into a
> destination of size 16
> 59 | sprintf(buf, "LSTATE 0x%02x", state);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Here is a patch to get rid of the compiler warnings.
>
> diff --git a/mfi_drive.c b/mfi_drive.c
> index 9f726f3..1450621 100644
> --- a/mfi_drive.c
> +++ b/mfi_drive.c
> @@ -135,7 +135,7 @@ mfi_drive_name(const struct mfi_pd_info *info_p,
> uint16_t device_id, uint32_t de
> const char *
> mfi_pdstate(enum mfi_pd_state state)
> {
> - static char buf[16];
> + static char buf[18];
>
> switch (state) {
> case MFI_PD_STATE_UNCONFIGURED_GOOD:
> @@ -375,7 +375,7 @@ mfi_pd_inq_string(const struct mfi_pd_info *info)
> {
> struct scsi_inquiry_data iqd, *inq_data = &iqd;
> char vendor[16], product[48], revision[16], rstr[12],
> serial[SID_VENDOR_SPECIFIC_0_SIZE];
> - static char inq_string[64];
> + static char inq_string[121];
>
> memcpy(inq_data, info->inquiry_data,
> (sizeof (iqd) < sizeof (info->inquiry_data))?
> diff --git a/mfi_evt.c b/mfi_evt.c
> index ad22bc8..b43be7e 100644
> --- a/mfi_evt.c
> +++ b/mfi_evt.c
> @@ -334,7 +334,7 @@ simple_hex(const void *ptr, size_t length, const char
> *separator)
> static const char *
> pdrive_location(const struct mfi_evt_pd *pd)
> {
> - static char buffer[16];
> + static char buffer[17];
>
> if (pd->enclosure_index == 0) {
> snprintf(buffer, sizeof(buffer), "%02d(s%d)", pd->device_id,
> diff --git a/mfi_volume.c b/mfi_volume.c
> index cde2bb4..571333c 100644
> --- a/mfi_volume.c
> +++ b/mfi_volume.c
> @@ -44,7 +44,7 @@ MFI_TABLE(top, volume);
> const char *
> mfi_ldstate(enum mfi_ld_state state)
> {
> - static char buf[16];
> + static char buf[18];
>
> switch (state) {
> case MFI_LD_STATE_OFFLINE:
>
> --
> Happy hacking
> Petter Reinholdtsen
>
Hello.
I was writing a new section for the man page today. I will commit this patch
by tomorrow. Thanks.
Reply to: