From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.1 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2D40CC433DF for ; Wed, 22 Jul 2020 17:26:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E08E5207BB for ; Wed, 22 Jul 2020 17:26:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=efficios.com header.i=@efficios.com header.b="uYOrROJL" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729015AbgGVR0s (ORCPT ); Wed, 22 Jul 2020 13:26:48 -0400 Received: from mail.efficios.com ([167.114.26.124]:44046 "EHLO mail.efficios.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726349AbgGVR0r (ORCPT ); Wed, 22 Jul 2020 13:26:47 -0400 Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id A4F712D73F9; Wed, 22 Jul 2020 13:26:46 -0400 (EDT) Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id DmHD8t4Me0BR; Wed, 22 Jul 2020 13:26:46 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 581DD2D75C8; Wed, 22 Jul 2020 13:26:46 -0400 (EDT) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.efficios.com 581DD2D75C8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficios.com; s=default; t=1595438806; bh=ie1s3yfaP5hYh71RbSEDgzXewJydGQnrL4wq2YH/vxs=; h=Date:From:To:Message-ID:MIME-Version; b=uYOrROJLsUCtksuqZGWZRv7FC2VaA1JCcO/bSko1tPTQy4YCK9yv+ZqLEf/3Ig/p3 ba2pmRRRH6Zz4yoeFAvr9GJ3L00viVw7mStmr4Y9OwgjPXH/NgXVAUWUa1xkahWCzk C0PQVuyv8T/4duQYbUkfZo5CzPDStEb4a5E+AghIMPxgpWhZ22Soa7lbfr4lL6B4Uh KtPyXcHpUerUvQkSZu8YxXcqTo23HDVS0PCYsz5RKuqiuLv2iP0GY+0DA1nIMUPqy2 rXL9O9aOjRcoENC/ZCrPv484ZptDFLgvbrRy+LV3U19DmQR5J3fDzFqfCkPeCMnkiQ Nzv9jsaO0pFhA== X-Virus-Scanned: amavisd-new at efficios.com Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id jVFannzj4RYG; Wed, 22 Jul 2020 13:26:46 -0400 (EDT) Received: from mail03.efficios.com (mail03.efficios.com [167.114.26.124]) by mail.efficios.com (Postfix) with ESMTP id 4BE4B2D7532; Wed, 22 Jul 2020 13:26:46 -0400 (EDT) Date: Wed, 22 Jul 2020 13:26:46 -0400 (EDT) From: Mathieu Desnoyers To: netdev , Alexey Kuznetsov , Hideaki YOSHIFUJI , David Ahern Cc: David Ahern , "David S. Miller" Message-ID: <1949069529.26392.1595438806291.JavaMail.zimbra@efficios.com> In-Reply-To: <20200720221118.26148-1-mathieu.desnoyers@efficios.com> References: <20200720221118.26148-1-mathieu.desnoyers@efficios.com> Subject: Re: [RFC PATCH] Fix: ipv4/icmp: icmp error route lookup performed on wrong routing table MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [167.114.26.124] X-Mailer: Zimbra 8.8.15_GA_3955 (ZimbraWebClient - FF78 (Linux)/8.8.15_GA_3953) Thread-Topic: ipv4/icmp: icmp error route lookup performed on wrong routing table Thread-Index: z8oEnN4BMpZegUplmtxPoLdCmaontg== Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Adding IPv4/IPv6 maintainers in CC, along with David Ahern's k.org email address. ----- On Jul 20, 2020, at 6:11 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote: > As per RFC792, ICMP errors should be sent to the source host. > > However, in configurations with Virtual Forwarding and Routing tables, > looking up which routing table to use is currently done by using the > destination net_device. > > commit 9d1a6c4ea43e ("net: icmp_route_lookup should use rt dev to > determine L3 domain") changes the interfaces passed to > l3mdev_master_ifindex() and inet_addr_type_dev_table() from skb_in->dev > to skb_dst(skb_in)->dev in order to fix a NULL pointer dereference. This > changes the interface used for routing table lookup from source to > destination. Therefore, if the source and destination interfaces are > within separate VFR, or one in the global routing table and the other in > a VFR, looking up the source host in the destination interface's routing > table is likely to fail. > > One observable effect of this issue is that traceroute does not work in > the following cases: > > - Route leaking between global routing table and VRF > - Route leaking between VRFs > > [ Note 1: I'm not entirely sure what routing table should be used when > param->replyopts.opt.opt.srr is set ? Is it valid to honor Strict > Source Route when sending an ICMP error ? ] > > [ Note 2: I notice that ipv6 icmp6_send() uses skb_dst(skb)->dev as > argument to l3mdev_master_ifindex(). I'm not sure if it is correct ? ] > > [ This patch is only compile-tested. ] > > Fixes: 9d1a6c4ea43e ("net: icmp_route_lookup should use rt dev to determine L3 > domain") > Link: https://tools.ietf.org/html/rfc792 > Signed-off-by: Mathieu Desnoyers > Cc: David Ahern > Cc: David S. Miller > Cc: netdev@vger.kernel.org > --- > net/ipv4/icmp.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c > index e30515f89802..3d1da70c7293 100644 > --- a/net/ipv4/icmp.c > +++ b/net/ipv4/icmp.c > @@ -465,6 +465,7 @@ static struct rtable *icmp_route_lookup(struct net *net, > int type, int code, > struct icmp_bxm *param) > { > + struct net_device *route_lookup_dev; > struct rtable *rt, *rt2; > struct flowi4 fl4_dec; > int err; > @@ -479,7 +480,14 @@ static struct rtable *icmp_route_lookup(struct net *net, > fl4->flowi4_proto = IPPROTO_ICMP; > fl4->fl4_icmp_type = type; > fl4->fl4_icmp_code = code; > - fl4->flowi4_oif = l3mdev_master_ifindex(skb_dst(skb_in)->dev); > + /* > + * The device used for looking up which routing table to use is > + * preferably the source whenever it is set, which should ensure > + * the icmp error can be sent to the source host, else fallback > + * on the destination device. > + */ > + route_lookup_dev = skb_in->dev ? skb_in->dev : skb_dst(skb_in)->dev; > + fl4->flowi4_oif = l3mdev_master_ifindex(route_lookup_dev); > > security_skb_classify_flow(skb_in, flowi4_to_flowi(fl4)); > rt = ip_route_output_key_hash(net, fl4, skb_in); > @@ -503,7 +511,7 @@ static struct rtable *icmp_route_lookup(struct net *net, > if (err) > goto relookup_failed; > > - if (inet_addr_type_dev_table(net, skb_dst(skb_in)->dev, > + if (inet_addr_type_dev_table(net, route_lookup_dev, > fl4_dec.saddr) == RTN_LOCAL) { > rt2 = __ip_route_output_key(net, &fl4_dec); > if (IS_ERR(rt2)) > -- > 2.11.0 -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com