All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/2] Fix vlan untag and insertion for bridge and vlan with reorder_hdr off
@ 2018-03-13  5:51 Toshiaki Makita
  2018-03-13  5:51 ` [PATCH net 1/2] net: Fix vlan untag for bridge and vlan_dev " Toshiaki Makita
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Toshiaki Makita @ 2018-03-13  5:51 UTC (permalink / raw)
  To: David S. Miller; +Cc: Toshiaki Makita, netdev, Brandon Carpenter, Vlad Yasevich

As Brandon Carpenter reported[1], sending non-vlan-offloaded packets from
bridge devices ends up with corrupted packets. He narrowed down this problem
and found that the root cause is in skb_reorder_vlan_header().

While I was working on fixing this problem, I found that the function does
not work properly for double tagged packets with reorder_hdr off as well.

Patch 1 fixes these 2 problems in skb_reorder_vlan_header().

And it turned out that fixing skb_reorder_vlan_header() is not sufficient
to receive double tagged packets with reorder_hdr off while I was testing the
fix. Vlan tags got out of order when vlan devices with reorder_hdr disabled
were stacked. Patch 2 fixes this problem.

[1] https://www.spinics.net/lists/linux-ethernet-bridging/msg07039.html

Toshiaki Makita (2):
  net: Fix vlan untag for bridge and vlan_dev with reorder_hdr off
  vlan: Fix out of order vlan headers with reorder header off

 include/linux/if_vlan.h       | 66 +++++++++++++++++++++++++++++++++++--------
 include/uapi/linux/if_ether.h |  1 +
 net/8021q/vlan_core.c         |  4 +--
 net/core/skbuff.c             |  7 +++--
 4 files changed, 63 insertions(+), 15 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net 1/2] net: Fix vlan untag for bridge and vlan_dev with reorder_hdr off
  2018-03-13  5:51 [PATCH net 0/2] Fix vlan untag and insertion for bridge and vlan with reorder_hdr off Toshiaki Makita
@ 2018-03-13  5:51 ` Toshiaki Makita
  2018-03-13  5:51 ` [PATCH net 2/2] vlan: Fix out of order vlan headers with reorder header off Toshiaki Makita
  2018-03-16 14:05 ` [PATCH net 0/2] Fix vlan untag and insertion for bridge and vlan with reorder_hdr off David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: Toshiaki Makita @ 2018-03-13  5:51 UTC (permalink / raw)
  To: David S. Miller; +Cc: Toshiaki Makita, netdev, Brandon Carpenter, Vlad Yasevich

When we have a bridge with vlan_filtering on and a vlan device on top of
it, packets would be corrupted in skb_vlan_untag() called from
br_dev_xmit().

The problem sits in skb_reorder_vlan_header() used in skb_vlan_untag(),
which makes use of skb->mac_len. In this function mac_len is meant for
handling rx path with vlan devices with reorder_header disabled, but in
tx path mac_len is typically 0 and cannot be used, which is the problem
in this case.

The current code even does not properly handle rx path (skb_vlan_untag()
called from __netif_receive_skb_core()) with reorder_header off actually.

In rx path single tag case, it works as follows:

- Before skb_reorder_vlan_header()

 mac_header                                data
   v                                        v
   +-------------------+-------------+------+----
   |        ETH        |    VLAN     | ETH  |
   |       ADDRS       | TPID | TCI  | TYPE |
   +-------------------+-------------+------+----
   <-------- mac_len --------->
                       <------------->
                        to be removed

- After skb_reorder_vlan_header()

            mac_header                     data
                 v                          v
                 +-------------------+------+----
                 |        ETH        | ETH  |
                 |       ADDRS       | TYPE |
                 +-------------------+------+----
                 <-------- mac_len --------->

This is ok, but in rx double tag case, it corrupts packets:

