Netdev List
 help / color / mirror / Atom feed
* [PATCH v3 19/19] MAINTAINERS: Add entry for Thunderbolt network driver
From: Mika Westerberg @ 2017-10-02 10:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman, David S . Miller
  Cc: Andreas Noever, Michael Jamet, Yehezkel Bernat, Amir Levy,
	Mario.Limonciello, Lukas Wunner, Andy Shevchenko, Andrew Lunn,
	Mika Westerberg, netdev, linux-kernel
In-Reply-To: <20171002103846.64602-1-mika.westerberg@linux.intel.com>

I will be maintaining the Thunderbolt network driver along with Michael
and Yehezkel.

Signed-off-by: Michael Jamet <michael.jamet@intel.com>
Signed-off-by: Yehezkel Bernat <yehezkel.bernat@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 34661b5ac9ad..745527d5e326 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13286,6 +13286,14 @@ S:	Maintained
 F:	drivers/thunderbolt/
 F:	include/linux/thunderbolt.h
 
+THUNDERBOLT NETWORK DRIVER
+M:	Michael Jamet <michael.jamet@intel.com>
+M:	Mika Westerberg <mika.westerberg@linux.intel.com>
+M:	Yehezkel Bernat <yehezkel.bernat@intel.com>
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	drivers/net/thunderbolt.c
+
 THUNDERX GPIO DRIVER
 M:	David Daney <david.daney@cavium.com>
 S:	Maintained
-- 
2.14.2

^ permalink raw reply related

* [PATCH v3 16/19] thunderbolt: Allocate ring HopID automatically if requested
From: Mika Westerberg @ 2017-10-02 10:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman, David S . Miller
  Cc: Andreas Noever, Michael Jamet, Yehezkel Bernat, Amir Levy,
	Mario.Limonciello, Lukas Wunner, Andy Shevchenko, Andrew Lunn,
	Mika Westerberg, netdev, linux-kernel
In-Reply-To: <20171002103846.64602-1-mika.westerberg@linux.intel.com>

Thunderbolt services should not care which HopID (ring) they use for
sending and receiving packets over the high-speed DMA path, so make
tb_ring_alloc_rx() and tb_ring_alloc_tx() accept negative HopID. This
means that the NHI will allocate next available HopID for the caller
automatically.

These HopIDs will be allocated from the range which is not reserved for
the Thunderbolt protocol (8 .. hop_count - 1).

