From e7fefb3214b5a1ed030cab9df513560c503a9851 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 1 Sep 2017 16:08:08 +0200 Subject: [PATCH 1/8] link_gre6: Fix for changing tclass/flowlabel When trying to change tclass or flowlabel of a GREv6 tunnel which has the respective value set already, the code accidentally bitwise OR'ed the old and the new value, leading to unexpected results. Fix this by clearing the relevant bits of flowinfo variable prior to assigning the new value. Fixes: af89576d7a8c4 ("iproute2: GRE over IPv6 tunnel support.") Signed-off-by: Phil Sutter --- ip/link_gre6.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ip/link_gre6.c b/ip/link_gre6.c index 4d3d4b54..447ac5d7 100644 --- a/ip/link_gre6.c +++ b/ip/link_gre6.c @@ -288,6 +288,7 @@ get_failed: else { if (get_u8(&uval, *argv, 16)) invarg("invalid TClass", *argv); + flowinfo &= ~IP6_FLOWINFO_TCLASS; flowinfo |= htonl((__u32)uval << 20) & IP6_FLOWINFO_TCLASS; flags &= ~IP6_TNL_F_USE_ORIG_TCLASS; } @@ -303,6 +304,7 @@ get_failed: invarg("invalid Flowlabel", *argv); if (uval > 0xFFFFF) invarg("invalid Flowlabel", *argv); + flowinfo &= ~IP6_FLOWINFO_FLOWLABEL; flowinfo |= htonl(uval) & IP6_FLOWINFO_FLOWLABEL; flags &= ~IP6_TNL_F_USE_ORIG_FLOWLABEL; } From 50f81afd4d24e9fc4a272f74072fc66c2cb2f08f Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 1 Sep 2017 16:08:09 +0200 Subject: [PATCH 2/8] link_gre6: Print the tunnel's tclass setting Print the value analogous to flowlabel. While being at it, also break the overlong lines to not exceed 80 characters boundary. Signed-off-by: Phil Sutter --- ip/link_gre6.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ip/link_gre6.c b/ip/link_gre6.c index 447ac5d7..78b5215c 100644 --- a/ip/link_gre6.c +++ b/ip/link_gre6.c @@ -462,7 +462,14 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[]) if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) fprintf(f, "flowlabel inherit "); else - fprintf(f, "flowlabel 0x%05x ", ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL)); + fprintf(f, "flowlabel 0x%05x ", + ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL)); + + if (flags & IP6_TNL_F_USE_ORIG_TCLASS) + fprintf(f, "tclass inherit "); + else + fprintf(f, "tclass 0x%02x ", + ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20); if (flags & IP6_TNL_F_RCV_DSCP_COPY) fprintf(f, "dscp inherit "); From 8d15e012a3227d79295cd95582bb6d8a6f0bdc92 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 1 Sep 2017 18:52:51 +0200 Subject: [PATCH 3/8] utils: Implement strlcpy() and strlcat() By making use of strncpy(), both implementations are really simple so there is no need to add libbsd as additional dependency. Signed-off-by: Phil Sutter --- include/utils.h | 3 +++ lib/utils.c | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/include/utils.h b/include/utils.h index 7a3b3fd2..f161588b 100644 --- a/include/utils.h +++ b/include/utils.h @@ -251,4 +251,7 @@ int make_path(const char *path, mode_t mode); char *find_cgroup2_mount(void); int get_command_name(const char *pid, char *comm, size_t len); +size_t strlcpy(char *dst, const char *src, size_t size); +size_t strlcat(char *dst, const char *src, size_t size); + #endif /* __UTILS_H__ */ diff --git a/lib/utils.c b/lib/utils.c index 9143ed22..330ab073 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -1230,3 +1230,22 @@ int get_real_family(int rtm_type, int rtm_family) return rtm_family; } + +size_t strlcpy(char *dst, const char *src, size_t size) +{ + if (size) { + strncpy(dst, src, size - 1); + dst[size - 1] = '\0'; + } + return strlen(src); +} + +size_t strlcat(char *dst, const char *src, size_t size) +{ + size_t dlen = strlen(dst); + + if (dlen > size) + return dlen + strlen(src); + + return dlen + strlcpy(dst + dlen, src, size - dlen); +} From 18f156bfecda20166c2fb543ba8c9c6559edef9c Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 1 Sep 2017 18:52:52 +0200 Subject: [PATCH 4/8] Convert the obvious cases to strlcpy() This converts the typical idiom of manually terminating the buffer after a call to strncpy(). Signed-off-by: Phil Sutter --- ip/ipnetns.c | 3 +-- ip/iproute_lwtunnel.c | 3 +-- ip/ipvrf.c | 3 +-- lib/bpf.c | 3 +-- lib/fs.c | 3 +-- lib/inet_proto.c | 3 +-- misc/ss.c | 3 +-- tc/em_ipset.c | 3 +-- 8 files changed, 8 insertions(+), 16 deletions(-) diff --git a/ip/ipnetns.c b/ip/ipnetns.c index 9ee1fe6a..afb4978a 100644 --- a/ip/ipnetns.c +++ b/ip/ipnetns.c @@ -518,8 +518,7 @@ int netns_identify_pid(const char *pidstr, char *name, int len) if ((st.st_dev == netst.st_dev) && (st.st_ino == netst.st_ino)) { - strncpy(name, entry->d_name, len - 1); - name[len - 1] = '\0'; + strlcpy(name, entry->d_name, len); } } closedir(dir); diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c index 1a3dc4d4..4c2d3b07 100644 --- a/ip/iproute_lwtunnel.c +++ b/ip/iproute_lwtunnel.c @@ -325,8 +325,7 @@ static int parse_encap_seg6(struct rtattr *rta, size_t len, int *argcp, invarg("\"segs\" provided before \"mode\"\n", *argv); - strncpy(segbuf, *argv, 1024); - segbuf[1023] = 0; + strlcpy(segbuf, *argv, 1024); } else if (strcmp(*argv, "hmac") == 0) { NEXT_ARG(); if (hmac_ok++) diff --git a/ip/ipvrf.c b/ip/ipvrf.c index e6fad32a..b74c501e 100644 --- a/ip/ipvrf.c +++ b/ip/ipvrf.c @@ -336,8 +336,7 @@ static int vrf_path(char *vpath, size_t len) if (vrf) *vrf = '\0'; - strncpy(vpath, start, len - 1); - vpath[len - 1] = '\0'; + strlcpy(vpath, start, len); /* if vrf path is just / then return nothing */ if (!strcmp(vpath, "/")) diff --git a/lib/bpf.c b/lib/bpf.c index 0bd0a95e..c180934a 100644 --- a/lib/bpf.c +++ b/lib/bpf.c @@ -512,8 +512,7 @@ static const char *bpf_find_mntpt_single(unsigned long magic, char *mnt, ret = bpf_valid_mntpt(mntpt, magic); if (!ret) { - strncpy(mnt, mntpt, len - 1); - mnt[len - 1] = 0; + strlcpy(mnt, mntpt, len); return mnt; } diff --git a/lib/fs.c b/lib/fs.c index ebe05cd4..86efd4ed 100644 --- a/lib/fs.c +++ b/lib/fs.c @@ -172,8 +172,7 @@ int get_command_name(const char *pid, char *comm, size_t len) if (nl) *nl = '\0'; - strncpy(comm, name, len - 1); - comm[len - 1] = '\0'; + strlcpy(comm, name, len); break; } diff --git a/lib/inet_proto.c b/lib/inet_proto.c index 53c02903..bdfd52fd 100644 --- a/lib/inet_proto.c +++ b/lib/inet_proto.c @@ -38,8 +38,7 @@ const char *inet_proto_n2a(int proto, char *buf, int len) free(ncache); icache = proto; ncache = strdup(pe->p_name); - strncpy(buf, pe->p_name, len - 1); - buf[len - 1] = '\0'; + strlcpy(buf, pe->p_name, len); return buf; } snprintf(buf, len, "ipproto-%d", proto); diff --git a/misc/ss.c b/misc/ss.c index 2c9e80e6..dd8dfaa4 100644 --- a/misc/ss.c +++ b/misc/ss.c @@ -425,8 +425,7 @@ static void user_ent_hash_build(void) user_ent_hash_build_init = 1; - strncpy(name, root, sizeof(name)-1); - name[sizeof(name)-1] = 0; + strlcpy(name, root, sizeof(name)); if (strlen(name) == 0 || name[strlen(name)-1] != '/') strcat(name, "/"); diff --git a/tc/em_ipset.c b/tc/em_ipset.c index b5975651..48b287f5 100644 --- a/tc/em_ipset.c +++ b/tc/em_ipset.c @@ -145,8 +145,7 @@ get_set_byname(const char *setname, struct xt_set_info *info) int res; req.op = IP_SET_OP_GET_BYNAME; - strncpy(req.set.name, setname, IPSET_MAXNAMELEN); - req.set.name[IPSET_MAXNAMELEN - 1] = '\0'; + strlcpy(req.set.name, setname, IPSET_MAXNAMELEN); res = do_getsockopt(&req); if (res != 0) return -1; From 532b8874fe545acaa8d45c4dd3b54b8f3bb41d9f Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 1 Sep 2017 18:52:53 +0200 Subject: [PATCH 5/8] Convert harmful calls to strncpy() to strlcpy() This patch converts spots where manual buffer termination was missing to strlcpy() since that does what is needed. Signed-off-by: Phil Sutter --- genl/ctrl.c | 2 +- ip/ipvrf.c | 2 +- ip/xfrm_state.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/genl/ctrl.c b/genl/ctrl.c index 6abd5258..448988eb 100644 --- a/genl/ctrl.c +++ b/genl/ctrl.c @@ -313,7 +313,7 @@ static int ctrl_list(int cmd, int argc, char **argv) if (matches(*argv, "name") == 0) { NEXT_ARG(); - strncpy(d, *argv, sizeof (d) - 1); + strlcpy(d, *argv, sizeof(d)); addattr_l(nlh, 128, CTRL_ATTR_FAMILY_NAME, d, strlen(d) + 1); } else if (matches(*argv, "id") == 0) { diff --git a/ip/ipvrf.c b/ip/ipvrf.c index b74c501e..f9277e1e 100644 --- a/ip/ipvrf.c +++ b/ip/ipvrf.c @@ -74,7 +74,7 @@ static int vrf_identify(pid_t pid, char *name, size_t len) if (end) *end = '\0'; - strncpy(name, vrf, len - 1); + strlcpy(name, vrf, len); break; } } diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c index e11c93bf..4483fb8f 100644 --- a/ip/xfrm_state.c +++ b/ip/xfrm_state.c @@ -125,7 +125,7 @@ static int xfrm_algo_parse(struct xfrm_algo *alg, enum xfrm_attr_type_t type, fprintf(stderr, "warning: ALGO-NAME/ALGO-KEYMAT values will be sent to the kernel promiscuously! (verifying them isn't implemented yet)\n"); #endif - strncpy(alg->alg_name, name, sizeof(alg->alg_name)); + strlcpy(alg->alg_name, name, sizeof(alg->alg_name)); if (slen > 2 && strncmp(key, "0x", 2) == 0) { /* split two chars "0x" from the top */ From 44cc6c792a6503e024f042c65f35cd44b3283b20 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 1 Sep 2017 18:52:54 +0200 Subject: [PATCH 6/8] ipxfrm: Replace STRBUF_CAT macro with strlcat() Signed-off-by: Phil Sutter --- ip/ipxfrm.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c index d5eb22e2..12c2f721 100644 --- a/ip/ipxfrm.c +++ b/ip/ipxfrm.c @@ -40,17 +40,6 @@ #include "ip_common.h" #define STRBUF_SIZE (128) -#define STRBUF_CAT(buf, str) \ - do { \ - int rest = sizeof(buf) - 1 - strlen(buf); \ - if (rest > 0) { \ - int len = strlen(str); \ - if (len > rest) \ - len = rest; \ - strncat(buf, str, len); \ - buf[sizeof(buf) - 1] = '\0'; \ - } \ - } while (0); struct xfrm_filter filter; @@ -902,8 +891,8 @@ void xfrm_state_info_print(struct xfrm_usersa_info *xsinfo, prefix, title); if (prefix) - STRBUF_CAT(buf, prefix); - STRBUF_CAT(buf, "\t"); + strlcat(buf, prefix, sizeof(buf)); + strlcat(buf, "\t", sizeof(buf)); fputs(buf, fp); fprintf(fp, "replay-window %u ", xsinfo->replay_window); @@ -944,7 +933,7 @@ void xfrm_state_info_print(struct xfrm_usersa_info *xsinfo, char sbuf[STRBUF_SIZE]; memcpy(sbuf, buf, sizeof(sbuf)); - STRBUF_CAT(sbuf, "sel "); + strlcat(sbuf, "sel ", sizeof(sbuf)); xfrm_selector_print(&xsinfo->sel, xsinfo->family, fp, sbuf); } @@ -992,8 +981,8 @@ void xfrm_policy_info_print(struct xfrm_userpolicy_info *xpinfo, } if (prefix) - STRBUF_CAT(buf, prefix); - STRBUF_CAT(buf, "\t"); + strlcat(buf, prefix, sizeof(buf)); + strlcat(buf, "\t", sizeof(buf)); fputs(buf, fp); if (xpinfo->dir >= XFRM_POLICY_MAX) { From 9376314b49a47eb42ade3fc0d41cb51438f8dbc6 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 1 Sep 2017 18:52:55 +0200 Subject: [PATCH 7/8] tc_util: No need to terminate an snprintf'ed buffer snprintf() won't leave the buffer unterminated, so manually terminating is not necessary here. Signed-off-by: Phil Sutter --- tc/tc_util.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tc/tc_util.c b/tc/tc_util.c index 37104683..50d35504 100644 --- a/tc/tc_util.c +++ b/tc/tc_util.c @@ -434,7 +434,6 @@ static const char *action_n2a(int action) return "trap"; default: snprintf(buf, 64, "%d", action); - buf[63] = '\0'; return buf; } } From bc4a57b87990b30c85fdf0efbc1f8f219466daf4 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 1 Sep 2017 18:52:56 +0200 Subject: [PATCH 8/8] lnstat_util: Make sure buffer is NUL-terminated Can't use strlcpy() here since lnstat is not linked against libutil. While being at it, fix coding style in that chunk as well. Signed-off-by: Phil Sutter --- misc/lnstat_util.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/misc/lnstat_util.c b/misc/lnstat_util.c index ec19238c..c2dc42ec 100644 --- a/misc/lnstat_util.c +++ b/misc/lnstat_util.c @@ -150,7 +150,8 @@ static int lnstat_scan_compat_rtstat_fields(struct lnstat_file *lf) { char buf[FGETS_BUF_SIZE]; - strncpy(buf, RTSTAT_COMPAT_LINE, sizeof(buf)-1); + strncpy(buf, RTSTAT_COMPAT_LINE, sizeof(buf) - 1); + buf[sizeof(buf) - 1] = '\0'; return __lnstat_scan_fields(lf, buf); }