- Before skb_reorder_vlan_header()

 mac_header                                              data
   v                                                      v
   +-------------------+-------------+-------------+------+----
   |        ETH        |    VLAN     |    VLAN     | ETH  |
   |       ADDRS       | TPID | TCI  | TPID | TCI  | TYPE |
   +-------------------+-------------+-------------+------+----
   <--------------- mac_len ---------------->
                                     <------------->
                                    should be removed
                       <--------------------------->
                         actually will be removed

- After skb_reorder_vlan_header()

            mac_header                                   data
                 v                                        v
                               +-------------------+------+----
                               |        ETH        | ETH  |
                               |       ADDRS       | TYPE |
                               +-------------------+------+----
                 <--------------- mac_len ---------------->

So, two of vlan tags are both removed while only inner one should be
removed and mac_header (and mac_len) is broken.

skb_vlan_untag() is meant for removing the vlan header at (skb->data - 2),
so use skb->data and skb->mac_header to calculate the right offset.

Reported-by: Brandon Carpenter <brandon.carpenter@cypherpath.com>
Fixes: a6e18ff11170 ("vlan: Fix untag operations of stacked vlans with REORDER_HEADER off")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 include/uapi/linux/if_ether.h | 1 +
 net/core/skbuff.c             | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/if_ether.h b/include/uapi/linux/if_ether.h
index 8bbbcb5..820de5d 100644
--- a/include/uapi/linux/if_ether.h
+++ b/include/uapi/linux/if_ether.h
@@ -30,6 +30,7 @@
  */
 
 #define ETH_ALEN	6		/* Octets in one ethernet addr	 */
