From 6fb0569065b56019850a25c2b235d7119bdb0ae6 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 12 Jul 2019 16:17:51 +0200
Subject: [PATCH 1/2] util-lib: [static] array argument sizes are apparently
 not OK for NULL parameters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Let's drop the 'static' logic when a parameter can be NULL.

I think asan/ubsan are right here, judging by the C99 spec language:

"A declaration of a parameter as ‘‘array of type’’ shall be adjusted to
‘‘qualified pointer to type’’, where the type qualifiers (if any) are
those specified within the [ and ] of the array type derivation. If the
keyword static also appears within the [ and ] of the array type
derivation, then for each call to the function, the value of the
corresponding actual argument shall provide access to the first element
of an array with at least as many elements as specified by the size
expression."

If we specify NULL, then we certainly don't pvode access to any valid
array.

Fixes: #13039
---
 src/basic/string-util.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/basic/string-util.c b/src/basic/string-util.c
index 7c487fb9a3..9586b3940e 100644
--- a/src/basic/string-util.c
+++ b/src/basic/string-util.c
@@ -725,10 +725,17 @@ char *strreplace(const char *text, const char *old_string, const char *new_strin
         return ret;
 }
 
-static void advance_offsets(ssize_t diff, size_t offsets[static 2], size_t shift[static 2], size_t size) {
+static void advance_offsets(
+                ssize_t diff,
+                size_t offsets[2], /* note: we can't use [static 2] here, since this may be NULL */
+                size_t shift[static 2],
+                size_t size) {
+
         if (!offsets)
                 return;
 
+        assert(shift);
+
         if ((size_t) diff < offsets[0])
                 shift[0] += size;
         if ((size_t) diff < offsets[1])
@@ -844,8 +851,7 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) {
 
         fclose(f);
 
-        free(*ibuf);
-        *ibuf = obuf;
+        free_and_replace(*ibuf, obuf);
 
         if (_isz)
                 *_isz = osz;
@@ -855,7 +861,7 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) {
                 highlight[1] += shift[1];
         }
 
-        return obuf;
+        return *ibuf;
 }
 
 char *strextend_with_separator(char **x, const char *separator, ...) {

From 2caa38e99f76788eb5bd84cbdef72c5e3912dd4b Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 12 Jul 2019 16:39:07 +0200
Subject: [PATCH 2/2] tree-wide: some more [static] related fixes

let's add [static] where it was missing so far

Drop [static] on parameters that can be NULL.

Add an assert() around parameters that have [static] and can't be NULL
hence.

Add some "const" where it was forgotten.
---
 src/core/execute.c             | 20 ++++++++++++--------
 src/core/namespace.c           |  4 ++--
 src/core/namespace.h           |  4 ++--
 src/network/netdev/wireguard.c |  2 ++
 src/shared/efivars.c           |  2 ++
 src/shared/ethtool-util.c      |  2 ++
 src/systemctl/systemctl.c      |  2 ++
 src/udev/scsi_id/scsi_serial.c |  3 +++
 src/udev/udev-event.c          |  1 +
 src/udev/udev-rules.c          |  2 ++
 10 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/src/core/execute.c b/src/core/execute.c
index 9c086103ae..911c369042 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -455,12 +455,13 @@ static int setup_input(
                 const ExecContext *context,
                 const ExecParameters *params,
                 int socket_fd,
-                int named_iofds[3]) {
+                const int named_iofds[static 3]) {
 
         ExecInput i;
 
         assert(context);
         assert(params);
+        assert(named_iofds);
 
         if (params->stdin_fd >= 0) {
                 if (dup2(params->stdin_fd, STDIN_FILENO) < 0)
@@ -570,7 +571,7 @@ static int setup_output(
                 const ExecParameters *params,
                 int fileno,
                 int socket_fd,
-                int named_iofds[3],
+                const int named_iofds[static 3],
                 const char *ident,
                 uid_t uid,
                 gid_t gid,
@@ -2721,9 +2722,7 @@ out:
 static void append_socket_pair(int *array, size_t *n, const int pair[static 2]) {
         assert(array);
         assert(n);
-
-        if (!pair)
-                return;
+        assert(pair);
 
         if (pair[0] >= 0)
                 array[(*n)++] = pair[0];
@@ -2914,7 +2913,7 @@ static int exec_child(
                 ExecRuntime *runtime,
                 DynamicCreds *dcreds,
                 int socket_fd,
-                int named_iofds[3],
+                const int named_iofds[static 3],
                 int *fds,
                 size_t n_socket_fds,
                 size_t n_storage_fds,
@@ -3782,7 +3781,7 @@ static int exec_child(
 }
 
 static int exec_context_load_environment(const Unit *unit, const ExecContext *c, char ***l);
-static int exec_context_named_iofds(const ExecContext *c, const ExecParameters *p, int named_iofds[3]);
+static int exec_context_named_iofds(const ExecContext *c, const ExecParameters *p, int named_iofds[static 3]);
 
 int exec_spawn(Unit *unit,
                ExecCommand *command,
@@ -4110,13 +4109,18 @@ const char* exec_context_fdname(const ExecContext *c, int fd_index) {
         }
 }
 
-static int exec_context_named_iofds(const ExecContext *c, const ExecParameters *p, int named_iofds[static 3]) {
+static int exec_context_named_iofds(
+                const ExecContext *c,
+                const ExecParameters *p,
+                int named_iofds[static 3]) {
+
         size_t i, targets;
         const char* stdio_fdname[3];
         size_t n_fds;
 
         assert(c);
         assert(p);
+        assert(named_iofds);
 
         targets = (c->std_input == EXEC_INPUT_NAMED_FD) +
                   (c->std_output == EXEC_OUTPUT_NAMED_FD) +
diff --git a/src/core/namespace.c b/src/core/namespace.c
index 7aab2f7593..973b64007c 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -1699,7 +1699,7 @@ int setup_tmp_dirs(const char *id, char **tmp_dir, char **var_tmp_dir) {
         return 0;
 }
 
-int setup_netns(int netns_storage_socket[static 2]) {
+int setup_netns(const int netns_storage_socket[static 2]) {
         _cleanup_close_ int netns = -1;
         int r, q;
 
@@ -1762,7 +1762,7 @@ fail:
         return r;
 }
 
-int open_netns_path(int netns_storage_socket[static 2], const char *path) {
+int open_netns_path(const int netns_storage_socket[static 2], const char *path) {
         _cleanup_close_ int netns = -1;
         int q, r;
 
diff --git a/src/core/namespace.h b/src/core/namespace.h
index 73fcb3fc82..e5cd8e5313 100644
--- a/src/core/namespace.h
+++ b/src/core/namespace.h
@@ -94,8 +94,8 @@ int setup_tmp_dirs(
                 char **tmp_dir,
                 char **var_tmp_dir);
 
-int setup_netns(int netns_storage_socket[static 2]);
-int open_netns_path(int netns_storage_socket[static 2], const char *path);
+int setup_netns(const int netns_storage_socket[static 2]);
+int open_netns_path(const int netns_storage_socket[static 2], const char *path);
 
 const char* protect_home_to_string(ProtectHome p) _const_;
 ProtectHome protect_home_from_string(const char *s) _pure_;
diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c
index 2e9651cceb..913ee2a058 100644
--- a/src/network/netdev/wireguard.c
+++ b/src/network/netdev/wireguard.c
@@ -899,6 +899,8 @@ static int wireguard_read_key_file(const char *filename, uint8_t dest[static WG_
         if (!filename)
                 return 0;
 
+        assert(dest);
+
         r = read_full_file_full(filename, READ_FULL_FILE_SECURE | READ_FULL_FILE_UNBASE64, &key, &key_len);
         if (r < 0)
                 return r;
diff --git a/src/shared/efivars.c b/src/shared/efivars.c
index f16c2db739..a4a3c200ac 100644
--- a/src/shared/efivars.c
+++ b/src/shared/efivars.c
@@ -660,6 +660,8 @@ int efi_set_boot_order(uint16_t *order, size_t n) {
 static int boot_id_hex(const char s[static 4]) {
         int id = 0, i;
 
+        assert(s);
+
         for (i = 0; i < 4; i++)
                 if (s[i] >= '0' && s[i] <= '9')
                         id |= (s[i] - '0') << (3 - i) * 4;
diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c
index d823931c76..b0961df72e 100644
--- a/src/shared/ethtool-util.c
+++ b/src/shared/ethtool-util.c
@@ -652,6 +652,8 @@ int ethtool_set_glinksettings(
         struct ifreq ifr = {};
         int r;
 
+        assert(advertise);
+
         if (autonegotiation != AUTONEG_DISABLE && memeqzero(advertise, sizeof(uint32_t) * N_ADVERTISE)) {
                 log_info("ethtool: autonegotiation is unset or enabled, the speed and duplex are not writable.");
                 return 0;
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 6490fe6f0d..52e3416cab 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -3021,6 +3021,8 @@ static enum action verb_to_action(const char *verb) {
 static const char** make_extra_args(const char *extra_args[static 4]) {
         size_t n = 0;
 
+        assert(extra_args);
+
         if (arg_scope != UNIT_FILE_SYSTEM)
                 extra_args[n++] = "--user";
 
diff --git a/src/udev/scsi_id/scsi_serial.c b/src/udev/scsi_id/scsi_serial.c
index 080b027d18..8deb692f13 100644
--- a/src/udev/scsi_id/scsi_serial.c
+++ b/src/udev/scsi_id/scsi_serial.c
@@ -400,6 +400,9 @@ static int append_vendor_model(
                 const struct scsi_id_device *dev_scsi,
                 char buf[static VENDOR_LENGTH + MODEL_LENGTH]) {
 
+        assert(dev_scsi);
+        assert(buf);
+
         if (strnlen(dev_scsi->vendor, VENDOR_LENGTH) != VENDOR_LENGTH)
                 return log_debug_errno(SYNTHETIC_ERRNO(EINVAL),
                                        "%s: bad vendor string \"%s\"",
diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
index dbdb9065dc..2dacd78968 100644
--- a/src/udev/udev-event.c
+++ b/src/udev/udev-event.c
@@ -156,6 +156,7 @@ static int get_subst_type(const char **str, bool strict, FormatSubstitutionType
         assert(str);
         assert(*str);
         assert(ret_type);
+        assert(ret_attr);
 
         if (*p == '$') {
                 p++;
diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c
index 303594fa51..e9a6453b6c 100644
--- a/src/udev/udev-rules.c
+++ b/src/udev/udev-rules.c
@@ -1464,6 +1464,8 @@ static int attr_subst_subdir(char attr[static UTIL_PATH_SIZE]) {
         const char *tail;
         size_t len, size;
 
+        assert(attr);
+
         tail = strstr(attr, "/*/");
         if (!tail)
             return 0;