From a05294ff05923563087b53c1db64816130be3b34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 28 Feb 2019 11:29:38 +0100 Subject: [PATCH 1/3] shared/install: do not use a temporary variable outside of its scope Coverity says: > Pointer to local outside scope (RETURN_LOCAL)9. > use_invalid: Using dirs, which points to an out-of-scope temporary variable of type char const *[5]. And indeed, the switch statement forms a scope. Let's use an if to avoid creating a scope. --- src/shared/install.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/shared/install.c b/src/shared/install.c index 9e88ac46bd..64d55532e2 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -2827,19 +2827,14 @@ static int presets_find_config(UnitFileScope scope, const char *root_dir, char * assert(scope >= 0); assert(scope < _UNIT_FILE_SCOPE_MAX); - switch (scope) { - case UNIT_FILE_SYSTEM: + if (scope == UNIT_FILE_SYSTEM) dirs = (const char* const*) CONF_PATHS_STRV("systemd/system-preset"); - break; - case UNIT_FILE_GLOBAL: - case UNIT_FILE_USER: + else if (IN_SET(scope, UNIT_FILE_GLOBAL, UNIT_FILE_USER)) dirs = (const char* const*) CONF_PATHS_USR_STRV("systemd/user-preset"); - break; - default: + else assert_not_reached("Invalid unit file scope"); - } return conf_files_list_strv(files, ".preset", root_dir, 0, dirs); } From 8bdca77c407d100b8c81e2108aba47a8810ead88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 28 Feb 2019 11:40:44 +0100 Subject: [PATCH 2/3] udev-builtin-usb_id: use strjoina to simplify code --- src/udev/udev-builtin-usb_id.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c index 3525d25048..489b232814 100644 --- a/src/udev/udev-builtin-usb_id.c +++ b/src/udev/udev-builtin-usb_id.c @@ -138,13 +138,12 @@ static void set_scsi_type(char *to, const char *from, size_t len) { #define USB_DT_INTERFACE 0x04 static int dev_if_packed_info(sd_device *dev, char *ifs_str, size_t len) { - _cleanup_free_ char *filename = NULL; _cleanup_close_ int fd = -1; ssize_t size; unsigned char buf[18 + 65535]; size_t pos = 0; unsigned strpos = 0; - const char *syspath; + const char *filename, *syspath; int r; struct usb_interface_descriptor { uint8_t bLength; @@ -161,12 +160,11 @@ static int dev_if_packed_info(sd_device *dev, char *ifs_str, size_t len) { r = sd_device_get_syspath(dev, &syspath); if (r < 0) return r; - if (asprintf(&filename, "%s/descriptors", syspath) < 0) - return log_oom(); + filename = strjoina(syspath, "/descriptors"); fd = open(filename, O_RDONLY|O_CLOEXEC); if (fd < 0) - return log_device_debug_errno(dev, errno, "Failed to open USB device 'descriptors' file: %m"); + return log_device_debug_errno(dev, errno, "Failed to open \"%s\": %m", filename); size = read(fd, buf, sizeof(buf)); if (size < 18 || (size_t) size >= sizeof(buf)) From 760034bebed2c4cfc8a6b114a207f817b2e61db3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 28 Feb 2019 11:57:51 +0100 Subject: [PATCH 3/3] udev-builtin-usb_id: guard against overflow when reading descriptor data CID#996458. Coverity warns that we trust desc->bLength as read in the input data to adjust our position in the buffer. This value could be anything, leading to overflow. It's unlikely that the kernel feeds us invalid data, but let's me more careful. If any error is encountered, more logs are given. --- src/udev/udev-builtin-usb_id.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c index 489b232814..7bdf6cfbb5 100644 --- a/src/udev/udev-builtin-usb_id.c +++ b/src/udev/udev-builtin-usb_id.c @@ -167,8 +167,10 @@ static int dev_if_packed_info(sd_device *dev, char *ifs_str, size_t len) { return log_device_debug_errno(dev, errno, "Failed to open \"%s\": %m", filename); size = read(fd, buf, sizeof(buf)); - if (size < 18 || (size_t) size >= sizeof(buf)) - return -EIO; + if (size < 18) + return log_device_warning_errno(dev, SYNTHETIC_ERRNO(EIO), + "Short read from \"%s\"", filename); + assert((size_t) size <= sizeof buf); ifs_str[0] = '\0'; while (pos + sizeof(struct usb_interface_descriptor) < (size_t) size && @@ -177,9 +179,12 @@ static int dev_if_packed_info(sd_device *dev, char *ifs_str, size_t len) { struct usb_interface_descriptor *desc; char if_str[8]; - desc = (struct usb_interface_descriptor *) &buf[pos]; + desc = (struct usb_interface_descriptor *) (buf + pos); if (desc->bLength < 3) break; + if (desc->bLength > size - sizeof(struct usb_interface_descriptor)) + return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EIO), + "Corrupt data read from \"%s\"", filename); pos += desc->bLength; if (desc->bDescriptorType != USB_DT_INTERFACE)