The allocated HopID can be retrieved from ring->hop field after the ring
has been allocated successfully if needed.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Michael Jamet <michael.jamet@intel.com>
Reviewed-by: Yehezkel Bernat <yehezkel.bernat@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/thunderbolt/nhi.c | 78 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 60 insertions(+), 18 deletions(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index af0a80ddf594..0e79eebfcbb7 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -26,6 +26,8 @@
  * use this ring for anything else.
  */
 #define RING_E2E_UNUSED_HOPID	2
+/* HopIDs 0-7 are reserved by the Thunderbolt protocol */
+#define RING_FIRST_USABLE_HOPID	8
 
 /*
  * Minimal number of vectors when we use MSI-X. Two for control channel
@@ -411,6 +413,62 @@ static void ring_release_msix(struct tb_ring *ring)
 	ring->irq = 0;
 }
 
+static int nhi_alloc_hop(struct tb_nhi *nhi, struct tb_ring *ring)
+{
+	int ret = 0;
+
+	spin_lock_irq(&nhi->lock);
+
+	if (ring->hop < 0) {
+		unsigned int i;
+
+		/*
+		 * Automatically allocate HopID from the non-reserved
+		 * range 8 .. hop_count - 1.
+		 */
+		for (i = RING_FIRST_USABLE_HOPID; i < nhi->hop_count; i++) {
+			if (ring->is_tx) {
+				if (!nhi->tx_rings[i]) {
+					ring->hop = i;
+					break;
+				}
+			} else {
+				if (!nhi->rx_rings[i]) {
+					ring->hop = i;
+					break;
+				}
+			}
+		}
+	}
+
+	if (ring->hop < 0 || ring->hop >= nhi->hop_count) {
+		dev_warn(&nhi->pdev->dev, "invalid hop: %d\n", ring->hop);
+		ret = -EINVAL;
+		goto err_unlock;
+	}
+	if (ring->is_tx && nhi->tx_rings[ring->hop]) {
+		dev_warn(&nhi->pdev->dev, "TX hop %d already allocated\n",
+			 ring->hop);
+		ret = -EBUSY;
+		goto err_unlock;
+	} else if (!ring->is_tx && nhi->rx_rings[ring->hop]) {
+		dev_warn(&nhi->pdev->dev, "RX hop %d already allocated\n",
+			 ring->hop);
+		ret = -EBUSY;
+		goto err_unlock;
+	}
+
+	if (ring->is_tx)
+		nhi->tx_rings[ring->hop] = ring;
+	else
+		nhi->rx_rings[ring->hop] = ring;
+
+err_unlock:
+	spin_unlock_irq(&nhi->lock);
+
+	return ret;
+}
+
 static struct tb_ring *tb_ring_alloc(struct tb_nhi *nhi, u32 hop, int size,
 				     bool transmit, unsigned int flags,
 				     u16 sof_mask, u16 eof_mask,
@@ -456,28 +514,12 @@ static struct tb_ring *tb_ring_alloc(struct tb_nhi *nhi, u32 hop, int size,
 	if (ring_request_msix(ring, flags & RING_FLAG_NO_SUSPEND))
 		goto err_free_descs;
 
-	spin_lock_irq(&nhi->lock);
-	if (hop >= nhi->hop_count) {
-		dev_WARN(&nhi->pdev->dev, "invalid hop: %d\n", hop);
+	if (nhi_alloc_hop(nhi, ring))
 		goto err_release_msix;
-	}
-	if (transmit && nhi->tx_rings[hop]) {
-		dev_WARN(&nhi->pdev->dev, "TX hop %d already allocated\n", hop);
-		goto err_release_msix;
-	} else if (!transmit && nhi->rx_rings[hop]) {
-		dev_WARN(&nhi->pdev->dev, "RX hop %d already allocated\n", hop);
-		goto err_release_msix;
-	}
-	if (transmit)
-		nhi->tx_rings[hop] = ring;
-	else
-		nhi->rx_rings[hop] = ring;
-	spin_unlock_irq(&nhi->lock);
 
 	return ring;
 
 err_release_msix:
-	spin_unlock_irq(&nhi->lock);
 	ring_release_msix(ring);
 err_free_descs:
 	dma_free_coherent(&ring->nhi->pdev->dev,
@@ -506,7 +548,7 @@ EXPORT_SYMBOL_GPL(tb_ring_alloc_tx);
 /**
  * tb_ring_alloc_rx() - Allocate DMA ring for receive
  * @nhi: Pointer to the NHI the ring is to be allocated
- * @hop: HopID (ring) to allocate
+ * @hop: HopID (ring) to allocate. Pass %-1 for automatic allocation.
  * @size: Number of entries in the ring
  * @flags: Flags for the ring
  * @sof_mask: Mask of PDF values that start a frame
-- 
2.14.2

^ permalink raw reply related

* v4.14-rc2/arm64 kernel BUG at net/core/skbuff.c:2626
From: Mark Rutland @ 2017-10-02 10:49 UTC (permalink / raw)
  To: linux-kernel, netdev, linux-arm-kernel, syzkaller
  Cc: David S. Miller, Willem de Bruijn, Eric Dumazet

Hi all,

I hit the below splat at net/core/skbuff.c:2626 while fuzzing v4.14-rc2
on arm64 with Syzkaller. This is the BUG_ON(len) at the end of
skb_copy_and_csum_bits().

I've uploaded a copy of the splat, my config, and (full) Syzkaller log
to my kernel.org web space [1]. I haven't had the opportunity to
reproduce this yet. 

This isn't a pure v4.14-rc2, as I have a not-yet-upstream fix [2]
applied to avoid a userfaultfd bug. However, per the Syzkaller log, the
userfaultfd syscall wasn't invoked, so I don't believe that should
matter.

Thanks,
Mark.

[1] https://www.kernel.org/pub/linux/kernel/people/mark/bugs/20171002-skbuff-bug/
[2] https://lkml.kernel.org/r/20170920180413.26713-1-aarcange@redhat.com

------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:2626!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
Modules linked in:
CPU: 3 PID: 0 Comm: swapper/3 Not tainted 4.14.0-rc2-00001-gd7ad33d #115
Hardware name: linux,dummy-virt (DT)
task: ffff80003a901a80 task.stack: ffff80003a908000
PC is at skb_copy_and_csum_bits+0x8dc/0xae0 net/core/skbuff.c:2626
LR is at skb_copy_and_csum_bits+0x8dc/0xae0 net/core/skbuff.c:2626
pc : [<ffff200009e03214>] lr : [<ffff200009e03214>] pstate: 00000145
sp : ffff80003efd7b50
x29: ffff80003efd7b50 x28: 000000000000003c 
x27: 00000000000001e8 x26: ffff80003a901a90 
x25: 000000000000003c x24: dfff200000000000 
x23: ffff800035723a80 x22: 000000000000003c 
x21: 0000000000000000 x20: 0000000000000000 
x19: 0000000000003a6d x18: ffff20000da58140 
x17: 0000000000000000 x16: 0000000000000001 
x15: ffff20000e1485a0 x14: ffff2000082f8980 
x13: ffff200009fc73d0 x12: ffff200009fc707c 
x11: 1ffff00002c2a3fc x10: ffff100002c2a3fc 
x9 : dfff200000000000 x8 : 07030301a8ff1127 
x7 : edff11270a080204 x6 : ffff800016151fe8 
x5 : ffff100002c2a3fd x4 : 000000000000000c 
x3 : 0000000000000030 x2 : 1ffff00006ae47a1 
x1 : 01f6cee936b5bc00 x0 : 0000000000000000 
Process swapper/3 (pid: 0, stack limit = 0xffff80003a908000)
Call trace:
Exception stack(0xffff80003efd7a10 to 0xffff80003efd7b50)
7a00:                                   0000000000000000 01f6cee936b5bc00
7a20: 1ffff00006ae47a1 0000000000000030 000000000000000c ffff100002c2a3fd
7a40: ffff800016151fe8 edff11270a080204 07030301a8ff1127 dfff200000000000
7a60: ffff100002c2a3fc 1ffff00002c2a3fc ffff200009fc707c ffff200009fc73d0
7a80: ffff2000082f8980 ffff20000e1485a0 0000000000000001 0000000000000000
7aa0: ffff20000da58140 0000000000003a6d 0000000000000000 0000000000000000
7ac0: 000000000000003c ffff800035723a80 dfff200000000000 000000000000003c
7ae0: ffff80003a901a90 00000000000001e8 000000000000003c ffff80003efd7b50
7b00: ffff200009e03214 ffff80003efd7b50 ffff200009e03214 0000000000000145
7b20: 0000000000003a6d 0000000000000000 0001000000000000 000000000000003c
7b40: ffff80003efd7b50 ffff200009e03214
[<ffff200009e03214>] skb_copy_and_csum_bits+0x8dc/0xae0 net/core/skbuff.c:2626
[<ffff20000a01d244>] icmp_glue_bits+0xa4/0x2a0 net/ipv4/icmp.c:357
[<ffff200009f3f0d4>] __ip_append_data+0x10e4/0x20a8 net/ipv4/ip_output.c:1018
[<ffff200009f41a88>] ip_append_data.part.3+0xe8/0x1a0 net/ipv4/ip_output.c:1170
[<ffff200009f46e74>] ip_append_data+0xa4/0xb0 net/ipv4/ip_output.c:1173
[<ffff20000a01ccc8>] icmp_push_reply+0x1b8/0x690 net/ipv4/icmp.c:375
[<ffff20000a0211b0>] icmp_send+0x1070/0x1890 net/ipv4/icmp.c:741
[<ffff200009f41d48>] ip_fragment.constprop.4+0x208/0x340 net/ipv4/ip_output.c:552
[<ffff200009f42228>] ip_finish_output+0x3a8/0xab0 net/ipv4/ip_output.c:315
[<ffff200009f468c4>] NF_HOOK_COND include/linux/netfilter.h:238 [inline]
[<ffff200009f468c4>] ip_output+0x284/0x790 net/ipv4/ip_output.c:405
[<ffff200009f43204>] dst_output include/net/dst.h:458 [inline]
[<ffff200009f43204>] ip_local_out+0x9c/0x1b8 net/ipv4/ip_output.c:124
[<ffff200009f445e8>] ip_queue_xmit+0x850/0x18e0 net/ipv4/ip_output.c:504
[<ffff200009fb091c>] tcp_transmit_skb+0x107c/0x3338 net/ipv4/tcp_output.c:1123
[<ffff200009fbbcc4>] __tcp_retransmit_skb+0x614/0x1d18 net/ipv4/tcp_output.c:2847
[<ffff200009fbd840>] tcp_send_loss_probe+0x478/0x7d0 net/ipv4/tcp_output.c:2457
[<ffff200009fc707c>] tcp_write_timer_handler+0x50c/0x7e8 net/ipv4/tcp_timer.c:557
[<ffff200009fc73d0>] tcp_write_timer+0x78/0x170 net/ipv4/tcp_timer.c:579
[<ffff2000082f8980>] call_timer_fn+0x1b8/0x430 kernel/time/timer.c:1281
[<ffff2000082f8dcc>] expire_timers+0x1d4/0x320 kernel/time/timer.c:1320
[<ffff2000082f912c>] __run_timers kernel/time/timer.c:1620 [inline]
[<ffff2000082f912c>] run_timer_softirq+0x214/0x5f0 kernel/time/timer.c:1646
[<ffff2000080826c0>] __do_softirq+0x350/0xc0c kernel/softirq.c:284
[<ffff200008170af4>] do_softirq_own_stack include/linux/interrupt.h:498 [inline]
[<ffff200008170af4>] invoke_softirq kernel/softirq.c:371 [inline]
[<ffff200008170af4>] irq_exit+0x1dc/0x2f8 kernel/softirq.c:405
[<ffff2000082a95bc>] __handle_domain_irq+0xdc/0x230 kernel/irq/irqdesc.c:647
[<ffff2000080820ac>] handle_domain_irq include/linux/irqdesc.h:175 [inline]
[<ffff2000080820ac>] gic_handle_irq+0x6c/0xe0 drivers/irqchip/irq-gic.c:367
Exception stack(0xffff80003a90bb70 to 0xffff80003a90bcb0)
bb60:                                   ffff80003a90234c 0000000000000007
bb80: 0000000000000000 1ffff00007520469 1fffe400017ad00c dfff200000000000
bba0: dfff200000000000 0000000000000000 ffff80003a902350 1ffff00007520469
bbc0: ffff80003a902348 ffff80003a902368 1ffff0000752046c 1ffff0000752046e
bbe0: 1ffff0000752046d ffff20000e1485a0 0000000000000000 0000000000000001
bc00: ffff20000da58140 ffff80003efd9800 ffff80003efd9800 ffff20000ae60000
bc20: ffff80003a971a80 1ffff000075217aa 0000000000000000 ffff20000ae60000
bc40: 0000000000000001 ffff20000a34fce0 0000dffff519f438 ffff80003a90bcb0
bc60: ffff20000a36134c ffff80003a90bcb0 ffff20000a361350 0000000010000145
bc80: ffff80003efd9800 ffff80003efd9800 ffffffffffffffff ffff80003efd9800
bca0: ffff80003a90bcb0 ffff20000a361350
[<ffff200008084034>] el1_irq+0xb4/0x12c arch/arm64/kernel/entry.S:569
[<ffff20000a361350>] arch_local_irq_enable arch/arm64/include/asm/irqflags.h:40 [inline]
[<ffff20000a361350>] __raw_spin_unlock_irq include/linux/spinlock_api_smp.h:168 [inline]
[<ffff20000a361350>] _raw_spin_unlock_irq+0x30/0x100 kernel/locking/spinlock.c:199
[<ffff2000081e0850>] finish_lock_switch kernel/sched/sched.h:1335 [inline]
[<ffff2000081e0850>] finish_task_switch+0x1d8/0x950 kernel/sched/core.c:2657
[<ffff20000a34fce0>] context_switch kernel/sched/core.c:2793 [inline]
[<ffff20000a34fce0>] __schedule+0x518/0x17b0 kernel/sched/core.c:3366
[<ffff20000a3520e8>] schedule_idle+0x58/0xc8 kernel/sched/core.c:3452
[<ffff200008254a00>] do_idle+0x1d8/0x370 kernel/sched/idle.c:269
[<ffff200008255138>] cpu_startup_entry+0x20/0x28 kernel/sched/idle.c:351
[<ffff2000080a2f4c>] secondary_start_kernel+0x2fc/0x498 arch/arm64/kernel/smp.c:280
Code: 97bcbfac 17fffe19 d503201f 97974258 (d4210000) 
---[ end trace 3359b414c3a12466 ]---

^ permalink raw reply

* Re: cross namespace interface notification for tun devices
From: Jason A. Donenfeld @ 2017-10-02 11:11 UTC (permalink / raw)
  To: nicolas.dichtel; +Cc: Netdev, Mathias
In-Reply-To: <f80d9afa-0ce2-903d-9bf9-b7bb8765086b@6wind.com>

On Mon, Oct 2, 2017 at 11:32 AM, Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
> 1. Move the process to netns B, open the netlink socket and move back the
> process to netns A. The socket will remain in netns B and you will receive all
> netlink messages related to netns B.
>
> 2. Assign a nsid to netns B in netns A and use NETLINK_LISTEN_ALL_NSID on your
> netlink socket (see iproute2).

Both of these seem to rely on the process knowing where the device is
being moved and having access to that namespace. I don't think these
two things are a given though. Unless I'm missing something?

Jason

^ permalink raw reply

* Re: [PATCH v3 02/19] thunderbolt: Remove __packed from ICM message structures
From: Andy Shevchenko @ 2017-10-02 11:45 UTC (permalink / raw)
  To: Mika Westerberg, Greg Kroah-Hartman, David S . Miller
  Cc: Andreas Noever, Michael Jamet, Yehezkel Bernat, Amir Levy,
	Mario.Limonciello, Lukas Wunner, Andrew Lunn, netdev,
	linux-kernel
In-Reply-To: <20171002103846.64602-3-mika.westerberg@linux.intel.com>

On Mon, 2017-10-02 at 13:38 +0300, Mika Westerberg wrote:
> These messages are all 32-byte aligned and they should be packed 

Obviously 32-bit.

Other than that,

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> without
> the __packed attribute just fine. It also allows compiler to generate
> better code on some architectures.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Michael Jamet <michael.jamet@intel.com>
> Reviewed-by: Yehezkel Bernat <yehezkel.bernat@intel.com>
> ---
>  drivers/thunderbolt/tb_msgs.h | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/thunderbolt/tb_msgs.h
> b/drivers/thunderbolt/tb_msgs.h
> index de6441e4a060..f3adf58a40ce 100644
> --- a/drivers/thunderbolt/tb_msgs.h
> +++ b/drivers/thunderbolt/tb_msgs.h
> @@ -130,7 +130,7 @@ struct icm_pkg_header {
>  	u8 flags;
>  	u8 packet_id;
>  	u8 total_packets;
> -} __packed;
> +};
>  
>  #define ICM_FLAGS_ERROR			BIT(0)
>  #define ICM_FLAGS_NO_KEY		BIT(1)
> @@ -139,20 +139,20 @@ struct icm_pkg_header {
>  
>  struct icm_pkg_driver_ready {
>  	struct icm_pkg_header hdr;
> -} __packed;
> +};
>  
>  struct icm_pkg_driver_ready_response {
>  	struct icm_pkg_header hdr;
>  	u8 romver;
>  	u8 ramver;
>  	u16 security_level;
> -} __packed;
> +};
>  
>  /* Falcon Ridge & Alpine Ridge common messages */
>  
>  struct icm_fr_pkg_get_topology {
>  	struct icm_pkg_header hdr;
> -} __packed;
> +};
>  
>  #define ICM_GET_TOPOLOGY_PACKETS	14
>  
> @@ -167,7 +167,7 @@ struct icm_fr_pkg_get_topology_response {
>  	u32 reserved[2];
>  	u32 ports[16];
>  	u32 port_hop_info[16];
> -} __packed;
> +};
>  
>  #define ICM_SWITCH_USED			BIT(0)
>  #define ICM_SWITCH_UPSTREAM_PORT_MASK	GENMASK(7, 1)
> @@ -184,7 +184,7 @@ struct icm_fr_event_device_connected {
>  	u8 connection_id;
>  	u16 link_info;
>  	u32 ep_name[55];
> -} __packed;
> +};
>  
>  #define ICM_LINK_INFO_LINK_MASK		0x7
>  #define ICM_LINK_INFO_DEPTH_SHIFT	4
> @@ -197,13 +197,13 @@ struct icm_fr_pkg_approve_device {
>  	u8 connection_key;
>  	u8 connection_id;
>  	u16 reserved;
> -} __packed;
> +};
>  
>  struct icm_fr_event_device_disconnected {
>  	struct icm_pkg_header hdr;
>  	u16 reserved;
>  	u16 link_info;
> -} __packed;
> +};
>  
>  struct icm_fr_pkg_add_device_key {
>  	struct icm_pkg_header hdr;
> @@ -212,7 +212,7 @@ struct icm_fr_pkg_add_device_key {
>  	u8 connection_id;
>  	u16 reserved;
>  	u32 key[8];
> -} __packed;
> +};
>  
>  struct icm_fr_pkg_add_device_key_response {
>  	struct icm_pkg_header hdr;
> @@ -220,7 +220,7 @@ struct icm_fr_pkg_add_device_key_response {
>  	u8 connection_key;
>  	u8 connection_id;
>  	u16 reserved;
> -} __packed;
> +};
>  
>  struct icm_fr_pkg_challenge_device {
>  	struct icm_pkg_header hdr;
> @@ -229,7 +229,7 @@ struct icm_fr_pkg_challenge_device {
>  	u8 connection_id;
>  	u16 reserved;
>  	u32 challenge[8];
> -} __packed;
> +};
>  
>  struct icm_fr_pkg_challenge_device_response {
>  	struct icm_pkg_header hdr;
> @@ -239,7 +239,7 @@ struct icm_fr_pkg_challenge_device_response {
>  	u16 reserved;
>  	u32 challenge[8];
>  	u32 response[8];
> -} __packed;
> +};
>  
>  /* Alpine Ridge only messages */
>  
> @@ -247,7 +247,7 @@ struct icm_ar_pkg_get_route {
>  	struct icm_pkg_header hdr;
>  	u16 reserved;
>  	u16 link_info;
> -} __packed;
> +};
>  
>  struct icm_ar_pkg_get_route_response {
>  	struct icm_pkg_header hdr;
> @@ -255,6 +255,6 @@ struct icm_ar_pkg_get_route_response {
>  	u16 link_info;
>  	u32 route_hi;
>  	u32 route_lo;
> -} __packed;
> +};
>  
>  #endif

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* [iproute PATCH v3 1/3] ip{6,}tunnel: Avoid copying user-supplied interface name around
From: Phil Sutter @ 2017-10-02 11:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20171002114637.25703-1-phil@nwl.cc>

