From 290843c3852c5d2fda6bef8f3214b23c7d6120ab Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 Jul 2017 21:36:59 +0200 Subject: [PATCH 1/5] mount: fix potential bad memory access when /proc/self/mountinfo is empty It's unlikely this can ever be triggered, but let's be safe rather than sorry, and handle the case where the list of mount points is zero, and the "l" array thus NULL. let's ensure we allocate at least one entry. --- src/mount/mount-tool.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index 0ab06ac3b9..9d56b40700 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -736,13 +736,13 @@ static int find_mount_points(const char *what, char ***list) { if (!GREEDY_REALLOC(l, bufsize, n + 2)) return log_oom(); - l[n] = strdup(where); - if (!l[n]) - return log_oom(); - - n++; + l[n++] = where; + where = NULL; } + if (!GREEDY_REALLOC(l, bufsize, n + 1)) + return log_oom(); + l[n] = NULL; *list = l; l = NULL; /* avoid freeing */ From 5c6803f5bd1852fb42230c928250d19f1942c885 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 Jul 2017 21:39:23 +0200 Subject: [PATCH 2/5] mount: rework find_loop_device() to log about no errors We should either log about all errors in a function, or about none (and then leave the logging about it to the caller who we propagate the error to). Given that the callers of find_loop_device() already log about the returned errors let's hence suppress the log messages in find_loop_device() itself. --- src/mount/mount-tool.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index 9d56b40700..21c3b1302d 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -759,11 +759,8 @@ static int find_loop_device(const char *backing_file, char **loop_dev) { assert(loop_dev); d = opendir("/sys/devices/virtual/block"); - if (!d) { - if (errno == ENOENT) - return -ENOENT; - return log_error_errno(errno, "Can't open directory /sys/devices/virtual/block: %m"); - } + if (!d) + return -errno; FOREACH_DIRENT(de, d, return -errno) { _cleanup_free_ char *sys = NULL, *fname = NULL; @@ -779,7 +776,7 @@ static int find_loop_device(const char *backing_file, char **loop_dev) { sys = strjoin("/sys/devices/virtual/block/", de->d_name, "/loop/backing_file"); if (!sys) - return log_oom(); + return -ENOMEM; r = read_one_line_file(sys, &fname); if (r < 0) @@ -790,7 +787,7 @@ static int find_loop_device(const char *backing_file, char **loop_dev) { l = strjoin("/dev/", de->d_name); if (!l) - return log_oom(); + return -ENOMEM; break; } From a53dceb709f39ddbc25bfb910525c7d77c7a7dcf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 Jul 2017 21:41:02 +0200 Subject: [PATCH 3/5] mount: add debug logging for the case when we knowingly ignore an error --- src/mount/mount-tool.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index 21c3b1302d..23083fd16e 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -779,8 +779,10 @@ static int find_loop_device(const char *backing_file, char **loop_dev) { return -ENOMEM; r = read_one_line_file(sys, &fname); - if (r < 0) + if (r < 0) { + log_debug_errno(r, "Failed to read %s, ignoring: %m", sys); continue; + } if (files_same(fname, backing_file, 0) <= 0) continue; From aa46fa6420c641552eda6cde5de712cbebbc8dc6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 Jul 2017 21:41:14 +0200 Subject: [PATCH 4/5] mount: change find_loop_device() error code when no loop device is found to ENXIO ENOENT is a bit too likely to be returned for various reasons, for example if /sys or /proc are not mounted and hence the files we need not around. Hence, let's use ENXIO instead, which is equally fitting for the purpose but has the benefit that the underlying calls won't generate this error on their own, hence any ambiguity is removed. --- src/mount/mount-tool.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index 23083fd16e..c0b0e4630c 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -795,7 +795,7 @@ static int find_loop_device(const char *backing_file, char **loop_dev) { } if (!l) - return -ENOENT; + return -ENXIO; *loop_dev = l; l = NULL; /* avoid freeing */ @@ -954,7 +954,7 @@ static int umount_loop(sd_bus *bus, const char *backing_file) { r = find_loop_device(backing_file, &loop_dev); if (r < 0) - return log_error_errno(r, r == -ENOENT ? "File %s is not mounted." : "Can't get loop device for %s: %m", backing_file); + return log_error_errno(r, r == -ENXIO ? "File %s is not mounted." : "Can't get loop device for %s: %m", backing_file); return umount_by_device(bus, loop_dev); } @@ -1229,10 +1229,10 @@ static int discover_loop_backing_file(void) { int r; r = find_loop_device(arg_mount_what, &loop_dev); - if (r < 0 && r != -ENOENT) + if (r < 0 && r != -ENXIO) return log_error_errno(errno, "Can't get loop device for %s: %m", arg_mount_what); - if (r == -ENOENT) { + if (r == -ENXIO) { _cleanup_free_ char *escaped = NULL; if (arg_mount_where) From 514368adfbc9104c6226e883b8a68b451780ed23 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 Jul 2017 21:42:39 +0200 Subject: [PATCH 5/5] mount: add missing validation error message We really should generate exactly one log message for each error, hence let's do that in this one case too. --- src/mount/mount-tool.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index c0b0e4630c..ed6578d540 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -1241,8 +1241,10 @@ static int discover_loop_backing_file(void) { escaped = xescape(basename(arg_mount_what), "\\"); if (!escaped) return log_oom(); - if (!filename_is_valid(escaped)) + if (!filename_is_valid(escaped)) { + log_error("Escaped name %s is not a valid filename.", escaped); return -EINVAL; + } arg_mount_where = strjoin("/run/media/system/", escaped); if (!arg_mount_where)