Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net] macsec: Fix header length if SCI is added if explicitily disabled
From: Sabrina Dubroca @ 2016-10-24 13:38 UTC (permalink / raw)
  To: Tobias Brunner; +Cc: David S. Miller, netdev
In-Reply-To: <253edab5-98e6-60e0-0c29-2f1205a4dd1a@strongswan.org>

2016-10-24, 15:32:40 +0200, Tobias Brunner wrote:
> > [snip]
> >> @@ -440,12 +448,12 @@ static void macsec_fill_sectag(struct macsec_eth_header *h,
> >>  			       const struct macsec_secy *secy, u32 pn)
> >>  {
> >>  	const struct macsec_tx_sc *tx_sc = &secy->tx_sc;
> >> +	bool sci_present = send_sci(secy);
> > 
> > You're already computing this in macsec_encrypt() just before calling
> > macsec_fill_sectag(), so you could pass it as argument instead of
> > recomputing it.
> 
> Right, I'll send a v2.  Would you like me to inline the send_sci()
> function, as it will only be called once afterwards.

I think keeping the send_sci() function is okay, but if you prefer to
inline it, I don't mind.

-- 
Sabrina

^ permalink raw reply

* Re: net/dccp: warning in dccp_set_state
From: Andrey Konovalov @ 2016-10-24 13:41 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Gerrit Renker, David S. Miller, dccp, netdev, LKML, syzkaller,
	Kostya Serebryany, Alexander Potapenko, Eric Dumazet,
	Dmitry Vyukov
In-Reply-To: <1477313555.7065.105.camel@edumazet-glaptop3.roam.corp.google.com>

Hi Eric,

I can confirm that with your patch the warning goes away.

Tested-by: Andrey Konovalov <andreyknvl@google.com>

On Mon, Oct 24, 2016 at 2:52 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2016-10-24 at 05:47 -0700, Eric Dumazet wrote:
>> On Mon, 2016-10-24 at 14:23 +0200, Andrey Konovalov wrote:
>> > Hi,
>> >
>> > I've got the following error report while running the syzkaller fuzzer:
>> >
>> > WARNING: CPU: 1 PID: 21072 at net/dccp/proto.c:83 dccp_set_state+0x229/0x290
>> > Kernel panic - not syncing: panic_on_warn set ...
>> >
>> > CPU: 1 PID: 21072 Comm: syz-executor Not tainted 4.9.0-rc1+ #293
>> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> >  ffff88003d4c7738 ffffffff81b474f4 0000000000000003 dffffc0000000000
>> >  ffffffff844f8b00 ffff88003d4c7804 ffff88003d4c7800 ffffffff8140c06a
>> >  0000000041b58ab3 ffffffff8479ab7d ffffffff8140beae ffffffff8140cd00
>> > Call Trace:
>> >  [<     inline     >] __dump_stack lib/dump_stack.c:15
>> >  [<ffffffff81b474f4>] dump_stack+0xb3/0x10f lib/dump_stack.c:51
>> >  [<ffffffff8140c06a>] panic+0x1bc/0x39d kernel/panic.c:179
>> >  [<ffffffff8111125c>] __warn+0x1cc/0x1f0 kernel/panic.c:542
>> >  [<ffffffff8111144c>] warn_slowpath_null+0x2c/0x40 kernel/panic.c:585
>> >  [<ffffffff8389e5d9>] dccp_set_state+0x229/0x290 net/dccp/proto.c:83
>> >  [<ffffffff838a0aa2>] dccp_close+0x612/0xc10 net/dccp/proto.c:1016
>> >  [<ffffffff8316bf1f>] inet_release+0xef/0x1c0 net/ipv4/af_inet.c:415
>> >  [<ffffffff82b6e89e>] sock_release+0x8e/0x1d0 net/socket.c:570
>> >  [<ffffffff82b6e9f6>] sock_close+0x16/0x20 net/socket.c:1017
>> >  [<ffffffff815256ad>] __fput+0x29d/0x720 fs/file_table.c:208
>> >  [<ffffffff81525bb5>] ____fput+0x15/0x20 fs/file_table.c:244
>> >  [<ffffffff811727d8>] task_work_run+0xf8/0x170 kernel/task_work.c:116
>> >  [<     inline     >] exit_task_work include/linux/task_work.h:21
>> >  [<ffffffff8111bc53>] do_exit+0x883/0x2ac0 kernel/exit.c:828
>> >  [<ffffffff811221fe>] do_group_exit+0x10e/0x340 kernel/exit.c:931
>> >  [<ffffffff81143c94>] get_signal+0x634/0x15a0 kernel/signal.c:2307
>> >  [<ffffffff81054aad>] do_signal+0x8d/0x1a30 arch/x86/kernel/signal.c:807
>> >  [<ffffffff81003a05>] exit_to_usermode_loop+0xe5/0x130
>> > arch/x86/entry/common.c:156
>> >  [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:190
>> >  [<ffffffff81006298>] syscall_return_slowpath+0x1a8/0x1e0
>> > arch/x86/entry/common.c:259
>> >  [<ffffffff83fc1a62>] entry_SYSCALL_64_fastpath+0xc0/0xc2
>> > Dumping ftrace buffer:
>> >    (ftrace buffer empty)
>> > Kernel Offset: disabled
>> >
>> > On commit 1a1891d762d6e64daf07b5be4817e3fbb29e3c59 (Oct 18).
>>
>> Not sure we we keep around DCCP. David could we kill it ?
>>
>> TCP seems to have an additional check, missing in DCCP.
>>
>> diff --git a/net/dccp/proto.c b/net/dccp/proto.c
>> index 41e65804ddf5..9fe25bf63296 100644
>> --- a/net/dccp/proto.c
>> +++ b/net/dccp/proto.c
>> @@ -1009,6 +1009,10 @@ void dccp_close(struct sock *sk, long timeout)
>>               __kfree_skb(skb);
>>       }
>>
>> +     /* If socket has been already reset kill it. */
>> +     if (sk->sk_state == DCCP_CLOSED)
>> +             goto adjudge_to_death;
>> +
>>       if (data_was_unread) {
>>               /* Unread data was tossed, send an appropriate Reset Code */
>>               DCCP_WARN("ABORT with %u bytes unread\n", data_was_unread);
>>
>
> The equivalent tcp fix was :
> https://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=565b7b2d2e632b5792879c0c9cccdd9eecd31195
>
>

^ permalink raw reply

* [PATCH v2 net] macsec: Fix header length if SCI is added if explicitly disabled
From: Tobias Brunner @ 2016-10-24 13:44 UTC (permalink / raw)
  To: Sabrina Dubroca, David S. Miller; +Cc: netdev

Even if sending SCIs is explicitly disabled, the code that creates the
Security Tag might still decide to add it (e.g. if multiple RX SCs are
defined on the MACsec interface).
But because the header length so far only depended on the configuration
option the SCI overwrote the original frame's contents (EtherType and
e.g. the beginning of the IP header) and if encrypted did not visibly
end up in the packet, while the SC flag in the TCI field of the Security
Tag was still set, resulting in invalid MACsec frames.

Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Signed-off-by: Tobias Brunner <tobias@strongswan.org>
---
 drivers/net/macsec.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 3ea47f28e143..d2e61e002926 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -397,6 +397,14 @@ static struct macsec_cb *macsec_skb_cb(struct sk_buff *skb)
 #define DEFAULT_ENCRYPT false
 #define DEFAULT_ENCODING_SA 0
 
+static bool send_sci(const struct macsec_secy *secy)
+{
+	const struct macsec_tx_sc *tx_sc = &secy->tx_sc;
+
+	return tx_sc->send_sci ||
+		(secy->n_rx_sc > 1 && !tx_sc->end_station && !tx_sc->scb);
+}
+
 static sci_t make_sci(u8 *addr, __be16 port)
 {
 	sci_t sci;
@@ -437,15 +445,15 @@ static unsigned int macsec_extra_len(bool sci_present)
 
 /* Fill SecTAG according to IEEE 802.1AE-2006 10.5.3 */
 static void macsec_fill_sectag(struct macsec_eth_header *h,
-			       const struct macsec_secy *secy, u32 pn)
+			       const struct macsec_secy *secy, u32 pn,
+			       bool sci_present)
 {
 	const struct macsec_tx_sc *tx_sc = &secy->tx_sc;
 
-	memset(&h->tci_an, 0, macsec_sectag_len(tx_sc->send_sci));
+	memset(&h->tci_an, 0, macsec_sectag_len(sci_present));
 	h->eth.h_proto = htons(ETH_P_MACSEC);
 
-	if (tx_sc->send_sci ||
-	    (secy->n_rx_sc > 1 && !tx_sc->end_station && !tx_sc->scb)) {
+	if (sci_present) {
 		h->tci_an |= MACSEC_TCI_SC;
 		memcpy(&h->secure_channel_id, &secy->sci,
 		       sizeof(h->secure_channel_id));
@@ -650,6 +658,7 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
 	struct macsec_tx_sc *tx_sc;
 	struct macsec_tx_sa *tx_sa;
 	struct macsec_dev *macsec = macsec_priv(dev);
+	bool sci_present;
 	u32 pn;
 
 	secy = &macsec->secy;
@@ -687,7 +696,8 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
 
 	unprotected_len = skb->len;
 	eth = eth_hdr(skb);
-	hh = (struct macsec_eth_header *)skb_push(skb, macsec_extra_len(tx_sc->send_sci));
+	sci_present = send_sci(secy);
+	hh = (struct macsec_eth_header *)skb_push(skb, macsec_extra_len(sci_present));
 	memmove(hh, eth, 2 * ETH_ALEN);
 
 	pn = tx_sa_update_pn(tx_sa, secy);
@@ -696,7 +706,7 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
 		kfree_skb(skb);
 		return ERR_PTR(-ENOLINK);
 	}
-	macsec_fill_sectag(hh, secy, pn);
+	macsec_fill_sectag(hh, secy, pn, sci_present);
 	macsec_set_shortlen(hh, unprotected_len - 2 * ETH_ALEN);
 
 	skb_put(skb, secy->icv_len);
@@ -726,10 +736,10 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
 	skb_to_sgvec(skb, sg, 0, skb->len);
 
 	if (tx_sc->encrypt) {
-		int len = skb->len - macsec_hdr_len(tx_sc->send_sci) -
+		int len = skb->len - macsec_hdr_len(sci_present) -
 			  secy->icv_len;
 		aead_request_set_crypt(req, sg, sg, len, iv);
-		aead_request_set_ad(req, macsec_hdr_len(tx_sc->send_sci));
+		aead_request_set_ad(req, macsec_hdr_len(sci_present));
 	} else {
 		aead_request_set_crypt(req, sg, sg, 0, iv);
 		aead_request_set_ad(req, skb->len - secy->icv_len);
-- 
1.9.1

^ permalink raw reply related

* [PATCH] LSO feature added to Cadence GEM driver
From: Rafal Ozieblo @ 2016-10-24 13:18 UTC (permalink / raw)
  To: Nicolas Ferre, netdev, linux-kernel; +Cc: Rafal Ozieblo

New Cadence GEM hardware support Large Segment Offload (LSO):
TCP segmentation offload (TSO) as well as UDP fragmentation
offload (UFO). Support for those features was added to the driver.

Signed-off-by: Rafal Ozieblo <rafalo@cadence.com>
---
 drivers/net/ethernet/cadence/macb.c | 141 +++++++++++++++++++++++++++++++++---
 drivers/net/ethernet/cadence/macb.h |  14 ++++
 2 files changed, 143 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index b32444a..f659d57 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -32,7 +32,9 @@
 #include <linux/of_gpio.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
-
+#include <linux/ip.h>
+#include <linux/udp.h>
+#include <linux/tcp.h>
 #include "macb.h"
 
 #define MACB_RX_BUFFER_SIZE	128
@@ -53,8 +55,10 @@
 					| MACB_BIT(TXERR))
 #define MACB_TX_INT_FLAGS	(MACB_TX_ERR_FLAGS | MACB_BIT(TCOMP))
 
-#define MACB_MAX_TX_LEN		((unsigned int)((1 << MACB_TX_FRMLEN_SIZE) - 1))
-#define GEM_MAX_TX_LEN		((unsigned int)((1 << GEM_TX_FRMLEN_SIZE) - 1))
+/* Max length of transmit frame must be a multiple of 8 bytes */
+#define MACB_TX_LEN_ALIGN	8
+#define MACB_MAX_TX_LEN		((unsigned int)((1 << MACB_TX_FRMLEN_SIZE) - 1) & ~((unsigned int)(MACB_TX_LEN_ALIGN - 1)))
+#define GEM_MAX_TX_LEN		((unsigned int)((1 << GEM_TX_FRMLEN_SIZE) - 1) & ~((unsigned int)(MACB_TX_LEN_ALIGN - 1)))
 
 #define GEM_MTU_MIN_SIZE	68
 
@@ -1212,7 +1216,8 @@ static void macb_poll_controller(struct net_device *dev)
 
 static unsigned int macb_tx_map(struct macb *bp,
 				struct macb_queue *queue,
-				struct sk_buff *skb)
+				struct sk_buff *skb,
+				unsigned int hdrlen)
 {
 	dma_addr_t mapping;
 	unsigned int len, entry, i, tx_head = queue->tx_head;
@@ -1220,14 +1225,27 @@ static unsigned int macb_tx_map(struct macb *bp,
 	struct macb_dma_desc *desc;
 	unsigned int offset, size, count = 0;
 	unsigned int f, nr_frags = skb_shinfo(skb)->nr_frags;
-	unsigned int eof = 1;
-	u32 ctrl;
+	unsigned int eof = 1, mss_mfs = 0;
+	u32 ctrl, lso_ctrl = 0, seq_ctrl = 0;
+
+	/* LSO */
+	if (skb_shinfo(skb)->gso_size != 0) {
+		if (IPPROTO_UDP == (((struct iphdr *)skb_network_header(skb))->protocol))
+			/* UDP - UFO */
+			lso_ctrl = MACB_LSO_UFO_ENABLE;
+		else
+			/* TCP - TSO */
+			lso_ctrl = MACB_LSO_TSO_ENABLE;
+	}
 
 	/* First, map non-paged data */
 	len = skb_headlen(skb);
+
+	/* first buffer length */
+	size = hdrlen;
+
 	offset = 0;
 	while (len) {
-		size = min(len, bp->max_tx_length);
 		entry = macb_tx_ring_wrap(tx_head);
 		tx_skb = &queue->tx_skb[entry];
 
@@ -1247,6 +1265,8 @@ static unsigned int macb_tx_map(struct macb *bp,
 		offset += size;
 		count++;
 		tx_head++;
+
+		size = min(len, bp->max_tx_length);
 	}
 
 	/* Then, map paged data from fragments */
@@ -1300,6 +1320,20 @@ static unsigned int macb_tx_map(struct macb *bp,
 	desc = &queue->tx_ring[entry];
 	desc->ctrl = ctrl;
 
+	if (lso_ctrl) {
+		if (lso_ctrl == MACB_LSO_UFO_ENABLE)
+			/* include header and FCS in value given to h/w */
+			mss_mfs = skb_shinfo(skb)->gso_size +
+					skb_transport_offset(skb) + 4;
+		else /* TSO */ {
+			mss_mfs = skb_shinfo(skb)->gso_size;
+			/* TCP Sequence Number Source Select
+			 * can be set only for TSO
+			 */
+			seq_ctrl = 0;
+		}
+	}
+
 	do {
 		i--;
 		entry = macb_tx_ring_wrap(i);
@@ -1314,6 +1348,16 @@ static unsigned int macb_tx_map(struct macb *bp,
 		if (unlikely(entry == (TX_RING_SIZE - 1)))
 			ctrl |= MACB_BIT(TX_WRAP);
 
+		/* First descriptor is header descriptor */
+		if (i == queue->tx_head) {
+			ctrl |= MACB_BF(TX_LSO, lso_ctrl);
+			ctrl |= MACB_BF(TX_TCP_SEQ_SRC, seq_ctrl);
+		} else
+			/* Only set MSS/MFS on payload descriptors
+			 * (second or later descriptor)
+			 */
+			ctrl |= MACB_BF(MSS_MFS, mss_mfs);
+
 		/* Set TX buffer descriptor */
 		macb_set_addr(desc, tx_skb->mapping);
 		/* desc->addr must be visible to hardware before clearing
@@ -1339,6 +1383,37 @@ static unsigned int macb_tx_map(struct macb *bp,
 	return 0;
 }
 
+static int macb_lso_check_compatibility(struct sk_buff *skb, unsigned int hdrlen)
+{
+	unsigned int nr_frags, f;
+
+	if (skb_shinfo(skb)->gso_size == 0)
+		/* not LSO */
+		return -EPERM;
+
+	/* there is only one buffer */
+	if (!skb_is_nonlinear(skb))
+		return 0;
+
+	/* For LSO:
+	 * When software supplies two or more payload buffers all payload buffers
+	 * apart from the last must be a multiple of 8 bytes in size.
+	 */
+	if (!IS_ALIGNED(skb_headlen(skb) - hdrlen, MACB_TX_LEN_ALIGN))
+		return -EPERM;
+
+	nr_frags = skb_shinfo(skb)->nr_frags;
+	/* No need to check last fragment */
+	nr_frags--;
+	for (f = 0; f < nr_frags; f++) {
+		const skb_frag_t *frag = &skb_shinfo(skb)->frags[f];
+
+		if (!IS_ALIGNED(skb_frag_size(frag), MACB_TX_LEN_ALIGN))
+			return -EPERM;
+	}
+	return 0;
+}
+
 static inline int macb_clear_csum(struct sk_buff *skb)
 {
 	/* no change for packets without checksum offloading */
@@ -1363,7 +1438,33 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct macb *bp = netdev_priv(dev);
 	struct macb_queue *queue = &bp->queues[queue_index];
 	unsigned long flags;
-	unsigned int count, nr_frags, frag_size, f;
+	unsigned int desc_cnt, nr_frags, frag_size, f;
+	unsigned int is_lso = 0, is_udp, hdrlen;
+
+	is_lso = (skb_shinfo(skb)->gso_size != 0);
+
+	if (is_lso) {
+		is_udp = (IPPROTO_UDP == (((struct iphdr *)skb_network_header(skb))->protocol));
+
+		/* length of headers */
+		if (is_udp)
+			/* only queue eth + ip headers separately for UDP */
+			hdrlen = skb_transport_offset(skb);
+		else
+			hdrlen = skb_transport_offset(skb) + tcp_hdrlen(skb);
+		if (skb_headlen(skb) < hdrlen) {
+			netdev_err(bp->dev, "Error - LSO headers fragmented!!!\n");
+			/* if this is required, would need to copy to single buffer */
+			return NETDEV_TX_BUSY;
+		}
+		if (macb_lso_check_compatibility(skb, hdrlen)) {
+			if (skb_linearize(skb)) {
+				netdev_err(bp->dev, "Can't linearize buffer\n");
+				return NETDEV_TX_BUSY;
+			}
+		}
+	} else
+		hdrlen = min(skb_headlen(skb), bp->max_tx_length);
 
 #if defined(DEBUG) && defined(VERBOSE_DEBUG)
 	netdev_vdbg(bp->dev,
@@ -1378,17 +1479,21 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 * socket buffer: skb fragments of jumbo frames may need to be
 	 * split into many buffer descriptors.
 	 */
-	count = DIV_ROUND_UP(skb_headlen(skb), bp->max_tx_length);
+	if (is_lso && (skb_headlen(skb) > hdrlen))
+		/* extra header descriptor if also payload in first buffer */
+		desc_cnt = DIV_ROUND_UP((skb_headlen(skb) - hdrlen), bp->max_tx_length) + 1;
+	else
+		desc_cnt = DIV_ROUND_UP(skb_headlen(skb), bp->max_tx_length);
 	nr_frags = skb_shinfo(skb)->nr_frags;
 	for (f = 0; f < nr_frags; f++) {
 		frag_size = skb_frag_size(&skb_shinfo(skb)->frags[f]);
-		count += DIV_ROUND_UP(frag_size, bp->max_tx_length);
+		desc_cnt += DIV_ROUND_UP(frag_size, bp->max_tx_length);
 	}
 
 	spin_lock_irqsave(&bp->lock, flags);
 
 	/* This is a hard error, log it. */
-	if (CIRC_SPACE(queue->tx_head, queue->tx_tail, TX_RING_SIZE) < count) {
+	if (CIRC_SPACE(queue->tx_head, queue->tx_tail, TX_RING_SIZE) < desc_cnt) {
 		netif_stop_subqueue(dev, queue_index);
 		spin_unlock_irqrestore(&bp->lock, flags);
 		netdev_dbg(bp->dev, "tx_head = %u, tx_tail = %u\n",
@@ -1396,13 +1501,19 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		return NETDEV_TX_BUSY;
 	}
 
+	if (is_lso) {
+		if (is_udp)
+			/* zero UDP checksum, not calculated by h/w for UFO */
+			udp_hdr(skb)->check = 0;
+	}
+
 	if (macb_clear_csum(skb)) {
 		dev_kfree_skb_any(skb);
 		goto unlock;
 	}
 
 	/* Map socket buffer for DMA transfer */
-	if (!macb_tx_map(bp, queue, skb)) {
+	if (!macb_tx_map(bp, queue, skb, hdrlen)) {
 		dev_kfree_skb_any(skb);
 		goto unlock;
 	}
@@ -2501,6 +2612,12 @@ static int macb_init(struct platform_device *pdev)
 
 	/* Set features */
 	dev->hw_features = NETIF_F_SG;
+
+	/* Check LSO capability */
+	if (GEM_BFEXT(PBUF_LSO, gem_readl(bp, DCFG6)))
+		dev->hw_features |= NETIF_F_TSO
+				| NETIF_F_UFO;
+
 	/* Checksum offload is only available on gem with packet buffer */
 	if (macb_is_gem(bp) && !(bp->caps & MACB_CAPS_FIFO_MODE))
 		dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_RXCSUM;
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 8bed4b5..534aded 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -382,6 +382,10 @@
 #define GEM_TX_PKT_BUFF_OFFSET			21
 #define GEM_TX_PKT_BUFF_SIZE			1
 
+/* Bitfields in DCFG6. */
+#define GEM_PBUF_LSO_OFFSET			27
+#define GEM_PBUF_LSO_SIZE			1
+
 /* Constants for CLK */
 #define MACB_CLK_DIV8				0
 #define MACB_CLK_DIV16				1
@@ -414,6 +418,10 @@
 #define MACB_CAPS_SG_DISABLED			0x40000000
 #define MACB_CAPS_MACB_IS_GEM			0x80000000
 
+/* LSO settings */
+#define MACB_LSO_UFO_ENABLE			0x01
+#define MACB_LSO_TSO_ENABLE			0x02
+
 /* Bit manipulation macros */
 #define MACB_BIT(name)					\
 	(1 << MACB_##name##_OFFSET)
@@ -545,6 +553,12 @@ struct macb_dma_desc {
 #define MACB_TX_LAST_SIZE			1
 #define MACB_TX_NOCRC_OFFSET			16
 #define MACB_TX_NOCRC_SIZE			1
+#define MACB_MSS_MFS_OFFSET			16
+#define MACB_MSS_MFS_SIZE			14
+#define MACB_TX_LSO_OFFSET			17
+#define MACB_TX_LSO_SIZE			2
+#define MACB_TX_TCP_SEQ_SRC_OFFSET		19
+#define MACB_TX_TCP_SEQ_SRC_SIZE		1
 #define MACB_TX_BUF_EXHAUSTED_OFFSET		27
 #define MACB_TX_BUF_EXHAUSTED_SIZE		1
 #define MACB_TX_UNDERRUN_OFFSET			28
-- 
2.4.5

^ permalink raw reply related

* Re: [PATCH net] sctp: fix the panic caused by route update
From: Neil Horman @ 2016-10-24 14:05 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
	Vlad Yasevich, daniel
In-Reply-To: <a0659fd9a23fc95fe9a586be4b57c8032c8a6060.1477242069.git.lucien.xin@gmail.com>

On Mon, Oct 24, 2016 at 01:01:09AM +0800, Xin Long wrote:
> Commit 7303a1475008 ("sctp: identify chunks that need to be fragmented
> at IP level") made the chunk be fragmented at IP level in the next round
> if it's size exceed PMTU.
> 
> But there still is another case, PMTU can be updated if transport's dst
> expires and transport's pmtu_pending is set in sctp_packet_transmit. If
> the new PMTU is less than the chunk, the same issue with that commit can
> be triggered.
> 
> So we should drop this packet and let it retransmit in another round
> where it would be fragmented at IP level.
> 
> This patch is to fix it by checking the chunk size after PMTU may be
> updated and dropping this packet if it's size exceed PMTU.
> 
> Fixes: 90017accff61 ("sctp: Add GSO support")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/sctp/output.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 2a5c189..6cb0df8 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -418,6 +418,7 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
>  	__u8 has_data = 0;
>  	int gso = 0;
>  	int pktcount = 0;
> +	int auth_len = 0;
>  	struct dst_entry *dst;
>  	unsigned char *auth = NULL;	/* pointer to auth in skb data */
>  
> @@ -510,7 +511,12 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
>  			list_for_each_entry(chunk, &packet->chunk_list, list) {
>  				int padded = SCTP_PAD4(chunk->skb->len);
>  
> -				if (pkt_size + padded > tp->pathmtu)
> +				if (chunk == packet->auth)
> +					auth_len = padded;
> +				else if (auth_len + padded + packet->overhead >
> +					 tp->pathmtu)
> +					goto nomem;
> +				else if (pkt_size + padded > tp->pathmtu)
>  					break;
>  				pkt_size += padded;
>  			}
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Acked-by: Neil Horman <nhorman@txudriver.com>

^ permalink raw reply

* Fwd: net/ipx: null-ptr-deref in ipxrtr_route_packet
From: Andrey Konovalov @ 2016-10-24 14:22 UTC (permalink / raw)
  To: acme, David S. Miller, netdev, LKML
  Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin,
	Eric Dumazet, Dmitry Vyukov
In-Reply-To: <CAAeHK+z1vkY=CdCHzvyzAgrsmOW_bH4OKSY6cCkiw_OJk3kXCQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2608 bytes --]

+acme@redhat.com

Hi,

I've got the following error report while running the syzkaller fuzzer:

kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN
Modules linked in:
CPU: 0 PID: 3953 Comm: syz-executor Not tainted 4.9.0-rc1+ #228
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff88006aa2ac00 task.stack: ffff880068a90000
RIP: 0010:[<ffffffff834ed8f4>]  [<ffffffff834ed8f4>]
ipxrtr_route_packet+0x4e4/0xbe0 net/ipx/ipx_route.c
:213
RSP: 0018:ffff880068a97b08  EFLAGS: 00010246
RAX: ffff88006b648500 RBX: ffff880068a97e40 RCX: dffffc0000000000
RDX: 0000000000000003 RSI: 0000000000000000 RDI: ffff88006b648960
RBP: ffff880068a97bc8 R08: dffffc0000000000 R09: 1ffff1000d4ddf97
R10: dffffc0000000000 R11: 0000000000000000 R12: ffff88006b410300
R13: 0000000000000000 R14: ffff88006444b68e R15: ffff88006a6efc80
FS:  00007f28cf665700(0000) GS:ffff88006cc00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000451f80 CR3: 0000000068a9a000 CR4: 00000000000006f0
Stack:
 ffff88006a6efd58 ffff880068a97dc0 ffff880068a97e44 1ffff1000d152f68
 000000000000001a 0000000000000000 ffff88006b648500 0000000041b58ab3
 ffffffff847fb90b ffffffff834ed410 ffffffff82b7cfea ffff8800ffffff97
Call Trace:
 [<ffffffff834e802e>] ipx_sendmsg+0x30e/0x550 net/ipx/af_ipx.c:1749
 [<     inline     >] sock_sendmsg_nosec net/socket.c:606
 [<ffffffff82b7075c>] sock_sendmsg+0xcc/0x110 net/socket.c:616
 [<ffffffff82b718a1>] SYSC_sendto+0x211/0x340 net/socket.c:1641
 [<ffffffff82b73e80>] SyS_sendto+0x40/0x50 net/socket.c:1609
 [<ffffffff83fc0141>] entry_SYSCALL_64_fastpath+0x1f/0xc2
arch/x86/entry/entry_64.S:209
Code: 41 80 7c 0d 00 00 0f 85 82 06 00 00 48 8b 85 70 ff ff ff 49 b8
00 00 00 00 00 fc ff df 4c 8b a8 60 04 00 00 4c 89 ee 48 c1 ee 03 <46>
0f b6 0c 06 45 84 c9 74 0a 41 80 f9 03 0f 8e e5 05 00 00 49
RIP  [<ffffffff834ed8f4>] ipxrtr_route_packet+0x4e4/0xbe0
net/ipx/ipx_route.c:213
 RSP <ffff880068a97b08>
---[ end trace f5bc9a28de6b2776 ]---
==================================================================

For some reason ipxs->intrfc ends up being NULL.

The reproducer is attached, you need to run a few instances simultaneously.

In case it's relevant, this is what I have in /etc/network/interfaces:

auto eth1
iface eth1 inet static
address 192.168.1.5
netmask 255.255.255.0
post-up arp -s 192.168.1.6 aa:aa:aa:aa:aa:aa
iface eth1 ipx static
frame EtherII
netnum 0x42424242

On commit 1a1891d762d6e64daf07b5be4817e3fbb29e3c59 (Oct 18).

[-- Attachment #2: ipx.c --]
[-- Type: text/x-csrc, Size: 7417 bytes --]

// autogenerated by syzkaller (http://github.com/google/syzkaller)

#ifndef __NR_sendto
#define __NR_sendto 44
#endif
#ifndef __NR_syz_fuse_mount
#define __NR_syz_fuse_mount 1000004
#endif
#ifndef __NR_syz_open_dev
#define __NR_syz_open_dev 1000002
#endif
#ifndef __NR_syz_test
#define __NR_syz_test 1000001
#endif
#ifndef __NR_mmap
#define __NR_mmap 9
#endif
#ifndef __NR_socket
#define __NR_socket 41
#endif
#ifndef __NR_bind
#define __NR_bind 49
#endif
#ifndef __NR_syz_fuseblk_mount
#define __NR_syz_fuseblk_mount 1000005
#endif
#ifndef __NR_syz_open_pts
#define __NR_syz_open_pts 1000003
#endif

#include <fcntl.h>
#include <pthread.h>
#include <setjmp.h>
#include <signal.h>
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <unistd.h>

__thread int skip_segv;
__thread jmp_buf segv_env;

static void segv_handler(int sig, siginfo_t* info, void* uctx)
{
  if (__atomic_load_n(&skip_segv, __ATOMIC_RELAXED))
    _longjmp(segv_env, 1);
  exit(sig);
}

static void install_segv_handler()
{
  struct sigaction sa;
  memset(&sa, 0, sizeof(sa));
  sa.sa_sigaction = segv_handler;
  sa.sa_flags = SA_NODEFER | SA_SIGINFO;
  sigaction(SIGSEGV, &sa, NULL);
  sigaction(SIGBUS, &sa, NULL);
}

#define NONFAILING(...)                                                \
  {                                                                    \
    __atomic_fetch_add(&skip_segv, 1, __ATOMIC_SEQ_CST);               \
    if (_setjmp(segv_env) == 0) {                                      \
      __VA_ARGS__;                                                     \
    }                                                                  \
    __atomic_fetch_sub(&skip_segv, 1, __ATOMIC_SEQ_CST);               \
  }

static uintptr_t syz_open_dev(uintptr_t a0, uintptr_t a1, uintptr_t a2)
{
  if (a0 == 0xc || a0 == 0xb) {
    char buf[128];
    sprintf(buf, "/dev/%s/%d:%d", a0 == 0xc ? "char" : "block",
            (uint8_t)a1, (uint8_t)a2);
    return open(buf, O_RDWR, 0);
  } else {
    char buf[1024];
    char* hash;
    strncpy(buf, (char*)a0, sizeof(buf));
    buf[sizeof(buf) - 1] = 0;
    while ((hash = strchr(buf, '#'))) {
      *hash = '0' + (char)(a1 % 10);
      a1 /= 10;
    }
    return open(buf, a2, 0);
  }
}

static uintptr_t syz_open_pts(uintptr_t a0, uintptr_t a1)
{
  int ptyno = 0;
  if (ioctl(a0, TIOCGPTN, &ptyno))
    return -1;
  char buf[128];
  sprintf(buf, "/dev/pts/%d", ptyno);
  return open(buf, a1, 0);
}

static uintptr_t syz_fuse_mount(uintptr_t a0, uintptr_t a1,
                                uintptr_t a2, uintptr_t a3,
                                uintptr_t a4, uintptr_t a5)
{
  uint64_t target = a0;
  uint64_t mode = a1;
  uint64_t uid = a2;
  uint64_t gid = a3;
  uint64_t maxread = a4;
  uint64_t flags = a5;

  int fd = open("/dev/fuse", O_RDWR);
  if (fd == -1)
    return fd;
  char buf[1024];
  sprintf(buf, "fd=%d,user_id=%ld,group_id=%ld,rootmode=0%o", fd,
          (long)uid, (long)gid, (unsigned)mode & ~3u);
  if (maxread != 0)
    sprintf(buf + strlen(buf), ",max_read=%ld", (long)maxread);
  if (mode & 1)
    strcat(buf, ",default_permissions");
  if (mode & 2)
    strcat(buf, ",allow_other");
  syscall(SYS_mount, "", target, "fuse", flags, buf);
  return fd;
}

static uintptr_t syz_fuseblk_mount(uintptr_t a0, uintptr_t a1,
                                   uintptr_t a2, uintptr_t a3,
                                   uintptr_t a4, uintptr_t a5,
                                   uintptr_t a6, uintptr_t a7)
{
  uint64_t target = a0;
  uint64_t blkdev = a1;
  uint64_t mode = a2;
  uint64_t uid = a3;
  uint64_t gid = a4;
  uint64_t maxread = a5;
  uint64_t blksize = a6;
  uint64_t flags = a7;

  int fd = open("/dev/fuse", O_RDWR);
  if (fd == -1)
    return fd;
  if (syscall(SYS_mknodat, AT_FDCWD, blkdev, S_IFBLK, makedev(7, 199)))
    return fd;
  char buf[256];
  sprintf(buf, "fd=%d,user_id=%ld,group_id=%ld,rootmode=0%o", fd,
          (long)uid, (long)gid, (unsigned)mode & ~3u);
  if (maxread != 0)
    sprintf(buf + strlen(buf), ",max_read=%ld", (long)maxread);
  if (blksize != 0)
    sprintf(buf + strlen(buf), ",blksize=%ld", (long)blksize);
  if (mode & 1)
    strcat(buf, ",default_permissions");
  if (mode & 2)
    strcat(buf, ",allow_other");
  syscall(SYS_mount, blkdev, target, "fuseblk", flags, buf);
  return fd;
}

static uintptr_t execute_syscall(int nr, uintptr_t a0, uintptr_t a1,
                                 uintptr_t a2, uintptr_t a3,
                                 uintptr_t a4, uintptr_t a5,
                                 uintptr_t a6, uintptr_t a7,
                                 uintptr_t a8)
{
  switch (nr) {
  default:
    return syscall(nr, a0, a1, a2, a3, a4, a5);
  case __NR_syz_test:
    return 0;
  case __NR_syz_open_dev:
    return syz_open_dev(a0, a1, a2);
  case __NR_syz_open_pts:
    return syz_open_pts(a0, a1);
  case __NR_syz_fuse_mount:
    return syz_fuse_mount(a0, a1, a2, a3, a4, a5);
  case __NR_syz_fuseblk_mount:
    return syz_fuseblk_mount(a0, a1, a2, a3, a4, a5, a6, a7);
  }
}

long r[18];

int main()
{
  install_segv_handler();
  memset(r, -1, sizeof(r));
  r[0] = execute_syscall(__NR_mmap, 0x20000000ul, 0xe000ul, 0x3ul,
                         0x32ul, 0xfffffffffffffffful, 0x0ul, 0, 0, 0);
  r[1] = execute_syscall(__NR_socket, 0x4ul, 0x2ul, 0x0ul, 0, 0, 0, 0,
                         0, 0);
  r[2] = execute_syscall(__NR_mmap, 0x2000e000ul, 0x1000ul, 0x3ul,
                         0x32ul, 0xfffffffffffffffful, 0x0ul, 0, 0, 0);
  NONFAILING(*(uint16_t*)0x2000e000 = (uint16_t)0x2);
  NONFAILING(*(uint16_t*)0x2000e002 = (uint16_t)0x4242);
  NONFAILING(*(uint32_t*)0x2000e004 = (uint32_t)0x0);
  NONFAILING(*(uint8_t*)0x2000e008 = (uint8_t)0x0);
  NONFAILING(*(uint8_t*)0x2000e009 = (uint8_t)0x0);
  NONFAILING(*(uint8_t*)0x2000e00a = (uint8_t)0x0);
  NONFAILING(*(uint8_t*)0x2000e00b = (uint8_t)0x0);
  NONFAILING(*(uint8_t*)0x2000e00c = (uint8_t)0x0);
  NONFAILING(*(uint8_t*)0x2000e00d = (uint8_t)0x0);
  NONFAILING(*(uint8_t*)0x2000e00e = (uint8_t)0x0);
  NONFAILING(*(uint8_t*)0x2000e00f = (uint8_t)0x0);
  r[14] = execute_syscall(__NR_bind, r[1], 0x2000e000ul, 0x10ul, 0, 0,
                          0, 0, 0, 0);
  NONFAILING(memcpy((void*)0x20007fe6, "\xae\x89\xfa\x41\xd0\x29\x69"
                                       "\xc3\xac\x79\xf0\x64\xc6\xad"
                                       "\x6b\x27\x63\x6b\x49\x0d\x99"
                                       "\x7b\x01\x9e\x3e\xb3",
                    26));
  NONFAILING(memcpy(
      (void*)0x2000d000,
      "\x04\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
      "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
      "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
      "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
      "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
      "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
      "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
      "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
      "\x00",
      128));
  r[17] = execute_syscall(__NR_sendto, r[1], 0x20007fe6ul, 0x1aul,
                          0x0ul, 0x2000d000ul, 0x80ul, 0, 0, 0);
  return 0;
}

^ permalink raw reply

* Re: UDP does not autobind on recv
From: Jiri Slaby @ 2016-10-24 14:23 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: ML netdev
In-Reply-To: <1477314180.7065.108.camel@edumazet-glaptop3.roam.corp.google.com>

On 10/24/2016, 03:03 PM, Eric Dumazet wrote:
> On Mon, 2016-10-24 at 14:54 +0200, Jiri Slaby wrote:
>> Hello,
>>
>> as per man 7 udp:
>>   In order to receive packets, the socket can be bound to
>>   a local  address first  by using bind(2).  Otherwise,
>>   the socket layer will automatically assign a free local
>>   port out of the range defined by /proc/sys/net/ipv4
>>   /ip_local_port_range and bind the socket to INADDR_ANY.
>>
>> I did not know that bind is unneeded, so I tried that. But it does not
>> work with this piece of code:
>> int main()
>> {
>>     char buf[128];
>>     int fd = socket(AF_INET, SOCK_DGRAM, 0);
>>     recv(fd, buf, sizeof(buf), 0);
>> }
> 
> autobind makes little sense at recv() time really.
> 
> How an application could expect to receive a frame to 'some socket'
> without even knowing its port ?

For example
        struct sockaddr_storage sa;
        socklen_t slen = sizeof(sa);
        recv(fd, buf, sizeof(buf), MSG_DONTWAIT);
        getsockname(fd, (struct sockaddr *)&sa, &slen);
        recv(fd, buf, sizeof(buf), 0);
works.

> How useful would that be exactly ?

No need for finding a free port and checking, for example.

> How TCP behaves ?

TCP is a completely different story. bind is documented to be required
there. (And listen and accept.)

> I would say, fix the documentation if it is not correct.

I don't have a problem with either. I have only found, that the
implementation differs from the documentation :). Is there some
supervisor documentation (like POSIX) which should we be in conformance to?

thanks,
-- 
js
suse labs

^ permalink raw reply

* Re: [PATCH] net: fec: hard phy reset on open
From: Manfred Schlaegl @ 2016-10-24 14:42 UTC (permalink / raw)
  To: Andy Duan; +Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <AM4PR0401MB22600E9D064FD15F23AAF68AFFA90@AM4PR0401MB2260.eurprd04.prod.outlook.com>

On 2016-10-24 16:03, Andy Duan wrote:
> From: manfred.schlaegl@gmx.at <manfred.schlaegl@gmx.at>  Sent: Monday, October 24, 2016 5:26 PM
>> To: Andy Duan <fugang.duan@nxp.com>
>> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: [PATCH] net: fec: hard phy reset on open
>>
>> We have seen some problems with auto negotiation on i.MX6 using LAN8720,
>> after interface down/up.
>>
>> In our configuration, the ptp clock is used externally as reference clock for
>> the phy. Some phys (e.g. LAN8720) needs a stable clock while and after hard
>> reset.
>> Before this patch, the driver disabled the clock on close but did no hard reset
>> on open, after enabling the clocks again.
>>
>> A solution that prevents disabling the clocks on close was considered, but
>> discarded because of bad power saving behavior.
>>
>> This patch saves the reset dt properties on probe and does a reset on every
>> open after clocks where enabled, to make sure the clock is stable while and
>> after hard reset.
>>
>> Tested on i.MX6 and i.MX28, both using LAN8720.
>>
>> Signed-off-by: Manfred Schlaegl <manfred.schlaegl@ginzinger.com>
>> ---

> This patch did hard reset to let phy stable.

> Firstly, you should do reset before clock enable.
I have to disagree here.
The phy demands(datasheet + tests) a stable clock at the time of (hard-)reset and after this. Therefore the clock has to be enabled before the hard reset.
(This is exactly the reason for the patch.)

Generally: The sense of a reset is to defer the start of digital circuit until the environment (power, clocks, ...) has stabilized.

Furthermore: Before this patch the hard reset was done in fec_probe. And here also after the clocks were enabled.

Whats was your argument to do it the other way in this special case?

> Secondly, we suggest to do phy reset in phy driver, not MAC driver.
I was not sure, if you meant a soft-, or hard-reset here.

In case you are talking about soft reset:
Yes, the phy drivers perform a soft reset. Sadly a soft reset is not sufficient in this case - The phy recovers only on a hard reset from lost clock. (datasheet + tests)

In case you're talking about hard reset:
Intuitively I would also think, that the (hard-)reset should be handled by the phy driver, but this is not reality in given implementations.

Documentation/devicetree/bindings/net/fsl-fec.txt says

- phy-reset-gpios : Should specify the gpio for phy reset

It is explicitly talked about phy-reset here. And the (hard-)reset was handled by the fec driver also before this patch (once on probe).

> 
> Regards,
> Andy

Thanks for your feedback!

Best regards,
Manfred



> 
>>  drivers/net/ethernet/freescale/fec.h      |  4 ++
>>  drivers/net/ethernet/freescale/fec_main.c | 77 +++++++++++++++++-------
>> -------
>>  2 files changed, 47 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/fec.h
>> b/drivers/net/ethernet/freescale/fec.h
>> index c865135..379e619 100644
>> --- a/drivers/net/ethernet/freescale/fec.h
>> +++ b/drivers/net/ethernet/freescale/fec.h
>> @@ -498,6 +498,10 @@ struct fec_enet_private {
>>  	struct clk *clk_enet_out;
>>  	struct clk *clk_ptp;
>>
>> +	int phy_reset;
>> +	bool phy_reset_active_high;
>> +	int phy_reset_msec;
>> +
>>  	bool ptp_clk_on;
>>  	struct mutex ptp_clk_mutex;
>>  	unsigned int num_tx_queues;
>> diff --git a/drivers/net/ethernet/freescale/fec_main.c
>> b/drivers/net/ethernet/freescale/fec_main.c
>> index 48a033e..8cc1ec5 100644
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>> @@ -2802,6 +2802,22 @@ static int fec_enet_alloc_buffers(struct
>> net_device *ndev)
>>  	return 0;
>>  }
>>
>> +static void fec_reset_phy(struct fec_enet_private *fep) {
>> +	if (!gpio_is_valid(fep->phy_reset))
>> +		return;
>> +
>> +	gpio_set_value_cansleep(fep->phy_reset, !!fep-
>>> phy_reset_active_high);
>> +
>> +	if (fep->phy_reset_msec > 20)
>> +		msleep(fep->phy_reset_msec);
>> +	else
>> +		usleep_range(fep->phy_reset_msec * 1000,
>> +			     fep->phy_reset_msec * 1000 + 1000);
>> +
>> +	gpio_set_value_cansleep(fep->phy_reset, !fep-
>>> phy_reset_active_high);
>> +}
>> +
>>  static int
>>  fec_enet_open(struct net_device *ndev)
>>  {
>> @@ -2817,6 +2833,8 @@ fec_enet_open(struct net_device *ndev)
>>  	if (ret)
>>  		goto clk_enable;
>>
>> +	fec_reset_phy(fep);
>> +
>>  	/* I should reset the ring buffers here, but I don't yet know
>>  	 * a simple way to do that.
>>  	 */
>> @@ -3183,52 +3201,39 @@ static int fec_enet_init(struct net_device *ndev)
>>  	return 0;
>>  }
>>
>> -#ifdef CONFIG_OF
>> -static void fec_reset_phy(struct platform_device *pdev)
>> +static int
>> +fec_get_reset_phy(struct platform_device *pdev, int *msec, int
>> *phy_reset,
>> +		  bool *active_high)
>>  {
>> -	int err, phy_reset;
>> -	bool active_high = false;
>> -	int msec = 1;
>> +	int err;
>>  	struct device_node *np = pdev->dev.of_node;
>>
>> -	if (!np)
>> -		return;
>> +	if (!np || !of_device_is_available(np))
>> +		return 0;
>>
>> -	of_property_read_u32(np, "phy-reset-duration", &msec);
>> +	of_property_read_u32(np, "phy-reset-duration", msec);
>>  	/* A sane reset duration should not be longer than 1s */
>> -	if (msec > 1000)
>> -		msec = 1;
>> +	if (*msec > 1000)
>> +		*msec = 1;
>>
>> -	phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
>> -	if (!gpio_is_valid(phy_reset))
>> -		return;
>> +	*phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
>> +	if (!gpio_is_valid(*phy_reset))
>> +		return 0;
>>
>> -	active_high = of_property_read_bool(np, "phy-reset-active-high");
>> +	*active_high = of_property_read_bool(np, "phy-reset-active-high");
>>
>> -	err = devm_gpio_request_one(&pdev->dev, phy_reset,
>> -			active_high ? GPIOF_OUT_INIT_HIGH :
>> GPIOF_OUT_INIT_LOW,
>> -			"phy-reset");
>> +	err = devm_gpio_request_one(&pdev->dev, *phy_reset,
>> +				    *active_high ?
>> +					GPIOF_OUT_INIT_HIGH :
>> +					GPIOF_OUT_INIT_LOW,
>> +				    "phy-reset");
>>  	if (err) {
>>  		dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n",
>> err);
>> -		return;
>> +		return err;
>>  	}
>>
>> -	if (msec > 20)
>> -		msleep(msec);
>> -	else
>> -		usleep_range(msec * 1000, msec * 1000 + 1000);
>> -
>> -	gpio_set_value_cansleep(phy_reset, !active_high);
>> -}
>> -#else /* CONFIG_OF */
>> -static void fec_reset_phy(struct platform_device *pdev) -{
>> -	/*
>> -	 * In case of platform probe, the reset has been done
>> -	 * by machine code.
>> -	 */
>> +	return 0;
>>  }
>> -#endif /* CONFIG_OF */
>>
>>  static void
>>  fec_enet_get_queue_num(struct platform_device *pdev, int *num_tx, int
>> *num_rx) @@ -3409,7 +3414,10 @@ fec_probe(struct platform_device
>> *pdev)
>>  	pm_runtime_set_active(&pdev->dev);
>>  	pm_runtime_enable(&pdev->dev);
>>
>> -	fec_reset_phy(pdev);
>> +	ret = fec_get_reset_phy(pdev, &fep->phy_reset_msec, &fep-
>>> phy_reset,
>> +				&fep->phy_reset_active_high);
>> +	if (ret)
>> +		goto failed_reset;
>>
>>  	if (fep->bufdesc_ex)
>>  		fec_ptp_init(pdev);
>> @@ -3467,6 +3475,7 @@ fec_probe(struct platform_device *pdev)
>>  failed_mii_init:
>>  failed_irq:
>>  failed_init:
>> +failed_reset:
>>  	fec_ptp_stop(pdev);
>>  	if (fep->reg_phy)
>>  		regulator_disable(fep->reg_phy);
>> --
>> 2.1.4

^ permalink raw reply

* Re: [PATCH v2 net] macsec: Fix header length if SCI is added if explicitly disabled
From: Sabrina Dubroca @ 2016-10-24 14:50 UTC (permalink / raw)
  To: Tobias Brunner; +Cc: David S. Miller, netdev
In-Reply-To: <77d75a4a-2307-6e3d-a4bb-7d7ff61bf3e8@strongswan.org>

2016-10-24, 15:44:26 +0200, Tobias Brunner wrote:
> Even if sending SCIs is explicitly disabled, the code that creates the
> Security Tag might still decide to add it (e.g. if multiple RX SCs are
> defined on the MACsec interface).
> But because the header length so far only depended on the configuration
> option the SCI overwrote the original frame's contents (EtherType and
> e.g. the beginning of the IP header) and if encrypted did not visibly
> end up in the packet, while the SC flag in the TCI field of the Security
> Tag was still set, resulting in invalid MACsec frames.
> 
> Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
> Signed-off-by: Tobias Brunner <tobias@strongswan.org>

Acked-by: Sabrina Dubroca <sd@queasysnail.net>

-- 
Sabrina

^ permalink raw reply

* [PATCH] ip6_tunnel: Clear IP6CB(skb)->frag_max_size in ip4ip6_tnl_xmit()
From: Eli Cooper @ 2016-10-24 15:07 UTC (permalink / raw)
  To: netdev, David S . Miller

skb->cb may contain data from previous layers, as shown in 5146d1f1511
("tunnel: Clear IPCB(skb)->opt before dst_link_failure called").
However, for ipip6 tunnels, clearing IPCB(skb)->opt alone is not enough,
because skb->cb is later misinterpreted as IP6CB(skb)->frag_max_size.

In the observed scenario, the garbage data made the max fragment size so
so small that packets sent through the tunnel are mistakenly fragmented.

This patch clears IP6CB(skb)->frag_max_size for ipip6 tunnels.

Signed-off-by: Eli Cooper <elicooper@gmx.com>
---
 net/ipv6/ip6_tunnel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 202d16a..4110562 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1205,6 +1205,7 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 	int err;
 
 	memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
+	IP6CB(skb)->frag_max_size = 0;
 
 	tproto = ACCESS_ONCE(t->parms.proto);
 	if (tproto != IPPROTO_IPIP && tproto != 0)
-- 
2.10.1

^ permalink raw reply related

* [PATCH v2 RESEND] xen-netback: prefer xenbus_scanf() over xenbus_gather()
From: Jan Beulich @ 2016-10-24 15:08 UTC (permalink / raw)
  To: Paul Durrant, Wei Liu
  Cc: david.vrabel, xen-devel, boris.ostrovsky, Juergen Gross, netdev

For single items being collected this should be preferred as being more
typesafe (as the compiler can check format string and to-be-written-to
variable match) and more efficient (requiring one less parameter to be
passed).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Avoid commit message to continue from subject.
---
 drivers/net/xen-netback/xenbus.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

--- 4.9-rc2/drivers/net/xen-netback/xenbus.c
+++ 4.9-rc2-xen-netback-prefer-xenbus_scanf/drivers/net/xen-netback/xenbus.c
@@ -889,16 +889,16 @@ static int connect_ctrl_ring(struct back
 	unsigned int evtchn;
 	int err;
 
-	err = xenbus_gather(XBT_NIL, dev->otherend,
-			    "ctrl-ring-ref", "%u", &val, NULL);
-	if (err)
+	err = xenbus_scanf(XBT_NIL, dev->otherend,
+			   "ctrl-ring-ref", "%u", &val);
+	if (err <= 0)
 		goto done; /* The frontend does not have a control ring */
 
 	ring_ref = val;
 
-	err = xenbus_gather(XBT_NIL, dev->otherend,
-			    "event-channel-ctrl", "%u", &val, NULL);
-	if (err) {
+	err = xenbus_scanf(XBT_NIL, dev->otherend,
+			   "event-channel-ctrl", "%u", &val);
+	if (err <= 0) {
 		xenbus_dev_fatal(dev, err,
 				 "reading %s/event-channel-ctrl",
 				 dev->otherend);
@@ -919,7 +919,7 @@ done:
 	return 0;
 
 fail:
-	return err;
+	return err ?: -ENODATA;
 }
 
 static void connect(struct backend_info *be)

^ permalink raw reply

* net/sctp: slab-out-of-bounds in sctp_sf_ootb
From: Andrey Konovalov @ 2016-10-24 15:30 UTC (permalink / raw)
  To: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML
  Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet,
	Dmitry Vyukov

Hi,

I've got the following error report while running the syzkaller fuzzer:

==================================================================
BUG: KASAN: slab-out-of-bounds in sctp_sf_ootb+0x634/0x6c0 at addr
ffff88006bc1f210
Read of size 2 by task syz-executor/13493
CPU: 3 PID: 13493 Comm: syz-executor Not tainted 4.9.0-rc2+ #300
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 ffff88003e256e40 ffffffff81b47264 ffff88003e80ccc0 ffff88006bc1eed8
 ffff88006bc1f210 ffff88006bc1eed0 ffff88003e256e68 ffffffff8150b34c
 ffff88003e256ef8 ffff88003e80ccc0 ffff8800ebc1f210 ffff88003e256ee8
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff81b47264>] dump_stack+0xb3/0x10f lib/dump_stack.c:51
 [<ffffffff8150b34c>] kasan_object_err+0x1c/0x70 mm/kasan/report.c:156
 [<     inline     >] print_address_description mm/kasan/report.c:194
 [<ffffffff8150b5e7>] kasan_report_error+0x1f7/0x4d0 mm/kasan/report.c:283
 [<     inline     >] kasan_report mm/kasan/report.c:303
 [<ffffffff8150bb7a>] __asan_report_load_n_noabort+0x3a/0x40
mm/kasan/report.c:334
 [<ffffffff838d6ad4>] sctp_sf_ootb+0x634/0x6c0 net/sctp/sm_statefuns.c:3448
 [<ffffffff838e5914>] sctp_do_sm+0x104/0x4ed0 net/sctp/sm_sideeffect.c:1108
 [<ffffffff838f13cd>] sctp_endpoint_bh_rcv+0x32d/0x9c0
net/sctp/endpointola.c:452
 [<ffffffff839168a4>] sctp_inq_push+0x134/0x1a0 net/sctp/inqueue.c:95
 [<ffffffff83950f68>] sctp_rcv+0x1fa8/0x2d00 net/sctp/input.c:268
 [<ffffffff8306aae2>] ip_local_deliver_finish+0x332/0xad0
net/ipv4/ip_input.c:216
 [<     inline     >] NF_HOOK_THRESH include/linux/netfilter.h:232
 [<     inline     >] NF_HOOK include/linux/netfilter.h:255
 [<ffffffff8306b992>] ip_local_deliver+0x1c2/0x4b0 net/ipv4/ip_input.c:257
 [<     inline     >] dst_input include/net/dst.h:507
 [<ffffffff830692c0>] ip_rcv_finish+0x750/0x1c40 net/ipv4/ip_input.c:396
 [<     inline     >] NF_HOOK_THRESH include/linux/netfilter.h:232
 [<     inline     >] NF_HOOK include/linux/netfilter.h:255
 [<ffffffff8306c5ef>] ip_rcv+0x96f/0x12f0 net/ipv4/ip_input.c:487
 [<ffffffff82bdafb7>] __netif_receive_skb_core+0x1897/0x2a50 net/core/dev.c:4212
 [<ffffffff82bdc19a>] __netif_receive_skb+0x2a/0x170 net/core/dev.c:4250
 [<ffffffff82bdc493>] netif_receive_skb_internal+0x1b3/0x390 net/core/dev.c:4278
 [<ffffffff82bdc6b8>] netif_receive_skb+0x48/0x250 net/core/dev.c:4302
 [<ffffffff82420705>] tun_get_user+0xbd5/0x28a0 drivers/net/tun.c:1308
 [<ffffffff824225ea>] tun_chr_write_iter+0xda/0x190 drivers/net/tun.c:1332
 [<     inline     >] new_sync_write fs/read_write.c:499
 [<ffffffff8151c954>] __vfs_write+0x334/0x570 fs/read_write.c:512
 [<ffffffff8152046b>] vfs_write+0x17b/0x500 fs/read_write.c:560
 [<     inline     >] SYSC_write fs/read_write.c:607
 [<ffffffff81523d94>] SyS_write+0xd4/0x1a0 fs/read_write.c:599
 [<ffffffff83fc0941>] entry_SYSCALL_64_fastpath+0x1f/0xc2
Object at ffff88006bc1eed8, in cache kmalloc-512 size: 512
Allocated:
PID = 9755
 [  182.804017] [<ffffffff8107e236>] save_stack_trace+0x16/0x20
arch/x86/kernel/stacktrace.c:57
 [  182.804017] [<ffffffff8150a6b6>] save_stack+0x46/0xd0 mm/kasan/kasan.c:495
 [  182.804017] [<     inline     >] set_track mm/kasan/kasan.c:507
 [  182.804017] [<ffffffff8150a92b>] kasan_kmalloc+0xab/0xe0
mm/kasan/kasan.c:598
 [  182.804017] [<ffffffff8150ae92>] kasan_slab_alloc+0x12/0x20
mm/kasan/kasan.c:537
 [  182.804017] [<     inline     >] slab_post_alloc_hook mm/slab.h:417
 [  182.804017] [<     inline     >] slab_alloc_node mm/slub.c:2708
 [  182.804017] [<ffffffff81509beb>]
__kmalloc_node_track_caller+0xcb/0x390 mm/slub.c:4270
 [  182.804017] [<ffffffff82b8a5d1>]
__kmalloc_reserve.isra.35+0x41/0xe0 net/core/skbuff.c:138
 [  182.804017] [<ffffffff82b90b00>] __alloc_skb+0xf0/0x600
net/core/skbuff.c:231
 [  182.804017] [<     inline     >] alloc_skb include/linux/skbuff.h:921
 [  182.804017] [<ffffffff82b7d5b3>] sock_wmalloc+0xa3/0xf0 net/core/sock.c:1757
 [  182.804017] [<ffffffff8307c948>]
__ip_append_data.isra.46+0x1e38/0x28c0 net/ipv4/ip_output.c:1010
 [  182.804017] [<ffffffff8307d4c1>] ip_append_data.part.47+0xf1/0x170
net/ipv4/ip_output.c:1201
 [  182.804017] [<     inline     >] ip_append_data net/ipv4/ip_output.c:1605
 [  182.804017] [<ffffffff83089851>] ip_send_unicast_reply+0x831/0xe20
net/ipv4/ip_output.c:1605
 [  182.804017] [<ffffffff8311132a>] tcp_v4_send_reset+0xb0a/0x1700
net/ipv4/tcp_ipv4.c:696
 [  182.804017] [<ffffffff831167cb>] tcp_v4_rcv+0x198b/0x2e50
net/ipv4/tcp_ipv4.c:1719
 [  182.804017] [<ffffffff8306aae2>]
ip_local_deliver_finish+0x332/0xad0 net/ipv4/ip_input.c:216
 [  182.804017] [<     inline     >] NF_HOOK_THRESH
include/linux/netfilter.h:232
 [  182.804017] [<     inline     >] NF_HOOK include/linux/netfilter.h:255
 [  182.804017] [<ffffffff8306b992>] ip_local_deliver+0x1c2/0x4b0
net/ipv4/ip_input.c:257
 [  182.804017] [<     inline     >] dst_input include/net/dst.h:507
 [  182.804017] [<ffffffff830692c0>] ip_rcv_finish+0x750/0x1c40
net/ipv4/ip_input.c:396
 [  182.804017] [<     inline     >] NF_HOOK_THRESH
include/linux/netfilter.h:232
 [  182.804017] [<     inline     >] NF_HOOK include/linux/netfilter.h:255
 [  182.804017] [<ffffffff8306c5ef>] ip_rcv+0x96f/0x12f0 net/ipv4/ip_input.c:487
 [  182.804017] [<ffffffff82bdafb7>]
__netif_receive_skb_core+0x1897/0x2a50 net/core/dev.c:4212
 [  182.804017] [<ffffffff82bdc19a>] __netif_receive_skb+0x2a/0x170
net/core/dev.c:4250
 [  182.804017] [<ffffffff82bdc493>]
netif_receive_skb_internal+0x1b3/0x390 net/core/dev.c:4278
 [  182.804017] [<ffffffff82bdc6b8>] netif_receive_skb+0x48/0x250
net/core/dev.c:4302
 [  182.804017] [<ffffffff82420705>] tun_get_user+0xbd5/0x28a0
drivers/net/tun.c:1308
 [  182.804017] [<ffffffff824225ea>] tun_chr_write_iter+0xda/0x190
drivers/net/tun.c:1332
 [  182.804017] [<     inline     >] new_sync_write fs/read_write.c:499
 [  182.804017] [<ffffffff8151c954>] __vfs_write+0x334/0x570 fs/read_write.c:512
 [  182.804017] [<ffffffff8152046b>] vfs_write+0x17b/0x500 fs/read_write.c:560
 [  182.804017] [<     inline     >] SYSC_write fs/read_write.c:607
 [  182.804017] [<ffffffff81523d94>] SyS_write+0xd4/0x1a0 fs/read_write.c:599
 [  182.804017] [<ffffffff83fc0941>] entry_SYSCALL_64_fastpath+0x1f/0xc2
Freed:
PID = 9207
 [  182.804017] [<ffffffff8107e236>] save_stack_trace+0x16/0x20
arch/x86/kernel/stacktrace.c:57
 [  182.804017] [<ffffffff8150a6b6>] save_stack+0x46/0xd0 mm/kasan/kasan.c:495
 [  182.804017] [<     inline     >] set_track mm/kasan/kasan.c:507
 [  182.804017] [<ffffffff8150af13>] kasan_slab_free+0x73/0xc0
mm/kasan/kasan.c:571
 [  182.804017] [<     inline     >] slab_free_hook mm/slub.c:1352
 [  182.804017] [<     inline     >] slab_free_freelist_hook mm/slub.c:1374
 [  182.804017] [<     inline     >] slab_free mm/slub.c:2951
 [  182.804017] [<ffffffff815073f8>] kfree+0xe8/0x2b0 mm/slub.c:3871
 [  182.804017] [<ffffffff82b896b8>] skb_free_head+0x78/0xb0
net/core/skbuff.c:580
 [  182.804017] [<ffffffff82b92748>] skb_release_data+0x308/0x3a0
net/core/skbuff.c:611
 [  182.804017] [<ffffffff82b9282a>] skb_release_all+0x4a/0x60
net/core/skbuff.c:670
 [  182.804017] [<ffffffff82b92855>] __kfree_skb+0x15/0x20 net/core/skbuff.c:684
 [  182.804017] [<ffffffff82b92964>] kfree_skb+0x104/0x300 net/core/skbuff.c:705
 [  182.804017] [<ffffffff8306902e>] ip_rcv_finish+0x4be/0x1c40
net/ipv4/ip_input.c:399
 [  182.804017] [<     inline     >] NF_HOOK_THRESH
include/linux/netfilter.h:232
 [  182.804017] [<     inline     >] NF_HOOK include/linux/netfilter.h:255
 [  182.804017] [<ffffffff8306c5ef>] ip_rcv+0x96f/0x12f0 net/ipv4/ip_input.c:487
 [  182.804017] [<ffffffff82bdafb7>]
__netif_receive_skb_core+0x1897/0x2a50 net/core/dev.c:4212
 [  182.804017] [<ffffffff82bdc19a>] __netif_receive_skb+0x2a/0x170
net/core/dev.c:4250
 [  182.804017] [<ffffffff82bdc493>]
netif_receive_skb_internal+0x1b3/0x390 net/core/dev.c:4278
 [  182.804017] [<ffffffff82bdc6b8>] netif_receive_skb+0x48/0x250
net/core/dev.c:4302
 [  182.804017] [<ffffffff82420705>] tun_get_user+0xbd5/0x28a0
drivers/net/tun.c:1308
 [  182.804017] [<ffffffff824225ea>] tun_chr_write_iter+0xda/0x190
drivers/net/tun.c:1332
 [  182.804017] [<     inline     >] new_sync_write fs/read_write.c:499
 [  182.804017] [<ffffffff8151c954>] __vfs_write+0x334/0x570 fs/read_write.c:512
 [  182.804017] [<ffffffff8152046b>] vfs_write+0x17b/0x500 fs/read_write.c:560
 [  182.804017] [<     inline     >] SYSC_write fs/read_write.c:607
 [  182.804017] [<ffffffff81523d94>] SyS_write+0xd4/0x1a0 fs/read_write.c:599
 [  182.804017] [<ffffffff83fc0941>] entry_SYSCALL_64_fastpath+0x1f/0xc2
Memory state around the buggy address:
 ffff88006bc1f100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 ffff88006bc1f180: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff88006bc1f200: fc fc fc fc fc fc fb fb fb fb fb fb fb fb fb fb
                         ^
 ffff88006bc1f280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff88006bc1f300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================

The problem is that sctp_walk_errors walks the chunk before its length
is checked for overflow.

On commit 07d9a380680d1c0eb51ef87ff2eab5c994949e69 (Oct 23).

^ permalink raw reply

* Re: question about function igmp_stop_timer() in net/ipv4/igmp.c
From: Andrew Lunn @ 2016-10-24 15:32 UTC (permalink / raw)
  To: Dongpo Li; +Cc: netdev
In-Reply-To: <580DF574.6070905@hisilicon.com>

On Mon, Oct 24, 2016 at 07:50:12PM +0800, Dongpo Li wrote:
> Hello
> 
> We encountered a multicast problem when two set-top box(STB) join the same multicast group and leave.
> The two boxes can join the same multicast group
> but only one box can send the IGMP leave group message when leave,
> the other box does not send the IGMP leave message.
> Our boxes use the IGMP version 2.
> 
> I added some debug info and found the whole procedure is like this:
> (1) Box A joins the multicast group 225.1.101.145 and send the IGMP v2 membership report(join group).
> (2) Box B joins the same multicast group 225.1.101.145 and also send the IGMP v2 membership report(join group).
> (3) Box A receives the IGMP membership report from Box B and kernel calls igmp_heard_report().
>     This function will call igmp_stop_timer(im).
>     In function igmp_stop_timer(im), it tries to delete IGMP timer and does the following:
>         im->tm_running = 0;
>         im->reporter = 0;
> (4) Box A leaves the multicast group 225.1.101.145 and kernel calls
>     ip_mc_leave_group -> ip_mc_dec_group -> igmp_group_dropped.
>     But in function igmp_group_dropped(), the im->reporter is 0, so the kernel does not send the IGMP leave message.

RFC 2236 says:

2.  Introduction

   The Internet Group Management Protocol (IGMP) is used by IP hosts to
   report their multicast group memberships to any immediately-
   neighboring multicast routers.

Are Box A or B multicast routers?

    Andrew

^ permalink raw reply

* [PATCH] netfilter: ip_vs_sync: fix bogus maybe-uninitialized warning
From: Arnd Bergmann @ 2016-10-24 15:34 UTC (permalink / raw)
  To: Wensong Zhang, Simon Horman, Julian Anastasov
  Cc: Arnd Bergmann, Pablo Neira Ayuso, Patrick McHardy,
	Jozsef Kadlecsik, David S. Miller, Quentin Armitage, netdev,
	lvs-devel, netfilter-devel, coreteam, linux-kernel

Building the ip_vs_sync code with CONFIG_OPTIMIZE_INLINING on x86
confuses the compiler to the point where it produces a rather
dubious warning message:

net/netfilter/ipvs/ip_vs_sync.c:1073:33: error: ‘opt.init_seq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  struct ip_vs_sync_conn_options opt;
                                 ^~~
net/netfilter/ipvs/ip_vs_sync.c:1073:33: error: ‘opt.delta’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
net/netfilter/ipvs/ip_vs_sync.c:1073:33: error: ‘opt.previous_delta’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
net/netfilter/ipvs/ip_vs_sync.c:1073:33: error: ‘*((void *)&opt+12).init_seq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
net/netfilter/ipvs/ip_vs_sync.c:1073:33: error: ‘*((void *)&opt+12).delta’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
net/netfilter/ipvs/ip_vs_sync.c:1073:33: error: ‘*((void *)&opt+12).previous_delta’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

The problem appears to be a combination of a number of factors, including
the __builtin_bswap32 compiler builtin being slightly odd, having a large
amount of code inlined into a single function, and the way that some
functions only get partially inlined here.

I've spent way too much time trying to work out a way to improve the
code, but the best I've come up with is to add an explicit memset
right before the ip_vs_seq structure is first initialized here. When
the compiler works correctly, this has absolutely no effect, but in the
case that produces the warning, the warning disappears.

In the process of analysing this warning, I also noticed that
we use memcpy to copy the larger ip_vs_sync_conn_options structure
over two members of the ip_vs_conn structure. This works because
the layout is identical, but seems error-prone, so I'm changing
this in the process to directly copy the two members. This change
seemed to have no effect on the object code or the warning, but
it deals with the same data, so I kept the two changes together.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/netfilter/ipvs/ip_vs_sync.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 1b07578bedf3..9350530c16c1 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -283,6 +283,7 @@ struct ip_vs_sync_buff {
  */
 static void ntoh_seq(struct ip_vs_seq *no, struct ip_vs_seq *ho)
 {
+	memset(ho, 0, sizeof(*ho));
 	ho->init_seq       = get_unaligned_be32(&no->init_seq);
 	ho->delta          = get_unaligned_be32(&no->delta);
 	ho->previous_delta = get_unaligned_be32(&no->previous_delta);
@@ -917,8 +918,10 @@ static void ip_vs_proc_conn(struct netns_ipvs *ipvs, struct ip_vs_conn_param *pa
 			kfree(param->pe_data);
 	}
 
-	if (opt)
-		memcpy(&cp->in_seq, opt, sizeof(*opt));
+	if (opt) {
+		cp->in_seq = opt->in_seq;
+		cp->out_seq = opt->out_seq;
+	}
 	atomic_set(&cp->in_pkts, sysctl_sync_threshold(ipvs));
 	cp->state = state;
 	cp->old_state = cp->state;
-- 
2.9.0

^ permalink raw reply related

* RE: [PATCH] net: fec: hard phy reset on open
From: Andy Duan @ 2016-10-24 14:03 UTC (permalink / raw)
  To: Manfred Schlaegl; +Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <7c3c37c0-7c24-a63c-b441-b1e8085a3c96@gmx.at>

From: manfred.schlaegl@gmx.at <manfred.schlaegl@gmx.at>  Sent: Monday, October 24, 2016 5:26 PM
> To: Andy Duan <fugang.duan@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH] net: fec: hard phy reset on open
> 
> We have seen some problems with auto negotiation on i.MX6 using LAN8720,
> after interface down/up.
> 
> In our configuration, the ptp clock is used externally as reference clock for
> the phy. Some phys (e.g. LAN8720) needs a stable clock while and after hard
> reset.
> Before this patch, the driver disabled the clock on close but did no hard reset
> on open, after enabling the clocks again.
> 
> A solution that prevents disabling the clocks on close was considered, but
> discarded because of bad power saving behavior.
> 
> This patch saves the reset dt properties on probe and does a reset on every
> open after clocks where enabled, to make sure the clock is stable while and
> after hard reset.
> 
> Tested on i.MX6 and i.MX28, both using LAN8720.
> 
> Signed-off-by: Manfred Schlaegl <manfred.schlaegl@ginzinger.com>
> ---
This patch did hard reset to let phy stable.
Firstly, you should do reset before clock enable.
Secondly, we suggest to do phy reset in phy driver, not MAC driver.

Regards,
Andy

>  drivers/net/ethernet/freescale/fec.h      |  4 ++
>  drivers/net/ethernet/freescale/fec_main.c | 77 +++++++++++++++++-------
> -------
>  2 files changed, 47 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec.h
> b/drivers/net/ethernet/freescale/fec.h
> index c865135..379e619 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -498,6 +498,10 @@ struct fec_enet_private {
>  	struct clk *clk_enet_out;
>  	struct clk *clk_ptp;
> 
> +	int phy_reset;
> +	bool phy_reset_active_high;
> +	int phy_reset_msec;
> +
>  	bool ptp_clk_on;
>  	struct mutex ptp_clk_mutex;
>  	unsigned int num_tx_queues;
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 48a033e..8cc1ec5 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -2802,6 +2802,22 @@ static int fec_enet_alloc_buffers(struct
> net_device *ndev)
>  	return 0;
>  }
> 
> +static void fec_reset_phy(struct fec_enet_private *fep) {
> +	if (!gpio_is_valid(fep->phy_reset))
> +		return;
> +
> +	gpio_set_value_cansleep(fep->phy_reset, !!fep-
> >phy_reset_active_high);
> +
> +	if (fep->phy_reset_msec > 20)
> +		msleep(fep->phy_reset_msec);
> +	else
> +		usleep_range(fep->phy_reset_msec * 1000,
> +			     fep->phy_reset_msec * 1000 + 1000);
> +
> +	gpio_set_value_cansleep(fep->phy_reset, !fep-
> >phy_reset_active_high);
> +}
> +
>  static int
>  fec_enet_open(struct net_device *ndev)
>  {
> @@ -2817,6 +2833,8 @@ fec_enet_open(struct net_device *ndev)
>  	if (ret)
>  		goto clk_enable;
> 
> +	fec_reset_phy(fep);
> +
>  	/* I should reset the ring buffers here, but I don't yet know
>  	 * a simple way to do that.
>  	 */
> @@ -3183,52 +3201,39 @@ static int fec_enet_init(struct net_device *ndev)
>  	return 0;
>  }
> 
> -#ifdef CONFIG_OF
> -static void fec_reset_phy(struct platform_device *pdev)
> +static int
> +fec_get_reset_phy(struct platform_device *pdev, int *msec, int
> *phy_reset,
> +		  bool *active_high)
>  {
> -	int err, phy_reset;
> -	bool active_high = false;
> -	int msec = 1;
> +	int err;
>  	struct device_node *np = pdev->dev.of_node;
> 
> -	if (!np)
> -		return;
> +	if (!np || !of_device_is_available(np))
> +		return 0;
> 
> -	of_property_read_u32(np, "phy-reset-duration", &msec);
> +	of_property_read_u32(np, "phy-reset-duration", msec);
>  	/* A sane reset duration should not be longer than 1s */
> -	if (msec > 1000)
> -		msec = 1;
> +	if (*msec > 1000)
> +		*msec = 1;
> 
> -	phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
> -	if (!gpio_is_valid(phy_reset))
> -		return;
> +	*phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
> +	if (!gpio_is_valid(*phy_reset))
> +		return 0;
> 
> -	active_high = of_property_read_bool(np, "phy-reset-active-high");
> +	*active_high = of_property_read_bool(np, "phy-reset-active-high");
> 
> -	err = devm_gpio_request_one(&pdev->dev, phy_reset,
> -			active_high ? GPIOF_OUT_INIT_HIGH :
> GPIOF_OUT_INIT_LOW,
> -			"phy-reset");
> +	err = devm_gpio_request_one(&pdev->dev, *phy_reset,
> +				    *active_high ?
> +					GPIOF_OUT_INIT_HIGH :
> +					GPIOF_OUT_INIT_LOW,
> +				    "phy-reset");
>  	if (err) {
>  		dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n",
> err);
> -		return;
> +		return err;
>  	}
> 
> -	if (msec > 20)
> -		msleep(msec);
> -	else
> -		usleep_range(msec * 1000, msec * 1000 + 1000);
> -
> -	gpio_set_value_cansleep(phy_reset, !active_high);
> -}
> -#else /* CONFIG_OF */
> -static void fec_reset_phy(struct platform_device *pdev) -{
> -	/*
> -	 * In case of platform probe, the reset has been done
> -	 * by machine code.
> -	 */
> +	return 0;
>  }
> -#endif /* CONFIG_OF */
> 
>  static void
>  fec_enet_get_queue_num(struct platform_device *pdev, int *num_tx, int
> *num_rx) @@ -3409,7 +3414,10 @@ fec_probe(struct platform_device
> *pdev)
>  	pm_runtime_set_active(&pdev->dev);
>  	pm_runtime_enable(&pdev->dev);
> 
> -	fec_reset_phy(pdev);
> +	ret = fec_get_reset_phy(pdev, &fep->phy_reset_msec, &fep-
> >phy_reset,
> +				&fep->phy_reset_active_high);
> +	if (ret)
> +		goto failed_reset;
> 
>  	if (fep->bufdesc_ex)
>  		fec_ptp_init(pdev);
> @@ -3467,6 +3475,7 @@ fec_probe(struct platform_device *pdev)
>  failed_mii_init:
>  failed_irq:
>  failed_init:
> +failed_reset:
>  	fec_ptp_stop(pdev);
>  	if (fep->reg_phy)
>  		regulator_disable(fep->reg_phy);
> --
> 2.1.4

^ permalink raw reply

* [PATCH] staging: rtl8192x: fix bogus maybe-uninitialized warning
From: Arnd Bergmann @ 2016-10-24 15:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: devel, Arnd Bergmann, netdev, linux-wireless, linux-kernel,
	Kurt Kanzenbach

The rtllib_rx_extract_addr() is supposed to set up the mac addresses
for four possible cases, based on two bits of input data. For
some reason, gcc decides that it's possible that none of the these
four cases apply and the addresses remain uninitialized:

drivers/staging/rtl8192e/rtllib_rx.c: In function ‘rtllib_rx_InfraAdhoc’:
include/linux/etherdevice.h:316:61: error: ‘*((void *)&dst+4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/staging/rtl8192e/rtllib_rx.c:1318:5: note: ‘*((void *)&dst+4)’ was declared here
ded from /git/arm-soc/drivers/staging/rtl8192e/rtllib_rx.c:40:0:
include/linux/etherdevice.h:316:36: error: ‘dst’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/staging/rtl8192e/rtllib_rx.c:1318:5: note: ‘dst’ was declared here

This warning is clearly nonsense, but changing the last case into
'default' makes it obvious to the compiler too, which avoids the
warning and probably leads to better object code too.

As the same warning appears in other files that have the exact
same code, I'm fixing it in both rtl8192e and rtl8192u, even
though I did not observe it for the latter.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/rtl8192e/rtllib_rx.c                      | 2 +-
 drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 2 +-
 drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c         | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib_rx.c b/drivers/staging/rtl8192e/rtllib_rx.c
index c743182b933e..d6777ecda64d 100644
--- a/drivers/staging/rtl8192e/rtllib_rx.c
+++ b/drivers/staging/rtl8192e/rtllib_rx.c
@@ -986,7 +986,7 @@ static void rtllib_rx_extract_addr(struct rtllib_device *ieee,
 		ether_addr_copy(src, hdr->addr4);
 		ether_addr_copy(bssid, ieee->current_network.bssid);
 		break;
-	case 0:
+	default:
 		ether_addr_copy(dst, hdr->addr1);
 		ether_addr_copy(src, hdr->addr2);
 		ether_addr_copy(bssid, hdr->addr3);
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 6fa96d57d316..e68850897adf 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -553,7 +553,7 @@ static void michael_mic_hdr(struct sk_buff *skb, u8 *hdr)
 		memcpy(hdr, hdr11->addr3, ETH_ALEN); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr4, ETH_ALEN); /* SA */
 		break;
-	case 0:
+	default:
 		memcpy(hdr, hdr11->addr1, ETH_ALEN); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr2, ETH_ALEN); /* SA */
 		break;
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
index 89cbc077a48d..2e4d2d7bc2e7 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
@@ -1079,7 +1079,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
 		memcpy(src, hdr->addr4, ETH_ALEN);
 		memcpy(bssid, ieee->current_network.bssid, ETH_ALEN);
 		break;
-	case 0:
+	default:
 		memcpy(dst, hdr->addr1, ETH_ALEN);
 		memcpy(src, hdr->addr2, ETH_ALEN);
 		memcpy(bssid, hdr->addr3, ETH_ALEN);
-- 
2.9.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related

* [PATCH] wireless: fix bogus maybe-uninitialized warning
From: Arnd Bergmann @ 2016-10-24 15:38 UTC (permalink / raw)
  To: Kalle Valo, Stanislav Yakovlev, Jouni Malinen, Johannes Berg,
	David S. Miller
  Cc: linux-wireless, netdev, Arnd Bergmann, linux-kernel
In-Reply-To: <20161024153918.2810634-1-arnd@arndb.de>

The hostap_80211_rx() function is supposed to set up the mac addresses
for four possible cases, based on two bits of input data. For
some reason, gcc decides that it's possible that none of the these
four cases apply and the addresses remain uninitialized:

drivers/net/wireless/intersil/hostap/hostap_80211_rx.c: In function ‘hostap_80211_rx’:
arch/x86/include/asm/string_32.h:77:14: warning: ‘src’ may be used uninitialized in this function [-Wmaybe-uninitialized]
drivers/net/wireless/intel/ipw2x00/libipw_rx.c: In function ‘libipw_rx’:
arch/x86/include/asm/string_32.h:77:14: error: ‘dst’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
arch/x86/include/asm/string_32.h:78:22: error: ‘*((void *)&dst+4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This warning is clearly nonsense, but changing the last case into
'default' makes it obvious to the compiler too, which avoids the
warning and probably leads to better object code too.

The same code is duplicated several times in the kernel, so this
patch uses the same workaround for all copies. The exact configuration
was hit only very rarely in randconfig builds and I only saw it
in three drivers, but I assume that all of them are potentially
affected, and it's better to keep the code consistent.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/wireless/ath/ath6kl/wmi.c                  | 8 ++++----
 drivers/net/wireless/intel/ipw2x00/libipw_rx.c         | 2 +-
 drivers/net/wireless/intersil/hostap/hostap_80211_rx.c | 2 +-
 net/wireless/lib80211_crypt_tkip.c                     | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index 3fd1cc98fd2f..84a6d12c3f8a 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -421,10 +421,6 @@ int ath6kl_wmi_dot11_hdr_remove(struct wmi *wmi, struct sk_buff *skb)
 
 	switch ((le16_to_cpu(wh.frame_control)) &
 		(IEEE80211_FCTL_FROMDS | IEEE80211_FCTL_TODS)) {
-	case 0:
-		memcpy(eth_hdr.h_dest, wh.addr1, ETH_ALEN);
-		memcpy(eth_hdr.h_source, wh.addr2, ETH_ALEN);
-		break;
 	case IEEE80211_FCTL_TODS:
 		memcpy(eth_hdr.h_dest, wh.addr3, ETH_ALEN);
 		memcpy(eth_hdr.h_source, wh.addr2, ETH_ALEN);
@@ -435,6 +431,10 @@ int ath6kl_wmi_dot11_hdr_remove(struct wmi *wmi, struct sk_buff *skb)
 		break;
 	case IEEE80211_FCTL_FROMDS | IEEE80211_FCTL_TODS:
 		break;
+	default:
+		memcpy(eth_hdr.h_dest, wh.addr1, ETH_ALEN);
+		memcpy(eth_hdr.h_source, wh.addr2, ETH_ALEN);
+		break;
 	}
 
 	skb_pull(skb, sizeof(struct ath6kl_llc_snap_hdr));
diff --git a/drivers/net/wireless/intel/ipw2x00/libipw_rx.c b/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
index cef7f7d79cd9..1c1ec7bb9302 100644
--- a/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
+++ b/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
@@ -507,7 +507,7 @@ int libipw_rx(struct libipw_device *ieee, struct sk_buff *skb,
 		memcpy(dst, hdr->addr3, ETH_ALEN);
 		memcpy(src, hdr->addr4, ETH_ALEN);
 		break;
-	case 0:
+	default:
 		memcpy(dst, hdr->addr1, ETH_ALEN);
 		memcpy(src, hdr->addr2, ETH_ALEN);
 		break;
diff --git a/drivers/net/wireless/intersil/hostap/hostap_80211_rx.c b/drivers/net/wireless/intersil/hostap/hostap_80211_rx.c
index 599f30f22841..34dbddbf3f9b 100644
--- a/drivers/net/wireless/intersil/hostap/hostap_80211_rx.c
+++ b/drivers/net/wireless/intersil/hostap/hostap_80211_rx.c
@@ -855,7 +855,7 @@ void hostap_80211_rx(struct net_device *dev, struct sk_buff *skb,
 		memcpy(dst, hdr->addr3, ETH_ALEN);
 		memcpy(src, hdr->addr4, ETH_ALEN);
 		break;
-	case 0:
+	default:
 		memcpy(dst, hdr->addr1, ETH_ALEN);
 		memcpy(src, hdr->addr2, ETH_ALEN);
 		break;
diff --git a/net/wireless/lib80211_crypt_tkip.c b/net/wireless/lib80211_crypt_tkip.c
index 71447cf86306..ba0a1f398ce5 100644
--- a/net/wireless/lib80211_crypt_tkip.c
+++ b/net/wireless/lib80211_crypt_tkip.c
@@ -556,7 +556,7 @@ static void michael_mic_hdr(struct sk_buff *skb, u8 * hdr)
 		memcpy(hdr, hdr11->addr3, ETH_ALEN);	/* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr4, ETH_ALEN);	/* SA */
 		break;
-	case 0:
+	default:
 		memcpy(hdr, hdr11->addr1, ETH_ALEN);	/* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr2, ETH_ALEN);	/* SA */
 		break;
-- 
2.9.0

^ permalink raw reply related

* net/ipv4: warning in inet_sock_destruct
From: Andrey Konovalov @ 2016-10-24 15:40 UTC (permalink / raw)
  To: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML
  Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Eric Dumazet,
	Dmitry Vyukov

Hi,

I've got the following error report while running the syzkaller fuzzer:

------------[ cut here ]------------
WARNING: CPU: 1 PID: 0 at net/ipv4/af_inet.c:153[<        none
>] inet_sock_destruct+0x64d/0x810 net/ipv4/af_inet.c:153
Modules linked in:
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.9.0-rc2+ #301
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 ffff88006cd07d88 ffffffff81b47264 0000000000000000 0000000000000000
 ffffffff84465d80 0000000000000000 ffff88006cd07dd0 ffffffff81111237
 ffff88006cd19100[   60.531224]  0000000000000099 ffffffff84465d80
0000000000000099
Call Trace:
 <IRQ> [   60.531224]  [<ffffffff81b47264>] dump_stack+0xb3/0x10f
 [<ffffffff81111237>] __warn+0x1a7/0x1f0 kernel/panic.c:550
 [<ffffffff8111144c>] warn_slowpath_null+0x2c/0x40 kernel/panic.c:585
 [<ffffffff8316e64d>] inet_sock_destruct+0x64d/0x810 net/ipv4/af_inet.c:153
 [<ffffffff82b7c3a1>] __sk_destruct+0x51/0x480 net/core/sock.c:1422
 [<     inline     >] __rcu_reclaim kernel/rcu/rcu.h:118
 [<     inline     >] rcu_do_batch kernel/rcu/tree.c:2776
 [<     inline     >] invoke_rcu_callbacks kernel/rcu/tree.c:3040
 [<     inline     >] __rcu_process_callbacks kernel/rcu/tree.c:3007
 [<ffffffff8125e090>] rcu_process_callbacks+0xa40/0x1190 kernel/rcu/tree.c:3024
 [<ffffffff83fc376f>] __do_softirq+0x23f/0x8e5 kernel/softirq.c:284
 [<     inline     >] invoke_softirq kernel/softirq.c:364
 [<ffffffff811262b7>] irq_exit+0x1a7/0x1e0 kernel/softirq.c:405
 [<     inline     >] exiting_irq ./arch/x86/include/asm/apic.h:659
 [<ffffffff83fc309b>] smp_apic_timer_interrupt+0x7b/0xa0
arch/x86/kernel/apic/apic.c:960
 [<ffffffff83fc214c>] apic_timer_interrupt+0x8c/0xa0
 <EOI> [   60.531224]  [<ffffffff83fbf866>] ? native_safe_halt+0x6/0x10
 [<     inline     >] arch_safe_halt ./arch/x86/include/asm/paravirt.h:103
 [<ffffffff83fbef92>] default_idle+0x22/0x2d0 arch/x86/kernel/process.c:308
 [<ffffffff8106ef6a>] arch_cpu_idle+0xa/0x10 arch/x86/kernel/process.c:299
 [<ffffffff83fbfc16>] default_idle_call+0x36/0x60 kernel/sched/idle.c:96
 [<     inline     >] cpuidle_idle_call kernel/sched/idle.c:154
 [<     inline     >] cpu_idle_loop kernel/sched/idle.c:247
 [<ffffffff811f8024>] cpu_startup_entry+0x244/0x300 kernel/sched/idle.c:302
 [<ffffffff810b3ec0>] start_secondary+0x250/0x2d0 arch/x86/kernel/smpboot.c:263
---[ end trace 3cd7480984cd70d8 ]---

===============================
[ INFO: suspicious RCU usage. ]
4.9.0-rc2+ #301 Tainted: G        W
-------------------------------
net/core/sock.c:1425 suspicious rcu_dereference_check() usage!

other info that might help us debug this:


rcu_scheduler_active = 1, debug_locks = 0
1 lock held by swapper/1/0:
 #0: [   60.560631]  (
rcu_callback[   60.560930] ){......}
, at: [   60.561271] [<ffffffff8125e03b>] rcu_process_callbacks+0x9eb/0x1190

stack backtrace:
CPU: 1 PID: 0 Comm: swapper/1 Tainted: G        W       4.9.0-rc2+ #301
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 ffff88006cd07e20 ffffffff81b47264 ffff88006c180000 0000000000000000
 0000000000000001 ffffffff843fe660 ffff88006cd07e50 ffffffff81204a4f
 ffff880066438440 ffff880066438000 ffff8800664381b0 0000000000000000
Call Trace:
 <IRQ> [   60.563304]  [<ffffffff81b47264>] dump_stack+0xb3/0x10f
 [<ffffffff81204a4f>] lockdep_rcu_suspicious+0x13f/0x190
kernel/locking/lockdep.c:4445
 [<ffffffff82b7c710>] __sk_destruct+0x3c0/0x480 net/core/sock.c:1424
 [<     inline     >] __rcu_reclaim kernel/rcu/rcu.h:118
 [<     inline     >] rcu_do_batch kernel/rcu/tree.c:2776
 [<     inline     >] invoke_rcu_callbacks kernel/rcu/tree.c:3040
 [<     inline     >] __rcu_process_callbacks kernel/rcu/tree.c:3007
 [<ffffffff8125e090>] rcu_process_callbacks+0xa40/0x1190 kernel/rcu/tree.c:3024
 [<ffffffff83fc376f>] __do_softirq+0x23f/0x8e5 kernel/softirq.c:284
 [<     inline     >] invoke_softirq kernel/softirq.c:364
 [<ffffffff811262b7>] irq_exit+0x1a7/0x1e0 kernel/softirq.c:405
 [<     inline     >] exiting_irq ./arch/x86/include/asm/apic.h:659
 [<ffffffff83fc309b>] smp_apic_timer_interrupt+0x7b/0xa0
arch/x86/kernel/apic/apic.c:960
 [<ffffffff83fc214c>] apic_timer_interrupt+0x8c/0xa0
 <EOI> [   60.563304]  [<ffffffff83fbf866>] ? native_safe_halt+0x6/0x10
 [<     inline     >] arch_safe_halt ./arch/x86/include/asm/paravirt.h:103
 [<ffffffff83fbef92>] default_idle+0x22/0x2d0 arch/x86/kernel/process.c:308
 [<ffffffff8106ef6a>] arch_cpu_idle+0xa/0x10 arch/x86/kernel/process.c:299
 [<ffffffff83fbfc16>] default_idle_call+0x36/0x60 kernel/sched/idle.c:96
 [<     inline     >] cpuidle_idle_call kernel/sched/idle.c:154
 [<     inline     >] cpu_idle_loop kernel/sched/idle.c:247
 [<ffffffff811f8024>] cpu_startup_entry+0x244/0x300 kernel/sched/idle.c:302
 [<ffffffff810b3ec0>] start_secondary+0x250/0x2d0 arch/x86/kernel/smpboot.c:263



On commit 07d9a380680d1c0eb51ef87ff2eab5c994949e69 (Oct 23).

^ permalink raw reply

* Re: [PATCH] LSO feature added to Cadence GEM driver
From: Eric Dumazet @ 2016-10-24 15:40 UTC (permalink / raw)
  To: Rafal Ozieblo; +Cc: Nicolas Ferre, netdev, linux-kernel
In-Reply-To: <1477315088-28396-1-git-send-email-rafalo@cadence.com>

On Mon, 2016-10-24 at 14:18 +0100, Rafal Ozieblo wrote:
> New Cadence GEM hardware support Large Segment Offload (LSO):
> TCP segmentation offload (TSO) as well as UDP fragmentation
> offload (UFO). Support for those features was added to the driver.
> 
> Signed-off-by: Rafal Ozieblo <rafalo@cadence.com>

...
>  
> +static int macb_lso_check_compatibility(struct sk_buff *skb, unsigned int hdrlen)
> +{
> +	unsigned int nr_frags, f;
> +
> +	if (skb_shinfo(skb)->gso_size == 0)
> +		/* not LSO */
> +		return -EPERM;
> +
> +	/* there is only one buffer */
> +	if (!skb_is_nonlinear(skb))
> +		return 0;
> +
> +	/* For LSO:
> +	 * When software supplies two or more payload buffers all payload buffers
> +	 * apart from the last must be a multiple of 8 bytes in size.
> +	 */
> +	if (!IS_ALIGNED(skb_headlen(skb) - hdrlen, MACB_TX_LEN_ALIGN))
> +		return -EPERM;
> +
> +	nr_frags = skb_shinfo(skb)->nr_frags;
> +	/* No need to check last fragment */
> +	nr_frags--;
> +	for (f = 0; f < nr_frags; f++) {
> +		const skb_frag_t *frag = &skb_shinfo(skb)->frags[f];
> +
> +		if (!IS_ALIGNED(skb_frag_size(frag), MACB_TX_LEN_ALIGN))
> +			return -EPERM;
> +	}
> +	return 0;
> +}
> +

Very strange hardware requirements ;(

You should implement an .ndo_features_check method
to perform the checks from core networking stack, and not from your
ndo_start_xmit()

This has the huge advantage of not falling back to skb_linearize(skb)
which is very likely to fail with ~64 KB skbs anyway.

(Your ndo_features_check() would request software GSO instead ...)

^ permalink raw reply

* [PATCH] cw1200: fix bogus maybe-uninitialized warning
From: Arnd Bergmann @ 2016-10-24 15:41 UTC (permalink / raw)
  To: Solomon Peachy, Kalle Valo
  Cc: Arnd Bergmann, Johannes Berg,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On x86, the cw1200 driver produces a rather silly warning about the
possible use of the 'ret' variable without an initialization
presumably after being confused by the architecture specific definition
of WARN_ON:

drivers/net/wireless/st/cw1200/wsm.c: In function ‘wsm_handle_rx’:
drivers/net/wireless/st/cw1200/wsm.c:1457:9: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

As the driver just checks the same variable twice here, we can simplify
it by removing the second condition, which makes it more readable and
avoids the warning.

Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
 drivers/net/wireless/st/cw1200/wsm.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/st/cw1200/wsm.c b/drivers/net/wireless/st/cw1200/wsm.c
index 680d60eabc75..094e6637ade2 100644
--- a/drivers/net/wireless/st/cw1200/wsm.c
+++ b/drivers/net/wireless/st/cw1200/wsm.c
@@ -385,14 +385,13 @@ static int wsm_multi_tx_confirm(struct cw1200_common *priv,
 	if (WARN_ON(count <= 0))
 		return -EINVAL;
 
-	if (count > 1) {
-		/* We already released one buffer, now for the rest */
-		ret = wsm_release_tx_buffer(priv, count - 1);
-		if (ret < 0)
-			return ret;
-		else if (ret > 0)
-			cw1200_bh_wakeup(priv);
-	}
+	/* We already released one buffer, now for the rest */
+	ret = wsm_release_tx_buffer(priv, count - 1);
+	if (ret < 0)
+		return ret;
+
+	if (ret > 0)
+		cw1200_bh_wakeup(priv);
 
 	cw1200_debug_txed_multi(priv, count);
 	for (i = 0; i < count; ++i) {
-- 
2.9.0

^ permalink raw reply related

* [PATCH] kalmia: avoid potential uninitialized variable use
From: Arnd Bergmann @ 2016-10-24 15:54 UTC (permalink / raw)
  To: netdev; +Cc: Arnd Bergmann, linux-usb, linux-kernel

The kalmia_send_init_packet() returns zero or a negative return
code, but gcc has no way of knowing that there cannot be a
positive return code, so it determines that copying the ethernet
address at the end of kalmia_bind() will access uninitialized
data:

drivers/net/usb/kalmia.c: In function ‘kalmia_bind’:
arch/x86/include/asm/string_32.h:78:22: error: ‘*((void *)&ethernet_addr+4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
   *((short *)to + 2) = *((short *)from + 2);
                      ^
drivers/net/usb/kalmia.c:138:5: note: ‘*((void *)&ethernet_addr+4)’ was declared here

This warning is harmless, but for consistency, we should make
the check for the return code match what the driver does everywhere
else and just progate it, which then gets rid of the warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/usb/kalmia.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/kalmia.c b/drivers/net/usb/kalmia.c
index 5662babf0583..3e37724d30ae 100644
--- a/drivers/net/usb/kalmia.c
+++ b/drivers/net/usb/kalmia.c
@@ -151,7 +151,7 @@ kalmia_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	status = kalmia_init_and_get_ethernet_addr(dev, ethernet_addr);
 
-	if (status < 0) {
+	if (status) {
 		usb_set_intfdata(intf, NULL);
 		usb_driver_release_interface(driver_of(intf), intf);
 		return status;
-- 
2.9.0

^ permalink raw reply related

* Re: [PATCH net-next] flow_dissector: fix vlan tag handling
From: Arnd Bergmann @ 2016-10-24 16:00 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Tom Herbert, David S. Miller, Alexander Duyck, Jiri Pirko,
	Hadar Hen Zion, Gao Feng, Amir Vadai,
	Linux Kernel Network Developers, LKML, Linus Torvalds
In-Reply-To: <20161024081736.GB2781@nanopsycho>

On Monday, October 24, 2016 10:17:36 AM CEST Jiri Pirko wrote:
> Sat, Oct 22, 2016 at 10:30:08PM CEST, arnd@arndb.de wrote:
> >diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> >index 44e6ba9d3a6b..17be1b66cc41 100644
> >--- a/net/core/flow_dissector.c
> >+++ b/net/core/flow_dissector.c
> >@@ -246,13 +246,10 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
> > 	case htons(ETH_P_8021AD):
> > 	case htons(ETH_P_8021Q): {
> > 		const struct vlan_hdr *vlan;
> >+		struct vlan_hdr _vlan;
> >+		bool vlan_tag_present = (skb && skb_vlan_tag_present(skb));
> 
> Drop the unnecessary "()"

ok

> > 
> >-		if (skb && skb_vlan_tag_present(skb))
> >-			proto = skb->protocol;
> 
> This does not look correct. I believe that you need to set proto for
> further processing.
> 

Ah, of course. I only looked at the usage in this 'case' statement,
but the variable is also used after the 'goto again' and at
the end of the function.

	Arnd

^ permalink raw reply

* net/can: warning in bcm_connect/proc_register
From: Andrey Konovalov @ 2016-10-24 16:21 UTC (permalink / raw)
  To: Oliver Hartkopp, Marc Kleine-Budde, David S. Miller, linux-can,
	netdev, LKML
  Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Dmitry Vyukov,
	Eric Dumazet

Hi,

I've got the following error report while running the syzkaller fuzzer:

WARNING: CPU: 0 PID: 32451 at fs/proc/generic.c:345 proc_register+0x25e/0x300
proc_dir_entry 'can-bcm/249757' already registered
Kernel panic - not syncing: panic_on_warn set ...

CPU: 0 PID: 32451 Comm: syz-executor Not tainted 4.9.0-rc1+ #293
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 ffff880037d8fae0 ffffffff81b474f4 0000000000000003 dffffc0000000000
 ffffffff840cbf00 ffff880037d8fb04 ffff880037d8fba8 ffffffff8140c06a
 0000000041b58ab3 ffffffff8479ab7d ffffffff8140beae ffffffff00000032
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff81b474f4>] dump_stack+0xb3/0x10f lib/dump_stack.c:51
 [<ffffffff8140c06a>] panic+0x1bc/0x39d kernel/panic.c:179
 [<ffffffff8111125c>] __warn+0x1cc/0x1f0 kernel/panic.c:542
 [<ffffffff8111132c>] warn_slowpath_fmt+0xac/0xd0 kernel/panic.c:565
 [<ffffffff8167d6ee>] proc_register+0x25e/0x300 fs/proc/generic.c:344
 [<ffffffff8167dc01>] proc_create_data+0x101/0x1a0 fs/proc/generic.c:507
 [<ffffffff835729ce>] bcm_connect+0x16e/0x380 net/can/bcm.c:1585
 [<ffffffff82b725e4>] SYSC_connect+0x244/0x2f0 net/socket.c:1533
 [<ffffffff82b74dd4>] SyS_connect+0x24/0x30 net/socket.c:1514
 [<ffffffff83fc19c1>] entry_SYSCALL_64_fastpath+0x1f/0xc2
Dumping ftrace buffer:
   (ftrace buffer empty)
Kernel Offset: disabled

On commit 07d9a380680d1c0eb51ef87ff2eab5c994949e69 (Oct 23).

^ permalink raw reply

* Re: Redundancy support through HSR and PRP
From: Murali Karicheri @ 2016-10-24 16:35 UTC (permalink / raw)
  To: open list:TI NETCP ETHERNET DRIVER, David Miller, arvid.brodin
In-Reply-To: <5808F9FC.1080802@ti.com>

On 10/20/2016 01:08 PM, Murali Karicheri wrote:
> David,
> 
> On 10/10/2016 02:34 PM, Murali Karicheri wrote:
>> All,
>>
>> Wondering if there plan to add PRP driver support, like HSR in Linux? AFAIK, PRP
>> adds trailor to Ethernet frame and is used for Redundancy management like HSR.
>> So wondering why this is not supported.
>>
>> Thanks
>>
> I need to work on a prp driver for Linux. So if there is already someone working
> on this, I would like to join and contribute. Either way please respond so that
> I can work to add this support. 
> 
> I am also working to add support for offload HSR functions to hardware and will
> need to modify the hsr driver to support the same. So any suggestion as to how this
> can be done, will be appreciated.
> 
> Here is what I believe should happen to support this at a higher level
> 
> hsr capable NIC (with firmware support) may able to
>  - duplicate packets at the egress. So only one copy needs to be forwarded to the
>    NIC
>  - Discard the duplicate at the ingress. So forward only one to copy to the ethernet
>    driver
>  - Manage supervision of the network. Keep node list and their status
> 
> It could be a subset of the above. So I am hoping this can be published by the Ethernet
> driver as a set of features. The hsr driver can then look at this features and
> decide to offload and disable same functionality at the hsr driver. Also the node list/status
> has to be polled from the underlying hardware.
> 
> PRP is similar to HSR in many respect. Redundancy management uses a suffix tag to the MAC
> frame instead of prefix used by HSR. So they are more transparently handled by 
> switches or routers. Probably i need to do
>   - rename net/hsr to net/hsr-prp
>   - restructure the current set of files to add prp support 
> 
> Thanks
> 
+ Arvid

Didn't copy HSR owner in my original email. Copying now.


-- 
Murali Karicheri
Linux Kernel, Keystone

^ permalink raw reply

* Re: net/can: warning in bcm_connect/proc_register
From: Cong Wang @ 2016-10-24 16:58 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Oliver Hartkopp, Marc Kleine-Budde, David S. Miller, linux-can,
	netdev, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Dmitry Vyukov, Eric Dumazet
In-Reply-To: <CAAeHK+x6w+Lg1GoCZXdjPtM6n72dpUbF_Bvtg80YNK-Wh8YJ4w@mail.gmail.com>

On Mon, Oct 24, 2016 at 9:21 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
> Hi,
>
> I've got the following error report while running the syzkaller fuzzer:
>
> WARNING: CPU: 0 PID: 32451 at fs/proc/generic.c:345 proc_register+0x25e/0x300
> proc_dir_entry 'can-bcm/249757' already registered
> Kernel panic - not syncing: panic_on_warn set ...

Looks like we have two problems here:

1) A check for bo->bcm_proc_read != NULL seems missing
2) We need to lock the sock in bcm_connect().

I will work on a patch. Meanwhile, it would help a lot if you could provide
a reproducer.

Thanks!

^ 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