From 79712feff47b2c275cf1cb1291863476ac45070a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Aug 2019 10:16:54 +0200 Subject: [PATCH 1/4] route: fix strncpy() warning from coverity about unterminated string Coverity says: Error: BUFFER_SIZE_WARNING (CWE-120): [#def1] libnl-3.4.0/lib/route/cls/ematch/text.c:94: buffer_size_warning: Calling strncpy with a maximum size argument of 16 bytes on destination array "t->cfg.algo" of size 16 bytes might leave the destination string unterminated. # 92| struct text_data *t = rtnl_ematch_data(e); # 93| # 94|-> strncpy(t->cfg.algo, algo, sizeof(t->cfg.algo)); # 95| } # 96| Error: BUFFER_SIZE_WARNING (CWE-120): [#def11] libnl-3.4.0/lib/xfrm/sa.c:1192: buffer_size_warning: Calling strncpy with a maximum size argument of 64 bytes on destination array "auth->alg_name" of size 64 bytes might leave the destination string unterminated. # 1190| } # 1191| # 1192|-> strncpy(auth->alg_name, tmpl->auth->alg_name, sizeof(auth->alg_name)); # 1193| auth->alg_key_len = tmpl->auth->alg_key_len; # 1194| memcpy(auth->alg_key, tmpl->auth->alg_key, (tmpl->auth->alg_key_len + 7) / 8); (cherry picked from commit f6f163d68e756d7ee69b93b0ccb4ab24f9764f77) --- lib/route/cls/ematch/text.c | 1 + lib/xfrm/sa.c | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/route/cls/ematch/text.c b/lib/route/cls/ematch/text.c index b14c4abb92a7..12a1e747b48a 100644 --- a/lib/route/cls/ematch/text.c +++ b/lib/route/cls/ematch/text.c @@ -92,6 +92,7 @@ void rtnl_ematch_text_set_algo(struct rtnl_ematch *e, const char *algo) struct text_data *t = rtnl_ematch_data(e); strncpy(t->cfg.algo, algo, sizeof(t->cfg.algo)); + t->cfg.algo[sizeof(t->cfg.algo) - 1] = '\0'; } char *rtnl_ematch_text_get_algo(struct rtnl_ematch *e) diff --git a/lib/xfrm/sa.c b/lib/xfrm/sa.c index 995df9fd9769..15a3661a9699 100644 --- a/lib/xfrm/sa.c +++ b/lib/xfrm/sa.c @@ -1190,6 +1190,7 @@ static int build_xfrm_sa_message(struct xfrmnl_sa *tmpl, int cmd, int flags, str } strncpy(auth->alg_name, tmpl->auth->alg_name, sizeof(auth->alg_name)); + auth->alg_name[sizeof(auth->alg_name) - 1] = '\0'; auth->alg_key_len = tmpl->auth->alg_key_len; memcpy(auth->alg_key, tmpl->auth->alg_key, (tmpl->auth->alg_key_len + 7) / 8); if (nla_put(msg, XFRMA_ALG_AUTH, len, auth) < 0) { -- 2.21.0 From d7b51a8a3d2f0ac0e2c306a77bdf479f64154d43 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Aug 2019 10:38:12 +0200 Subject: [PATCH 2/4] link/sriov: fix memleak in rtnl_link_sriov_clone() Found by Coverity. (cherry picked from commit f1a085994a78a69abcd583d682b9850bc20ed482) --- lib/route/link/sriov.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/lib/route/link/sriov.c b/lib/route/link/sriov.c index 5c20ecff68f6..2a87cfe5ff3f 100644 --- a/lib/route/link/sriov.c +++ b/lib/route/link/sriov.c @@ -109,8 +109,10 @@ int rtnl_link_sriov_clone(struct rtnl_link *dst, struct rtnl_link *src) { if (s_vf->ce_mask & SRIOV_ATTR_ADDR) { vf_addr = nl_addr_clone(s_vf->vf_lladdr); - if (!vf_addr) + if (!vf_addr) { + rtnl_link_vf_put(d_vf); return -NLE_NOMEM; + } d_vf->vf_lladdr = vf_addr; } @@ -120,8 +122,10 @@ int rtnl_link_sriov_clone(struct rtnl_link *dst, struct rtnl_link *src) { err = rtnl_link_vf_vlan_alloc(&dst_vlans, src_vlans->size); - if (err < 0) + if (err < 0) { + rtnl_link_vf_put(d_vf); return err; + } dst_vlan_info = dst_vlans->vlans; memcpy(dst_vlans, src_vlans, sizeof(nl_vf_vlans_t)); memcpy(dst_vlan_info, src_vlan_info, @@ -558,8 +562,10 @@ int rtnl_link_sriov_parse_vflist(struct rtnl_link *link, struct nlattr **tb) { vf_data->vf_lladdr = nl_addr_build(AF_LLC, vf_lladdr->mac, 6); - if (vf_data->vf_lladdr == NULL) + if (vf_data->vf_lladdr == NULL) { + rtnl_link_vf_put(vf_data); return -NLE_NOMEM; + } nl_addr_set_family(vf_data->vf_lladdr, AF_LLC); vf_data->ce_mask |= SRIOV_ATTR_ADDR; } @@ -576,8 +582,10 @@ int rtnl_link_sriov_parse_vflist(struct rtnl_link *link, struct nlattr **tb) { err = rtnl_link_vf_vlan_info(list_len, vf_vlan_info, &vf_vlans); - if (err < 0) + if (err < 0) { + rtnl_link_vf_put(vf_data); return err; + } vf_data->vf_vlans = vf_vlans; vf_data->ce_mask |= SRIOV_ATTR_VLAN; @@ -586,8 +594,10 @@ int rtnl_link_sriov_parse_vflist(struct rtnl_link *link, struct nlattr **tb) { if (vf_vlan->vlan) { err = rtnl_link_vf_vlan_alloc(&vf_vlans, 1); - if (err < 0) + if (err < 0) { + rtnl_link_vf_put(vf_data); return err; + } vf_vlans->vlans[0].vf_vlan = vf_vlan->vlan; vf_vlans->vlans[0].vf_vlan_qos = vf_vlan->qos; @@ -649,8 +659,10 @@ int rtnl_link_sriov_parse_vflist(struct rtnl_link *link, struct nlattr **tb) { err = nla_parse_nested(stb, IFLA_VF_STATS_MAX, t[IFLA_VF_STATS], sriov_stats_policy); - if (err < 0) + if (err < 0) { + rtnl_link_vf_put(vf_data); return err; + } SET_VF_STAT(link, cur, stb, RTNL_LINK_VF_STATS_RX_PACKETS, -- 2.21.0 From 9f910abd4b39015cfdcc78566915ed1d852c0fd1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 27 Aug 2019 14:43:54 +0200 Subject: [PATCH 3/4] lib: accept %NULL arguments for nl_addr_cmp() Just be more forgiving. Also, this avoids a coverity warning: Error: FORWARD_NULL (CWE-476): [#def1] libnl-3.4.0/lib/route/addr.c:502: var_compare_op: Comparing "a->a_peer" to null implies that "a->a_peer" might be null. libnl-3.4.0/lib/route/addr.c:513: var_deref_model: Passing null pointer "a->a_peer" to "nl_addr_cmp", which dereferences it. libnl-3.4.0/lib/addr.c:587:8: deref_parm: Directly dereferencing parameter "a". # 585| int nl_addr_cmp(const struct nl_addr *a, const struct nl_addr *b) # 586| { # 587|-> int d = a->a_family - b->a_family; # 588| # 589| if (d == 0) { https://bugzilla.redhat.com/show_bug.cgi?id=1606988 (cherry picked from commit 34708e2ef048f3788f3f2d5018735b27b156d244) --- lib/addr.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/addr.c b/lib/addr.c index c299b402a12b..b43791d52179 100644 --- a/lib/addr.c +++ b/lib/addr.c @@ -584,8 +584,16 @@ int nl_addr_shared(const struct nl_addr *addr) */ int nl_addr_cmp(const struct nl_addr *a, const struct nl_addr *b) { - int d = a->a_family - b->a_family; + int d; + + if (a == b) + return 0; + if (!a) + return -1; + if (!b) + return 1; + d = a->a_family - b->a_family; if (d == 0) { d = a->a_len - b->a_len; -- 2.21.0 From 0fd322bb429228a200cc7935a5b597748faaadf8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 27 Aug 2019 14:58:35 +0200 Subject: [PATCH 4/4] lib: fix error code from nfnl_exp_build_message() Otherwise we return success but don't actually set the output result. This can lead to a crash, in case of out-of-memory. Found by Coverity. https://bugzilla.redhat.com/show_bug.cgi?id=1606988 (cherry picked from commit f3d5c44d21243d5eb59bfc2878d4977df2fd1369) --- lib/netfilter/exp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/netfilter/exp.c b/lib/netfilter/exp.c index 24ec55f4c374..947eea0d4bcd 100644 --- a/lib/netfilter/exp.c +++ b/lib/netfilter/exp.c @@ -490,6 +490,8 @@ static int nfnl_exp_build_message(const struct nfnl_exp *exp, int cmd, int flags return 0; nla_put_failure: + err = -NLE_NOMEM; + err_out: nlmsg_free(msg); return err; -- 2.21.0