From 60e4595101fd3f0ce6193ef87320c94641779708 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 21 Feb 2019 08:37:40 +0100 Subject: [PATCH] platform: cleanup parsing of RTA_MULTIPATH in _new_from_nl_route() I think the code before was correct. At the very least because we only run the while-loop at most once because multipath routes are not supported. However, it seems odd that the while loop checks for "tlen >= rtnh->rtnh_len" but later we do "tlen -= RTNH_ALIGN (rtnh->rtnh_len)" Well, arguably, tlen itself is aligned to 4 bytes (as kernel sends the netlink message that way). So, it was indeed fine. Still, confusing. Try to check more explicitly for the buffer sizes. --- src/platform/nm-linux-platform.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 1fa0c451e5..6b5ee8635c 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -3148,18 +3148,25 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) *****************************************************************/ if (tb[RTA_MULTIPATH]) { - struct rtnexthop *rtnh = nla_data (tb[RTA_MULTIPATH]); size_t tlen = nla_len (tb[RTA_MULTIPATH]); + struct rtnexthop *rtnh; - while ( tlen >= sizeof (*rtnh) - && tlen >= rtnh->rtnh_len) { + if (tlen < sizeof (*rtnh)) + goto rta_multipath_done; + + rtnh = nla_data_as (struct rtnexthop, tb[RTA_MULTIPATH]); + + if (tlen < rtnh->rtnh_len) + goto rta_multipath_done; + + while (TRUE) { if (nh.is_present) { /* we don't support multipath routes. */ return NULL; } - nh.is_present = TRUE; + nh.is_present = TRUE; nh.ifindex = rtnh->rtnh_ifindex; if (rtnh->rtnh_len > sizeof (*rtnh)) { @@ -3175,9 +3182,14 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) memcpy (&nh.gateway, nla_data (ntb[RTA_GATEWAY]), addr_len); } + if (tlen < RTNH_ALIGN (rtnh->rtnh_len) + sizeof (*rtnh)) + goto rta_multipath_done; + tlen -= RTNH_ALIGN (rtnh->rtnh_len); rtnh = RTNH_NEXT (rtnh); } +rta_multipath_done: + ; } if ( tb[RTA_OIF]