From 21a9905c7a6742b0cf9866f1a6f9ee29f896291e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 15 Feb 2018 09:20:36 +0100 Subject: [PATCH 1/5] libsystemd-network: unitialized return value r will *usually* be set, but for some packets it might not. CID #1385308. --- src/libsystemd-network/sd-dhcp6-client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index ec3484383b..2d38e51424 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -973,7 +973,7 @@ static int client_parse_message( lease->pd.ia_pd.lifetime_t2 = htobe32(lt_t2); } - return r; + return 0; } static int client_receive_reply(sd_dhcp6_client *client, DHCP6Message *reply, size_t len) { From 99f1d3fc5043b33dea5faa88f7015a487965333f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 15 Feb 2018 09:37:44 +0100 Subject: [PATCH 2/5] sd-dhcp6: fix check if serverid is set Ever since the initial implementation in 631bbe71298ec892f77f44f94feb612646fe6853, client_parse_message() was supposed to check that the message contains exactly one serverid. The check that no more than one is given was implemented correctly, but the check that at least one is given was not. Simplify the whole thing by making dhcp6_lease_get_serverid() return an error if the id is not set, and do not require the arguments to be present if the contents of the id are not needed. --- src/libsystemd-network/sd-dhcp6-client.c | 30 ++++++++++++------------ src/libsystemd-network/sd-dhcp6-lease.c | 11 +++++---- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index 2d38e51424..056bfa3d3d 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -771,8 +771,6 @@ static int client_parse_message( size_t pos = 0; int r; bool clientid = false; - uint8_t *id = NULL; - size_t id_len; uint32_t lt_t1 = ~0, lt_t2 = ~0; assert(client); @@ -817,8 +815,8 @@ static int client_parse_message( break; case SD_DHCP6_OPTION_SERVERID: - r = dhcp6_lease_get_serverid(lease, &id, &id_len); - if (r >= 0 && id) { + r = dhcp6_lease_get_serverid(lease, NULL, NULL); + if (r >= 0) { log_dhcp6_client(client, "%s contains multiple serverids", dhcp6_message_type_to_string(message->type)); return -EINVAL; @@ -956,21 +954,23 @@ static int client_parse_message( } if (client->state != DHCP6_STATE_INFORMATION_REQUEST) { - r = dhcp6_lease_get_serverid(lease, &id, &id_len); - if (r < 0) + r = dhcp6_lease_get_serverid(lease, NULL, NULL); + if (r < 0) { log_dhcp6_client(client, "%s has no server id", dhcp6_message_type_to_string(message->type)); - return r; - } + return -EINVAL; + } - if (lease->ia.addresses) { - lease->ia.ia_na.lifetime_t1 = htobe32(lt_t1); - lease->ia.ia_na.lifetime_t2 = htobe32(lt_t2); - } + } else { + if (lease->ia.addresses) { + lease->ia.ia_na.lifetime_t1 = htobe32(lt_t1); + lease->ia.ia_na.lifetime_t2 = htobe32(lt_t2); + } - if (lease->pd.addresses) { - lease->pd.ia_pd.lifetime_t1 = htobe32(lt_t1); - lease->pd.ia_pd.lifetime_t2 = htobe32(lt_t2); + if (lease->pd.addresses) { + lease->pd.ia_pd.lifetime_t1 = htobe32(lt_t1); + lease->pd.ia_pd.lifetime_t2 = htobe32(lt_t2); + } } return 0; diff --git a/src/libsystemd-network/sd-dhcp6-lease.c b/src/libsystemd-network/sd-dhcp6-lease.c index 1f3a782f8c..0089fbb737 100644 --- a/src/libsystemd-network/sd-dhcp6-lease.c +++ b/src/libsystemd-network/sd-dhcp6-lease.c @@ -95,11 +95,14 @@ int dhcp6_lease_set_serverid(sd_dhcp6_lease *lease, const uint8_t *id, int dhcp6_lease_get_serverid(sd_dhcp6_lease *lease, uint8_t **id, size_t *len) { assert_return(lease, -EINVAL); - assert_return(id, -EINVAL); - assert_return(len, -EINVAL); - *id = lease->serverid; - *len = lease->serverid_len; + if (!lease->serverid) + return -ENOMSG; + + if (id) + *id = lease->serverid; + if (len) + *len = lease->serverid_len; return 0; } From f863b1c6faf8de851a064b5009850b0fca40143b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 15 Feb 2018 10:10:01 +0100 Subject: [PATCH 3/5] core: move very long argument to a separate statement I like compact, but this was a bit too much. --- src/core/namespace.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index 70089f212a..72f0d038d1 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -735,6 +735,7 @@ static int mount_entry_chase( char *chased; int r; + unsigned flags = 0; assert(m); @@ -742,9 +743,19 @@ static int mount_entry_chase( * chase the symlinks on our own first. This is called for the destination path, as well as the source path (if * that applies). The result is stored in "location". */ - r = chase_symlinks(path, root_directory, - IN_SET(m->mode, BIND_MOUNT, BIND_MOUNT_RECURSIVE, PRIVATE_TMP, PRIVATE_VAR_TMP, PRIVATE_DEV, BIND_DEV, EMPTY_DIR, SYSFS, PROCFS) ? CHASE_NONEXISTENT : 0, - &chased); + if (IN_SET(m->mode, + BIND_MOUNT, + BIND_MOUNT_RECURSIVE, + PRIVATE_TMP, + PRIVATE_VAR_TMP, + PRIVATE_DEV, + BIND_DEV, + EMPTY_DIR, + SYSFS, + PROCFS)) + flags |= CHASE_NONEXISTENT; + + r = chase_symlinks(path, root_directory, flags, &chased); if (r == -ENOENT && m->ignore) { log_debug_errno(r, "Path %s does not exist, ignoring.", path); return 0; From 8405dcf752487a38d92896486ced2889aea8e746 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 15 Feb 2018 10:18:25 +0100 Subject: [PATCH 4/5] nspawn: make sure we don't leak the fd in chase_symlinks_and_update No callers use CHASE_OPEN right now, but let's be defensive. --- src/nspawn/nspawn.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 0f05ecff03..7405359cc7 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2233,10 +2233,8 @@ static int chase_symlinks_and_update(char **p, unsigned flags) { if (r < 0) return log_error_errno(r, "Failed to resolve path %s: %m", *p); - free(*p); - *p = chased; - - return 0; + free_and_replace(*p, chased); + return r; /* r might be an fd here in case we ever use CHASE_OPEN in flags */ } static int determine_uid_shift(const char *directory) { From 1b11339179296c6ccd3a8c717d1d8d530ee61ebc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 15 Feb 2018 10:30:46 +0100 Subject: [PATCH 5/5] udevadm: fix fd leak on oom Unlikely, but let's be correct. CID #1386003. --- src/udev/udevadm-trigger.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c index 05ddcdf48b..d979c1bc24 100644 --- a/src/udev/udevadm-trigger.c +++ b/src/udev/udevadm-trigger.c @@ -42,7 +42,7 @@ static int exec_list(struct udev_enumerate *udev_enumerate, const char *action, udev_list_entry_foreach(entry, udev_enumerate_get_list_entry(udev_enumerate)) { char filename[UTIL_PATH_SIZE]; const char *syspath; - int fd; + _cleanup_close_ int fd = -1; syspath = udev_list_entry_get_name(entry); if (verbose) @@ -54,14 +54,15 @@ static int exec_list(struct udev_enumerate *udev_enumerate, const char *action, fd = open(filename, O_WRONLY|O_CLOEXEC); if (fd < 0) continue; + if (settle_set) { r = set_put_strdup(settle_set, syspath); if (r < 0) return log_oom(); } + if (write(fd, action, strlen(action)) < 0) log_debug_errno(errno, "error writing '%s' to '%s': %m", action, filename); - close(fd); } return 0;