From f19966efeedf5292f003aaccecc3ac3a8a069b28 Mon Sep 17 00:00:00 2001 From: Guillaume Nault Date: Fri, 8 Nov 2019 18:00:12 +0100 Subject: [PATCH 1/5] ipnetns: treat NETNSA_NSID and NETNSA_CURRENT_NSID as signed These attributes are signed (with -1 meaning NETNSA_NSID_NOT_ASSIGNED). So let's use rta_getattr_s32() and print_int() instead of their unsigned counterpart to avoid confusion. Signed-off-by: Guillaume Nault Signed-off-by: David Ahern --- ip/ipnetns.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/ip/ipnetns.c b/ip/ipnetns.c index fc58a04b..06a773e4 100644 --- a/ip/ipnetns.c +++ b/ip/ipnetns.c @@ -137,7 +137,7 @@ int get_netnsid_from_name(const char *name) parse_rtattr(tb, NETNSA_MAX, NETNS_RTA(rthdr), len); if (tb[NETNSA_NSID]) { - ret = rta_getattr_u32(tb[NETNSA_NSID]); + ret = rta_getattr_s32(tb[NETNSA_NSID]); } out: @@ -317,20 +317,20 @@ int print_nsid(struct nlmsghdr *n, void *arg) if (n->nlmsg_type == RTM_DELNSID) print_bool(PRINT_ANY, "deleted", "Deleted ", true); - nsid = rta_getattr_u32(tb[NETNSA_NSID]); + nsid = rta_getattr_s32(tb[NETNSA_NSID]); if (nsid < 0) print_string(PRINT_ANY, "nsid", "nsid %s ", "not-assigned"); else - print_uint(PRINT_ANY, "nsid", "nsid %u ", nsid); + print_int(PRINT_ANY, "nsid", "nsid %d ", nsid); if (tb[NETNSA_CURRENT_NSID]) { - current = rta_getattr_u32(tb[NETNSA_CURRENT_NSID]); + current = rta_getattr_s32(tb[NETNSA_CURRENT_NSID]); if (current < 0) print_string(PRINT_ANY, "current-nsid", "current-nsid %s ", "not-assigned"); else - print_uint(PRINT_ANY, "current-nsid", - "current-nsid %u ", current); + print_int(PRINT_ANY, "current-nsid", + "current-nsid %d ", current); } c = netns_map_get_by_nsid(tb[NETNSA_CURRENT_NSID] ? current : nsid); @@ -491,8 +491,7 @@ static int netns_list(int argc, char **argv) if (ipnetns_have_nsid()) { id = get_netnsid_from_name(entry->d_name); if (id >= 0) - print_uint(PRINT_ANY, "id", - " (id: %d)", id); + print_int(PRINT_ANY, "id", " (id: %d)", id); } print_string(PRINT_FP, NULL, "\n", NULL); close_json_object(); From df6da60bcba47e02483484a726d021d5b61ab5d1 Mon Sep 17 00:00:00 2001 From: Guillaume Nault Date: Fri, 8 Nov 2019 18:00:14 +0100 Subject: [PATCH 2/5] ipnetns: fix misleading comment about 'ip monitor nsid' 'ip monitor nsid' doesn't call print_nsid(). Signed-off-by: Guillaume Nault Signed-off-by: David Ahern --- ip/ipnetns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ip/ipnetns.c b/ip/ipnetns.c index 06a773e4..5ab99a68 100644 --- a/ip/ipnetns.c +++ b/ip/ipnetns.c @@ -340,7 +340,7 @@ int print_nsid(struct nlmsghdr *n, void *arg) netns_map_del(c); } - /* During 'ip monitor nsid', no chance to have new nsid in cache. */ + /* nsid might not be in cache */ if (c == NULL && n->nlmsg_type == RTM_NEWNSID) if (netns_get_name(nsid, name) == 0) { print_string(PRINT_ANY, "name", From 08ba67db7b9861d858f540369ec849d0c7f283c7 Mon Sep 17 00:00:00 2001 From: Guillaume Nault Date: Fri, 8 Nov 2019 18:00:15 +0100 Subject: [PATCH 3/5] ipnetns: harden helper functions wrt. negative netns ids Negative values are invalid netns ids. Ensure that helper functions don't accidentally try to process them. Signed-off-by: Guillaume Nault Signed-off-by: David Ahern --- ip/ipnetns.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/ip/ipnetns.c b/ip/ipnetns.c index 5ab99a68..355455db 100644 --- a/ip/ipnetns.c +++ b/ip/ipnetns.c @@ -161,9 +161,13 @@ static struct hlist_head name_head[NSIDMAP_SIZE]; static struct nsid_cache *netns_map_get_by_nsid(int nsid) { - uint32_t h = NSID_HASH_NSID(nsid); struct hlist_node *n; + uint32_t h; + if (nsid < 0) + return NULL; + + h = NSID_HASH_NSID(nsid); hlist_for_each(n, &nsid_head[h]) { struct nsid_cache *c = container_of(n, struct nsid_cache, nsid_hash); @@ -178,6 +182,9 @@ char *get_name_from_nsid(int nsid) { struct nsid_cache *c; + if (nsid < 0) + return NULL; + netns_nsid_socket_init(); netns_map_init(); @@ -266,6 +273,9 @@ static int netns_get_name(int nsid, char *name) DIR *dir; int id; + if (nsid < 0) + return -EINVAL; + dir = opendir(NETNS_RUN_DIR); if (!dir) return -ENOENT; @@ -277,7 +287,7 @@ static int netns_get_name(int nsid, char *name) continue; id = get_netnsid_from_name(entry->d_name); - if (nsid == id) { + if (id >= 0 && nsid == id) { strcpy(name, entry->d_name); closedir(dir); return 0; From 1c9b69276c76e44234e62d3e1f678d25172a03f4 Mon Sep 17 00:00:00 2001 From: Guillaume Nault Date: Fri, 8 Nov 2019 18:00:18 +0100 Subject: [PATCH 4/5] ipnetns: don't print unassigned nsid in json export Don't output the nsid and current-nsid json keys if they're not set. Otherwise a parser would have to special case the "not-assigned" string. Signed-off-by: Guillaume Nault Signed-off-by: David Ahern --- ip/ipnetns.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ip/ipnetns.c b/ip/ipnetns.c index 355455db..cb8c0019 100644 --- a/ip/ipnetns.c +++ b/ip/ipnetns.c @@ -329,15 +329,15 @@ int print_nsid(struct nlmsghdr *n, void *arg) nsid = rta_getattr_s32(tb[NETNSA_NSID]); if (nsid < 0) - print_string(PRINT_ANY, "nsid", "nsid %s ", "not-assigned"); + print_string(PRINT_FP, NULL, "nsid unassigned ", NULL); else print_int(PRINT_ANY, "nsid", "nsid %d ", nsid); if (tb[NETNSA_CURRENT_NSID]) { current = rta_getattr_s32(tb[NETNSA_CURRENT_NSID]); if (current < 0) - print_string(PRINT_ANY, "current-nsid", - "current-nsid %s ", "not-assigned"); + print_string(PRINT_FP, NULL, + "current-nsid unassigned ", NULL); else print_int(PRINT_ANY, "current-nsid", "current-nsid %d ", current); From d0b645a51e5b5315e14ae2f5538e1e9df6c9f419 Mon Sep 17 00:00:00 2001 From: Guillaume Nault Date: Fri, 8 Nov 2019 18:00:20 +0100 Subject: [PATCH 5/5] ipnetns: remove blank lines printed by invarg() messages Since invarg() automatically adds a '\n' character, having one in the error message generates an extra blank line. Signed-off-by: Guillaume Nault Signed-off-by: David Ahern --- ip/ipnetns.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ip/ipnetns.c b/ip/ipnetns.c index cb8c0019..46cc235b 100644 --- a/ip/ipnetns.c +++ b/ip/ipnetns.c @@ -439,10 +439,10 @@ static int netns_list_id(int argc, char **argv) NEXT_ARG(); if (get_integer(&filter.target_nsid, *argv, 0)) - invarg("\"target-nsid\" value is invalid\n", + invarg("\"target-nsid\" value is invalid", *argv); else if (filter.target_nsid < 0) - invarg("\"target-nsid\" value should be >= 0\n", + invarg("\"target-nsid\" value should be >= 0", argv[1]); } else if (strcmp(*argv, "nsid") == 0) { if (nsid >= 0) @@ -450,9 +450,9 @@ static int netns_list_id(int argc, char **argv) NEXT_ARG(); if (get_integer(&nsid, *argv, 0)) - invarg("\"nsid\" value is invalid\n", *argv); + invarg("\"nsid\" value is invalid", *argv); else if (nsid < 0) - invarg("\"nsid\" value should be >= 0\n", + invarg("\"nsid\" value should be >= 0", argv[1]); } else usage(); @@ -932,9 +932,9 @@ static int netns_set(int argc, char **argv) if (strcmp(argv[1], "auto") == 0) nsid = -1; else if (get_integer(&nsid, argv[1], 0)) - invarg("Invalid \"netnsid\" value\n", argv[1]); + invarg("Invalid \"netnsid\" value", argv[1]); else if (nsid < 0) - invarg("\"netnsid\" value should be >= 0\n", argv[1]); + invarg("\"netnsid\" value should be >= 0", argv[1]); snprintf(netns_path, sizeof(netns_path), "%s/%s", NETNS_RUN_DIR, name); netns = open(netns_path, O_RDONLY | O_CLOEXEC);