+#define ETH_TLEN	2		/* Octets in ethernet type field */
 #define ETH_HLEN	14		/* Total octets in header.	 */
 #define ETH_ZLEN	60		/* Min. octets in frame sans FCS */
 #define ETH_DATA_LEN	1500		/* Max. octets in payload	 */
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index baf9905..b103f46 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5020,13 +5020,16 @@ bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len)
 
 static struct sk_buff *skb_reorder_vlan_header(struct sk_buff *skb)
 {
+	int mac_len;
+
 	if (skb_cow(skb, skb_headroom(skb)) < 0) {
 		kfree_skb(skb);
 		return NULL;
 	}
 
-	memmove(skb->data - ETH_HLEN, skb->data - skb->mac_len - VLAN_HLEN,
-		2 * ETH_ALEN);
+	mac_len = skb->data - skb_mac_header(skb);
+	memmove(skb_mac_header(skb) + VLAN_HLEN, skb_mac_header(skb),
+		mac_len - VLAN_HLEN - ETH_TLEN);
 	skb->mac_header += VLAN_HLEN;
 	return skb;
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net 2/2] vlan: Fix out of order vlan headers with reorder header off
  2018-03-13  5:51 [PATCH net 0/2] Fix vlan untag and insertion for bridge and vlan with reorder_hdr off Toshiaki Makita
  2018-03-13  5:51 ` [PATCH net 1/2] net: Fix vlan untag for bridge and vlan_dev " Toshiaki Makita
@ 2018-03-13  5:51 ` Toshiaki Makita
  2018-03-16 14:05 ` [PATCH net 0/2] Fix vlan untag and insertion for bridge and vlan with reorder_hdr off David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: Toshiaki Makita @ 2018-03-13  5:51 UTC (permalink / raw)
  To: David S. Miller; +Cc: Toshiaki Makita, netdev, Brandon Carpenter, Vlad Yasevich

With reorder header off, received packets are untagged in skb_vlan_untag()
called from within __netif_receive_skb_core(), and later the tag will be
inserted back in vlan_do_receive().

This caused out of order vlan headers when we create a vlan device on top
of another vlan device, because vlan_do_receive() inserts a tag as the
outermost vlan tag. E.g. the outer tag is first removed in skb_vlan_untag()
and inserted back in vlan_do_receive(), then the inner tag is next removed
and inserted back as the outermost tag.

This patch fixes the behaviour by inserting the inner tag at the right
position.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 include/linux/if_vlan.h | 66 ++++++++++++++++++++++++++++++++++++++++---------
 net/8021q/vlan_core.c   |  4 +--
 2 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 5e6a2d4..c4a1cff 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -300,30 +300,34 @@ static inline bool vlan_hw_offload_capable(netdev_features_t features,
 }
 
 /**
- * __vlan_insert_tag - regular VLAN tag inserting
+ * __vlan_insert_inner_tag - inner VLAN tag inserting
  * @skb: skbuff to tag
  * @vlan_proto: VLAN encapsulation protocol
  * @vlan_tci: VLAN TCI to insert
+ * @mac_len: MAC header length including outer vlan headers
  *
- * Inserts the VLAN tag into @skb as part of the payload
+ * Inserts the VLAN tag into @skb as part of the payload at offset mac_len
  * Returns error if skb_cow_head failes.
  *
  * Does not change skb->protocol so this function can be used during receive.
  */
-static inline int __vlan_insert_tag(struct sk_buff *skb,
-				    __be16 vlan_proto, u16 vlan_tci)
+static inline int __vlan_insert_inner_tag(struct sk_buff *skb,
+					  __be16 vlan_proto, u16 vlan_tci,
+					  unsigned int mac_len)
 {
 	struct vlan_ethhdr *veth;
 
 	if (skb_cow_head(skb, VLAN_HLEN) < 0)
 		return -ENOMEM;
 
-	veth = skb_push(skb, VLAN_HLEN);
+	skb_push(skb, VLAN_HLEN);
 
-	/* Move the mac addresses to the beginning of the new header. */
-	memmove(skb->data, skb->data + VLAN_HLEN, 2 * ETH_ALEN);
+	/* Move the mac header sans proto to the beginning of the new header. */
+	memmove(skb->data, skb->data + VLAN_HLEN, mac_len - ETH_TLEN);
 	skb->mac_header -= VLAN_HLEN;
 
+	veth = (struct vlan_ethhdr *)(skb->data + mac_len - ETH_HLEN);
+
 	/* first, the ethernet type */
 	veth->h_vlan_proto = vlan_proto;
 
@@ -334,12 +338,30 @@ static inline int __vlan_insert_tag(struct sk_buff *skb,
 }
 
 /**
- * vlan_insert_tag - regular VLAN tag inserting
+ * __vlan_insert_tag - regular VLAN tag inserting
  * @skb: skbuff to tag
  * @vlan_proto: VLAN encapsulation protocol
  * @vlan_tci: VLAN TCI to insert
  *
  * Inserts the VLAN tag into @skb as part of the payload
+ * Returns error if skb_cow_head failes.
+ *
+ * Does not change skb->protocol so this function can be used during receive.
+ */
+static inline int __vlan_insert_tag(struct sk_buff *skb,
+				    __be16 vlan_proto, u16 vlan_tci)
+{
+	return __vlan_insert_inner_tag(skb, vlan_proto, vlan_tci, ETH_HLEN);
+}
+
+/**
+ * vlan_insert_inner_tag - inner VLAN tag inserting
+ * @skb: skbuff to tag
+ * @vlan_proto: VLAN encapsulation protocol
+ * @vlan_tci: VLAN TCI to insert
+ * @mac_len: MAC header length including outer vlan headers
+ *
+ * Inserts the VLAN tag into @skb as part of the payload at offset mac_len
  * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
  *
  * Following the skb_unshare() example, in case of error, the calling function
@@ -347,12 +369,14 @@ static inline int __vlan_insert_tag(struct sk_buff *skb,
  *
  * Does not change skb->protocol so this function can be used during receive.
  */
-static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
-					      __be16 vlan_proto, u16 vlan_tci)
+static inline struct sk_buff *vlan_insert_inner_tag(struct sk_buff *skb,
+						    __be16 vlan_proto,
+						    u16 vlan_tci,
+						    unsigned int mac_len)
 {
 	int err;
 
-	err = __vlan_insert_tag(skb, vlan_proto, vlan_tci);
+	err = __vlan_insert_inner_tag(skb, vlan_proto, vlan_tci, mac_len);
 	if (err) {
 		dev_kfree_skb_any(skb);
 		return NULL;
@@ -361,6 +385,26 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
 }
 
 /**
+ * vlan_insert_tag - regular VLAN tag inserting
+ * @skb: skbuff to tag
+ * @vlan_proto: VLAN encapsulation protocol
+ * @vlan_tci: VLAN TCI to insert
+ *
+ * Inserts the VLAN tag into @skb as part of the payload
+ * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
+ *
+ * Following the skb_unshare() example, in case of error, the calling function
+ * doesn't have to worry about freeing the original skb.
+ *
+ * Does not change skb->protocol so this function can be used during receive.
+ */
+static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
+					      __be16 vlan_proto, u16 vlan_tci)
+{
+	return vlan_insert_inner_tag(skb, vlan_proto, vlan_tci, ETH_HLEN);
+}
+
+/**
  * vlan_insert_tag_set_proto - regular VLAN tag inserting
  * @skb: skbuff to tag
  * @vlan_proto: VLAN encapsulation protocol
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 64aa9f7..45c9bf5 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -48,8 +48,8 @@ bool vlan_do_receive(struct sk_buff **skbp)
 		 * original position later
 		 */
 		skb_push(skb, offset);
-		skb = *skbp = vlan_insert_tag(skb, skb->vlan_proto,
-					      skb->vlan_tci);
+		skb = *skbp = vlan_insert_inner_tag(skb, skb->vlan_proto,
+						    skb->vlan_tci, skb->mac_len);
 		if (!skb)
 			return false;
 		skb_pull(skb, offset + VLAN_HLEN);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net 0/2] Fix vlan untag and insertion for bridge and vlan with reorder_hdr off
  2018-03-13  5:51 [PATCH net 0/2] Fix vlan untag and insertion for bridge and vlan with reorder_hdr off Toshiaki Makita
  2018-03-13  5:51 ` [PATCH net 1/2] net: Fix vlan untag for bridge and vlan_dev " Toshiaki Makita
  2018-03-13  5:51 ` [PATCH net 2/2] vlan: Fix out of order vlan headers with reorder header off Toshiaki Makita
@ 2018-03-16 14:05 ` David Miller
  2018-03-28  6:11   ` Eric Dumazet
  2 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2018-03-16 14:05 UTC (permalink / raw)
  To: makita.toshiaki; +Cc: netdev, brandon.carpenter, vyasevic

From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Date: Tue, 13 Mar 2018 14:51:26 +0900

> As Brandon Carpenter reported[1], sending non-vlan-offloaded packets from
> bridge devices ends up with corrupted packets. He narrowed down this problem
> and found that the root cause is in skb_reorder_vlan_header().
> 
> While I was working on fixing this problem, I found that the function does
> not work properly for double tagged packets with reorder_hdr off as well.
> 
> Patch 1 fixes these 2 problems in skb_reorder_vlan_header().
> 
> And it turned out that fixing skb_reorder_vlan_header() is not sufficient
> to receive double tagged packets with reorder_hdr off while I was testing the
> fix. Vlan tags got out of order when vlan devices with reorder_hdr disabled
> were stacked. Patch 2 fixes this problem.
> 
> [1] https://www.spinics.net/lists/linux-ethernet-bridging/msg07039.html

Series applied and queued up for -stable, thanks.

I was thinking of pushing back on the addition of the ETH_TLEN UAPI visible
macro, because I don't see any other system providing that define.  But in
the end I decided that it's harmless and really that header file is the
correct location for such a definition.

Thank you.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net 0/2] Fix vlan untag and insertion for bridge and vlan with reorder_hdr off
  2018-03-16 14:05 ` [PATCH net 0/2] Fix vlan untag and insertion for bridge and vlan with reorder_hdr off David Miller