In both files' parse_args() functions as well as in iptunnel's do_prl()
and do_6rd() functions, a user-supplied 'dev' parameter is uselessly
copied into a temporary buffer before passing it to ll_name_to_index()
or copying into a struct ifreq.  Avoid this by just caching the argv
pointer value until the later lookup/strcpy.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 ip/ip6tunnel.c |  6 +++---
 ip/iptunnel.c  | 22 +++++++++-------------
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index b4a7def144226..c12d700e74189 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -136,7 +136,7 @@ static void print_tunnel(struct ip6_tnl_parm2 *p)
 static int parse_args(int argc, char **argv, int cmd, struct ip6_tnl_parm2 *p)
 {
 	int count = 0;
-	char medium[IFNAMSIZ] = {};
+	const char *medium = NULL;
 
 	while (argc > 0) {
 		if (strcmp(*argv, "mode") == 0) {
@@ -180,7 +180,7 @@ static int parse_args(int argc, char **argv, int cmd, struct ip6_tnl_parm2 *p)
 			memcpy(&p->laddr, &laddr.data, sizeof(p->laddr));
 		} else if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
-			strncpy(medium, *argv, IFNAMSIZ - 1);
+			medium = *argv;
 		} else if (strcmp(*argv, "encaplimit") == 0) {
 			NEXT_ARG();
 			if (strcmp(*argv, "none") == 0) {
@@ -285,7 +285,7 @@ static int parse_args(int argc, char **argv, int cmd, struct ip6_tnl_parm2 *p)
 		count++;
 		argc--; argv++;
 	}
-	if (medium[0]) {
+	if (medium) {
 		p->link = ll_name_to_index(medium);
 		if (p->link == 0) {
 			fprintf(stderr, "Cannot find device \"%s\"\n", medium);
diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index 105d0f5576f1a..0acfd0793d3cd 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -60,7 +60,7 @@ static void set_tunnel_proto(struct ip_tunnel_parm *p, int proto)
 static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
 {
 	int count = 0;
-	char medium[IFNAMSIZ] = {};
+	const char *medium = NULL;
 	int isatap = 0;
 
 	memset(p, 0, sizeof(*p));
@@ -139,7 +139,7 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
 				p->iph.saddr = htonl(INADDR_ANY);
 		} else if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
-			strncpy(medium, *argv, IFNAMSIZ - 1);
+			medium = *argv;
 		} else if (strcmp(*argv, "ttl") == 0 ||
 			   strcmp(*argv, "hoplimit") == 0 ||
 			   strcmp(*argv, "hlim") == 0) {
@@ -216,7 +216,7 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
 		}
 	}
 
-	if (medium[0]) {
+	if (medium) {
 		p->link = ll_name_to_index(medium);
 		if (p->link == 0) {
 			fprintf(stderr, "Cannot find device \"%s\"\n", medium);
@@ -465,9 +465,8 @@ static int do_prl(int argc, char **argv)
 {
 	struct ip_tunnel_prl p = {};
 	int count = 0;
-	int devname = 0;
 	int cmd = 0;
-	char medium[IFNAMSIZ] = {};
+	const char *medium = NULL;
 
 	while (argc > 0) {
 		if (strcmp(*argv, "prl-default") == 0) {
@@ -488,8 +487,7 @@ static int do_prl(int argc, char **argv)
 			count++;
 		} else if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
-			strncpy(medium, *argv, IFNAMSIZ-1);
-			devname++;
+			medium = *argv;
 		} else {
 			fprintf(stderr,
 				"Invalid PRL parameter \"%s\"\n", *argv);
@@ -502,7 +500,7 @@ static int do_prl(int argc, char **argv)
 		}
 		argc--; argv++;
 	}
-	if (devname == 0) {
+	if (!medium) {
 		fprintf(stderr, "Must specify device\n");
 		exit(-1);
 	}
@@ -513,9 +511,8 @@ static int do_prl(int argc, char **argv)
 static int do_6rd(int argc, char **argv)
 {
 	struct ip_tunnel_6rd ip6rd = {};
-	int devname = 0;
 	int cmd = 0;
-	char medium[IFNAMSIZ] = {};
+	const char *medium = NULL;
 	inet_prefix prefix;
 
 	while (argc > 0) {
@@ -537,8 +534,7 @@ static int do_6rd(int argc, char **argv)
 			cmd = SIOCDEL6RD;
 		} else if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
-			strncpy(medium, *argv, IFNAMSIZ-1);
-			devname++;
+			medium = *argv;
 		} else {
 			fprintf(stderr,
 				"Invalid 6RD parameter \"%s\"\n", *argv);
@@ -546,7 +542,7 @@ static int do_6rd(int argc, char **argv)
 		}
 		argc--; argv++;
 	}
-	if (devname == 0) {
+	if (!medium) {
 		fprintf(stderr, "Must specify device\n");
 		exit(-1);
 	}
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH v3 3/3] Check user supplied interface name lengths
From: Phil Sutter @ 2017-10-02 11:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20171002114637.25703-1-phil@nwl.cc>

The original problem was that something like:

| strncpy(ifr.ifr_name, *argv, IFNAMSIZ);

might leave ifr.ifr_name unterminated if length of *argv exceeds
IFNAMSIZ. In order to fix this, I thought about replacing all those
cases with (equivalent) calls to snprintf() or even introducing
strlcpy(). But as Ulrich Drepper correctly pointed out when rejecting
the latter from being added to glibc, truncating a string without
notifying the user is not to be considered good practice. So let's
excercise what he suggested and reject empty, overlong or otherwise
invalid interface names right from the start - this way calls to
strncpy() like shown above become safe and the user has a chance to
reconsider what he was trying to do.

Note that this doesn't add calls to check_ifname() to all places where
user supplied interface name is parsed. In many cases, the interface
must exist already and is therefore looked up using ll_name_to_index(),
so if_nametoindex() will perform the necessary checks already.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v2:
- Change implementation of check_ifname() and add get_ifname() just as
  Stephen suggested with one exception: Call strncpy() with length of
  IFNAMSIZ, otherwise it might leave destination unterminated.
- Change callers accordingly.

Changes since v1:
- Added missing check to tc/f_flower.c.
- Drop some useless checks from ip/ip{6,}tunnel.c (ll_name_to_index()
  will detect illegal interface names for us).
- Renamed assert_valid_dev_name() to the shorter check_ifname().
- iplink: Check 'name' and 'dev' parameters right where they are parsed.
- ipl2tp: Drop needless check for p->ifname[0].
---
 include/utils.h |  2 ++
 ip/ip6tunnel.c  |  3 ++-
 ip/ipl2tp.c     |  4 +++-
 ip/iplink.c     | 31 ++++++++++++-------------------
 ip/ipmaddr.c    |  3 ++-
 ip/iprule.c     | 10 ++++++++--
 ip/iptunnel.c   |  7 ++++++-
 ip/iptuntap.c   |  6 ++++--
 lib/utils.c     | 29 +++++++++++++++++++++++++++++
 misc/arpd.c     |  3 ++-
 tc/f_flower.c   |  2 ++
 11 files changed, 72 insertions(+), 28 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index c9ed230b96044..76addb3258f59 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -133,6 +133,8 @@ void missarg(const char *) __attribute__((noreturn));
 void invarg(const char *, const char *) __attribute__((noreturn));
 void duparg(const char *, const char *) __attribute__((noreturn));
 void duparg2(const char *, const char *) __attribute__((noreturn));
+int check_ifname(const char *);
+int get_ifname(char *, const char *);
 int matches(const char *arg, const char *pattern);
 int inet_addr_match(const inet_prefix *a, const inet_prefix *b, int bits);
 
diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index c12d700e74189..bc44bef7f030c 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -273,7 +273,8 @@ static int parse_args(int argc, char **argv, int cmd, struct ip6_tnl_parm2 *p)
 				usage();
 			if (p->name[0])
 				duparg2("name", *argv);
-			strncpy(p->name, *argv, IFNAMSIZ - 1);
+			if (get_ifname(p->name, *argv))
+				invarg("\"name\" not a valid ifname", *argv);
 			if (cmd == SIOCCHGTUNNEL && count == 0) {
 				struct ip6_tnl_parm2 old_p = {};
 
diff --git a/ip/ipl2tp.c b/ip/ipl2tp.c
index 88664c909e11f..1e37b175e3315 100644
--- a/ip/ipl2tp.c
+++ b/ip/ipl2tp.c
@@ -182,7 +182,7 @@ static int create_session(struct l2tp_parm *p)
 	if (p->peer_cookie_len)
 		addattr_l(&req.n, 1024, L2TP_ATTR_PEER_COOKIE,
 			  p->peer_cookie,  p->peer_cookie_len);
-	if (p->ifname && p->ifname[0])
+	if (p->ifname)
 		addattrstrz(&req.n, 1024, L2TP_ATTR_IFNAME, p->ifname);
 
 	if (rtnl_talk(&genl_rth, &req.n, NULL, 0) < 0)
@@ -545,6 +545,8 @@ static int parse_args(int argc, char **argv, int cmd, struct l2tp_parm *p)
 			}
 		} else if (strcmp(*argv, "name") == 0) {
 			NEXT_ARG();
+			if (check_ifname(*argv))
+				invarg("\"name\" not a valid ifname", *argv);
 			p->ifname = *argv;
 		} else if (strcmp(*argv, "remote") == 0) {
 			NEXT_ARG();
diff --git a/ip/iplink.c b/ip/iplink.c
index ff5b56c038d28..6a96ea9ff56a7 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -573,6 +573,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			req->i.ifi_flags &= ~IFF_UP;
 		} else if (strcmp(*argv, "name") == 0) {
 			NEXT_ARG();
+			if (check_ifname(*argv))
+				invarg("\"name\" not a valid ifname", *argv);
 			*name = *argv;
 		} else if (strcmp(*argv, "index") == 0) {
 			NEXT_ARG();
@@ -848,6 +850,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 				NEXT_ARG();
 			if (*dev)
 				duparg2("dev", *argv);
+			if (check_ifname(*argv))
+				invarg("\"dev\" not a valid ifname", *argv);
 			*dev = *argv;
 			dev_index = ll_name_to_index(*dev);
 		}
@@ -870,7 +874,6 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 
 static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 {
-	int len;
 	char *dev = NULL;
 	char *name = NULL;
 	char *link = NULL;
@@ -960,13 +963,8 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 	}
 
 	if (name) {
-		len = strlen(name) + 1;
-		if (len == 1)
-			invarg("\"\" is not a valid device identifier\n",
-			       "name");
-		if (len > IFNAMSIZ)
-			invarg("\"name\" too long\n", name);
-		addattr_l(&req.n, sizeof(req), IFLA_IFNAME, name, len);
+		addattr_l(&req.n, sizeof(req),
+			  IFLA_IFNAME, name, strlen(name) + 1);
 	}
 
 	if (type) {
@@ -1016,7 +1014,6 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 
 int iplink_get(unsigned int flags, char *name, __u32 filt_mask)
 {
-	int len;
 	struct iplink_req req = {
 		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
 		.n.nlmsg_flags = NLM_F_REQUEST | flags,
@@ -1029,13 +1026,8 @@ int iplink_get(unsigned int flags, char *name, __u32 filt_mask)
 	} answer;
 
 	if (name) {
-		len = strlen(name) + 1;
-		if (len == 1)
-			invarg("\"\" is not a valid device identifier\n",
-				   "name");
-		if (len > IFNAMSIZ)
-			invarg("\"name\" too long\n", name);
-		addattr_l(&req.n, sizeof(req), IFLA_IFNAME, name, len);
+		addattr_l(&req.n, sizeof(req),
+			  IFLA_IFNAME, name, strlen(name) + 1);
 	}
 	addattr32(&req.n, sizeof(req), IFLA_EXT_MASK, filt_mask);
 
@@ -1265,6 +1257,8 @@ static int do_set(int argc, char **argv)
 			flags &= ~IFF_UP;
 		} else if (strcmp(*argv, "name") == 0) {
 			NEXT_ARG();
+			if (check_ifname(*argv))
+				invarg("\"name\" not a valid ifname", *argv);
 			newname = *argv;
 		} else if (matches(*argv, "address") == 0) {
 			NEXT_ARG();
@@ -1355,6 +1349,8 @@ static int do_set(int argc, char **argv)
 
 			if (dev)
 				duparg2("dev", *argv);
+			if (check_ifname(*argv))
+				invarg("\"dev\" not a valid ifname", *argv);
 			dev = *argv;
 		}
 		argc--; argv++;
@@ -1383,9 +1379,6 @@ static int do_set(int argc, char **argv)
 	}
 
 	if (newname && strcmp(dev, newname)) {
-		if (strlen(newname) == 0)
-			invarg("\"\" is not a valid device identifier\n",
-			       "name");
 		if (do_changename(dev, newname) < 0)
 			return -1;
 		dev = newname;
diff --git a/ip/ipmaddr.c b/ip/ipmaddr.c
index 85a69e779563d..5683f6fa830c1 100644
--- a/ip/ipmaddr.c
+++ b/ip/ipmaddr.c
@@ -284,7 +284,8 @@ static int multiaddr_modify(int cmd, int argc, char **argv)
 			NEXT_ARG();
 			if (ifr.ifr_name[0])
 				duparg("dev", *argv);
-			strncpy(ifr.ifr_name, *argv, IFNAMSIZ);
+			if (get_ifname(ifr.ifr_name, *argv))
+				invarg("\"dev\" not a valid ifname", *argv);
 		} else {
 			if (matches(*argv, "address") == 0) {
 				NEXT_ARG();
diff --git a/ip/iprule.c b/ip/iprule.c
index 8313138db815f..36c57fa70b74a 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -472,11 +472,13 @@ static int iprule_list_flush_or_save(int argc, char **argv, int action)
 		} else if (strcmp(*argv, "dev") == 0 ||
 			   strcmp(*argv, "iif") == 0) {
 			NEXT_ARG();
-			strncpy(filter.iif, *argv, IFNAMSIZ);
+			if (get_ifname(filter.iif, *argv))
+				invarg("\"iif\"/\"dev\" not a valid ifname", *argv);
 			filter.iifmask = 1;
 		} else if (strcmp(*argv, "oif") == 0) {
 			NEXT_ARG();
-			strncpy(filter.oif, *argv, IFNAMSIZ);
+			if (get_ifname(filter.oif, *argv))
+				invarg("\"oif\" not a valid ifname", *argv);
 			filter.oifmask = 1;
 		} else if (strcmp(*argv, "l3mdev") == 0) {
 			filter.l3mdev = 1;
@@ -695,10 +697,14 @@ static int iprule_modify(int cmd, int argc, char **argv)
 		} else if (strcmp(*argv, "dev") == 0 ||
 			   strcmp(*argv, "iif") == 0) {
 			NEXT_ARG();
+			if (check_ifname(*argv))
+				invarg("\"iif\"/\"dev\" not a valid ifname", *argv);
 			addattr_l(&req.n, sizeof(req), FRA_IFNAME,
 				  *argv, strlen(*argv)+1);
 		} else if (strcmp(*argv, "oif") == 0) {
 			NEXT_ARG();
+			if (check_ifname(*argv))
+				invarg("\"oif\" not a valid ifname", *argv);
 			addattr_l(&req.n, sizeof(req), FRA_OIFNAME,
 				  *argv, strlen(*argv)+1);
 		} else if (strcmp(*argv, "l3mdev") == 0) {
diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index 0acfd0793d3cd..208a1f06ab12f 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -178,7 +178,8 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
 
 			if (p->name[0])
 				duparg2("name", *argv);
-			strncpy(p->name, *argv, IFNAMSIZ - 1);
+			if (get_ifname(p->name, *argv))
+				invarg("\"name\" not a valid ifname", *argv);
 			if (cmd == SIOCCHGTUNNEL && count == 0) {
 				struct ip_tunnel_parm old_p = {};
 
@@ -487,6 +488,8 @@ static int do_prl(int argc, char **argv)
 			count++;
 		} else if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
+			if (check_ifname(*argv))
+				invarg("\"dev\" not a valid ifname", *argv);
 			medium = *argv;
 		} else {
 			fprintf(stderr,
@@ -534,6 +537,8 @@ static int do_6rd(int argc, char **argv)
 			cmd = SIOCDEL6RD;
 		} else if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
+			if (check_ifname(*argv))
+				invarg("\"dev\" not a valid ifname", *argv);
 			medium = *argv;
 		} else {
 			fprintf(stderr,
diff --git a/ip/iptuntap.c b/ip/iptuntap.c
index 451f7f0eac6bb..b46e452f21278 100644
--- a/ip/iptuntap.c
+++ b/ip/iptuntap.c
@@ -176,7 +176,8 @@ static int parse_args(int argc, char **argv,
 			ifr->ifr_flags |= IFF_MULTI_QUEUE;
 		} else if (matches(*argv, "dev") == 0) {
 			NEXT_ARG();
-			strncpy(ifr->ifr_name, *argv, IFNAMSIZ-1);
+			if (get_ifname(ifr->ifr_name, *argv))
+				invarg("\"dev\" not a valid ifname", *argv);
 		} else {
 			if (matches(*argv, "name") == 0) {
 				NEXT_ARG();
@@ -184,7 +185,8 @@ static int parse_args(int argc, char **argv,
 				usage();
 			if (ifr->ifr_name[0])
 				duparg2("name", *argv);
-			strncpy(ifr->ifr_name, *argv, IFNAMSIZ);
+			if (get_ifname(ifr->ifr_name, *argv))
+				invarg("\"name\" not a valid ifname", *argv);
 		}
 		count++;
 		argc--; argv++;
diff --git a/lib/utils.c b/lib/utils.c
index bbd3cbc46a0e5..0cf99619c3021 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -20,6 +20,7 @@
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <string.h>
+#include <ctype.h>
 #include <netdb.h>
 #include <arpa/inet.h>
 #include <asm/types.h>
@@ -699,6 +700,34 @@ void duparg2(const char *key, const char *arg)
 	exit(-1);
 }
 
+int check_ifname(const char *name)
+{
+	/* These checks mimic kernel checks in dev_valid_name */
+	if (*name == '\0')
+		return -1;
+	if (strlen(name) >= IFNAMSIZ)
+		return -1;
+
+	while (*name) {
+		if (*name == '/' || isspace(*name))
+			return -1;
+		++name;
+	}
+	return 0;
+}
+
+/* buf is assumed to be IFNAMSIZ */
+int get_ifname(char *buf, const char *name)
+{
+	int ret;
+
+	ret = check_ifname(name);
+	if (ret == 0)
+		strncpy(buf, name, IFNAMSIZ);
+
+	return ret;
+}
+
 int matches(const char *cmd, const char *pattern)
 {
 	int len = strlen(cmd);
diff --git a/misc/arpd.c b/misc/arpd.c
index bfab44544ee1d..c2666f76fd5e9 100644
--- a/misc/arpd.c
+++ b/misc/arpd.c
@@ -664,7 +664,8 @@ int main(int argc, char **argv)
 		struct ifreq ifr = {};
 
 		for (i = 0; i < ifnum; i++) {
-			strncpy(ifr.ifr_name, ifnames[i], IFNAMSIZ);
+			if (get_ifname(ifr.ifr_name, ifnames[i]))
+				invarg("not a valid ifname", ifnames[i]);
 			if (ioctl(udp_sock, SIOCGIFINDEX, &ifr)) {
 				perror("ioctl(SIOCGIFINDEX)");
 				exit(-1);
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 99e62a382dec6..b180210717394 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -630,6 +630,8 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 			flags |= TCA_CLS_FLAGS_SKIP_SW;
 		} else if (matches(*argv, "indev") == 0) {
 			NEXT_ARG();
+			if (check_ifname(*argv))
+				invarg("\"indev\" not a valid ifname", *argv);
 			addattrstrz(n, MAX_MSG, TCA_FLOWER_INDEV, *argv);
 		} else if (matches(*argv, "vlan_id") == 0) {
 			__u16 vid;
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH v3 2/3] tc: flower: No need to cache indev arg
From: Phil Sutter @ 2017-10-02 11:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20171002114637.25703-1-phil@nwl.cc>

Since addattrstrz() will copy the provided string into the attribute
payload, there is no need to cache the data.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tc/f_flower.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tc/f_flower.c b/tc/f_flower.c
index 934832e2bbe90..99e62a382dec6 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -629,11 +629,8 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 		} else if (matches(*argv, "skip_sw") == 0) {
 			flags |= TCA_CLS_FLAGS_SKIP_SW;
 		} else if (matches(*argv, "indev") == 0) {
-			char ifname[IFNAMSIZ] = {};
-
 			NEXT_ARG();
-			strncpy(ifname, *argv, sizeof(ifname) - 1);
-			addattrstrz(n, MAX_MSG, TCA_FLOWER_INDEV, ifname);
+			addattrstrz(n, MAX_MSG, TCA_FLOWER_INDEV, *argv);
 		} else if (matches(*argv, "vlan_id") == 0) {
 			__u16 vid;
 
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH v3 0/3] Check user supplied interface name lengths
From: Phil Sutter @ 2017-10-02 11:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

This series adds explicit checks for user-supplied interface names to
make sure they fit Linux's requirements.

The first two patches simplify interface name parsing in some places -
these are side-effects of working on the actual implementation provided
in patch three.

Changes since v2:
- Changed patch 3 as suggested in review.

Changes since v1:
- Patches 1 and 2 introduced.
- Changes to patch 3 are listed in there.

Phil Sutter (3):
  ip{6,}tunnel: Avoid copying user-supplied interface name around
  tc: flower: No need to cache indev arg
  Check user supplied interface name lengths

 include/utils.h |  2 ++
 ip/ip6tunnel.c  |  9 +++++----
 ip/ipl2tp.c     |  4 +++-
 ip/iplink.c     | 31 ++++++++++++-------------------
 ip/ipmaddr.c    |  3 ++-
 ip/iprule.c     | 10 ++++++++--
 ip/iptunnel.c   | 29 +++++++++++++++--------------
 ip/iptuntap.c   |  6 ++++--
 lib/utils.c     | 29 +++++++++++++++++++++++++++++
 misc/arpd.c     |  3 ++-
 tc/f_flower.c   |  7 +++----
 11 files changed, 85 insertions(+), 48 deletions(-)

-- 
2.13.1

^ permalink raw reply

* v4.14-rc2/arm64 misaligned atomic in ip_expire() / skb_clone()
From: Mark Rutland @ 2017-10-02 11:57 UTC (permalink / raw)
  To: linux-kernel, netdev, linux-arm-kernel, syzkaller
  Cc: David S. Miller, Willem de Bruijn, Eric Dumazet

Hi all,

I'm intermittently hitting splats like below in skb_clone() while
fuzzing v4.14-rc2 on arm64 with Syzkaller. It looks like the
atomic_inc() at the end of __skb_clone() is being passed a misaligned
pointer.

I've uploaded a number of splats and their associated (full) Syzkaller
logs, along with my kernel config to my kernel.org webspace [1]. It
might take a while for that to appear.

This isn't a pure v4.14-rc2, as I have a not-yet-upstream fix [2]
applied to avoid a userfaultfd bug. The userfaultfd syscall appears in
all of the Syzkaller logs, so there is the chance that this is related,
but as I've not seen any other issues I suspect that's unlikely.

Thanks,
Mark.

[1] https://www.kernel.org/pub/linux/kernel/people/mark/bugs/20171002-skb_clone-misaligned-atomic
[2] https://lkml.kernel.org/r/20170920180413.26713-1-aarcange@redhat.com

Unable to handle kernel paging request at virtual address ffff80002fd714a2
Mem abort info:
  Exception class = DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
Data abort info:
  ISV = 0, ISS = 0x00000033
  CM = 0, WnR = 0
swapper pgtable: 4k pages, 48-bit VAs, pgd = ffff20000eeb2000
[ffff80002fd714a2] *pgd=000000007eff7003, *pud=000000007eff6003, *pmd=00f800006fc00711
Internal error: Oops: 96000021 [#1] PREEMPT SMP
Modules linked in:
CPU: 3 PID: 0 Comm: swapper/3 Not tainted 4.14.0-rc2-00001-gd7ad33d #115
Hardware name: linux,dummy-virt (DT)
task: ffff80003a901a80 task.stack: ffff80003a908000
PC is at __ll_sc_atomic_add+0x4/0x18 arch/arm64/include/asm/atomic_ll_sc.h:113
LR is at atomic_add arch/arm64/include/asm/atomic_lse.h:45 [inline]
LR is at __skb_clone+0x4a8/0x6c0 net/core/skbuff.c:873
pc : [<ffff20000a30ce44>] lr : [<ffff200009dffb58>] pstate: 10000145
sp : ffff80003efd86e0
x29: ffff80003efd86e0 x28: 000060003418b000 
x27: ffff20000ae55360 x26: ffff8000182c1608 
x25: ffff80002fd7137e x24: ffff8000182c1610 
x23: ffff20000ae60000 x22: ffff80001577871c 
x21: 1ffff00007dfb0e8 x20: ffff8000182c1540 
x19: ffff800015778640 x18: ffff20000da58140 
x17: 0000000000000000 x16: 0000000000000002 
x15: ffff20000e1485a0 x14: ffff2000082f912c 
x13: ffff2000082f8dcc x12: ffff2000082f8980 
x11: 1ffff00002aef0df x10: ffff100002aef0df 
x9 : dfff200000000000 x8 : 0082009000a40008 
x7 : 0000000000000000 x6 : ffff800015778700 
x5 : ffff100002aef0e0 x4 : 0000000000000000 
x3 : 1ffff00002aef0e3 x2 : ffff80002fd7147e 
x1 : ffff80002fd714a2 x0 : 0000000000000001 
Process swapper/3 (pid: 0, stack limit = 0xffff80003a908000)
Call trace:
Exception stack(0xffff80003efd85a0 to 0xffff80003efd86e0)
85a0: 0000000000000001 ffff80002fd714a2 ffff80002fd7147e 1ffff00002aef0e3
85c0: 0000000000000000 ffff100002aef0e0 ffff800015778700 0000000000000000
85e0: 0082009000a40008 dfff200000000000 ffff100002aef0df 1ffff00002aef0df
8600: ffff2000082f8980 ffff2000082f8dcc ffff2000082f912c ffff20000e1485a0
8620: 0000000000000002 0000000000000000 ffff20000da58140 ffff800015778640
8640: ffff8000182c1540 1ffff00007dfb0e8 ffff80001577871c ffff20000ae60000
8660: ffff8000182c1610 ffff80002fd7137e ffff8000182c1608 ffff20000ae55360
8680: 000060003418b000 ffff80003efd86e0 ffff200009dffb58 ffff80003efd86e0
86a0: ffff20000a30ce44 0000000010000145 ffff800015778640 ffff8000182c1540
86c0: 0001000000000000 ffff8000182c15ce ffff80003efd86e0 ffff20000a30ce44
[<ffff20000a30ce44>] __ll_sc_atomic_add+0x4/0x18 arch/arm64/include/asm/atomic_ll_sc.h:113
[<ffff200009e1009c>] skb_clone+0x1c4/0x3b0 net/core/skbuff.c:1286
[<ffff200009f2ff80>] ip_expire+0x4e8/0x7c0 net/ipv4/ip_fragment.c:239
[<ffff2000082f8980>] call_timer_fn+0x1b8/0x430 kernel/time/timer.c:1281
[<ffff2000082f8dcc>] expire_timers+0x1d4/0x320 kernel/time/timer.c:1320
[<ffff2000082f912c>] __run_timers kernel/time/timer.c:1620 [inline]
[<ffff2000082f912c>] run_timer_softirq+0x214/0x5f0 kernel/time/timer.c:1646
[<ffff2000080826c0>] __do_softirq+0x350/0xc0c kernel/softirq.c:284
[<ffff200008170af4>] do_softirq_own_stack include/linux/interrupt.h:498 [inline]
[<ffff200008170af4>] invoke_softirq kernel/softirq.c:371 [inline]
[<ffff200008170af4>] irq_exit+0x1dc/0x2f8 kernel/softirq.c:405
[<ffff2000082a95bc>] __handle_domain_irq+0xdc/0x230 kernel/irq/irqdesc.c:647
[<ffff2000080820ac>] handle_domain_irq include/linux/irqdesc.h:175 [inline]
[<ffff2000080820ac>] gic_handle_irq+0x6c/0xe0 drivers/irqchip/irq-gic.c:367
Exception stack(0xffff80003a90bd70 to 0xffff80003a90beb0)
bd60:                                   ffff80003a90234c 0000000000000007
bd80: 0000000000000000 1ffff00007520469 1fffe400017ad00c ffffffffffffe540
bda0: 0000000000000000 0000000000000000 ffff80003a902350 1ffff00007520469
bdc0: ffff80003a902348 ffff80003a902368 1ffff0000752046c 1ffff0000752046e
bde0: 1ffff0000752046d ffff20000e1485a0 0000000000000000 0000000000029d44
be00: ffff20000da58140 ffff80003a901a80 ffff80003a901a80 dfff200000000000
be20: ffff20000ae60e98 ffff0400015cc1d3 0000000000000000 ffff20000ae60df8
be40: ffff20000ae60df8 0000000000000000 0000000000000000 ffff80003a90beb0
be60: ffff200008089b50 ffff80003a90beb0 ffff200008089b54 0000000010000145
be80: ffff80003a901a80 ffff80003a901a80 ffffffffffffffff 01f6cee936b5bc00
bea0: ffff80003a90beb0 ffff200008089b54
[<ffff200008084034>] el1_irq+0xb4/0x12c arch/arm64/kernel/entry.S:569
[<ffff200008089b54>] arch_local_irq_enable arch/arm64/include/asm/irqflags.h:40 [inline]
[<ffff200008089b54>] arch_cpu_idle+0x1c/0x28 arch/arm64/kernel/process.c:87
[<ffff20000a360a94>] default_idle_call+0x34/0x78 kernel/sched/idle.c:98
[<ffff200008254a34>] cpuidle_idle_call kernel/sched/idle.c:156 [inline]
[<ffff200008254a34>] do_idle+0x20c/0x370 kernel/sched/idle.c:246
[<ffff20000825513c>] cpu_startup_entry+0x24/0x28 kernel/sched/idle.c:351
[<ffff2000080a2f4c>] secondary_start_kernel+0x2fc/0x498 arch/arm64/kernel/smp.c:280
Code: 978b7cfd 17ffff91 00000000 f9800031 (885f7c31) 
---[ end trace e4e9a51ab15d3a5f ]---

^ permalink raw reply

* Re: [PATCH] mac80211: aead api to reduce redundancy
From: Johannes Berg @ 2017-10-02 12:04 UTC (permalink / raw)
  To: Xiang Gao, davem, linux-kernel, linux-wireless, netdev
In-Reply-To: <20170926131945.3962-1-qasdfgtyuiop@gmail.com>

Please use "v2" tag or so in the subject line, having the same patch
again is really not helpful.

The next should be v3, obviously.

> +++ b/net/mac80211/aead_api.c
> @@ -1,7 +1,4 @@
> -/*
> - * Copyright 2014-2015, Qualcomm Atheros, Inc.
> - *
> - * This program is free software; you can redistribute it and/or
> modify
> +/* This program is free software; you can redistribute it and/or
> modify

I see no reason to make this change, why remove copyright?

> +++ b/net/mac80211/wpa.c
> @@ -464,7 +464,8 @@ static int ccmp_encrypt_skb(struct
> ieee80211_tx_data *tx, struct sk_buff *skb,
>  	pos += IEEE80211_CCMP_HDR_LEN;
>  	ccmp_special_blocks(skb, pn, b_0, aad);
>  	return ieee80211_aes_ccm_encrypt(key->u.ccmp.tfm, b_0, aad,
> pos, len,
> -					 skb_put(skb, mic_len),
> mic_len);
> +					 skb_put(skb,
> +						 key->u.ccmp.tfm-
> >authsize));
>  }

I see no reason for the change from mic_len to authsize here?

> @@ -540,10 +541,11 @@ ieee80211_crypto_ccmp_decrypt(struct
> ieee80211_rx_data *rx,
>  			ccmp_special_blocks(skb, pn, b_0, aad);
>  
>  			if (ieee80211_aes_ccm_decrypt(
> -				    key->u.ccmp.tfm, b_0, aad,
> -				    skb->data + hdrlen + IEEE80211_CCMP_HDR_LEN,
> -				    data_len,
> -				    skb->data + skb->len - mic_len, mic_len))
> +				key->u.ccmp.tfm, b_0, aad,
> +				skb->data + hdrlen + IEEE80211_CCMP_HDR_LEN,
> +				data_len,
> +				skb->data + skb->len - key->u.ccmp.tfm->authsize
> +			))
>  				return RX_DROP_UNUSABLE;

That's a really really strange way of writing this ...

Please reformat.

johannes

^ permalink raw reply

* Re: cross namespace interface notification for tun devices
From: Nicolas Dichtel @ 2017-10-02 12:06 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Netdev, Mathias
In-Reply-To: <CAHmME9pAvv7ebKC-uZGPJRi9Jasgrd2tgCvS1Lji+cgM1mV2qw@mail.gmail.com>

Le 02/10/2017 à 13:11, Jason A. Donenfeld a écrit :
> On Mon, Oct 2, 2017 at 11:32 AM, Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
>> 1. Move the process to netns B, open the netlink socket and move back the
>> process to netns A. The socket will remain in netns B and you will receive all
>> netlink messages related to netns B.
>>
>> 2. Assign a nsid to netns B in netns A and use NETLINK_LISTEN_ALL_NSID on your
>> netlink socket (see iproute2).
> 
> Both of these seem to rely on the process knowing where the device is
> being moved and having access to that namespace. I don't think these
> two things are a given though. Unless I'm missing something?
I didn't understand correctly.
Your control process cannot monitor or control an interface which is in a
unkown/hidden netns. But x-netns interfaces are special. We already add a way to
identify peer netns for this kind of interfaces.
If an handler get_link_net was added to the rtnl_link_ops of the tun driver, it
will help to identify netns A when you are in netns B. But you need the opposite.
I already try a patch to advertise via netlink the dst netns when an interface
moves to a new netns. I think that it is valid for x-netns interfaces.
As soon as you can identify the dst netns, your problem is solved, right?


Nicolas

^ permalink raw reply

* Re: [net-next V2 PATCH 5/5] samples/bpf: add cpumap sample program xdp_redirect_cpu
From: Jesper Dangaard Brouer @ 2017-10-02 12:07 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: netdev, jakub.kicinski, Michael S. Tsirkin, Jason Wang, mchan,
	John Fastabend, peter.waskiewicz.jr, Daniel Borkmann,
	Andy Gospodarek, brouer
In-Reply-To: <20170930030607.sk2wzjxxlbhkkt7k@ast-mbp>

On Fri, 29 Sep 2017 20:06:09 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> > +/*** Trace point code ***/
> > +
> > +/* Tracepoint format: /sys/kernel/debug/tracing/events/xdp/xdp_redirect/format
> > + * Code in:                kernel/include/trace/events/xdp.h
> > + */
> > +struct xdp_redirect_ctx {
> > +	unsigned short common_type;	//	offset:0;  size:2; signed:0;
> > +	unsigned char common_flags;	//	offset:2;  size:1; signed:0;
> > +	unsigned char common_preempt_count;//	offset:3;  size:1; signed:0;
> > +	int common_pid;			//	offset:4;  size:4; signed:1;  
> 
> this part is not right. First 8 bytes are not accessible by bpf code.
> Please use __u64 pad; or similar here.

I've corrected this in V3.

Can you explain why BPF cannot access these (first 8 bytes) struct members?


> Just noticed that samples/bpf/xdp_monitor_kern.c has the same problem.
> 
> > +
> > +	int prog_id;			//	offset:8;  size:4; signed:1;
> > +	u32 act;			//	offset:12  size:4; signed:0;
> > +	int ifindex;			//	offset:16  size:4; signed:1;
> > +	int err;			//	offset:20  size:4; signed:1;
> > +	int to_ifindex;			//	offset:24  size:4; signed:1;
> > +	u32 map_id;			//	offset:28  size:4; signed:0;
> > +	int map_index;			//	offset:32  size:4; signed:1;
> > +};					//	offset:36  
> 
> the second part of fields is correct.


-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [RFC net-next 0/5] net: dsa: LAG support
From: Andrew Lunn @ 2017-10-02 12:51 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, vivien.didelot, jiri, idosch, Woojung.Huh, john,
	sean.wang
In-Reply-To: <20171001194639.8647-1-f.fainelli@gmail.com>

> - not sure what to do with a switch fabric, naively, if adding two ports
>   of two distinct switches as a LAG group, we may have to propagate that
>   to "dsa" cross-chip interfaces as well

Hi Florian

Marvell switches do support this. If i remember correctly, it requires
some setup for forwarding over the DSA ports.

But for a first implementation, i would be tempted to disallow such
setups. Force the LAG members to be on the same switch.

	Andrew

^ permalink raw reply

* Re: [jkirsher/next-queue PATCH] ixgbe: Update adaptive ITR algorithm
From: Jesper Dangaard Brouer @ 2017-10-02 12:56 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: netdev, intel-wired-lan, john.fastabend, brouer
In-Reply-To: <20170925215225.15616.63705.stgit@localhost.localdomain>

On Mon, 25 Sep 2017 14:55:36 -0700
Alexander Duyck <alexander.duyck@gmail.com> wrote:

> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> The following change is meant to update the adaptive ITR algorithm to
> better support the needs of the network. Specifically with this change what
> I have done is make it so that our ITR algorithm will try to prevent either
> starving a socket buffer for memory in the case of Tx, or overruing an Rx
> socket buffer on receive.
> 
> In addition a side effect of the calculations used is that we should
> function better with new features such as XDP which can handle small
> packets at high rates without needing to lock us into NAPI polling mode.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> 
> So I am putting this out to a wider distribution list than normal for a
> patch like this in order to get feedback on if there are any areas I may
> have overlooked. With this patch is should address many of the performance
> limitations seen with pktgen and XDP in terms of workloads that the old
> adaptive scheme wasn't handling.

Thanks a lot Alex!

I've tested the patch with XDP redirect (map), and the issue I reported
in [1] is solved with this patch.

[1] Subject: "XDP redirect measurements, gotchas and tracepoints"
 http://lkml.kernel.org/r/20170821212506.1cb0d5d6@redhat.com

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [RFC net-next 0/5] net: dsa: LAG support
From: Andrew Lunn @ 2017-10-02 12:59 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: Florian Fainelli, netdev, vivien.didelot, jiri, idosch,
	Woojung.Huh, john, sean.wang
In-Reply-To: <20171002065023.GA11832@shredder.mtl.com>

> > - not sure what to do with a switch fabric, naively, if adding two ports
> >   of two distinct switches as a LAG group, we may have to propagate that
> >   to "dsa" cross-chip interfaces as well
> 
> At least in mlxsw case, enslaving switch and non-switch ports to the
> same LAG doesn't make sense. Any traffic routed by the switch will only
> be load-balanced between the switch ports. One way to solve that is to
> forbid such enslavements during NETDEV_PRECHANGEUPPER in case the lower
> devices in the adjacency list of the LAG don't belong to the same
> switch.
> 
> Note that such configurations are bound to fail anyway, as the
> non-switch ports will not have `switchdev_ops` configured and thus fail
> during __switchdev_port_obj_add() / __switchdev_port_attr_set().

Hi Ido

Here Florian is thinking about the D in DSA. Marvell switches have the
capabilities of building a switch fabric out of multiple
interconnected switches. To switchdev, they appear as a single switch.
switchdev has no idea of the mapping of interfaces to switches, nor
the routing of frames between switches. This all happens in the layers
bellow. The hardware does support LAG members on different switches
within the same fabric. But it requires some additional setup for the
ports which link switches together. We have the same issues with MDB,
where additional setup is required for group members spread over the
switch fabric.

      Andrew

^ permalink raw reply

* Re: [RFC net-next 0/5] net: dsa: LAG support
From: Ido Schimmel @ 2017-10-02 13:05 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, netdev, vivien.didelot, jiri, idosch,
	Woojung.Huh, john, sean.wang
In-Reply-To: <20171002125932.GB4765@lunn.ch>

Hi Andrew,

On Mon, Oct 02, 2017 at 02:59:32PM +0200, Andrew Lunn wrote:
> > > - not sure what to do with a switch fabric, naively, if adding two ports
> > >   of two distinct switches as a LAG group, we may have to propagate that
> > >   to "dsa" cross-chip interfaces as well
> > 
> > At least in mlxsw case, enslaving switch and non-switch ports to the
> > same LAG doesn't make sense. Any traffic routed by the switch will only
> > be load-balanced between the switch ports. One way to solve that is to
> > forbid such enslavements during NETDEV_PRECHANGEUPPER in case the lower
> > devices in the adjacency list of the LAG don't belong to the same
> > switch.
> > 
> > Note that such configurations are bound to fail anyway, as the
> > non-switch ports will not have `switchdev_ops` configured and thus fail
> > during __switchdev_port_obj_add() / __switchdev_port_attr_set().
> 
> Hi Ido
> 
> Here Florian is thinking about the D in DSA. Marvell switches have the
> capabilities of building a switch fabric out of multiple
> interconnected switches. To switchdev, they appear as a single switch.
> switchdev has no idea of the mapping of interfaces to switches, nor
> the routing of frames between switches. This all happens in the layers
> bellow. The hardware does support LAG members on different switches
> within the same fabric. But it requires some additional setup for the
> ports which link switches together. We have the same issues with MDB,
> where additional setup is required for group members spread over the
> switch fabric.

Yes, I understood that. I was simply referring to the more general
problem of any two net devices and how to solve it. Not currently
implemented in mlxsw, but should be necessary for DSA as well.

Agree with your previous mail about keeping it simple for the first
implementation.

^ permalink raw reply

* Re: [PATCH 05/18] net: use ARRAY_SIZE
From: Andy Shevchenko @ 2017-10-02 13:07 UTC (permalink / raw)
  To: Jérémy Lefaure
  Cc: Sathya Perla, Ajit Khaparde, Sriharsha Basavapatna, Somnath Kotur,
	Jeff Kirsher, Arend van Spriel, Franky Lin, Hante Meuleman,
	Chi-Hsien Lin, Wright Feng, Kalle Valo, Larry Finger, Chaoming Li,
	David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
	"linux-kernel@vger.kernel.org" <
In-Reply-To: <20171001193101.8898-6-jeremy.lefaure@lse.epita.fr>

On Sun, Oct 1, 2017 at 10:30 PM, Jérémy Lefaure
<jeremy.lefaure@lse.epita.fr> wrote:
> Using the ARRAY_SIZE macro improves the readability of the code. Also,
> it is not always useful to use a variable to store this constant
> calculated at compile time.
>

> +       {&gainctrl_lut_core0_rev0, ARRAY_SIZE(gainctrl_lut_core0_rev0), 26, 192,
> +        32},

For all such cases I would rather put on one line disregard checkpatch
warning for better readability.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: v4.14-rc2/arm64 kernel BUG at net/core/skbuff.c:2626
From: Eric Dumazet @ 2017-10-02 13:36 UTC (permalink / raw)
  To: Mark Rutland
  Cc: LKML, netdev, linux-arm-kernel, syzkaller, David S. Miller,
	Willem de Bruijn
In-Reply-To: <20171002104947.GE20737@leverpostej>

On Mon, Oct 2, 2017 at 3:49 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi all,
>
> I hit the below splat at net/core/skbuff.c:2626 while fuzzing v4.14-rc2
> on arm64 with Syzkaller. This is the BUG_ON(len) at the end of
> skb_copy_and_csum_bits().
>
> I've uploaded a copy of the splat, my config, and (full) Syzkaller log
> to my kernel.org web space [1]. I haven't had the opportunity to
> reproduce this yet.
>
> This isn't a pure v4.14-rc2, as I have a not-yet-upstream fix [2]
> applied to avoid a userfaultfd bug. However, per the Syzkaller log, the
> userfaultfd syscall wasn't invoked, so I don't believe that should
> matter.
>
> Thanks,
> Mark.
>
> [1] https://www.kernel.org/pub/linux/kernel/people/mark/bugs/20171002-skbuff-bug/
> [2] https://lkml.kernel.org/r/20170920180413.26713-1-aarcange@redhat.com
>
> ------------[ cut here ]------------
> kernel BUG at net/core/skbuff.c:2626!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 3 PID: 0 Comm: swapper/3 Not tainted 4.14.0-rc2-00001-gd7ad33d #115
> Hardware name: linux,dummy-virt (DT)
> task: ffff80003a901a80 task.stack: ffff80003a908000
> PC is at skb_copy_and_csum_bits+0x8dc/0xae0 net/core/skbuff.c:2626
> LR is at skb_copy_and_csum_bits+0x8dc/0xae0 net/core/skbuff.c:2626
> pc : [<ffff200009e03214>] lr : [<ffff200009e03214>] pstate: 00000145
> sp : ffff80003efd7b50
> x29: ffff80003efd7b50 x28: 000000000000003c
> x27: 00000000000001e8 x26: ffff80003a901a90
> x25: 000000000000003c x24: dfff200000000000
> x23: ffff800035723a80 x22: 000000000000003c
> x21: 0000000000000000 x20: 0000000000000000
> x19: 0000000000003a6d x18: ffff20000da58140
> x17: 0000000000000000 x16: 0000000000000001
> x15: ffff20000e1485a0 x14: ffff2000082f8980
> x13: ffff200009fc73d0 x12: ffff200009fc707c
> x11: 1ffff00002c2a3fc x10: ffff100002c2a3fc
> x9 : dfff200000000000 x8 : 07030301a8ff1127
> x7 : edff11270a080204 x6 : ffff800016151fe8
> x5 : ffff100002c2a3fd x4 : 000000000000000c
> x3 : 0000000000000030 x2 : 1ffff00006ae47a1
> x1 : 01f6cee936b5bc00 x0 : 0000000000000000
> Process swapper/3 (pid: 0, stack limit = 0xffff80003a908000)
> Call trace:
> Exception stack(0xffff80003efd7a10 to 0xffff80003efd7b50)
> 7a00:                                   0000000000000000 01f6cee936b5bc00
> 7a20: 1ffff00006ae47a1 0000000000000030 000000000000000c ffff100002c2a3fd
> 7a40: ffff800016151fe8 edff11270a080204 07030301a8ff1127 dfff200000000000
> 7a60: ffff100002c2a3fc 1ffff00002c2a3fc ffff200009fc707c ffff200009fc73d0
> 7a80: ffff2000082f8980 ffff20000e1485a0 0000000000000001 0000000000000000
> 7aa0: ffff20000da58140 0000000000003a6d 0000000000000000 0000000000000000
> 7ac0: 000000000000003c ffff800035723a80 dfff200000000000 000000000000003c
> 7ae0: ffff80003a901a90 00000000000001e8 000000000000003c ffff80003efd7b50
> 7b00: ffff200009e03214 ffff80003efd7b50 ffff200009e03214 0000000000000145
> 7b20: 0000000000003a6d 0000000000000000 0001000000000000 000000000000003c
> 7b40: ffff80003efd7b50 ffff200009e03214
> [<ffff200009e03214>] skb_copy_and_csum_bits+0x8dc/0xae0 net/core/skbuff.c:2626
> [<ffff20000a01d244>] icmp_glue_bits+0xa4/0x2a0 net/ipv4/icmp.c:357
> [<ffff200009f3f0d4>] __ip_append_data+0x10e4/0x20a8 net/ipv4/ip_output.c:1018
> [<ffff200009f41a88>] ip_append_data.part.3+0xe8/0x1a0 net/ipv4/ip_output.c:1170
> [<ffff200009f46e74>] ip_append_data+0xa4/0xb0 net/ipv4/ip_output.c:1173
> [<ffff20000a01ccc8>] icmp_push_reply+0x1b8/0x690 net/ipv4/icmp.c:375
> [<ffff20000a0211b0>] icmp_send+0x1070/0x1890 net/ipv4/icmp.c:741
> [<ffff200009f41d48>] ip_fragment.constprop.4+0x208/0x340 net/ipv4/ip_output.c:552
> [<ffff200009f42228>] ip_finish_output+0x3a8/0xab0 net/ipv4/ip_output.c:315
> [<ffff200009f468c4>] NF_HOOK_COND include/linux/netfilter.h:238 [inline]
> [<ffff200009f468c4>] ip_output+0x284/0x790 net/ipv4/ip_output.c:405
> [<ffff200009f43204>] dst_output include/net/dst.h:458 [inline]
> [<ffff200009f43204>] ip_local_out+0x9c/0x1b8 net/ipv4/ip_output.c:124
> [<ffff200009f445e8>] ip_queue_xmit+0x850/0x18e0 net/ipv4/ip_output.c:504
> [<ffff200009fb091c>] tcp_transmit_skb+0x107c/0x3338 net/ipv4/tcp_output.c:1123
> [<ffff200009fbbcc4>] __tcp_retransmit_skb+0x614/0x1d18 net/ipv4/tcp_output.c:2847
> [<ffff200009fbd840>] tcp_send_loss_probe+0x478/0x7d0 net/ipv4/tcp_output.c:2457
> [<ffff200009fc707c>] tcp_write_timer_handler+0x50c/0x7e8 net/ipv4/tcp_timer.c:557
> [<ffff200009fc73d0>] tcp_write_timer+0x78/0x170 net/ipv4/tcp_timer.c:579
> [<ffff2000082f8980>] call_timer_fn+0x1b8/0x430 kernel/time/timer.c:1281
> [<ffff2000082f8dcc>] expire_timers+0x1d4/0x320 kernel/time/timer.c:1320
> [<ffff2000082f912c>] __run_timers kernel/time/timer.c:1620 [inline]
> [<ffff2000082f912c>] run_timer_softirq+0x214/0x5f0 kernel/time/timer.c:1646
> [<ffff2000080826c0>] __do_softirq+0x350/0xc0c kernel/softirq.c:284
> [<ffff200008170af4>] do_softirq_own_stack include/linux/interrupt.h:498 [inline]
> [<ffff200008170af4>] invoke_softirq kernel/softirq.c:371 [inline]
> [<ffff200008170af4>] irq_exit+0x1dc/0x2f8 kernel/softirq.c:405
> [<ffff2000082a95bc>] __handle_domain_irq+0xdc/0x230 kernel/irq/irqdesc.c:647
> [<ffff2000080820ac>] handle_domain_irq include/linux/irqdesc.h:175 [inline]
> [<ffff2000080820ac>] gic_handle_irq+0x6c/0xe0 drivers/irqchip/irq-gic.c:367
> Exception stack(0xffff80003a90bb70 to 0xffff80003a90bcb0)
> bb60:                                   ffff80003a90234c 0000000000000007
> bb80: 0000000000000000 1ffff00007520469 1fffe400017ad00c dfff200000000000
> bba0: dfff200000000000 0000000000000000 ffff80003a902350 1ffff00007520469
> bbc0: ffff80003a902348 ffff80003a902368 1ffff0000752046c 1ffff0000752046e
> bbe0: 1ffff0000752046d ffff20000e1485a0 0000000000000000 0000000000000001
> bc00: ffff20000da58140 ffff80003efd9800 ffff80003efd9800 ffff20000ae60000
> bc20: ffff80003a971a80 1ffff000075217aa 0000000000000000 ffff20000ae60000
> bc40: 0000000000000001 ffff20000a34fce0 0000dffff519f438 ffff80003a90bcb0
> bc60: ffff20000a36134c ffff80003a90bcb0 ffff20000a361350 0000000010000145
> bc80: ffff80003efd9800 ffff80003efd9800 ffffffffffffffff ffff80003efd9800
> bca0: ffff80003a90bcb0 ffff20000a361350
> [<ffff200008084034>] el1_irq+0xb4/0x12c arch/arm64/kernel/entry.S:569
> [<ffff20000a361350>] arch_local_irq_enable arch/arm64/include/asm/irqflags.h:40 [inline]
> [<ffff20000a361350>] __raw_spin_unlock_irq include/linux/spinlock_api_smp.h:168 [inline]
> [<ffff20000a361350>] _raw_spin_unlock_irq+0x30/0x100 kernel/locking/spinlock.c:199
> [<ffff2000081e0850>] finish_lock_switch kernel/sched/sched.h:1335 [inline]
> [<ffff2000081e0850>] finish_task_switch+0x1d8/0x950 kernel/sched/core.c:2657
> [<ffff20000a34fce0>] context_switch kernel/sched/core.c:2793 [inline]
> [<ffff20000a34fce0>] __schedule+0x518/0x17b0 kernel/sched/core.c:3366
> [<ffff20000a3520e8>] schedule_idle+0x58/0xc8 kernel/sched/core.c:3452
> [<ffff200008254a00>] do_idle+0x1d8/0x370 kernel/sched/idle.c:269
> [<ffff200008255138>] cpu_startup_entry+0x20/0x28 kernel/sched/idle.c:351
> [<ffff2000080a2f4c>] secondary_start_kernel+0x2fc/0x498 arch/arm64/kernel/smp.c:280
> Code: 97bcbfac 17fffe19 d503201f 97974258 (d4210000)
> ---[ end trace 3359b414c3a12466 ]---

This is most likely a bug caused by syzkaller setting a ridiculous MTU
on loopback device, below minimum size of ipv4 MTU.

I tried to track it in August [1], but it seems hard to find all the
issues with this.

commit c780a049f9bf442314335372c9abc4548bfe3e44
Author: Eric Dumazet <edumazet@google.com>
Date:   Wed Aug 16 11:09:12 2017 -0700

    ipv4: better IP_MAX_MTU enforcement

    While working on yet another syzkaller report, I found
    that our IP_MAX_MTU enforcements were not properly done.

    gcc seems to reload dev->mtu for min(dev->mtu, IP_MAX_MTU), and
    final result can be bigger than IP_MAX_MTU :/

    This is a problem because device mtu can be changed on other cpus or
    threads.

    While this patch does not fix the issue I am working on, it is
    probably worth addressing it.

^ permalink raw reply

* Re: Fw: [Bug 197099] New: Kernel panic in interrupt [l2tp_ppp]
From: James Chapman @ 2017-10-02 13:32 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20171001102110.24184f1b@xeon-e3>

This seems to be a NULL pointer exception caused by tunnel->sock being
NULL at the call to bh_lock_sock() in l2tp_xmit_skb() at
l2tp_core.c:1135.

tunnel->sock is set NULL in l2tp_core's tunnel socket destructor.

At the moment, I don't understand how this happens because
pppol2tp_xmit() does a sock_hold() on the tunnel socket before
l2tp_xmit_skb() is called. I'm still looking at this.

Has this problem only recently started happening?





On 1 October 2017 at 18:21, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
>
> Begin forwarded message:
>
> Date: Sun, 01 Oct 2017 16:22:33 +0000
> From: bugzilla-daemon@bugzilla.kernel.org
> To: stephen@networkplumber.org
> Subject: [Bug 197099] New: Kernel panic in interrupt [l2tp_ppp]
>
>
> https://bugzilla.kernel.org/show_bug.cgi?id=197099
>
>             Bug ID: 197099
>            Summary: Kernel panic in interrupt [l2tp_ppp]
>            Product: Networking
>            Version: 2.5
>     Kernel Version: 4.8.13-1.el6.elrepo.x86_64
>           Hardware: x86-64
>                 OS: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Other
>           Assignee: stephen@networkplumber.org
>           Reporter: svimik@gmail.com
>         Regression: No
>
> Created attachment 258685
>   --> https://bugzilla.kernel.org/attachment.cgi?id=258685&action=edit
> stacktrace screenshot
>
> Hello!
>
> Getting kernel panics on multiple servers. Since it mentions l2tp_core,
> l2tp_ppp and ppp_generic, I decided to report it to Networking (correct me if
> I'm wrong).
>
> Unfortunately I'm still struggling with making kdump work, so the trace
> screenshot is all I have at this moment. The only hope is that this stacktrace
> means something to the guys that wrote the code.
>
> --
> You are receiving this mail because:
> You are the assignee for the bug.

^ permalink raw reply

* Re: v4.14-rc2/arm64 misaligned atomic in ip_expire() / skb_clone()
From: Eric Dumazet @ 2017-10-02 13:44 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-kernel, netdev, linux-arm-kernel, syzkaller,
	David S. Miller, Willem de Bruijn, Eric Dumazet
In-Reply-To: <20171002115730.GA21696@leverpostej>

On Mon, 2017-10-02 at 12:57 +0100, Mark Rutland wrote:
> Hi all,
> 
> I'm intermittently hitting splats like below in skb_clone() while
> fuzzing v4.14-rc2 on arm64 with Syzkaller. It looks like the
> atomic_inc() at the end of __skb_clone() is being passed a misaligned
> pointer.
> 
> I've uploaded a number of splats and their associated (full) Syzkaller
> logs, along with my kernel config to my kernel.org webspace [1]. It
> might take a while for that to appear.
> 
> This isn't a pure v4.14-rc2, as I have a not-yet-upstream fix [2]
> applied to avoid a userfaultfd bug. The userfaultfd syscall appears in
> all of the Syzkaller logs, so there is the chance that this is related,
> but as I've not seen any other issues I suspect that's unlikely.
> 
> Thanks,
> Mark.
> 
> [1] https://www.kernel.org/pub/linux/kernel/people/mark/bugs/20171002-skb_clone-misaligned-atomic
> [2] https://lkml.kernel.org/r/20170920180413.26713-1-aarcange@redhat.com
> 
> Unable to handle kernel paging request at virtual address ffff80002fd714a2
> Mem abort info:
>   Exception class = DABT (current EL), IL = 32 bits
>   SET = 0, FnV = 0
>   EA = 0, S1PTW = 0
> Data abort info:
>   ISV = 0, ISS = 0x00000033
>   CM = 0, WnR = 0
> swapper pgtable: 4k pages, 48-bit VAs, pgd = ffff20000eeb2000
> [ffff80002fd714a2] *pgd=000000007eff7003, *pud=000000007eff6003, *pmd=00f800006fc00711
> Internal error: Oops: 96000021 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 3 PID: 0 Comm: swapper/3 Not tainted 4.14.0-rc2-00001-gd7ad33d #115
> Hardware name: linux,dummy-virt (DT)
> task: ffff80003a901a80 task.stack: ffff80003a908000
> PC is at __ll_sc_atomic_add+0x4/0x18 arch/arm64/include/asm/atomic_ll_sc.h:113
> LR is at atomic_add arch/arm64/include/asm/atomic_lse.h:45 [inline]
> LR is at __skb_clone+0x4a8/0x6c0 net/core/skbuff.c:873
> pc : [<ffff20000a30ce44>] lr : [<ffff200009dffb58>] pstate: 10000145
> sp : ffff80003efd86e0
> x29: ffff80003efd86e0 x28: 000060003418b000 
> x27: ffff20000ae55360 x26: ffff8000182c1608 
> x25: ffff80002fd7137e x24: ffff8000182c1610 
> x23: ffff20000ae60000 x22: ffff80001577871c 
> x21: 1ffff00007dfb0e8 x20: ffff8000182c1540 
> x19: ffff800015778640 x18: ffff20000da58140 
> x17: 0000000000000000 x16: 0000000000000002 
> x15: ffff20000e1485a0 x14: ffff2000082f912c 
> x13: ffff2000082f8dcc x12: ffff2000082f8980 
> x11: 1ffff00002aef0df x10: ffff100002aef0df 
> x9 : dfff200000000000 x8 : 0082009000a40008 
> x7 : 0000000000000000 x6 : ffff800015778700 
> x5 : ffff100002aef0e0 x4 : 0000000000000000 
> x3 : 1ffff00002aef0e3 x2 : ffff80002fd7147e 
> x1 : ffff80002fd714a2 x0 : 0000000000000001 
> Process swapper/3 (pid: 0, stack limit = 0xffff80003a908000)
> Call trace:
> Exception stack(0xffff80003efd85a0 to 0xffff80003efd86e0)
> 85a0: 0000000000000001 ffff80002fd714a2 ffff80002fd7147e 1ffff00002aef0e3
> 85c0: 0000000000000000 ffff100002aef0e0 ffff800015778700 0000000000000000
> 85e0: 0082009000a40008 dfff200000000000 ffff100002aef0df 1ffff00002aef0df
> 8600: ffff2000082f8980 ffff2000082f8dcc ffff2000082f912c ffff20000e1485a0
> 8620: 0000000000000002 0000000000000000 ffff20000da58140 ffff800015778640
> 8640: ffff8000182c1540 1ffff00007dfb0e8 ffff80001577871c ffff20000ae60000
> 8660: ffff8000182c1610 ffff80002fd7137e ffff8000182c1608 ffff20000ae55360
> 8680: 000060003418b000 ffff80003efd86e0 ffff200009dffb58 ffff80003efd86e0
> 86a0: ffff20000a30ce44 0000000010000145 ffff800015778640 ffff8000182c1540
> 86c0: 0001000000000000 ffff8000182c15ce ffff80003efd86e0 ffff20000a30ce44
> [<ffff20000a30ce44>] __ll_sc_atomic_add+0x4/0x18 arch/arm64/include/asm/atomic_ll_sc.h:113
> [<ffff200009e1009c>] skb_clone+0x1c4/0x3b0 net/core/skbuff.c:1286
> [<ffff200009f2ff80>] ip_expire+0x4e8/0x7c0 net/ipv4/ip_fragment.c:239
> [<ffff2000082f8980>] call_timer_fn+0x1b8/0x430 kernel/time/timer.c:1281
> [<ffff2000082f8dcc>] expire_timers+0x1d4/0x320 kernel/time/timer.c:1320
> [<ffff2000082f912c>] __run_timers kernel/time/timer.c:1620 [inline]
> [<ffff2000082f912c>] run_timer_softirq+0x214/0x5f0 kernel/time/timer.c:1646
> [<ffff2000080826c0>] __do_softirq+0x350/0xc0c kernel/softirq.c:284
> [<ffff200008170af4>] do_softirq_own_stack include/linux/interrupt.h:498 [inline]
> [<ffff200008170af4>] invoke_softirq kernel/softirq.c:371 [inline]
> [<ffff200008170af4>] irq_exit+0x1dc/0x2f8 kernel/softirq.c:405
> [<ffff2000082a95bc>] __handle_domain_irq+0xdc/0x230 kernel/irq/irqdesc.c:647
> [<ffff2000080820ac>] handle_domain_irq include/linux/irqdesc.h:175 [inline]
> [<ffff2000080820ac>] gic_handle_irq+0x6c/0xe0 drivers/irqchip/irq-gic.c:367
> Exception stack(0xffff80003a90bd70 to 0xffff80003a90beb0)
> bd60:                                   ffff80003a90234c 0000000000000007
> bd80: 0000000000000000 1ffff00007520469 1fffe400017ad00c ffffffffffffe540
> bda0: 0000000000000000 0000000000000000 ffff80003a902350 1ffff00007520469
> bdc0: ffff80003a902348 ffff80003a902368 1ffff0000752046c 1ffff0000752046e
> bde0: 1ffff0000752046d ffff20000e1485a0 0000000000000000 0000000000029d44
> be00: ffff20000da58140 ffff80003a901a80 ffff80003a901a80 dfff200000000000
> be20: ffff20000ae60e98 ffff0400015cc1d3 0000000000000000 ffff20000ae60df8
> be40: ffff20000ae60df8 0000000000000000 0000000000000000 ffff80003a90beb0
> be60: ffff200008089b50 ffff80003a90beb0 ffff200008089b54 0000000010000145
> be80: ffff80003a901a80 ffff80003a901a80 ffffffffffffffff 01f6cee936b5bc00
> bea0: ffff80003a90beb0 ffff200008089b54
> [<ffff200008084034>] el1_irq+0xb4/0x12c arch/arm64/kernel/entry.S:569
> [<ffff200008089b54>] arch_local_irq_enable arch/arm64/include/asm/irqflags.h:40 [inline]
> [<ffff200008089b54>] arch_cpu_idle+0x1c/0x28 arch/arm64/kernel/process.c:87
> [<ffff20000a360a94>] default_idle_call+0x34/0x78 kernel/sched/idle.c:98
> [<ffff200008254a34>] cpuidle_idle_call kernel/sched/idle.c:156 [inline]
> [<ffff200008254a34>] do_idle+0x20c/0x370 kernel/sched/idle.c:246
> [<ffff20000825513c>] cpu_startup_entry+0x24/0x28 kernel/sched/idle.c:351
> [<ffff2000080a2f4c>] secondary_start_kernel+0x2fc/0x498 arch/arm64/kernel/smp.c:280
> Code: 978b7cfd 17ffff91 00000000 f9800031 (885f7c31) 
> ---[ end trace e4e9a51ab15d3a5f ]---
> 

skb->head is allocated by a kmalloc() call or similar.

This would happen if skb->end is mangled to not be a multiple of
NET_SKB_PAD  (or at least 4 in your case)

^ permalink raw reply

* Re: [PATCH 05/18] net: use ARRAY_SIZE
From: Kalle Valo @ 2017-10-02 13:46 UTC (permalink / raw)
  To: Jérémy Lefaure
  Cc: Sathya Perla, Ajit Khaparde, Sriharsha Basavapatna, Somnath Kotur,
	Jeff Kirsher, Arend van Spriel, Franky Lin, Hante Meuleman,
	Chi-Hsien Lin, Wright Feng, Larry Finger, Chaoming Li,
	David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
	linux-kernel, intel-wired-lan, linux-usb
In-Reply-To: <20171001193101.8898-6-jeremy.lefaure@lse.epita.fr>

Jérémy Lefaure <jeremy.lefaure@lse.epita.fr> writes:

> Using the ARRAY_SIZE macro improves the readability of the code. Also,
> it is not always useful to use a variable to store this constant
> calculated at compile time.
>
> Found with Coccinelle with the following semantic patch:
> @r depends on (org || report)@
> type T;
> T[] E;
> position p;
> @@
> (
>  (sizeof(E)@p /sizeof(*E))
> |
>  (sizeof(E)@p /sizeof(E[...]))
> |
>  (sizeof(E)@p /sizeof(T))
> )
>
> Signed-off-by: Jérémy Lefaure <jeremy.lefaure@lse.epita.fr>
> ---
>  drivers/net/ethernet/emulex/benet/be_cmds.c        |   4 +-
>  drivers/net/ethernet/intel/i40e/i40e_adminq.h      |   3 +-
>  drivers/net/ethernet/intel/i40evf/i40e_adminq.h    |   3 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c      |   3 +-
>  drivers/net/ethernet/intel/ixgbevf/vf.c            |  17 +-
>  drivers/net/usb/kalmia.c                           |   9 +-
>  .../broadcom/brcm80211/brcmsmac/phy/phytbl_n.c     | 473 ++++++---------------
>  .../net/wireless/realtek/rtlwifi/rtl8723be/hw.c    |   9 +-
>  .../net/wireless/realtek/rtlwifi/rtl8723be/phy.c   |  12 +-
>  .../net/wireless/realtek/rtlwifi/rtl8723be/table.c |  14 +-
>  .../net/wireless/realtek/rtlwifi/rtl8821ae/table.c |  34 +-
>  include/net/bond_3ad.h                             |   3 +-
>  net/ipv6/seg6_local.c                              |   6 +-
>  13 files changed, 177 insertions(+), 413 deletions(-)

We have a tree for wireless so usually it's better to submit wireless
changes on their own but here I assume Dave will apply this to his tree.
If not, please resubmit the wireless part in a separate patch.

-- 
Kalle Valo

^ permalink raw reply

* Re: [v4, 1/9] brcmsmac: make some local variables 'static const' to reduce stack size
From: Kalle Valo @ 2017-10-02 13:53 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, Arnd Bergmann, Mauro Carvalho Chehab, Jiri Pirko,
	David S. Miller, Andrey Ryabinin, Alexander Potapenko,
	Dmitry Vyukov, Masahiro Yamada, Michal Marek, Andrew Morton,
	Kees Cook, Geert Uytterhoeven, Greg Kroah-Hartman, linux-media
In-Reply-To: <20170922212930.620249-2-arnd@arndb.de>

Arnd Bergmann <arnd@arndb.de> wrote:

> With KASAN and a couple of other patches applied, this driver is one
> of the few remaining ones that actually use more than 2048 bytes of
> kernel stack:
> 
> broadcom/brcm80211/brcmsmac/phy/phy_n.c: In function 'wlc_phy_workarounds_nphy_gainctrl':
> broadcom/brcm80211/brcmsmac/phy/phy_n.c:16065:1: warning: the frame size of 3264 bytes is larger than 2048 bytes [-Wframe-larger-than=]
> broadcom/brcm80211/brcmsmac/phy/phy_n.c: In function 'wlc_phy_workarounds_nphy':
> broadcom/brcm80211/brcmsmac/phy/phy_n.c:17138:1: warning: the frame size of 2864 bytes is larger than 2048 bytes [-Wframe-larger-than=]
> 
> Here, I'm reducing the stack size by marking as many local variables as
> 'static const' as I can without changing the actual code.
> 
> This is the first of three patches to improve the stack usage in this
> driver. It would be good to have this backported to stabl kernels
> to get all drivers in 'allmodconfig' below the 2048 byte limit so
> we can turn on the frame warning again globally, but I realize that
> the patch is larger than the normal limit for stable backports.
> 
> The other two patches do not need to be backported.
> 
> Cc: <stable@vger.kernel.org>
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied to wireless-drivers.git, thanks.

c503dd38f850 brcmsmac: make some local variables 'static const' to reduce stack size

-- 
https://patchwork.kernel.org/patch/9967145/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [v4,2/9] brcmsmac: split up wlc_phy_workarounds_nphy
From: Kalle Valo @ 2017-10-02 13:55 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, Arnd Bergmann, Mauro Carvalho Chehab, Jiri Pirko,
	David S. Miller, Andrey Ryabinin, Alexander Potapenko,
	Dmitry Vyukov, Masahiro Yamada, Michal Marek, Andrew Morton,
	Kees Cook, Geert Uytterhoeven, Greg Kroah-Hartman, linux-media
In-Reply-To: <20170922212930.620249-3-arnd@arndb.de>

Arnd Bergmann <arnd@arndb.de> wrote:

> The stack consumption in this driver is still relatively high, with one
> remaining warning if the warning level is lowered to 1536 bytes:
> 
> drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:17135:1: error: the frame size of 1880 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> 
> The affected function is actually a collection of three separate implementations,
> and each of them is fairly large by itself. Splitting them up is done easily
> and improves readability at the same time.
> 
> I'm leaving the original indentation to make the review easier.
> 
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I'll queue this for v4.15. Depends on:

c503dd38f850 brcmsmac: make some local variables 'static const' to reduce stack size

-- 
https://patchwork.kernel.org/patch/9967141/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply

* Re: Fw: [Bug 197099] New: Kernel panic in interrupt [l2tp_ppp]
From: Eric Dumazet @ 2017-10-02 13:56 UTC (permalink / raw)
  To: James Chapman, svimik; +Cc: Stephen Hemminger, netdev
In-Reply-To: <CAEwTi7QkF73cc1hShzOoM9bzb5mBzyFW8t68OCn+XECrJ9d61Q@mail.gmail.com>

CC svimik@gmail.com so that he is aware of this netdev thread.

On Mon, 2017-10-02 at 14:32 +0100, James Chapman wrote:
> This seems to be a NULL pointer exception caused by tunnel->sock being
> NULL at the call to bh_lock_sock() in l2tp_xmit_skb() at
> l2tp_core.c:1135.
> 
> tunnel->sock is set NULL in l2tp_core's tunnel socket destructor.
> 
> At the moment, I don't understand how this happens because
> pppol2tp_xmit() does a sock_hold() on the tunnel socket before
> l2tp_xmit_skb() is called. I'm still looking at this.
> 
> Has this problem only recently started happening?
> 
> 
> 
> 
> 
> On 1 October 2017 at 18:21, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> >
> > Begin forwarded message:
> >
> > Date: Sun, 01 Oct 2017 16:22:33 +0000
> > From: bugzilla-daemon@bugzilla.kernel.org
> > To: stephen@networkplumber.org
> > Subject: [Bug 197099] New: Kernel panic in interrupt [l2tp_ppp]
> >
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=197099
> >
> >             Bug ID: 197099
> >            Summary: Kernel panic in interrupt [l2tp_ppp]
> >            Product: Networking
> >            Version: 2.5
> >     Kernel Version: 4.8.13-1.el6.elrepo.x86_64
> >           Hardware: x86-64
> >                 OS: Linux
> >               Tree: Mainline
> >             Status: NEW
> >           Severity: normal
> >           Priority: P1
> >          Component: Other
> >           Assignee: stephen@networkplumber.org
> >           Reporter: svimik@gmail.com
> >         Regression: No
> >
> > Created attachment 258685
> >   --> https://bugzilla.kernel.org/attachment.cgi?id=258685&action=edit
> > stacktrace screenshot
> >
> > Hello!
> >
> > Getting kernel panics on multiple servers. Since it mentions l2tp_core,
> > l2tp_ppp and ppp_generic, I decided to report it to Networking (correct me if
> > I'm wrong).
> >
> > Unfortunately I'm still struggling with making kdump work, so the trace
> > screenshot is all I have at this moment. The only hope is that this stacktrace
> > means something to the guys that wrote the code.
> >
> > --
> > You are receiving this mail because:
> > You are the assignee for the bug.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox