From 2aa2860bc3ec70e39217795b9ceac759b4bf1735 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 25 Apr 2018 09:45:28 +0200 Subject: [PATCH 1/2] gpt-auto-generator: use stack variables and fix minor memleak --- src/gpt-auto-generator/gpt-auto-generator.c | 40 ++++++++------------- 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index 31087a0812..5b03f45372 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -46,9 +46,9 @@ static bool arg_root_enabled = true; static bool arg_root_rw = false; static int add_cryptsetup(const char *id, const char *what, bool rw, bool require, char **device) { - _cleanup_free_ char *e = NULL, *n = NULL, *p = NULL, *d = NULL, *id_escaped = NULL, *what_escaped = NULL; + _cleanup_free_ char *e = NULL, *n = NULL, *d = NULL, *id_escaped = NULL, *what_escaped = NULL; _cleanup_fclose_ FILE *f = NULL; - char *ret; + const char *p; int r; assert(id); @@ -74,10 +74,7 @@ static int add_cryptsetup(const char *id, const char *what, bool rw, bool requir if (!what_escaped) return log_oom(); - p = strjoin(arg_dest, "/", n); - if (!p) - return log_oom(); - + p = strjoina(arg_dest, "/", n); f = fopen(p, "wxe"); if (!f) return log_error_errno(errno, "Failed to create unit file %s: %m", p); @@ -125,11 +122,7 @@ static int add_cryptsetup(const char *id, const char *what, bool rw, bool requir return r; } - free(p); - p = strjoin(arg_dest, "/dev-mapper-", e, ".device.d/50-job-timeout-sec-0.conf"); - if (!p) - return log_oom(); - + p = strjoina(arg_dest, "/dev-mapper-", e, ".device.d/50-job-timeout-sec-0.conf"); mkdir_parents_label(p, 0755); r = write_string_file(p, "# Automatically generated by systemd-gpt-auto-generator\n\n" @@ -139,12 +132,16 @@ static int add_cryptsetup(const char *id, const char *what, bool rw, bool requir if (r < 0) return log_error_errno(r, "Failed to write device drop-in: %m"); - ret = strappend("/dev/mapper/", id); - if (!ret) - return log_oom(); + if (device) { + char *ret; + + ret = strappend("/dev/mapper/", id); + if (!ret) + return log_oom(); - if (device) *device = ret; + } + return 0; } @@ -337,8 +334,8 @@ static int add_automount( usec_t timeout) { _cleanup_free_ char *unit = NULL; - _cleanup_free_ char *opt, *p = NULL; _cleanup_fclose_ FILE *f = NULL; + const char *opt = "noauto", *p; int r; assert(id); @@ -346,11 +343,7 @@ static int add_automount( assert(description); if (options) - opt = strjoin(options, ",noauto"); - else - opt = strdup("noauto"); - if (!opt) - return log_oom(); + opt = strjoina(options, ",", opt); r = add_mount(id, what, @@ -367,10 +360,7 @@ static int add_automount( if (r < 0) return log_error_errno(r, "Failed to generate unit name: %m"); - p = strjoin(arg_dest, "/", unit); - if (!p) - return log_oom(); - + p = strjoina(arg_dest, "/", unit); f = fopen(p, "wxe"); if (!f) return log_error_errno(errno, "Failed to create unit file %s: %m", unit); From e137880b2a58bd899c7d0e15dc24aa20fe02ab31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 25 Apr 2018 10:06:23 +0200 Subject: [PATCH 2/2] gpt-auto-generator: emit warnings and fail if we fail to query mount points Right now gpt-auto-generator will iterate through all mount entries, and silently ignore failure to check if the mount point target is empty. This can hide real errors (in particular from MAC), so instead let's warn and return failure at the end if this happens. We will still iterate over other candidates, so there should be no change in behaviour. Logging is moved into path_is_busy() to avoid the duplication of the same logging code in the two callers. --- src/gpt-auto-generator/gpt-auto-generator.c | 29 ++++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index 5b03f45372..66f916b4de 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -230,7 +230,7 @@ static int add_mount( return 0; } -static bool path_is_busy(const char *where) { +static int path_is_busy(const char *where) { int r; /* already a mountpoint; generators run during reload */ @@ -243,13 +243,17 @@ static bool path_is_busy(const char *where) { return false; if (r < 0) - return true; + return log_warning_errno(r, "Cannot check if \"%s\" is a mount point: %m", where); /* not a mountpoint but it contains files */ - if (dir_is_empty(where) <= 0) - return true; + r = dir_is_empty(where); + if (r < 0) + return log_warning_errno(r, "Cannot check if \"%s\" is empty: %m", where); + if (r > 0) + return false; - return false; + log_debug("\"%s\" already populated, ignoring.", where); + return true; } static int add_partition_mount( @@ -258,12 +262,12 @@ static int add_partition_mount( const char *where, const char *description) { + int r; assert(p); - if (path_is_busy(where)) { - log_debug("%s already populated, ignoring.", where); - return 0; - } + r = path_is_busy(where); + if (r != 0) + return r < 0 ? r : 0; return add_mount( id, @@ -407,10 +411,9 @@ static int add_esp(DissectedPartition *p) { return 0; } - if (path_is_busy(esp)) { - log_debug("%s already populated, ignoring.", esp); - return 0; - } + r = path_is_busy(esp); + if (r != 0) + return r < 0 ? r : 0; if (is_efi_boot()) { sd_id128_t loader_uuid;