From: Petr Štetiar Date: Sat, 28 Dec 2019 18:00:39 +0000 (+0100) Subject: blobmsg: fix wrong payload len passed from blobmsg_check_array X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=cd75136b1342e1e9dabf921be13240c6653640ed;p=oweals%2Flibubox.git blobmsg: fix wrong payload len passed from blobmsg_check_array 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 Tested-by: Kevin Darbyshire-Bryant Signed-off-by: Petr Štetiar --- diff --git a/blobmsg.c b/blobmsg.c index 37821c3..1dd57e1 100644 --- 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 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 index 0000000..87404d5 --- /dev/null +++ b/tests/cram/test_blobmsg_procd_instance.t @@ -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 index 0000000..a5b4706 --- /dev/null +++ b/tests/test-blobmsg-procd-instance.c @@ -0,0 +1,110 @@ +#include +#include +#include +#include + +#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 \n", argv[0]); + return 3; + } + + test_blobmsg_procd_instance(argv[1]); + + return 0; +}