From patchwork Tue Sep 4 17:15:44 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 966018 X-Patchwork-Delegate: shemminger@vyatta.com Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=nwl.cc Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 424YQ82Sxbz9sBy for ; Wed, 5 Sep 2018 03:16:00 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727688AbeIDVl7 (ORCPT ); Tue, 4 Sep 2018 17:41:59 -0400 Received: from orbyte.nwl.cc ([151.80.46.58]:55430 "EHLO orbyte.nwl.cc" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726528AbeIDVl7 (ORCPT ); Tue, 4 Sep 2018 17:41:59 -0400 Received: from localhost ([::1]:34732 helo=tatos) by orbyte.nwl.cc with esmtp (Exim 4.90_1) (envelope-from ) id 1fxEvi-0005eN-Su; Tue, 04 Sep 2018 19:15:54 +0200 From: Phil Sutter To: Stephen Hemminger Cc: netdev@vger.kernel.org Subject: [iproute PATCH] ip-route: Fix segfault with many nexthops Date: Tue, 4 Sep 2018 19:15:44 +0200 Message-Id: <20180904171544.9337-1-phil@nwl.cc> X-Mailer: git-send-email 2.18.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org It was possible to crash ip-route by adding an IPv6 route with 37 nexthop statements. A simple reproducer is: | for i in `seq 37`; do | nhs="nexthop via 1111::$i "$nhs | done | ip -6 route add 3333::/64 $nhs The related code was broken in multiple ways: * parse_one_nh() assumed that rta points to 4kB of storage but caller provided just 1kB. Fixed by passing 'len' parameter with the correct value. * Error checking of rta_addattr*() calls in parse_one_nh() and called functions was completely absent, so with above fix in place output flood would occur due to parser looping forever. Note that it is still not possible to add a route with more than 36 nexthops due to stack buffer sizes, this patch merely fixes error path. Signed-off-by: Phil Sutter --- ip/iproute.c | 41 ++++++++++------ ip/iproute_lwtunnel.c | 108 +++++++++++++++++++++++++----------------- 2 files changed, 91 insertions(+), 58 deletions(-) diff --git a/ip/iproute.c b/ip/iproute.c index 30833414a3f7f..9e5ae48c0715c 100644 --- a/ip/iproute.c +++ b/ip/iproute.c @@ -941,7 +941,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg) } static int parse_one_nh(struct nlmsghdr *n, struct rtmsg *r, - struct rtattr *rta, struct rtnexthop *rtnh, + struct rtattr *rta, size_t len, struct rtnexthop *rtnh, int *argcp, char ***argvp) { int argc = *argcp; @@ -962,11 +962,16 @@ static int parse_one_nh(struct nlmsghdr *n, struct rtmsg *r, if (r->rtm_family == AF_UNSPEC) r->rtm_family = addr.family; if (addr.family == r->rtm_family) { - rta_addattr_l(rta, 4096, RTA_GATEWAY, &addr.data, addr.bytelen); - rtnh->rtnh_len += sizeof(struct rtattr) + addr.bytelen; + if (rta_addattr_l(rta, len, RTA_GATEWAY, + &addr.data, addr.bytelen)) + return -1; + rtnh->rtnh_len += sizeof(struct rtattr) + + addr.bytelen; } else { - rta_addattr_l(rta, 4096, RTA_VIA, &addr.family, addr.bytelen+2); - rtnh->rtnh_len += RTA_SPACE(addr.bytelen+2); + if (rta_addattr_l(rta, len, RTA_VIA, + &addr.family, addr.bytelen + 2)) + return -1; + rtnh->rtnh_len += RTA_SPACE(addr.bytelen + 2); } } else if (strcmp(*argv, "dev") == 0) { NEXT_ARG(); @@ -988,13 +993,15 @@ static int parse_one_nh(struct nlmsghdr *n, struct rtmsg *r, NEXT_ARG(); if (get_rt_realms_or_raw(&realm, *argv)) invarg("\"realm\" value is invalid\n", *argv); - rta_addattr32(rta, 4096, RTA_FLOW, realm); + if (rta_addattr32(rta, len, RTA_FLOW, realm)) + return -1; rtnh->rtnh_len += sizeof(struct rtattr) + 4; } else if (strcmp(*argv, "encap") == 0) { - int len = rta->rta_len; + int old_len = rta->rta_len; - lwt_parse_encap(rta, 4096, &argc, &argv); - rtnh->rtnh_len += rta->rta_len - len; + if (lwt_parse_encap(rta, len, &argc, &argv)) + return -1; + rtnh->rtnh_len += rta->rta_len - old_len; } else if (strcmp(*argv, "as") == 0) { inet_prefix addr; @@ -1002,8 +1009,9 @@ static int parse_one_nh(struct nlmsghdr *n, struct rtmsg *r, if (strcmp(*argv, "to") == 0) NEXT_ARG(); get_addr(&addr, *argv, r->rtm_family); - rta_addattr_l(rta, 4096, RTA_NEWDST, &addr.data, - addr.bytelen); + if (rta_addattr_l(rta, len, RTA_NEWDST, + &addr.data, addr.bytelen)) + return -1; rtnh->rtnh_len += sizeof(struct rtattr) + addr.bytelen; } else break; @@ -1036,15 +1044,18 @@ static int parse_nexthops(struct nlmsghdr *n, struct rtmsg *r, memset(rtnh, 0, sizeof(*rtnh)); rtnh->rtnh_len = sizeof(*rtnh); rta->rta_len += rtnh->rtnh_len; - if (parse_one_nh(n, r, rta, rtnh, &argc, &argv)) { + if (parse_one_nh(n, r, rta, 1024, rtnh, &argc, &argv)) { fprintf(stderr, "Error: cannot parse nexthop\n"); exit(-1); } rtnh = RTNH_NEXT(rtnh); } + return 0; + if (rta->rta_len > RTA_LENGTH(0)) - addattr_l(n, 1024, RTA_MULTIPATH, RTA_DATA(rta), RTA_PAYLOAD(rta)); + return addattr_l(n, 1024, RTA_MULTIPATH, + RTA_DATA(rta), RTA_PAYLOAD(rta)); return 0; } @@ -1484,8 +1495,8 @@ static int iproute_modify(int cmd, unsigned int flags, int argc, char **argv) addattr_l(&req.n, sizeof(req), RTA_METRICS, RTA_DATA(mxrta), RTA_PAYLOAD(mxrta)); } - if (nhs_ok) - parse_nexthops(&req.n, &req.r, argc, argv); + if (nhs_ok && parse_nexthops(&req.n, &req.r, argc, argv)) + return -1; if (req.r.rtm_family == AF_UNSPEC) req.r.rtm_family = AF_INET; diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c index e604481142ec1..969a4763df71d 100644 --- a/ip/iproute_lwtunnel.c +++ b/ip/iproute_lwtunnel.c @@ -538,8 +538,9 @@ static int parse_encap_seg6(struct rtattr *rta, size_t len, int *argcp, memcpy(tuninfo->srh, srh, srhlen); - rta_addattr_l(rta, len, SEG6_IPTUNNEL_SRH, tuninfo, - sizeof(*tuninfo) + srhlen); + if (rta_addattr_l(rta, len, SEG6_IPTUNNEL_SRH, tuninfo, + sizeof(*tuninfo) + srhlen)) + return -1; free(tuninfo); free(srh); @@ -611,6 +612,7 @@ static int parse_encap_seg6local(struct rtattr *rta, size_t len, int *argcp, char segbuf[1024]; inet_prefix addr; __u32 hmac = 0; + int ret = 0; while (argc > 0) { if (strcmp(*argv, "action") == 0) { @@ -620,27 +622,28 @@ static int parse_encap_seg6local(struct rtattr *rta, size_t len, int *argcp, action = read_action_type(*argv); if (!action) invarg("\"action\" value is invalid\n", *argv); - rta_addattr32(rta, len, SEG6_LOCAL_ACTION, action); + ret = rta_addattr32(rta, len, SEG6_LOCAL_ACTION, + action); } else if (strcmp(*argv, "table") == 0) { NEXT_ARG(); if (table_ok++) duparg2("table", *argv); get_u32(&table, *argv, 0); - rta_addattr32(rta, len, SEG6_LOCAL_TABLE, table); + ret = rta_addattr32(rta, len, SEG6_LOCAL_TABLE, table); } else if (strcmp(*argv, "nh4") == 0) { NEXT_ARG(); if (nh4_ok++) duparg2("nh4", *argv); get_addr(&addr, *argv, AF_INET); - rta_addattr_l(rta, len, SEG6_LOCAL_NH4, &addr.data, - addr.bytelen); + ret = rta_addattr_l(rta, len, SEG6_LOCAL_NH4, + &addr.data, addr.bytelen); } else if (strcmp(*argv, "nh6") == 0) { NEXT_ARG(); if (nh6_ok++) duparg2("nh6", *argv); get_addr(&addr, *argv, AF_INET6); - rta_addattr_l(rta, len, SEG6_LOCAL_NH6, &addr.data, - addr.bytelen); + ret = rta_addattr_l(rta, len, SEG6_LOCAL_NH6, + &addr.data, addr.bytelen); } else if (strcmp(*argv, "iif") == 0) { NEXT_ARG(); if (iif_ok++) @@ -648,7 +651,7 @@ static int parse_encap_seg6local(struct rtattr *rta, size_t len, int *argcp, iif = ll_name_to_index(*argv); if (!iif) exit(nodev(*argv)); - rta_addattr32(rta, len, SEG6_LOCAL_IIF, iif); + ret = rta_addattr32(rta, len, SEG6_LOCAL_IIF, iif); } else if (strcmp(*argv, "oif") == 0) { NEXT_ARG(); if (oif_ok++) @@ -656,7 +659,7 @@ static int parse_encap_seg6local(struct rtattr *rta, size_t len, int *argcp, oif = ll_name_to_index(*argv); if (!oif) exit(nodev(*argv)); - rta_addattr32(rta, len, SEG6_LOCAL_OIF, oif); + ret = rta_addattr32(rta, len, SEG6_LOCAL_OIF, oif); } else if (strcmp(*argv, "srh") == 0) { NEXT_ARG(); if (srh_ok++) @@ -691,6 +694,8 @@ static int parse_encap_seg6local(struct rtattr *rta, size_t len, int *argcp, } else { break; } + if (ret) + return ret; argc--; argv++; } @@ -705,14 +710,14 @@ static int parse_encap_seg6local(struct rtattr *rta, size_t len, int *argcp, srh = parse_srh(segbuf, hmac, action == SEG6_LOCAL_ACTION_END_B6_ENCAP); srhlen = (srh->hdrlen + 1) << 3; - rta_addattr_l(rta, len, SEG6_LOCAL_SRH, srh, srhlen); + ret = rta_addattr_l(rta, len, SEG6_LOCAL_SRH, srh, srhlen); free(srh); } *argcp = argc + 1; *argvp = argv - 1; - return 0; + return ret; } static int parse_encap_mpls(struct rtattr *rta, size_t len, @@ -730,8 +735,9 @@ static int parse_encap_mpls(struct rtattr *rta, size_t len, exit(1); } - rta_addattr_l(rta, len, MPLS_IPTUNNEL_DST, &addr.data, - addr.bytelen); + if (rta_addattr_l(rta, len, MPLS_IPTUNNEL_DST, + &addr.data, addr.bytelen)) + return -1; argc--; argv++; @@ -745,7 +751,8 @@ static int parse_encap_mpls(struct rtattr *rta, size_t len, duparg2("ttl", *argv); if (get_u8(&ttl, *argv, 0)) invarg("\"ttl\" value is invalid\n", *argv); - rta_addattr8(rta, len, MPLS_IPTUNNEL_TTL, ttl); + if (rta_addattr8(rta, len, MPLS_IPTUNNEL_TTL, ttl)) + return -1; } else { break; } @@ -768,6 +775,7 @@ static int parse_encap_ip(struct rtattr *rta, size_t len, int id_ok = 0, dst_ok = 0, tos_ok = 0, ttl_ok = 0; char **argv = *argvp; int argc = *argcp; + int ret = 0; while (argc > 0) { if (strcmp(*argv, "id") == 0) { @@ -778,7 +786,7 @@ static int parse_encap_ip(struct rtattr *rta, size_t len, duparg2("id", *argv); if (get_be64(&id, *argv, 0)) invarg("\"id\" value is invalid\n", *argv); - rta_addattr64(rta, len, LWTUNNEL_IP_ID, id); + ret = rta_addattr64(rta, len, LWTUNNEL_IP_ID, id); } else if (strcmp(*argv, "dst") == 0) { inet_prefix addr; @@ -786,8 +794,8 @@ static int parse_encap_ip(struct rtattr *rta, size_t len, if (dst_ok++) duparg2("dst", *argv); get_addr(&addr, *argv, AF_INET); - rta_addattr_l(rta, len, LWTUNNEL_IP_DST, - &addr.data, addr.bytelen); + ret = rta_addattr_l(rta, len, LWTUNNEL_IP_DST, + &addr.data, addr.bytelen); } else if (strcmp(*argv, "tos") == 0) { __u32 tos; @@ -796,7 +804,7 @@ static int parse_encap_ip(struct rtattr *rta, size_t len, duparg2("tos", *argv); if (rtnl_dsfield_a2n(&tos, *argv)) invarg("\"tos\" value is invalid\n", *argv); - rta_addattr8(rta, len, LWTUNNEL_IP_TOS, tos); + ret = rta_addattr8(rta, len, LWTUNNEL_IP_TOS, tos); } else if (strcmp(*argv, "ttl") == 0) { __u8 ttl; @@ -805,10 +813,12 @@ static int parse_encap_ip(struct rtattr *rta, size_t len, duparg2("ttl", *argv); if (get_u8(&ttl, *argv, 0)) invarg("\"ttl\" value is invalid\n", *argv); - rta_addattr8(rta, len, LWTUNNEL_IP_TTL, ttl); + ret = rta_addattr8(rta, len, LWTUNNEL_IP_TTL, ttl); } else { break; } + if (ret) + break; argc--; argv++; } @@ -819,7 +829,7 @@ static int parse_encap_ip(struct rtattr *rta, size_t len, *argcp = argc + 1; *argvp = argv - 1; - return 0; + return ret; } static int parse_encap_ila(struct rtattr *rta, size_t len, @@ -828,6 +838,7 @@ static int parse_encap_ila(struct rtattr *rta, size_t len, __u64 locator; int argc = *argcp; char **argv = *argvp; + int ret = 0; if (get_addr64(&locator, *argv) < 0) { fprintf(stderr, "Bad locator: %s\n", *argv); @@ -836,7 +847,8 @@ static int parse_encap_ila(struct rtattr *rta, size_t len, argc--; argv++; - rta_addattr64(rta, 1024, ILA_ATTR_LOCATOR, locator); + if (rta_addattr64(rta, 1024, ILA_ATTR_LOCATOR, locator)) + return -1; while (argc > 0) { if (strcmp(*argv, "csum-mode") == 0) { @@ -849,8 +861,8 @@ static int parse_encap_ila(struct rtattr *rta, size_t len, invarg("\"csum-mode\" value is invalid\n", *argv); - rta_addattr8(rta, 1024, ILA_ATTR_CSUM_MODE, - (__u8)csum_mode); + ret = rta_addattr8(rta, 1024, ILA_ATTR_CSUM_MODE, + (__u8)csum_mode); argc--; argv++; } else if (strcmp(*argv, "ident-type") == 0) { @@ -863,8 +875,8 @@ static int parse_encap_ila(struct rtattr *rta, size_t len, invarg("\"ident-type\" value is invalid\n", *argv); - rta_addattr8(rta, 1024, ILA_ATTR_IDENT_TYPE, - (__u8)ident_type); + ret = rta_addattr8(rta, 1024, ILA_ATTR_IDENT_TYPE, + (__u8)ident_type); argc--; argv++; } else if (strcmp(*argv, "hook-type") == 0) { @@ -877,13 +889,15 @@ static int parse_encap_ila(struct rtattr *rta, size_t len, invarg("\"hook-type\" value is invalid\n", *argv); - rta_addattr8(rta, 1024, ILA_ATTR_HOOK_TYPE, - (__u8)hook_type); + ret = rta_addattr8(rta, 1024, ILA_ATTR_HOOK_TYPE, + (__u8)hook_type); argc--; argv++; } else { break; } + if (ret) + break; } /* argv is currently the first unparsed argument, @@ -893,7 +907,7 @@ static int parse_encap_ila(struct rtattr *rta, size_t len, *argcp = argc + 1; *argvp = argv - 1; - return 0; + return ret; } static int parse_encap_ip6(struct rtattr *rta, size_t len, @@ -902,6 +916,7 @@ static int parse_encap_ip6(struct rtattr *rta, size_t len, int id_ok = 0, dst_ok = 0, tos_ok = 0, ttl_ok = 0; char **argv = *argvp; int argc = *argcp; + int ret = 0; while (argc > 0) { if (strcmp(*argv, "id") == 0) { @@ -912,7 +927,7 @@ static int parse_encap_ip6(struct rtattr *rta, size_t len, duparg2("id", *argv); if (get_be64(&id, *argv, 0)) invarg("\"id\" value is invalid\n", *argv); - rta_addattr64(rta, len, LWTUNNEL_IP6_ID, id); + ret = rta_addattr64(rta, len, LWTUNNEL_IP6_ID, id); } else if (strcmp(*argv, "dst") == 0) { inet_prefix addr; @@ -920,8 +935,8 @@ static int parse_encap_ip6(struct rtattr *rta, size_t len, if (dst_ok++) duparg2("dst", *argv); get_addr(&addr, *argv, AF_INET6); - rta_addattr_l(rta, len, LWTUNNEL_IP6_DST, - &addr.data, addr.bytelen); + ret = rta_addattr_l(rta, len, LWTUNNEL_IP6_DST, + &addr.data, addr.bytelen); } else if (strcmp(*argv, "tc") == 0) { __u32 tc; @@ -930,7 +945,7 @@ static int parse_encap_ip6(struct rtattr *rta, size_t len, duparg2("tc", *argv); if (rtnl_dsfield_a2n(&tc, *argv)) invarg("\"tc\" value is invalid\n", *argv); - rta_addattr8(rta, len, LWTUNNEL_IP6_TC, tc); + ret = rta_addattr8(rta, len, LWTUNNEL_IP6_TC, tc); } else if (strcmp(*argv, "hoplimit") == 0) { __u8 hoplimit; @@ -940,10 +955,13 @@ static int parse_encap_ip6(struct rtattr *rta, size_t len, if (get_u8(&hoplimit, *argv, 0)) invarg("\"hoplimit\" value is invalid\n", *argv); - rta_addattr8(rta, len, LWTUNNEL_IP6_HOPLIMIT, hoplimit); + ret = rta_addattr8(rta, len, LWTUNNEL_IP6_HOPLIMIT, + hoplimit); } else { break; } + if (ret) + break; argc--; argv++; } @@ -954,7 +972,7 @@ static int parse_encap_ip6(struct rtattr *rta, size_t len, *argcp = argc + 1; *argvp = argv - 1; - return 0; + return ret; } static void lwt_bpf_usage(void) @@ -1021,6 +1039,7 @@ int lwt_parse_encap(struct rtattr *rta, size_t len, int *argcp, char ***argvp) int argc = *argcp; char **argv = *argvp; __u16 type; + int ret = 0; NEXT_ARG(); type = read_encap_type(*argv); @@ -1037,37 +1056,40 @@ int lwt_parse_encap(struct rtattr *rta, size_t len, int *argcp, char ***argvp) nest = rta_nest(rta, 1024, RTA_ENCAP); switch (type) { case LWTUNNEL_ENCAP_MPLS: - parse_encap_mpls(rta, len, &argc, &argv); + ret = parse_encap_mpls(rta, len, &argc, &argv); break; case LWTUNNEL_ENCAP_IP: - parse_encap_ip(rta, len, &argc, &argv); + ret = parse_encap_ip(rta, len, &argc, &argv); break; case LWTUNNEL_ENCAP_ILA: - parse_encap_ila(rta, len, &argc, &argv); + ret = parse_encap_ila(rta, len, &argc, &argv); break; case LWTUNNEL_ENCAP_IP6: - parse_encap_ip6(rta, len, &argc, &argv); + ret = parse_encap_ip6(rta, len, &argc, &argv); break; case LWTUNNEL_ENCAP_BPF: if (parse_encap_bpf(rta, len, &argc, &argv) < 0) exit(-1); break; case LWTUNNEL_ENCAP_SEG6: - parse_encap_seg6(rta, len, &argc, &argv); + ret = parse_encap_seg6(rta, len, &argc, &argv); break; case LWTUNNEL_ENCAP_SEG6_LOCAL: - parse_encap_seg6local(rta, len, &argc, &argv); + ret = parse_encap_seg6local(rta, len, &argc, &argv); break; default: fprintf(stderr, "Error: unsupported encap type\n"); break; } + if (ret) + return ret; + rta_nest_end(rta, nest); - rta_addattr16(rta, 1024, RTA_ENCAP_TYPE, type); + ret = rta_addattr16(rta, 1024, RTA_ENCAP_TYPE, type); *argcp = argc; *argvp = argv; - return 0; + return ret; }