@ 2018-03-28  6:11   ` Eric Dumazet
  2018-03-28  8:03     ` Toshiaki Makita
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2018-03-28  6:11 UTC (permalink / raw)
  To: David Miller, makita.toshiaki; +Cc: netdev, brandon.carpenter, vyasevic



On 03/16/2018 07:05 AM, David Miller wrote:
> From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> Date: Tue, 13 Mar 2018 14:51:26 +0900
> 
>> As Brandon Carpenter reported[1], sending non-vlan-offloaded packets from
>> bridge devices ends up with corrupted packets. He narrowed down this problem
>> and found that the root cause is in skb_reorder_vlan_header().
>>
>> While I was working on fixing this problem, I found that the function does
>> not work properly for double tagged packets with reorder_hdr off as well.
>>
>> Patch 1 fixes these 2 problems in skb_reorder_vlan_header().
>>
>> And it turned out that fixing skb_reorder_vlan_header() is not sufficient
>> to receive double tagged packets with reorder_hdr off while I was testing the
>> fix. Vlan tags got out of order when vlan devices with reorder_hdr disabled
>> were stacked. Patch 2 fixes this problem.
>>
>> [1] https://www.spinics.net/lists/linux-ethernet-bridging/msg07039.html
> 
> Series applied and queued up for -stable, thanks.
> 
> I was thinking of pushing back on the addition of the ETH_TLEN UAPI visible
> macro, because I don't see any other system providing that define.  But in
> the end I decided that it's harmless and really that header file is the
> correct location for such a definition.
> 
> Thank you.
> 

