blobmsg: fix wrong payload len passed from blobmsg_check_array
authorPetr Štetiar <ynezz@true.cz>
Sat, 28 Dec 2019 18:00:39 +0000 (19:00 +0100)
committerPetr Štetiar <ynezz@true.cz>
Sat, 28 Dec 2019 20:17:46 +0000 (21:17 +0100)
Fix incorrect use of blob_raw_len() on passed blobmsg to
blobmsg_check_array_len()  introduced in commit b0e21553ae8c ("blobmsg:
add _len variants for all attribute checking methods") by using correct
blobmsg_len().

This wrong (higher) length was then for example causing issues in
procd's instance_config_parse_command() where blobmsg_check_attr_list()
was failing sanity checking of service command, thus resulting in the
startup failures of some services like collectd, nlbwmon and samba4.

Ref: http://lists.infradead.org/pipermail/openwrt-devel/2019-December/020840.html
Fixes: b0e21553ae8c ("blobmsg: add _len variants for all attribute checking methods")
Reported-by: Hannu Nyman <hannu.nyman@welho.com>
Tested-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
Signed-off-by: Petr Štetiar <ynezz@true.cz>
blobmsg.c
tests/cram/inputs/procd-instance-nlbwmon.bin [new file with mode: 0644]
tests/cram/test_blobmsg_procd_instance.t [new file with mode: 0644]
tests/test-blobmsg-procd-instance.c [new file with mode: 0644]

index 37821c33fa1b2a889af0bb04ee48921cb5cc55df..1dd57e156122ec06d1797f9a17fb10938c2fc356 100644 (file)
--- a/blobmsg.c
+++ b/blobmsg.c
@@ -95,7 +95,7 @@ bool blobmsg_check_attr_len(const struct blob_attr *attr, bool name, size_t len)
 
 int blobmsg_check_array(const struct blob_attr *attr, int type)
 {
-       return blobmsg_check_array_len(attr, type, blob_raw_len(attr));
+       return blobmsg_check_array_len(attr, type, blobmsg_len(attr));
 }
 
 int blobmsg_check_array_len(const struct blob_attr *attr, int type, size_t len)
diff --git a/tests/cram/inputs/procd-instance-nlbwmon.bin b/tests/cram/inputs/procd-instance-nlbwmon.bin
new file mode 100644 (file)
index 0000000..db7748a
Binary files /dev/null and b/tests/cram/inputs/procd-instance-nlbwmon.bin differ
diff --git a/tests/cram/test_blobmsg_procd_instance.t b/tests/cram/test_blobmsg_procd_instance.t
new file mode 100644 (file)
index 0000000..87404d5
--- /dev/null
@@ -0,0 +1,11 @@
+check that blobmsg parsing/checking would produce expected results in procd:
+
+  $ [ -n "$TEST_BIN_DIR" ] && export PATH="$TEST_BIN_DIR:$PATH"
+  $ export INPUTS="$TESTDIR/inputs"
+
+  $ for blob in $(LC_ALL=C find $INPUTS -type f | sort ); do
+  >   valgrind --quiet --leak-check=full test-blobmsg-procd-instance $blob; \
+  >   test-blobmsg-procd-instance-san $blob; \
+  > done
+  procd-instance-nlbwmon.bin: OK
+  procd-instance-nlbwmon.bin: OK
diff --git a/tests/test-blobmsg-procd-instance.c b/tests/test-blobmsg-procd-instance.c
new file mode 100644 (file)
index 0000000..a5b4706
--- /dev/null
@@ -0,0 +1,110 @@
+#include <stdio.h>
+#include <stdint.h>
+#include <stddef.h>
+#include <libgen.h>
+
+#include "blobmsg.h"
+#include "utils.h"
+
+enum {
+       INSTANCE_ATTR_COMMAND,
+       INSTANCE_ATTR_ENV,
+       INSTANCE_ATTR_DATA,
+       INSTANCE_ATTR_NETDEV,
+       INSTANCE_ATTR_FILE,
+       INSTANCE_ATTR_TRIGGER,
+       INSTANCE_ATTR_RESPAWN,
+       INSTANCE_ATTR_NICE,
+       INSTANCE_ATTR_LIMITS,
+       INSTANCE_ATTR_WATCH,
+       INSTANCE_ATTR_ERROR,
+       INSTANCE_ATTR_USER,
+       INSTANCE_ATTR_GROUP,
+       INSTANCE_ATTR_STDOUT,
+       INSTANCE_ATTR_STDERR,
+       INSTANCE_ATTR_NO_NEW_PRIVS,
+       INSTANCE_ATTR_JAIL,
+       INSTANCE_ATTR_TRACE,
+       INSTANCE_ATTR_SECCOMP,
+       INSTANCE_ATTR_PIDFILE,
+       INSTANCE_ATTR_RELOADSIG,
+       INSTANCE_ATTR_TERMTIMEOUT,
+       INSTANCE_ATTR_FACILITY,
+       __INSTANCE_ATTR_MAX
+};
+
+static const struct blobmsg_policy instance_attr[__INSTANCE_ATTR_MAX] = {
+       [INSTANCE_ATTR_COMMAND] = { "command", BLOBMSG_TYPE_ARRAY },
+       [INSTANCE_ATTR_ENV] = { "env", BLOBMSG_TYPE_TABLE },
+       [INSTANCE_ATTR_DATA] = { "data", BLOBMSG_TYPE_TABLE },
+       [INSTANCE_ATTR_NETDEV] = { "netdev", BLOBMSG_TYPE_ARRAY },
+       [INSTANCE_ATTR_FILE] = { "file", BLOBMSG_TYPE_ARRAY },
+       [INSTANCE_ATTR_TRIGGER] = { "triggers", BLOBMSG_TYPE_ARRAY },
+       [INSTANCE_ATTR_RESPAWN] = { "respawn", BLOBMSG_TYPE_ARRAY },
+       [INSTANCE_ATTR_NICE] = { "nice", BLOBMSG_TYPE_INT32 },
+       [INSTANCE_ATTR_LIMITS] = { "limits", BLOBMSG_TYPE_TABLE },
+       [INSTANCE_ATTR_WATCH] = { "watch", BLOBMSG_TYPE_ARRAY },
+       [INSTANCE_ATTR_ERROR] = { "error", BLOBMSG_TYPE_ARRAY },
+       [INSTANCE_ATTR_USER] = { "user", BLOBMSG_TYPE_STRING },
+       [INSTANCE_ATTR_GROUP] = { "group", BLOBMSG_TYPE_STRING },
+       [INSTANCE_ATTR_STDOUT] = { "stdout", BLOBMSG_TYPE_BOOL },
+       [INSTANCE_ATTR_STDERR] = { "stderr", BLOBMSG_TYPE_BOOL },
+       [INSTANCE_ATTR_NO_NEW_PRIVS] = { "no_new_privs", BLOBMSG_TYPE_BOOL },
+       [INSTANCE_ATTR_JAIL] = { "jail", BLOBMSG_TYPE_TABLE },
+       [INSTANCE_ATTR_TRACE] = { "trace", BLOBMSG_TYPE_BOOL },
+       [INSTANCE_ATTR_SECCOMP] = { "seccomp", BLOBMSG_TYPE_STRING },
+       [INSTANCE_ATTR_PIDFILE] = { "pidfile", BLOBMSG_TYPE_STRING },
+       [INSTANCE_ATTR_RELOADSIG] = { "reload_signal", BLOBMSG_TYPE_INT32 },
+       [INSTANCE_ATTR_TERMTIMEOUT] = { "term_timeout", BLOBMSG_TYPE_INT32 },
+       [INSTANCE_ATTR_FACILITY] = { "facility", BLOBMSG_TYPE_STRING },
+};
+
+static void test_blobmsg_procd_instance(const char *filename)
+{
+#define BUF_LEN 2048
+       int r = 0;
+       FILE *fd = NULL;
+       size_t len = 0;
+       char buf[BUF_LEN+1] = { 0 };
+       struct blob_attr *tb[__INSTANCE_ATTR_MAX];
+       const char *fname = basename((char *) filename);
+
+       fd = fopen(filename, "r");
+       if (!fd) {
+               fprintf(stderr, "unable to open %s\n", fname);
+               return;
+       }
+
+       len = fread(&buf, 1, BUF_LEN, fd);
+       fclose(fd);
+
+       r = blobmsg_parse(instance_attr, __INSTANCE_ATTR_MAX, tb, buf, len);
+       if (r)
+               return;
+
+       if (!tb[INSTANCE_ATTR_COMMAND] || !tb[INSTANCE_ATTR_NICE] || !tb[INSTANCE_ATTR_STDERR])
+               return;
+
+       if (!blobmsg_check_attr_list(tb[INSTANCE_ATTR_COMMAND], BLOBMSG_TYPE_STRING))
+               return;
+
+       if (blobmsg_get_u32(tb[INSTANCE_ATTR_NICE]) != 19)
+               return;
+
+       if (!blobmsg_get_bool(tb[INSTANCE_ATTR_STDERR]))
+               return;
+
+       fprintf(stderr, "%s: OK\n", fname);
+}
+
+int main(int argc, char *argv[])
+{
+       if (argc != 2) {
+               fprintf(stderr, "Usage: %s <blobmsg.bin>\n", argv[0]);
+               return 3;
+       }
+
+       test_blobmsg_procd_instance(argv[1]);
+
+       return 0;
+}