diff --git a/src/udev/scsi_id/scsi.h b/src/udev/scsi_id/scsi.h index ebb8ae9008..71c532e427 100644 --- a/src/udev/scsi_id/scsi.h +++ b/src/udev/scsi_id/scsi.h @@ -24,7 +24,7 @@ struct scsi_ioctl_command { * as this is a nice value for some devices, especially some of the usb * mass storage devices. */ -#define SCSI_INQ_BUFF_LEN 254 +#define SCSI_INQ_BUFF_LEN 254U /* * SCSI INQUIRY vendor and model (really product) lengths. diff --git a/src/udev/scsi_id/scsi_id.c b/src/udev/scsi_id/scsi_id.c index e343889719..6a31f9c4c8 100644 --- a/src/udev/scsi_id/scsi_id.c +++ b/src/udev/scsi_id/scsi_id.c @@ -389,6 +389,10 @@ static int set_inq_values(struct scsi_id_device *dev_scsi, const char *path) { return 0; } +static bool scsi_string_is_valid(const char *s) { + return !isempty(s) && utf8_is_valid(s) && !string_has_cc(s, /* ok= */ NULL); +} + /* * scsi_id: try to get an id, if one is found, printf it to stdout. * returns a value passed to exit() - 0 if printed an id, else 1. @@ -432,17 +436,17 @@ static int scsi_id(char *maj_min_dev) { udev_replace_chars(serial_str, NULL); printf("ID_SERIAL_SHORT=%s\n", serial_str); } - if (dev_scsi.wwn[0] != '\0') { + if (scsi_string_is_valid(dev_scsi.wwn)) { printf("ID_WWN=0x%s\n", dev_scsi.wwn); - if (dev_scsi.wwn_vendor_extension[0] != '\0') { + if (scsi_string_is_valid(dev_scsi.wwn_vendor_extension)) { printf("ID_WWN_VENDOR_EXTENSION=0x%s\n", dev_scsi.wwn_vendor_extension); printf("ID_WWN_WITH_EXTENSION=0x%s%s\n", dev_scsi.wwn, dev_scsi.wwn_vendor_extension); } else printf("ID_WWN_WITH_EXTENSION=0x%s\n", dev_scsi.wwn); } - if (dev_scsi.tgpt_group[0] != '\0') + if (scsi_string_is_valid(dev_scsi.tgpt_group)) printf("ID_TARGET_PORT=%s\n", dev_scsi.tgpt_group); - if (dev_scsi.unit_serial_number[0] != '\0' && utf8_is_valid(dev_scsi.unit_serial_number) && !string_has_cc(dev_scsi.unit_serial_number, /* ok= */ NULL)) + if (scsi_string_is_valid(dev_scsi.unit_serial_number)) printf("ID_SCSI_SERIAL=%s\n", dev_scsi.unit_serial_number); goto out; } diff --git a/src/udev/scsi_id/scsi_id.h b/src/udev/scsi_id/scsi_id.h index db49c7f3d9..984bbc05f4 100644 --- a/src/udev/scsi_id/scsi_id.h +++ b/src/udev/scsi_id/scsi_id.h @@ -1,6 +1,8 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ #pragma once +#include "basic-forward.h" + /* * Copyright © IBM Corp. 2003 */ @@ -50,7 +52,7 @@ struct scsi_id_device { int scsi_std_inquiry(struct scsi_id_device *dev_scsi, const char *devname); int scsi_get_serial(struct scsi_id_device *dev_scsi, const char *devname, - int page_code, int len); + int page_code, size_t len); /* * Page code values. diff --git a/src/udev/scsi_id/scsi_serial.c b/src/udev/scsi_id/scsi_serial.c index 7de9999257..95268635cb 100644 --- a/src/udev/scsi_id/scsi_serial.c +++ b/src/udev/scsi_id/scsi_serial.c @@ -81,7 +81,7 @@ static const struct scsi_id_search_values id_search_list[] = { #define SG_ERR_CAT_OTHER 99 /* Some other error/warning */ static int do_scsi_page80_inquiry(struct scsi_id_device *dev_scsi, int fd, - char *serial, char *serial_short, int max_len); + char *serial, char *serial_short, size_t max_len); static int sg_err_category_new(int scsi_status, int msg_status, int host_status, int driver_status, const @@ -419,12 +419,12 @@ static int append_vendor_model( * serial number. */ static int check_fill_0x83_id(struct scsi_id_device *dev_scsi, - unsigned char *page_83, + uint8_t *page_83, const struct scsi_id_search_values *id_search, char *serial, char *serial_short, - int max_len, char *wwn, + size_t max_len, char *wwn, char *wwn_vendor_extension, char *tgpt_group) { - int i, j, s, len; + size_t i, j, s, len; /* * ASSOCIATION must be with the device (value 0) @@ -470,7 +470,7 @@ static int check_fill_0x83_id(struct scsi_id_device *dev_scsi, len += VENDOR_LENGTH + MODEL_LENGTH; if (max_len < len) { - log_debug("%s: length %d too short - need %d", + log_debug("%s: length %zu too short - need %zu", dev_scsi->kernel, max_len, len); return 1; } @@ -506,14 +506,14 @@ static int check_fill_0x83_id(struct scsi_id_device *dev_scsi, /* * ASCII descriptor. */ - while (i < (4 + page_83[3])) + while (i < (4U + page_83[3])) serial[j++] = page_83[i++]; } else { /* * Binary descriptor, convert to ASCII, using two bytes of * ASCII for each byte in the page_83. */ - while (i < (4 + page_83[3])) { + while (i < (4U + page_83[3])) { serial[j++] = hexchar(page_83[i] >> 4); serial[j++] = hexchar(page_83[i]); i++; @@ -533,16 +533,20 @@ static int check_fill_0x83_id(struct scsi_id_device *dev_scsi, /* Extract the raw binary from VPD 0x83 pre-SPC devices */ static int check_fill_0x83_prespc3(struct scsi_id_device *dev_scsi, - unsigned char *page_83, + uint8_t page_83[static SCSI_INQ_BUFF_LEN], const struct scsi_id_search_values - *id_search, char *serial, char *serial_short, int max_len) { - int i, j; + *id_search, char *serial, char *serial_short, size_t max_len) { + size_t j; + + assert(max_len > 0); serial[0] = hexchar(SCSI_ID_NAA); /* serial has been memset to zero before */ j = strlen(serial); /* j = 1; */ - for (i = 0; (i < page_83[3]) && (j < max_len-3); ++i) { + /* Cap reported page length to buffer size in case of malformed responses */ + size_t page_len = MIN((size_t)page_83[3], SCSI_INQ_BUFF_LEN - 4); + for (size_t i = 0; (i < page_len) && (j + 3 < max_len); ++i) { serial[j++] = hexchar(page_83[4+i] >> 4); serial[j++] = hexchar(page_83[4+i]); } @@ -553,11 +557,11 @@ static int check_fill_0x83_prespc3(struct scsi_id_device *dev_scsi, /* Get device identification VPD page */ static int do_scsi_page83_inquiry(struct scsi_id_device *dev_scsi, int fd, - char *serial, char *serial_short, int len, + char *serial, char *serial_short, size_t len, char *unit_serial_number, char *wwn, char *wwn_vendor_extension, char *tgpt_group) { int retval; - unsigned char page_83[SCSI_INQ_BUFF_LEN]; + uint8_t page_83[SCSI_INQ_BUFF_LEN]; /* also pick up the page 80 serial number */ do_scsi_page80_inquiry(dev_scsi, fd, NULL, unit_serial_number, MAX_SERIAL_LEN); @@ -610,12 +614,25 @@ static int do_scsi_page83_inquiry(struct scsi_id_device *dev_scsi, int fd, * Search for a match in the prioritized id_search_list - since WWN ids * come first we can pick up the WWN in check_fill_0x83_id(). */ + + /* Cap reported page length to buffer size in case of malformed responses. + * Below, j can equal page_end, and at that point page_83[j + 3] (the first descriptor data byte) + * must still be readable before the inner bounds check, so page_end + 4 < SCSI_INQ_BUFF_LEN + * requires page_end <= SCSI_INQ_BUFF_LEN - 5. */ + unsigned page_end = MIN(((unsigned)page_83[2] << 8) + (unsigned)page_83[3] + 3U, + SCSI_INQ_BUFF_LEN - 5U); + FOREACH_ELEMENT(search_value, id_search_list) { /* * Examine each descriptor returned. There is normally only * one or a small number of descriptors. */ - for (unsigned j = 4; j <= ((unsigned)page_83[2] << 8) + (unsigned)page_83[3] + 3; j += page_83[j + 3] + 4) { + for (unsigned j = 4; j <= page_end; j += page_83[j + 3] + 4) { + /* Ensure the full descriptor fits within the buffer, including + * fixed-offset accesses up to page_83[7] in the TGTGROUP path + * of check_fill_0x83_id(), so require at least 8 bytes from j */ + if (j + MAX(4U + (unsigned)page_83[j + 3], 8U) > SCSI_INQ_BUFF_LEN) + break; retval = check_fill_0x83_id(dev_scsi, page_83 + j, search_value, serial, serial_short, len, @@ -638,10 +655,10 @@ static int do_scsi_page83_inquiry(struct scsi_id_device *dev_scsi, int fd, * conformant to the SCSI-2 format. */ static int do_scsi_page83_prespc3_inquiry(struct scsi_id_device *dev_scsi, int fd, - char *serial, char *serial_short, int len) { + char *serial, char *serial_short, size_t len) { int retval; - int i, j; - unsigned char page_83[SCSI_INQ_BUFF_LEN]; + size_t i, j; + uint8_t page_83[SCSI_INQ_BUFF_LEN]; memzero(page_83, SCSI_INQ_BUFF_LEN); retval = scsi_inquiry(dev_scsi, fd, 1, PAGE_83, page_83, SCSI_INQ_BUFF_LEN); @@ -688,7 +705,9 @@ static int do_scsi_page83_prespc3_inquiry(struct scsi_id_device *dev_scsi, int f * using two bytes of ASCII for each byte * in the page_83. */ - while (i < (page_83[3]+4)) { + /* Cap reported page length to buffer size in case of malformed responses */ + size_t page_len = MIN((size_t)page_83[3] + 4, SCSI_INQ_BUFF_LEN); + while (i < page_len && j + 2 < len) { serial[j++] = hexchar(page_83[i] >> 4); serial[j++] = hexchar(page_83[i]); i++; @@ -698,12 +717,11 @@ static int do_scsi_page83_prespc3_inquiry(struct scsi_id_device *dev_scsi, int f /* Get unit serial number VPD page */ static int do_scsi_page80_inquiry(struct scsi_id_device *dev_scsi, int fd, - char *serial, char *serial_short, int max_len) { + char *serial, char *serial_short, size_t max_len) { int retval; int ser_ind; - int i; - int len; - unsigned char buf[SCSI_INQ_BUFF_LEN]; + size_t page_len; + uint8_t buf[SCSI_INQ_BUFF_LEN]; memzero(buf, SCSI_INQ_BUFF_LEN); retval = scsi_inquiry(dev_scsi, fd, 1, PAGE_80, buf, SCSI_INQ_BUFF_LEN); @@ -715,29 +733,30 @@ static int do_scsi_page80_inquiry(struct scsi_id_device *dev_scsi, int fd, return 1; } - len = 1 + VENDOR_LENGTH + MODEL_LENGTH + buf[3]; - if (max_len < len) { - log_debug("%s: length %d too short - need %d", - dev_scsi->kernel, max_len, len); + page_len = 1 + VENDOR_LENGTH + MODEL_LENGTH + buf[3]; + if (max_len < page_len) { + log_debug("%s: length %zu too short - need %zu", + dev_scsi->kernel, max_len, page_len); return 1; } /* * Prepend 'S' to avoid unlikely collision with page 0x83 vendor * specific type where we prepend '0' + vendor + model. */ - len = buf[3]; + /* Cap reported page length to buffer size in case of malformed responses */ + page_len = MIN((size_t)buf[3], SCSI_INQ_BUFF_LEN - 4); if (serial) { serial[0] = 'S'; ser_ind = append_vendor_model(dev_scsi, serial + 1); if (ser_ind < 0) return 1; ser_ind++; /* for the leading 'S' */ - for (i = 4; i < len + 4; i++, ser_ind++) + for (size_t i = 4; i < page_len + 4; i++, ser_ind++) serial[ser_ind] = buf[i]; } if (serial_short) { - memcpy(serial_short, buf + 4, len); - serial_short[len] = '\0'; + memcpy(serial_short, buf + 4, page_len); + serial_short[page_len] = '\0'; } return 0; } @@ -781,11 +800,11 @@ out: } int scsi_get_serial(struct scsi_id_device *dev_scsi, const char *devname, - int page_code, int len) { - unsigned char page0[SCSI_INQ_BUFF_LEN]; + int page_code, size_t len) { + uint8_t page0[SCSI_INQ_BUFF_LEN]; int fd = -EBADF; int cnt; - int ind; + size_t ind; int retval; memzero(dev_scsi->serial, len); @@ -860,7 +879,10 @@ int scsi_get_serial(struct scsi_id_device *dev_scsi, const char *devname, goto completed; } - for (ind = 4; ind <= page0[3] + 3; ind++) + /* Cap reported page length to buffer size in case of malformed responses */ + size_t page0_end = MIN((size_t)page0[3] + 3, SCSI_INQ_BUFF_LEN - 1); + + for (ind = 4; ind <= page0_end; ind++) if (page0[ind] == PAGE_83) if (!do_scsi_page83_inquiry(dev_scsi, fd, dev_scsi->serial, dev_scsi->serial_short, len, dev_scsi->unit_serial_number, dev_scsi->wwn, dev_scsi->wwn_vendor_extension, dev_scsi->tgpt_group)) { @@ -871,7 +893,7 @@ int scsi_get_serial(struct scsi_id_device *dev_scsi, const char *devname, goto completed; } - for (ind = 4; ind <= page0[3] + 3; ind++) + for (ind = 4; ind <= page0_end; ind++) if (page0[ind] == PAGE_80) if (!do_scsi_page80_inquiry(dev_scsi, fd, dev_scsi->serial, dev_scsi->serial_short, len)) {