syzbot reported some crashes caused by a memmove(..., ..., count=-2)

So something needs to be refined I guess.

BUG: unable to handle kernel paging request at ffff8801cccb8000
IP: __memmove+0x24/0x1a0 arch/x86/lib/memmove_64.S:43
PGD 9cee067 P4D 9cee067 PUD 1d9401063 PMD 1cccb7063 PTE 2810100028101
Oops: 000b [#1] SMP KASAN
Dumping ftrace buffer:
   (ftrace buffer empty)
Modules linked in:
CPU: 1 PID: 17663 Comm: syz-executor2 Not tainted 4.16.0-rc7+ #368
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:__memmove+0x24/0x1a0 arch/x86/lib/memmove_64.S:43
RSP: 0018:ffff8801cc046e28 EFLAGS: 00010287
RAX: ffff8801ccc244c4 RBX: fffffffffffffffe RCX: fffffffffff6c4c2
RDX: fffffffffffffffe RSI: ffff8801cccb7ffc RDI: ffff8801cccb8000
RBP: ffff8801cc046e48 R08: ffff8801ccc244be R09: ffffed0039984899
R10: 0000000000000001 R11: ffffed0039984898 R12: ffff8801ccc244c4
R13: ffff8801ccc244c0 R14: ffff8801d96b7c06 R15: ffff8801d96b7b40
FS:  00007febd562d700(0000) GS:ffff8801db300000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffff8801cccb8000 CR3: 00000001ccb2f006 CR4: 00000000001606e0
DR0: 0000000020000000 DR1: 0000000020000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000600
Call Trace:
 memmove include/linux/string.h:360 [inline]
 skb_reorder_vlan_header net/core/skbuff.c:5031 [inline]
 skb_vlan_untag+0x470/0xc40 net/core/skbuff.c:5061
 __netif_receive_skb_core+0x119c/0x3460 net/core/dev.c:4460
 __netif_receive_skb+0x2c/0x1b0 net/core/dev.c:4627
 netif_receive_skb_internal+0x10b/0x670 net/core/dev.c:4701
 netif_receive_skb+0xae/0x390 net/core/dev.c:4725
 tun_rx_batched.isra.50+0x5ee/0x870 drivers/net/tun.c:1555
 tun_get_user+0x299e/0x3c20 drivers/net/tun.c:1962
 tun_chr_write_iter+0xb9/0x160 drivers/net/tun.c:1990
 call_write_iter include/linux/fs.h:1782 [inline]
 new_sync_write fs/read_write.c:469 [inline]
 __vfs_write+0x684/0x970 fs/read_write.c:482
 vfs_write+0x189/0x510 fs/read_write.c:544
 SYSC_write fs/read_write.c:589 [inline]
 SyS_write+0xef/0x220 fs/read_write.c:581
 do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
 entry_SYSCALL_64_after_hwframe+0x42/0xb7
RIP: 0033:0x454879
RSP: 002b:00007febd562cc68 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007febd562d6d4 RCX: 0000000000454879
RDX: 0000000000000157 RSI: 0000000020000180 RDI: 0000000000000014
RBP: 000000000072bea0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 00000000000006b0 R14: 00000000006fc120 R15: 0000000000000000
Code: 90 90 90 90 90 90 90 48 89 f8 48 83 fa 20 0f 82 03 01 00 00 48 39 fe 7d 0f 49 89 f0 49 01 d0 49 39 f8 0f 8f 9f 00 00 00 48 89 d1 <f3> a4 c3 48 81 fa a8 02 00 00 72 05 40 38 fe 74 3b 48 83 ea 20 
RIP: __memmove+0x24/0x1a0 arch/x86/lib/memmove_64.S:43 RSP: ffff8801cc046e28
CR2: ffff8801cccb8000
---[ end trace b21c0866ee797d6d ]---
BUG: unable to handle kernel 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net 0/2] Fix vlan untag and insertion for bridge and vlan with reorder_hdr off
  2018-03-28  6:11   ` Eric Dumazet
@ 2018-03-28  8:03     ` Toshiaki Makita
  0 siblings, 0 replies; 6+ messages in thread
