From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by passt.top (Postfix, from userid 1000) id 86A745A0274; Tue, 23 Apr 2024 22:41:25 +0200 (CEST) From: Stefano Brivio To: passt-dev@passt.top Subject: [PATCH 2/2] netlink: Don't duplicate routes referring to unrelated host interfaces Date: Tue, 23 Apr 2024 22:41:25 +0200 Message-ID: <20240423204125.3424982-3-sbrivio@redhat.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240423204125.3424982-1-sbrivio@redhat.com> References: <20240423204125.3424982-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: LE2FR6AVM63XO2XQNNWMGNWEFS7VMN26 X-Message-ID-Hash: LE2FR6AVM63XO2XQNNWMGNWEFS7VMN26 X-MailFrom: sbrivio@passt.top X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: runsisi , David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: We take care of this in nl_addr_dup(): if the interface index associated to an address doesn't match the selected host interface (ifa->ifa_index != ifi_src), we don't copy that address. But for routes, we just unconditionally update the interface index to match the index in the target namespace, even if the source interface didn't match. This might happen in two cases: with a pre-4.20 kernel without support for NETLINK_GET_STRICT_CHK, which won't filter routes based on the interface we pass in the request, as reported by runsisi, and any kernel with support for multipath routes where any of the nexthops refers to an unrelated host interface. In both cases, check the index of the source interface, and avoid copying unrelated routes. Reported-by: runsisi Link: https://bugs.passt.top/show_bug.cgi?id=86 Signed-off-by: Stefano Brivio --- netlink.c | 43 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/netlink.c b/netlink.c index a5a4870..e8325c7 100644 --- a/netlink.c +++ b/netlink.c @@ -554,21 +554,32 @@ int nl_route_dup(int s_src, unsigned int ifi_src, NLMSG_OK(nh, left) && (status = nl_status(nh, left, seq)) > 0; nh = NLMSG_NEXT(nh, left)) { struct rtmsg *rtm = (struct rtmsg *)NLMSG_DATA(nh); + bool discard = false; struct rtattr *rta; size_t na; if (nh->nlmsg_type != RTM_NEWROUTE) continue; - dup_routes++; - for (rta = RTM_RTA(rtm), na = RTM_PAYLOAD(nh); RTA_OK(rta, na); rta = RTA_NEXT(rta, na)) { /* RTA_OIF and RTA_MULTIPATH attributes carry the - * identifier of a host interface. Change them to match - * the corresponding identifier in the target namespace. - */ + * identifier of a host interface. If they match the + * host interface we're copying from, change them to + * match the corresponding identifier in the target + * namespace. + * + * If RTA_OIF doesn't match (NETLINK_GET_STRICT_CHK not + * available), or if any interface index in nexthop + * objects differ from the host interface, discard the + * route altogether. + */ if (rta->rta_type == RTA_OIF) { + if (*(unsigned int *)RTA_DATA(rta) != ifi_src) { + discard = true; + break; + } + *(unsigned int *)RTA_DATA(rta) = ifi_dst; } else if (rta->rta_type == RTA_MULTIPATH) { size_t nh_len = RTA_PAYLOAD(rta); @@ -576,8 +587,19 @@ int nl_route_dup(int s_src, unsigned int ifi_src, for (rtnh = (struct rtnexthop *)RTA_DATA(rta); RTNH_OK(rtnh, nh_len); - rtnh = RTNH_NEXT_AND_DEC(rtnh, nh_len)) + rtnh = RTNH_NEXT_AND_DEC(rtnh, nh_len)) { + int src = (int)ifi_src; + + if (rtnh->rtnh_ifindex != src) { + discard = true; + break; + } + rtnh->rtnh_ifindex = ifi_dst; + } + + if (discard) + break; } else if (rta->rta_type == RTA_PREFSRC) { /* Host routes might include a preferred source * address, which must be one of the host's @@ -588,6 +610,11 @@ int nl_route_dup(int s_src, unsigned int ifi_src, rta->rta_type = RTA_UNSPEC; } } + + if (discard) + rtm->rtm_family = AF_UNSPEC; + else + dup_routes++; } if (!NLMSG_OK(nh, left)) { @@ -619,10 +646,12 @@ int nl_route_dup(int s_src, unsigned int ifi_src, for (nh = (struct nlmsghdr *)buf, left = nlmsgs_size; NLMSG_OK(nh, left); nh = NLMSG_NEXT(nh, left)) { + struct rtmsg *rtm = (struct rtmsg *)NLMSG_DATA(nh); uint16_t flags = nh->nlmsg_flags; int rc; - if (nh->nlmsg_type != RTM_NEWROUTE) + if (nh->nlmsg_type != RTM_NEWROUTE || + rtm->rtm_family == AF_UNSPEC) continue; rc = nl_do(s_dst, nh, RTM_NEWROUTE, -- 2.43.0