netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: netdev@vger.kernel.org
Cc: Florian Westphal <fw@strlen.de>
Subject: [PATCH] macvlan: fix oops with vlan-on-top and HW_VLAN_CTAG_TX lowerdev
Date: Sat, 28 Dec 2013 14:38:40 +0100	[thread overview]
Message-ID: <1388237920-5140-1-git-send-email-fw@strlen.de> (raw)

commit 797f87f83b60685ff8a13fa0572d2f10393c50d3
(macvlan: fix netdev feature propagation from lower device) can result
in oops:

[   81.011639] 8021q: adding VLAN 0 to HW filter on device wan0
[   81.030402] BUG: unable to handle kernel NULL pointer dereference at 00000000000001e0
[   81.032267] IP: [<ffffffff813269d0>] macvlan_hard_header+0x40/0x60
[..]
[   81.034359] RIP: 0010:[<ffffffff813269d0>]  [<ffffffff813269d0>] macvlan_hard_header+0x40/0x60
[..]
[   81.034359]  <IRQ> 
[   81.034359]  [<ffffffff8139c7ed>] ? neigh_resolve_output+0x16d/0x2b0
[   81.034359]  [<ffffffff81484246>] ? ip6_finish_output2+0x176/0x600
[   81.034359]  [<ffffffff81484246>] ip6_finish_output2+0x176/0x600
[   81.034359]  [<ffffffff81484129>] ? ip6_finish_output2+0x59/0x600
[   81.034359]  [<ffffffff814865c6>] ip6_finish_output+0x96/0x1f0
[   81.034359]  [<ffffffff81486773>] ip6_output+0x53/0x1c0
[   81.034359]  [<ffffffff814a7dc2>] mld_sendpack+0x2b2/0x330
[   81.034359]  [<ffffffff814a8774>] mld_ifc_timer_expire+0x194/0x2c0

...if the lower device supports NETIF_F_HW_VLAN_CTAG_TX flag and a vlan
is created on top of the macvlan device, i.e.

ip link add link eth0 name wan0 type macvlan
ip link add link wan0 name wan1 type vlan id 2
ip link set wan0 up

reason is that 8021q sets dev->header_ops to the realdev in
'NETIF_F_HW_VLAN_CTAG_TX present' case - but macvlan_heard_header
assumes that the *dev pointer passed is a macvlan device.

But thats not the case in the above scenario.
macvlan_hard_header is invokes with *dev being the 8021q interface,
which then oopses since the netdev_priv area is something completely different.

Cap lowerdev feature set to the one explicitly set via MACVLAN_FEATURES
before trying to increment any features.

Fixes: 797f87f83b ("macvlan: fix netdev feature propagation from lower device")
Reported-by: Will Trives <renevant@internode.on.net>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 WARNING: I am not sure this is correct.

 We lose flags that the lowerdev of the macvlan could handle.

 What 8021q is doing in net/8021q/vlan_dev.c:vlan_dev_init seems strange to me.

 Where does it say that is ok to just do
  dev->header_ops      = real_dev->header_ops;

 and then assume that header_ops->create() et al. will cope
 with dev being a 8021q device instead of real_dev?

 To me this was completely unexpected.

 Or is the real bug the use of netdev_priv in macvlan_hard_header()?

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 60406b0..cd2791b 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -690,13 +690,13 @@ static netdev_features_t macvlan_fix_features(struct net_device *dev,
 					      netdev_features_t features)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
-	netdev_features_t mask;
+	netdev_features_t mask, lowerdev_features;
 
-	features |= NETIF_F_ALL_FOR_ALL;
 	features &= (vlan->set_features | ~MACVLAN_FEATURES);
+	lowerdev_features = vlan->lowerdev->features & MACVLAN_FEATURES;
 	mask = features;
 
-	features = netdev_increment_features(vlan->lowerdev->features,
+	features = netdev_increment_features(lowerdev_features,
 					     features,
 					     mask);
 	if (!vlan->fwd_priv)
-- 
1.8.1.5

             reply	other threads:[~2013-12-28 13:41 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-28 13:38 Florian Westphal [this message]
2013-12-29 22:01 ` [PATCH] macvlan: fix oops with vlan-on-top and HW_VLAN_CTAG_TX lowerdev David Miller
2013-12-30 10:27   ` Florian Westphal
2013-12-31 21:24     ` David Miller
2014-01-03 11:39       ` Florian Westphal
2014-01-03 20:33         ` David Miller
2014-01-04 13:51           ` Florian Westphal
2014-01-05  1:13             ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1388237920-5140-1-git-send-email-fw@strlen.de \
    --to=fw@strlen.de \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).