From: Toshiaki Makita @ 2018-03-28  8:03 UTC (permalink / raw)
  To: Eric Dumazet, David Miller; +Cc: netdev, brandon.carpenter, vyasevic

On 2018/03/28 15:11, Eric Dumazet wrote:
> On 03/16/2018 07:05 AM, David Miller wrote:
>> From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>> Date: Tue, 13 Mar 2018 14:51:26 +0900
>>
>>> As Brandon Carpenter reported[1], sending non-vlan-offloaded packets from
>>> bridge devices ends up with corrupted packets. He narrowed down this problem
>>> and found that the root cause is in skb_reorder_vlan_header().
>>>
>>> While I was working on fixing this problem, I found that the function does
>>> not work properly for double tagged packets with reorder_hdr off as well.
>>>
>>> Patch 1 fixes these 2 problems in skb_reorder_vlan_header().
>>>
>>> And it turned out that fixing skb_reorder_vlan_header() is not sufficient
>>> to receive double tagged packets with reorder_hdr off while I was testing the
>>> fix. Vlan tags got out of order when vlan devices with reorder_hdr disabled
>>> were stacked. Patch 2 fixes this problem.
>>>
>>> [1] https://www.spinics.net/lists/linux-ethernet-bridging/msg07039.html
>>
>> Series applied and queued up for -stable, thanks.
>>
>> I was thinking of pushing back on the addition of the ETH_TLEN UAPI visible
>> macro, because I don't see any other system providing that define.  But in
>> the end I decided that it's harmless and really that header file is the
>> correct location for such a definition.
>>
>> Thank you.
>>
> 
> syzbot reported some crashes caused by a memmove(..., ..., count=-2)

Thank you for your report.
Interesting, tun device can make vlan packets which does not have
ethernet header.

> 
> So something needs to be refined I guess.

Probably we can skip memmove() for such packets, since there is no
header to copy.
I'll make a fix. Thanks.

