From 70556c1632e6fdbc8489bf3c59e0588ece2e2f37 Mon Sep 17 00:00:00 2001 From: Julien Fortin Date: Tue, 26 Sep 2017 16:45:39 -0700 Subject: [PATCH 1/5] lib: json_print: rework 'new_json_obj' drop FILE* argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As Stephen Hemminger mentioned on the last submission the new_json_obj function is always called with fp == stdout, so right now, there's no need of this extra argument. The background for the rework is the following: The ip monitor didn't call `new_json_obj` (even for in non json context), so the static FILE* _fp variable wasn't initialized, thus raising a SIGSEGV in ipaddress.c. This patch should fix this issue for good, new paths won't have to call `new_json_obj`. How to reproduce: $ ip -t mon label link (gdb) bt .#0 _IO_vfprintf_internal (s=s@entry=0x0, format=format@entry=0x45460d “%d: “, ap=ap@entry=0x7fffffff7f18) at vfprintf.c:1278 .#1 0x0000000000451310 in color_fprintf (fp=0x0, attr=, fmt=0x45460d “%d: “) at color.c:108 .#2 0x000000000044a856 in print_color_int (t=t@entry=PRINT_ANY, color=color@entry=4294967295, key=key@entry=0x4545fc “ifindex”, fmt=fmt@entry=0x45460d “%d: “, value=) at ip_print.c:132 .#3 0x000000000040ccd2 in print_int (value=, fmt=0x45460d “%d: “, key=0x4545fc “ifindex”, t=PRINT_ANY) at ip_common.h:189 .#4 print_linkinfo (who=, n=0x7fffffffa380, arg=0x7ffff77a82a0 <_IO_2_1_stdout_>) at ipaddress.c:1107 .#5 0x0000000000422e13 in accept_msg (who=0x7fffffff8320, ctrl=0x7fffffff8310, n=0x7fffffffa380, arg=0x7ffff77a82a0 <_IO_2_1_stdout_>) at ipmonitor.c:89 .#6 0x000000000044c58f in rtnl_listen (rtnl=0x672160 , handler=handler@entry=0x422c70 , jarg=0x7ffff77a82a0 <_IO_2_1_stdout_>) at libnetlink.c:761 .#7 0x00000000004233db in do_ipmonitor (argc=, argv=0x7fffffffe5a0) at ipmonitor.c:310 .#8 0x0000000000408f74 in do_cmd (argv0=0x7fffffffe7f5 “mon”, argc=3, argv=0x7fffffffe588) at ip.c:116 .#9 0x0000000000408a94 in main (argc=4, argv=0x7fffffffe580) at ip.c:311 Fixes: 6377572f ("ip: ip_print: add new API to print JSON or regular format output") Reported-by: David Ahern Signed-off-by: Julien Fortin --- include/json_print.h | 4 +--- ip/ipaddress.c | 4 ++-- lib/json_print.c | 31 ++++++++++--------------------- 3 files changed, 13 insertions(+), 26 deletions(-) diff --git a/include/json_print.h b/include/json_print.h index 44cf5ac5..b6ce1f9f 100644 --- a/include/json_print.h +++ b/include/json_print.h @@ -29,13 +29,11 @@ enum output_type { PRINT_ANY = 4, }; -void new_json_obj(int json, FILE *fp); +void new_json_obj(int json); void delete_json_obj(void); bool is_json_context(void); -void set_current_fp(FILE *fp); - void fflush_fp(void); void open_json_object(const char *str); diff --git a/ip/ipaddress.c b/ip/ipaddress.c index b8bc387a..9e9a7e0a 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -1815,7 +1815,7 @@ static int ipaddr_showdump(void) if (ipadd_dump_check_magic()) exit(-1); - new_json_obj(json, stdout); + new_json_obj(json); open_json_object(NULL); open_json_array(PRINT_JSON, "addr_info"); @@ -2176,7 +2176,7 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action) * Initialize a json_writer and open an array object * if -json was specified. */ - new_json_obj(json, stdout); + new_json_obj(json); /* * If only filter_dev present and none of the other diff --git a/lib/json_print.c b/lib/json_print.c index 93b4119d..aa527af6 100644 --- a/lib/json_print.c +++ b/lib/json_print.c @@ -16,15 +16,14 @@ #include "json_print.h" static json_writer_t *_jw; -static FILE *_fp; #define _IS_JSON_CONTEXT(type) ((type & PRINT_JSON || type & PRINT_ANY) && _jw) #define _IS_FP_CONTEXT(type) (!_jw && (type & PRINT_FP || type & PRINT_ANY)) -void new_json_obj(int json, FILE *fp) +void new_json_obj(int json) { if (json) { - _jw = jsonw_new(fp); + _jw = jsonw_new(stdout); if (!_jw) { perror("json object"); exit(1); @@ -32,7 +31,6 @@ void new_json_obj(int json, FILE *fp) jsonw_pretty(_jw, true); jsonw_start_array(_jw); } - set_current_fp(fp); } void delete_json_obj(void) @@ -48,15 +46,6 @@ bool is_json_context(void) return _jw != NULL; } -void set_current_fp(FILE *fp) -{ - if (!fp) { - fprintf(stderr, "Error: invalid file pointer.\n"); - exit(1); - } - _fp = fp; -} - json_writer_t *get_json_writer(void) { return _jw; @@ -89,7 +78,7 @@ void open_json_array(enum output_type type, const char *str) jsonw_name(_jw, str); jsonw_start_array(_jw); } else if (_IS_FP_CONTEXT(type)) { - fprintf(_fp, "%s", str); + printf("%s", str); } } @@ -103,7 +92,7 @@ void close_json_array(enum output_type type, const char *str) jsonw_end_array(_jw); jsonw_pretty(_jw, true); } else if (_IS_FP_CONTEXT(type)) { - fprintf(_fp, "%s", str); + printf("%s", str); } } @@ -124,7 +113,7 @@ void close_json_array(enum output_type type, const char *str) else \ jsonw_##type_name##_field(_jw, key, value); \ } else if (_IS_FP_CONTEXT(t)) { \ - color_fprintf(_fp, color, fmt, value); \ + color_fprintf(stdout, color, fmt, value); \ } \ } _PRINT_FUNC(int, int); @@ -147,7 +136,7 @@ void print_color_string(enum output_type type, else jsonw_string_field(_jw, key, value); } else if (_IS_FP_CONTEXT(type)) { - color_fprintf(_fp, color, fmt, value); + color_fprintf(stdout, color, fmt, value); } } @@ -168,7 +157,7 @@ void print_color_bool(enum output_type type, else jsonw_bool(_jw, value); } else if (_IS_FP_CONTEXT(type)) { - color_fprintf(_fp, color, fmt, value ? "true" : "false"); + color_fprintf(stdout, color, fmt, value ? "true" : "false"); } } @@ -187,7 +176,7 @@ void print_color_0xhex(enum output_type type, snprintf(b1, sizeof(b1), "%#x", hex); print_string(PRINT_JSON, key, NULL, b1); } else if (_IS_FP_CONTEXT(type)) { - color_fprintf(_fp, color, fmt, hex); + color_fprintf(stdout, color, fmt, hex); } } @@ -206,7 +195,7 @@ void print_color_hex(enum output_type type, else jsonw_string(_jw, b1); } else if (_IS_FP_CONTEXT(type)) { - color_fprintf(_fp, color, fmt, hex); + color_fprintf(stdout, color, fmt, hex); } } @@ -226,6 +215,6 @@ void print_color_null(enum output_type type, else jsonw_null(_jw); } else if (_IS_FP_CONTEXT(type)) { - color_fprintf(_fp, color, fmt, value); + color_fprintf(stdout, color, fmt, value); } } From 1267c0b924de0297510efd48f8f849af774be369 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Wed, 18 Oct 2017 19:58:13 +0200 Subject: [PATCH 2/5] ss: Distinguish between IPv4 and IPv6 wildcard sockets Commit aba9c23a6e1cb ("ss: enclose IPv6 address in brackets") unified display of wildcard sockets in IPv4 and IPv6 to print the unspecified address as '*'. Users then complained that they can't distinguish between address families anymore, so change this again to what Stephen Hemminger suggested: | *:80 << both IPV6 and IPV4 | [::]:80 << IPV6_ONLY | 0.0.0.0:80 << IPV4_ONLY Note that on older kernels which don't support INET_DIAG_SKV6ONLY attribute, pure IPv6 sockets will still show as '*'. Cc: Humberto Alves Cc: Eric Dumazet Signed-off-by: Phil Sutter --- misc/ss.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/misc/ss.c b/misc/ss.c index 09bff8a7..e37aba60 100644 --- a/misc/ss.c +++ b/misc/ss.c @@ -1041,7 +1041,8 @@ do_numeric: return buf; } -static void inet_addr_print(const inet_prefix *a, int port, unsigned int ifindex) +static void inet_addr_print(const inet_prefix *a, int port, + unsigned int ifindex, bool v6only) { char buf[1024]; const char *ap = buf; @@ -1049,14 +1050,10 @@ static void inet_addr_print(const inet_prefix *a, int port, unsigned int ifindex const char *ifname = NULL; if (a->family == AF_INET) { - if (a->data[0] == 0) { - buf[0] = '*'; - buf[1] = 0; - } else { - ap = format_host(AF_INET, 4, a->data); - } + ap = format_host(AF_INET, 4, a->data); } else { - if (!memcmp(a->data, &in6addr_any, sizeof(in6addr_any))) { + if (!v6only && + !memcmp(a->data, &in6addr_any, sizeof(in6addr_any))) { buf[0] = '*'; buf[1] = 0; } else { @@ -1728,12 +1725,12 @@ static void proc_ctx_print(struct sockstat *s) } } -static void inet_stats_print(struct sockstat *s) +static void inet_stats_print(struct sockstat *s, bool v6only) { sock_state_print(s); - inet_addr_print(&s->local, s->lport, s->iface); - inet_addr_print(&s->remote, s->rport, 0); + inet_addr_print(&s->local, s->lport, s->iface, v6only); + inet_addr_print(&s->remote, s->rport, 0, v6only); proc_ctx_print(s); } @@ -2065,7 +2062,7 @@ static int tcp_show_line(char *line, const struct filter *f, int family) s.rto = s.rto != 3 * hz ? s.rto / hz : 0; s.ss.type = IPPROTO_TCP; - inet_stats_print(&s.ss); + inet_stats_print(&s.ss, false); if (show_options) tcp_timer_print(&s); @@ -2411,6 +2408,7 @@ static int inet_show_sock(struct nlmsghdr *nlh, { struct rtattr *tb[INET_DIAG_MAX+1]; struct inet_diag_msg *r = NLMSG_DATA(nlh); + unsigned char v6only = 0; parse_rtattr(tb, INET_DIAG_MAX, (struct rtattr *)(r+1), nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*r))); @@ -2418,7 +2416,10 @@ static int inet_show_sock(struct nlmsghdr *nlh, if (tb[INET_DIAG_PROTOCOL]) s->type = rta_getattr_u8(tb[INET_DIAG_PROTOCOL]); - inet_stats_print(s); + if (s->local.family == AF_INET6 && tb[INET_DIAG_SKV6ONLY]) + v6only = rta_getattr_u8(tb[INET_DIAG_SKV6ONLY]); + + inet_stats_print(s, v6only); if (show_options) { struct tcpstat t = {}; @@ -2434,12 +2435,9 @@ static int inet_show_sock(struct nlmsghdr *nlh, if (show_details) { sock_details_print(s); - if (s->local.family == AF_INET6 && tb[INET_DIAG_SKV6ONLY]) { - unsigned char v6only; - - v6only = rta_getattr_u8(tb[INET_DIAG_SKV6ONLY]); + if (s->local.family == AF_INET6 && tb[INET_DIAG_SKV6ONLY]) printf(" v6only:%u", v6only); - } + if (tb[INET_DIAG_SHUTDOWN]) { unsigned char mask; @@ -2910,7 +2908,7 @@ static int dgram_show_line(char *line, const struct filter *f, int family) opt[0] = 0; s.type = dg_proto == UDP_PROTO ? IPPROTO_UDP : 0; - inet_stats_print(&s); + inet_stats_print(&s, false); if (show_details && opt[0]) printf(" opt:\"%s\"", opt); From 572e8936130440d0e180cf62ed55fface2ae8719 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Wed, 18 Oct 2017 20:08:26 +0200 Subject: [PATCH 3/5] ss: Detect IPPROTO_ICMPV6 sockets Prefix IPPROTO_ICMPV6 sockets with 'icmp6' instead of '???'. Signed-off-by: Phil Sutter --- misc/ss.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/misc/ss.c b/misc/ss.c index e37aba60..b5c6bbc0 100644 --- a/misc/ss.c +++ b/misc/ss.c @@ -784,6 +784,8 @@ static const char *proto_name(int protocol) return "sctp"; case IPPROTO_DCCP: return "dccp"; + case IPPROTO_ICMPV6: + return "icmp6"; } return "???"; From 21503ed2af233ffe795926f6641ac84ec1b15bf9 Mon Sep 17 00:00:00 2001 From: Michal Kubecek Date: Thu, 19 Oct 2017 10:21:08 +0200 Subject: [PATCH 4/5] ip maddr: fix filtering by device Commit 530903dd9003 ("ip: fix igmp parsing when iface is long") uses variable len to keep trailing colon from interface name comparison. This variable is local to loop body but we set it in one pass and use it in following one(s) so that we are actually using (pseudo)random length for comparison. This became apparent since commit b48a1161f5f9 ("ipmaddr: Avoid accessing uninitialized data") always initializes len to zero so that the name comparison is always true. As a result, "ip maddr show dev eth0" shows IPv4 multicast addresses for all interfaces. Instead of keeping the length, let's simply replace the trailing colon with a null byte. The bonus is that we get correct interface name in ma.name. Fixes: 530903dd9003 ("ip: fix igmp parsing when iface is long") Signed-off-by: Michal Kubecek Acked-by: Phil Sutter Acked-by: Petr Vorel --- ip/ipmaddr.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ip/ipmaddr.c b/ip/ipmaddr.c index 5683f6fa..46b86a3a 100644 --- a/ip/ipmaddr.c +++ b/ip/ipmaddr.c @@ -136,17 +136,18 @@ static void read_igmp(struct ma_info **result_p) while (fgets(buf, sizeof(buf), fp)) { struct ma_info *ma; - size_t len = 0; if (buf[0] != '\t') { + size_t len; + sscanf(buf, "%d%s", &m.index, m.name); len = strlen(m.name); if (m.name[len - 1] == ':') - len--; + m.name[len - 1] = '\0'; continue; } - if (filter.dev && strncmp(filter.dev, m.name, len)) + if (filter.dev && strcmp(filter.dev, m.name)) continue; sscanf(buf, "%08x%d", (__u32 *)&m.addr.data, &m.users); From c4be5febaaae1d86a5003df4e7352a04b6a63587 Mon Sep 17 00:00:00 2001 From: Roman Mashak Date: Wed, 18 Oct 2017 15:44:01 -0400 Subject: [PATCH 5/5] ss: initialize 'fackets' member of tcpstat structure 'fackets' has never been initialized with kernel extracted information, thus never really printed. Signed-off-by: Roman Mashak --- misc/ss.c | 1 + 1 file changed, 1 insertion(+) diff --git a/misc/ss.c b/misc/ss.c index b5c6bbc0..dfb438f9 100644 --- a/misc/ss.c +++ b/misc/ss.c @@ -2225,6 +2225,7 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r, s.retrans_total = info->tcpi_total_retrans; s.lost = info->tcpi_lost; s.sacked = info->tcpi_sacked; + s.fackets = info->tcpi_fackets; s.reordering = info->tcpi_reordering; s.rcv_space = info->tcpi_rcv_space; s.cwnd = info->tcpi_snd_cwnd;