netdev - Re: Why revert commit 2271c95 ("vrf: mark skb for multicast or link-local as enslaved to VRF")?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201018160624.GB11729@ICIPI.localdomain>
Date:   Sun, 18 Oct 2020 12:06:24 -0400
From:   Stephen Suryaputra <ssuryaextr@...il.com>
To:     David Ahern <dsahern@...il.com>
Cc:     netdev@...r.kernel.org, sashal@...nel.org,
        mmanning@...tta.att-mail.com
Subject: Re: Why revert commit 2271c95 ("vrf: mark skb for multicast or
 link-local as enslaved to VRF")?

On Sun, Oct 18, 2020 at 09:27:16AM -0600, David Ahern wrote:
> On 10/18/20 7:24 AM, Stephen Suryaputra wrote:
> > Greetings,
> > 
> > We noticed that the commit was reverted after upgrading to v4.14.200.
> > Any reason why it is reverted? We rely on it.
> > 
> 
> $ git show 2271c95
> fatal: ambiguous argument '2271c95': unknown revision or path not in the
> working tree.
> Use '--' to separate paths from revisions, like this:
> 'git <command> [<revision>...] -- [<file>...]'

$ git clone git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
$ git checkout v4.14.200
$ git --no-pager show 2271c95

commit 2271c9500434af2a26b2c9eadeb3c0b075409fb5
Author: Mike Manning <mmanning@...tta.att-mail.com>
Date:   Wed Nov 7 15:36:07 2018 +0000

    vrf: mark skb for multicast or link-local as enslaved to VRF
    
    [ Upstream commit 6f12fa775530195a501fb090d092c637f32d0cc5 ]
    
    The skb for packets that are multicast or to a link-local address are
    not marked as being enslaved to a VRF, if they are received on a socket
    bound to the VRF. This is needed for ND and it is preferable for the
    kernel not to have to deal with the additional use-cases if ll or mcast
    packets are handled as enslaved. However, this does not allow service
    instances listening on unbound and bound to VRF sockets to distinguish
    the VRF used, if packets are sent as multicast or to a link-local
    address. The fix is for the VRF driver to also mark these skb as being
    enslaved to the VRF.
    
    Signed-off-by: Mike Manning <mmanning@...tta.att-mail.com>
    Reviewed-by: David Ahern <dsahern@...il.com>
    Tested-by: David Ahern <dsahern@...il.com>
    Signed-off-by: David S. Miller <davem@...emloft.net>
    Signed-off-by: Sasha Levin <sashal@...nel.org>

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 03e4fcdfeab7..e0cea5c05f0e 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -996,24 +996,23 @@ static struct sk_buff *vrf_ip6_rcv(struct net_device *vrf_dev,
 				   struct sk_buff *skb)
 {
 	int orig_iif = skb->skb_iif;
-	bool need_strict;
+	bool need_strict = rt6_need_strict(&ipv6_hdr(skb)->daddr);
+	bool is_ndisc = ipv6_ndisc_frame(skb);
 
-	/* loopback traffic; do not push through packet taps again.
-	 * Reset pkt_type for upper layers to process skb
+	/* loopback, multicast & non-ND link-local traffic; do not push through
+	 * packet taps again. Reset pkt_type for upper layers to process skb
 	 */
-	if (skb->pkt_type == PACKET_LOOPBACK) {
+	if (skb->pkt_type == PACKET_LOOPBACK || (need_strict && !is_ndisc)) {
 		skb->dev = vrf_dev;
 		skb->skb_iif = vrf_dev->ifindex;
 		IP6CB(skb)->flags |= IP6SKB_L3SLAVE;
-		skb->pkt_type = PACKET_HOST;
+		if (skb->pkt_type == PACKET_LOOPBACK)
+			skb->pkt_type = PACKET_HOST;
 		goto out;
 	}
 
-	/* if packet is NDISC or addressed to multicast or link-local
-	 * then keep the ingress interface
-	 */
-	need_strict = rt6_need_strict(&ipv6_hdr(skb)->daddr);
-	if (!ipv6_ndisc_frame(skb) && !need_strict) {
+	/* if packet is NDISC then keep the ingress interface */
+	if (!is_ndisc) {
 		vrf_rx_stats(vrf_dev, skb->len);
 		skb->dev = vrf_dev;
 		skb->skb_iif = vrf_dev->ifindex;

$ git --no-pager show afed1a4

commit afed1a4dbb76c81900f10fd77397fb91ad442702
Author: Sasha Levin <sashal@...nel.org>
Date:   Mon Mar 23 16:21:31 2020 -0400

    Revert "vrf: mark skb for multicast or link-local as enslaved to VRF"
    
    This reverts commit 2271c9500434af2a26b2c9eadeb3c0b075409fb5.
    
    This patch shouldn't have been backported to 4.14.
    
    Signed-off-by: Sasha Levin <sashal@...nel.org>

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index e0cea5c05f0e..03e4fcdfeab7 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -996,23 +996,24 @@ static struct sk_buff *vrf_ip6_rcv(struct net_device *vrf_dev,
 				   struct sk_buff *skb)
 {
 	int orig_iif = skb->skb_iif;
-	bool need_strict = rt6_need_strict(&ipv6_hdr(skb)->daddr);
-	bool is_ndisc = ipv6_ndisc_frame(skb);
+	bool need_strict;
 
-	/* loopback, multicast & non-ND link-local traffic; do not push through
-	 * packet taps again. Reset pkt_type for upper layers to process skb
+	/* loopback traffic; do not push through packet taps again.
+	 * Reset pkt_type for upper layers to process skb
 	 */
-	if (skb->pkt_type == PACKET_LOOPBACK || (need_strict && !is_ndisc)) {
+	if (skb->pkt_type == PACKET_LOOPBACK) {
 		skb->dev = vrf_dev;
 		skb->skb_iif = vrf_dev->ifindex;
 		IP6CB(skb)->flags |= IP6SKB_L3SLAVE;
-		if (skb->pkt_type == PACKET_LOOPBACK)
-			skb->pkt_type = PACKET_HOST;
+		skb->pkt_type = PACKET_HOST;
 		goto out;
 	}
 
-	/* if packet is NDISC then keep the ingress interface */
-	if (!is_ndisc) {
+	/* if packet is NDISC or addressed to multicast or link-local
+	 * then keep the ingress interface
+	 */
+	need_strict = rt6_need_strict(&ipv6_hdr(skb)->daddr);
+	if (!ipv6_ndisc_frame(skb) && !need_strict) {
 		vrf_rx_stats(vrf_dev, skb->len);
 		skb->dev = vrf_dev;
 		skb->skb_iif = vrf_dev->ifindex;