> 
> BUG: unable to handle kernel paging request at ffff8801cccb8000
> IP: __memmove+0x24/0x1a0 arch/x86/lib/memmove_64.S:43
> PGD 9cee067 P4D 9cee067 PUD 1d9401063 PMD 1cccb7063 PTE 2810100028101
> Oops: 000b [#1] SMP KASAN
> Dumping ftrace buffer:
>    (ftrace buffer empty)
> Modules linked in:
> CPU: 1 PID: 17663 Comm: syz-executor2 Not tainted 4.16.0-rc7+ #368
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> RIP: 0010:__memmove+0x24/0x1a0 arch/x86/lib/memmove_64.S:43
> RSP: 0018:ffff8801cc046e28 EFLAGS: 00010287
> RAX: ffff8801ccc244c4 RBX: fffffffffffffffe RCX: fffffffffff6c4c2
> RDX: fffffffffffffffe RSI: ffff8801cccb7ffc RDI: ffff8801cccb8000
> RBP: ffff8801cc046e48 R08: ffff8801ccc244be R09: ffffed0039984899
> R10: 0000000000000001 R11: ffffed0039984898 R12: ffff8801ccc244c4
> R13: ffff8801ccc244c0 R14: ffff8801d96b7c06 R15: ffff8801d96b7b40
> FS:  00007febd562d700(0000) GS:ffff8801db300000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffff8801cccb8000 CR3: 00000001ccb2f006 CR4: 00000000001606e0
> DR0: 0000000020000000 DR1: 0000000020000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000600
> Call Trace:
>  memmove include/linux/string.h:360 [inline]
>  skb_reorder_vlan_header net/core/skbuff.c:5031 [inline]
>  skb_vlan_untag+0x470/0xc40 net/core/skbuff.c:5061
>  __netif_receive_skb_core+0x119c/0x3460 net/core/dev.c:4460
>  __netif_receive_skb+0x2c/0x1b0 net/core/dev.c:4627
>  netif_receive_skb_internal+0x10b/0x670 net/core/dev.c:4701
>  netif_receive_skb+0xae/0x390 net/core/dev.c:4725
>  tun_rx_batched.isra.50+0x5ee/0x870 drivers/net/tun.c:1555
>  tun_get_user+0x299e/0x3c20 drivers/net/tun.c:1962
>  tun_chr_write_iter+0xb9/0x160 drivers/net/tun.c:1990
>  call_write_iter include/linux/fs.h:1782 [inline]
>  new_sync_write fs/read_write.c:469 [inline]
>  __vfs_write+0x684/0x970 fs/read_write.c:482
>  vfs_write+0x189/0x510 fs/read_write.c:544
>  SYSC_write fs/read_write.c:589 [inline]
>  SyS_write+0xef/0x220 fs/read_write.c:581
>  do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
>  entry_SYSCALL_64_after_hwframe+0x42/0xb7
> RIP: 0033:0x454879
> RSP: 002b:00007febd562cc68 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 00007febd562d6d4 RCX: 0000000000454879
> RDX: 0000000000000157 RSI: 0000000020000180 RDI: 0000000000000014
> RBP: 000000000072bea0 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
> R13: 00000000000006b0 R14: 00000000006fc120 R15: 0000000000000000
> Code: 90 90 90 90 90 90 90 48 89 f8 48 83 fa 20 0f 82 03 01 00 00 48 39 fe 7d 0f 49 89 f0 49 01 d0 49 39 f8 0f 8f 9f 00 00 00 48 89 d1 <f3> a4 c3 48 81 fa a8 02 00 00 72 05 40 38 fe 74 3b 48 83 ea 20 
> RIP: __memmove+0x24/0x1a0 arch/x86/lib/memmove_64.S:43 RSP: ffff8801cc046e28
> CR2: ffff8801cccb8000
> ---[ end trace b21c0866ee797d6d ]---
> BUG: unable to handle kernel 
> 
> 
> 

-- 
Toshiaki Makita

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-03-28  8:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-13  5:51 [PATCH net 0/2] Fix vlan untag and insertion for bridge and vlan with reorder_hdr off Toshiaki Makita
2018-03-13  5:51 ` [PATCH net 1/2] net: Fix vlan untag for bridge and vlan_dev " Toshiaki Makita
2018-03-13  5:51 ` [PATCH net 2/2] vlan: Fix out of order vlan headers with reorder header off Toshiaki Makita
2018-03-16 14:05 ` [PATCH net 0/2] Fix vlan untag and insertion for bridge and vlan with reorder_hdr off David Miller
2018-03-28  6:11   ` Eric Dumazet
2018-03-28  8:03     ` Toshiaki Makita

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.