* Re: [PATCH net] macsec: avoid heap overflow in skb_to_sgvec on receive
From: Jason A. Donenfeld @ 2017-04-25 15:59 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: Netdev
In-Reply-To: <20170425155140.GA13414@bistromath.localdomain>
On Tue, Apr 25, 2017 at 5:51 PM, Sabrina Dubroca <sd@queasysnail.net> wrote:
> 2017-04-25, 17:39:09 +0200, Jason A. Donenfeld wrote:
>> Hi Sabrina,
>>
>> I think I may have beaten you to the punch here by a few minutes. :)
>
> I said I was going to post a patch.
> Mail headers seem to disagree with you ;)
Oh, whoops, sorry -- thought you wanted me to. Misread, but happy to
have done it anyway.
> Unless I missed something, encrypt was already handling fragments
> correctly. An skb with ->frag_list should have no skb_tailroom, so it
> will be linearized skb_copy_expand().
You're right. In that case, you might as well add back the FRAGLIST
annotation to the FEATURES, so that packets with frag_lists aren't
copied twice -- once upon linearization into xmit and again when
calling copy_expand in case they need more head or tail space. In
general, too, using the precise number of frags saves memory. But
above all, it seems to me the important thing is that it's very
_clear_ there isn't going to be an overflow, that the code doesn't
rely on so many odd particulars that then get violated later in its
life. Using the explicit length makes things a lot more clear. So
maybe better to go with mine?
^ permalink raw reply
* Re: [PATCH net 1/1] qed: Fix error in the dcbx app meta data initialization.
From: David Miller @ 2017-04-25 15:58 UTC (permalink / raw)
To: sudarsana.kalluru; +Cc: netdev, Yuval.Mintz
In-Reply-To: <20170425035910.29296-1-sudarsana.kalluru@cavium.com>
From: Sudarsana Reddy Kalluru <sudarsana.kalluru@cavium.com>
Date: Mon, 24 Apr 2017 20:59:10 -0700
> DCBX app_data array is initialized with the incorrect values for
> personality field. This would prevent offloaded protocols from
> honoring the PFC.
>
> Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] bpf: map_get_next_key to return first key on NULL
From: David Miller @ 2017-04-25 15:57 UTC (permalink / raw)
To: ast; +Cc: daniel, qinteng, netdev, kernel-team
In-Reply-To: <20170425020037.1114047-1-ast@fb.com>
From: Alexei Starovoitov <ast@fb.com>
Date: Mon, 24 Apr 2017 19:00:37 -0700
> From: Teng Qin <qinteng@fb.com>
>
> When iterating through a map, we need to find a key that does not exist
> in the map so map_get_next_key will give us the first key of the map.
> This often requires a lot of guessing in production systems.
>
> This patch makes map_get_next_key return the first key when the key
> pointer in the parameter is NULL.
>
> Signed-off-by: Teng Qin <qinteng@fb.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Applied, thanks Alexei.
^ permalink raw reply
* Re: net: heap out-of-bounds in fib6_clean_node/rt6_fill_node/fib6_age/fib6_prune_clone
From: David Ahern @ 2017-04-25 15:57 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Eric Dumazet, Mahesh Bandewar, Eric Dumazet, David Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, LKML, Cong Wang, syzkaller
In-Reply-To: <CACT4Y+aDzAYs9mbNGeFEkYSB2wTwomZTGEKn4hDH0PAb4uiLqw@mail.gmail.com>
On 3/7/17 2:21 AM, Dmitry Vyukov wrote:
> ------------[ cut here ]------------
> WARNING: CPU: 2 PID: 3990 at net/ipv6/ip6_fib.c:991
> fib6_add+0x2e12/0x3290 net/ipv6/ip6_fib.c:991 net/ipv6/ip6_fib.c:991
> Kernel panic - not syncing: panic_on_warn set ...
>
> CPU: 2 PID: 3990 Comm: kworker/2:4 Not tainted 4.11.0-rc1+ #311
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: ipv6_addrconf addrconf_dad_work
> Call Trace:
> __dump_stack lib/dump_stack.c:16 [inline]
> __dump_stack lib/dump_stack.c:16 [inline] lib/dump_stack.c:52
> dump_stack+0x2fb/0x3fd lib/dump_stack.c:52 lib/dump_stack.c:52
> panic+0x20f/0x426 kernel/panic.c:180 kernel/panic.c:180
> __warn+0x1c4/0x1e0 kernel/panic.c:541 kernel/panic.c:541
> warn_slowpath_null+0x2c/0x40 kernel/panic.c:584 kernel/panic.c:584
> fib6_add+0x2e12/0x3290 net/ipv6/ip6_fib.c:991 net/ipv6/ip6_fib.c:991
> __ip6_ins_rt+0x60/0x80 net/ipv6/route.c:948 net/ipv6/route.c:948
> ip6_ins_rt+0x19b/0x220 net/ipv6/route.c:959 net/ipv6/route.c:959
> __ipv6_ifa_notify+0x62e/0x7a0 net/ipv6/addrconf.c:5485 net/ipv6/addrconf.c:5485
> ipv6_ifa_notify+0xdf/0x1d0 net/ipv6/addrconf.c:5518 net/ipv6/addrconf.c:5518
> addrconf_dad_completed+0xe6/0x950 net/ipv6/addrconf.c:3983
> net/ipv6/addrconf.c:3983
> addrconf_dad_begin net/ipv6/addrconf.c:3797 [inline]
Similarly for this one.
^ permalink raw reply
* Re: [PATCH net] netvsc: fix calculation of available send sections
From: David Miller @ 2017-04-25 15:57 UTC (permalink / raw)
To: stephen; +Cc: netdev, sthemmin
In-Reply-To: <20170425013338.2653-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 24 Apr 2017 18:33:38 -0700
> My change (introduced in 4.11) to use find_first_clear_bit
> incorrectly assumed that the size argument was words, not bits.
> The effect was only a small limited number of the available send
> sections were being actually used. This can cause performance loss
> with some workloads.
>
> Since map_words is now used only during initialization, it can
> be on stack instead of in per-device data.
>
> Fixes: b58a185801da ("netvsc: simplify get next send section")
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Applied, thanks.
^ permalink raw reply
* Re: net: heap out-of-bounds in fib6_clean_node/rt6_fill_node/fib6_age/fib6_prune_clone
From: David Ahern @ 2017-04-25 15:56 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Mahesh Bandewar, Eric Dumazet, David Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML,
Cong Wang, syzkaller
In-Reply-To: <CACT4Y+YCPxKkaZBfAabxtAV71R0ghLT-vUKVewJpm9wb2GXaEQ@mail.gmail.com>
On 3/4/17 11:57 AM, Dmitry Vyukov wrote:
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in rt6_dump_route+0x293/0x2f0
> net/ipv6/route.c:3551 at addr ffff88007e523694
> Read of size 4 by task syz-executor3/24426
> CPU: 2 PID: 24426 Comm: syz-executor3 Not tainted 4.10.0+ #293
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:16 [inline]
> dump_stack+0x2fb/0x3fd lib/dump_stack.c:52
> kasan_object_err+0x1c/0x90 mm/kasan/report.c:166
> print_address_description mm/kasan/report.c:208 [inline]
> kasan_report_error mm/kasan/report.c:292 [inline]
> kasan_report.part.2+0x1b0/0x460 mm/kasan/report.c:314
> kasan_report mm/kasan/report.c:334 [inline]
> __asan_report_load4_noabort+0x29/0x30 mm/kasan/report.c:334
> rt6_dump_route+0x293/0x2f0 net/ipv6/route.c:3551
> fib6_dump_node+0x101/0x1a0 net/ipv6/ip6_fib.c:315
> fib6_walk_continue+0x4b3/0x620 net/ipv6/ip6_fib.c:1576
> fib6_walk+0x91/0xf0 net/ipv6/ip6_fib.c:1621
> fib6_dump_table net/ipv6/ip6_fib.c:374 [inline]
> inet6_dump_fib+0x832/0xea0 net/ipv6/ip6_fib.c:447
My expectation is that this one is fixed with the ipv6 patch I have sent
(not yet committed). Are you seeing this backtrace with that patch?
^ permalink raw reply
* Re: [PATCH net-next] selftests/net: Fix broken test case in psock_fanout
From: David Miller @ 2017-04-25 15:56 UTC (permalink / raw)
To: maloneykernel; +Cc: netdev, maloney
In-Reply-To: <20170425012911.903-1-maloneykernel@gmail.com>
From: Mike Maloney <maloneykernel@gmail.com>
Date: Mon, 24 Apr 2017 21:29:11 -0400
> From: Mike Maloney <maloney@google.com>
>
> The error return falue form sock_fanout_open is -1, not zero. One test
> case was checking for 0 instead of -1.
>
> Tested: Built and tested in clean client.
> Signed-off-by: Mike Maloney <maloney@google.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net: ethernet: ti: netcp_core: remove unused compl queue mapping
From: David Miller @ 2017-04-25 15:55 UTC (permalink / raw)
To: ivan.khoronzhuk
Cc: netdev, w-kwok2, m-karicheri2, linux-kernel, grygorii.strashko
In-Reply-To: <1493067246-3247-1-git-send-email-ivan.khoronzhuk@linaro.org>
From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Date: Mon, 24 Apr 2017 23:54:06 +0300
> This code is unused and probably was unintentionally left while
> moving completion queue mapping in submit function.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Applied.
^ permalink raw reply
* Re: [PATCH v3 net] net: ipv6: regenerate host route if moved to gc list
From: David Ahern @ 2017-04-25 15:54 UTC (permalink / raw)
To: Andrey Konovalov; +Cc: netdev, Dmitry Vyukov, mmanning, Martin KaFai Lau
In-Reply-To: <CAAeHK+yLe1PoFMqmGK0B6wXqfc0AZzRAmMLxks-vSkOp5Eq3VQ@mail.gmail.com>
On 4/25/17 6:50 AM, Andrey Konovalov wrote:
> I've been running syzkaller with your patch and got another report
> from ip6_pol_route.
In general the existing patch cleans up all of the ipv6 fib kasan and
WARN_ON traces that were seen?
>
> It happened only once so far and I couldn't reproduce it.
Some similarity in the sense of a ipv4 route in the ipv6 fib. That's
only going to happen if it hits the dst gc list and then back.
This duplicates what Dmitry reported on March 3rd.
^ permalink raw reply
* Re: [PATCH] net: hso: fix module unloading
From: David Miller @ 2017-04-25 15:52 UTC (permalink / raw)
To: andreas; +Cc: joe, peter, linux-usb, netdev, linux-kernel, letux-kernel
In-Reply-To: <1493061519-15785-1-git-send-email-andreas@kemnade.info>
From: Andreas Kemnade <andreas@kemnade.info>
Date: Mon, 24 Apr 2017 21:18:39 +0200
> keep tty driver until usb driver is unregistered
> rmmod hso
> produces traces like this without that:
...
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Applied, thank you.
^ permalink raw reply
* [PATCH v5 2/5] ipsec: check return value of skb_to_sgvec always
From: Jason A. Donenfeld @ 2017-04-25 15:52 UTC (permalink / raw)
To: netdev, linux-kernel, David.Laight, kernel-hardening, davem
Cc: Jason A. Donenfeld
In-Reply-To: <20170425155215.4835-1-Jason@zx2c4.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
net/ipv4/ah4.c | 8 ++++++--
net/ipv4/esp4.c | 30 ++++++++++++++++++++----------
net/ipv6/ah6.c | 8 ++++++--
net/ipv6/esp6.c | 31 +++++++++++++++++++++----------
4 files changed, 53 insertions(+), 24 deletions(-)
diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index 22377c8ff14b..e8f862358518 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c
@@ -220,7 +220,9 @@ static int ah_output(struct xfrm_state *x, struct sk_buff *skb)
ah->seq_no = htonl(XFRM_SKB_CB(skb)->seq.output.low);
sg_init_table(sg, nfrags + sglists);
- skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ err = skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ if (unlikely(err < 0))
+ goto out_free;
if (x->props.flags & XFRM_STATE_ESN) {
/* Attach seqhi sg right after packet payload */
@@ -393,7 +395,9 @@ static int ah_input(struct xfrm_state *x, struct sk_buff *skb)
skb_push(skb, ihl);
sg_init_table(sg, nfrags + sglists);
- skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ err = skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ if (unlikely(err < 0))
+ goto out_free;
if (x->props.flags & XFRM_STATE_ESN) {
/* Attach seqhi sg right after packet payload */
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index b1e24446e297..42cb09cc8533 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -360,9 +360,13 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
esph = esp_output_set_extra(skb, esph, extra);
sg_init_table(sg, nfrags);
- skb_to_sgvec(skb, sg,
- (unsigned char *)esph - skb->data,
- assoclen + ivlen + clen + alen);
+ err = skb_to_sgvec(skb, sg,
+ (unsigned char *)esph - skb->data,
+ assoclen + ivlen + clen + alen);
+ if (unlikely(err < 0)) {
+ spin_unlock_bh(&x->lock);
+ goto error;
+ }
allocsize = ALIGN(skb->data_len, L1_CACHE_BYTES);
@@ -381,11 +385,13 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
pfrag->offset = pfrag->offset + allocsize;
sg_init_table(dsg, skb_shinfo(skb)->nr_frags + 1);
- skb_to_sgvec(skb, dsg,
- (unsigned char *)esph - skb->data,
- assoclen + ivlen + clen + alen);
+ err = skb_to_sgvec(skb, dsg,
+ (unsigned char *)esph - skb->data,
+ assoclen + ivlen + clen + alen);
spin_unlock_bh(&x->lock);
+ if (unlikely(err < 0))
+ goto error;
goto skip_cow2;
}
@@ -422,9 +428,11 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
esph = esp_output_set_extra(skb, esph, extra);
sg_init_table(sg, nfrags);
- skb_to_sgvec(skb, sg,
- (unsigned char *)esph - skb->data,
- assoclen + ivlen + clen + alen);
+ err = skb_to_sgvec(skb, sg,
+ (unsigned char *)esph - skb->data,
+ assoclen + ivlen + clen + alen);
+ if (unlikely(err < 0))
+ goto error;
skip_cow2:
if ((x->props.flags & XFRM_STATE_ESN))
@@ -658,7 +666,9 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
esp_input_set_header(skb, seqhi);
sg_init_table(sg, nfrags);
- skb_to_sgvec(skb, sg, 0, skb->len);
+ err = skb_to_sgvec(skb, sg, 0, skb->len);
+ if (unlikely(err < 0))
+ goto out;
skb->ip_summed = CHECKSUM_NONE;
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index dda6035e3b84..755f38271dd5 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -423,7 +423,9 @@ static int ah6_output(struct xfrm_state *x, struct sk_buff *skb)
ah->seq_no = htonl(XFRM_SKB_CB(skb)->seq.output.low);
sg_init_table(sg, nfrags + sglists);
- skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ err = skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ if (unlikely(err < 0))
+ goto out_free;
if (x->props.flags & XFRM_STATE_ESN) {
/* Attach seqhi sg right after packet payload */
@@ -606,7 +608,9 @@ static int ah6_input(struct xfrm_state *x, struct sk_buff *skb)
ip6h->hop_limit = 0;
sg_init_table(sg, nfrags + sglists);
- skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ err = skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ if (unlikely(err < 0))
+ goto out_free;
if (x->props.flags & XFRM_STATE_ESN) {
/* Attach seqhi sg right after packet payload */
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index ff54faa75631..017e2c2d36e1 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -339,9 +339,13 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
esph = esp_output_set_esn(skb, esph, seqhi);
sg_init_table(sg, nfrags);
- skb_to_sgvec(skb, sg,
- (unsigned char *)esph - skb->data,
- assoclen + ivlen + clen + alen);
+ err = skb_to_sgvec(skb, sg,
+ (unsigned char *)esph - skb->data,
+ assoclen + ivlen + clen + alen);
+ if (unlikely(err < 0)) {
+ spin_unlock_bh(&x->lock);
+ goto error;
+ }
allocsize = ALIGN(skb->data_len, L1_CACHE_BYTES);
@@ -360,12 +364,15 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
pfrag->offset = pfrag->offset + allocsize;
sg_init_table(dsg, skb_shinfo(skb)->nr_frags + 1);
- skb_to_sgvec(skb, dsg,
- (unsigned char *)esph - skb->data,
- assoclen + ivlen + clen + alen);
+ err = skb_to_sgvec(skb, dsg,
+ (unsigned char *)esph - skb->data,
+ assoclen + ivlen + clen + alen);
spin_unlock_bh(&x->lock);
+ if (unlikely(err < 0))
+ goto error;
+
goto skip_cow2;
}
}
@@ -403,9 +410,11 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
esph = esp_output_set_esn(skb, esph, seqhi);
sg_init_table(sg, nfrags);
- skb_to_sgvec(skb, sg,
- (unsigned char *)esph - skb->data,
- assoclen + ivlen + clen + alen);
+ err = skb_to_sgvec(skb, sg,
+ (unsigned char *)esph - skb->data,
+ assoclen + ivlen + clen + alen);
+ if (unlikely(err < 0))
+ goto error;
skip_cow2:
if ((x->props.flags & XFRM_STATE_ESN))
@@ -600,7 +609,9 @@ static int esp6_input(struct xfrm_state *x, struct sk_buff *skb)
esp_input_set_header(skb, seqhi);
sg_init_table(sg, nfrags);
- skb_to_sgvec(skb, sg, 0, skb->len);
+ ret = skb_to_sgvec(skb, sg, 0, skb->len);
+ if (unlikely(ret < 0))
+ goto out;
skb->ip_summed = CHECKSUM_NONE;
--
2.12.2
^ permalink raw reply related
* Re: [PATCH net-next 0/6] qed/qede: VF tunnelling support
From: David Miller @ 2017-04-25 15:52 UTC (permalink / raw)
To: manish.chopra; +Cc: netdev, Yuval.Mintz
In-Reply-To: <20170424170049.16027-1-manish.chopra@cavium.com>
From: Manish Chopra <manish.chopra@cavium.com>
Date: Mon, 24 Apr 2017 10:00:43 -0700
> With this series VFs can run vxlan/geneve/gre tunnels over it.
> Please consider applying this series to "net-next"
Series applied.
^ permalink raw reply
* [PATCH v5 5/5] virtio_net: check return value of skb_to_sgvec always
From: Jason A. Donenfeld @ 2017-04-25 15:52 UTC (permalink / raw)
To: netdev, linux-kernel, David.Laight, kernel-hardening, davem
Cc: Jason A. Donenfeld
In-Reply-To: <20170425155215.4835-1-Jason@zx2c4.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
drivers/net/virtio_net.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f36584616e7d..1709fd0b4bf7 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1081,7 +1081,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
struct virtio_net_hdr_mrg_rxbuf *hdr;
const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
struct virtnet_info *vi = sq->vq->vdev->priv;
- unsigned num_sg;
+ int num_sg;
unsigned hdr_len = vi->hdr_len;
bool can_push;
@@ -1114,6 +1114,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
sg_set_buf(sq->sg, hdr, hdr_len);
num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1;
}
+ if (unlikely(num_sg < 0))
+ return num_sg;
return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb, GFP_ATOMIC);
}
--
2.12.2
^ permalink raw reply related
* [PATCH v5 4/5] macsec: check return value of skb_to_sgvec always
From: Jason A. Donenfeld @ 2017-04-25 15:52 UTC (permalink / raw)
To: netdev, linux-kernel, David.Laight, kernel-hardening, davem
Cc: Jason A. Donenfeld
In-Reply-To: <20170425155215.4835-1-Jason@zx2c4.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
drivers/net/macsec.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index dbab05afcdbe..d846f42b99ec 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -733,7 +733,12 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
macsec_fill_iv(iv, secy->sci, pn);
sg_init_table(sg, MAX_SKB_FRAGS + 1);
- skb_to_sgvec(skb, sg, 0, skb->len);
+ ret = skb_to_sgvec(skb, sg, 0, skb->len);
+ if (unlikely(ret < 0)) {
+ macsec_txsa_put(tx_sa);
+ kfree_skb(skb);
+ return ERR_PTR(ret);
+ }
if (tx_sc->encrypt) {
int len = skb->len - macsec_hdr_len(sci_present) -
@@ -937,7 +942,11 @@ static struct sk_buff *macsec_decrypt(struct sk_buff *skb,
macsec_fill_iv(iv, sci, ntohl(hdr->packet_number));
sg_init_table(sg, MAX_SKB_FRAGS + 1);
- skb_to_sgvec(skb, sg, 0, skb->len);
+ ret = skb_to_sgvec(skb, sg, 0, skb->len);
+ if (unlikely(ret < 0)) {
+ kfree_skb(skb);
+ return ERR_PTR(ret);
+ }
if (hdr->tci_an & MACSEC_TCI_E) {
/* confidentiality: ethernet + macsec header
--
2.12.2
^ permalink raw reply related
* [PATCH v5 3/5] rxrpc: check return value of skb_to_sgvec always
From: Jason A. Donenfeld @ 2017-04-25 15:52 UTC (permalink / raw)
To: netdev, linux-kernel, David.Laight, kernel-hardening, davem
Cc: Jason A. Donenfeld
In-Reply-To: <20170425155215.4835-1-Jason@zx2c4.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
net/rxrpc/rxkad.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index 4374e7b9c7bf..dcf46c9c3ece 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -229,7 +229,9 @@ static int rxkad_secure_packet_encrypt(const struct rxrpc_call *call,
len &= ~(call->conn->size_align - 1);
sg_init_table(sg, nsg);
- skb_to_sgvec(skb, sg, 0, len);
+ err = skb_to_sgvec(skb, sg, 0, len);
+ if (unlikely(err < 0))
+ goto out;
skcipher_request_set_crypt(req, sg, sg, len, iv.x);
crypto_skcipher_encrypt(req);
@@ -342,7 +344,8 @@ static int rxkad_verify_packet_1(struct rxrpc_call *call, struct sk_buff *skb,
goto nomem;
sg_init_table(sg, nsg);
- skb_to_sgvec(skb, sg, offset, 8);
+ if (unlikely(skb_to_sgvec(skb, sg, offset, 8) < 0))
+ goto nomem;
/* start the decryption afresh */
memset(&iv, 0, sizeof(iv));
@@ -429,7 +432,8 @@ static int rxkad_verify_packet_2(struct rxrpc_call *call, struct sk_buff *skb,
}
sg_init_table(sg, nsg);
- skb_to_sgvec(skb, sg, offset, len);
+ if (unlikely(skb_to_sgvec(skb, sg, offset, len) < 0))
+ goto nomem;
/* decrypt from the session key */
token = call->conn->params.key->payload.data[0];
--
2.12.2
^ permalink raw reply related
* [PATCH v5 1/5] skbuff: return -EMSGSIZE in skb_to_sgvec to prevent overflow
From: Jason A. Donenfeld @ 2017-04-25 15:52 UTC (permalink / raw)
To: netdev, linux-kernel, David.Laight, kernel-hardening, davem
Cc: Jason A. Donenfeld
In-Reply-To: <20170425.111717.1973239615715123840.davem@davemloft.net>
This is a defense-in-depth measure in response to bugs like
4d6fa57b4dab ("macsec: avoid heap overflow in skb_to_sgvec")
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
This is a resend of v4 with all the other child commits along with it.
net/core/skbuff.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index f86bf69cfb8d..e75640006d78 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3489,7 +3489,9 @@ void __init skb_init(void)
* @len: Length of buffer space to be mapped
*
* Fill the specified scatter-gather list with mappings/pointers into a
- * region of the buffer space attached to a socket buffer.
+ * region of the buffer space attached to a socket buffer. Returns either
+ * the number of scatterlist items used, or -EMSGSIZE if the contents
+ * could not fit.
*/
static int
__skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len)
@@ -3517,6 +3519,8 @@ __skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len)
end = start + skb_frag_size(&skb_shinfo(skb)->frags[i]);
if ((copy = end - offset) > 0) {
skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+ if (elt && sg_is_last(&sg[elt - 1]))
+ return -EMSGSIZE;
if (copy > len)
copy = len;
@@ -3537,6 +3541,9 @@ __skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len)
end = start + frag_iter->len;
if ((copy = end - offset) > 0) {
+ if (elt && sg_is_last(&sg[elt - 1]))
+ return -EMSGSIZE;
+
if (copy > len)
copy = len;
elt += __skb_to_sgvec(frag_iter, sg+elt, offset - start,
@@ -3581,6 +3588,9 @@ int skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int le
{
int nsg = __skb_to_sgvec(skb, sg, offset, len);
+ if (nsg <= 0)
+ return nsg;
+
sg_mark_end(&sg[nsg - 1]);
return nsg;
--
2.12.2
^ permalink raw reply related
* Re: net: heap out-of-bounds in fib6_clean_node/rt6_fill_node/fib6_age/fib6_prune_clone
From: David Ahern @ 2017-04-25 15:51 UTC (permalink / raw)
To: Andrey Konovalov, Dmitry Vyukov
Cc: Eric Dumazet, Mahesh Bandewar, Eric Dumazet, David Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, LKML, Cong Wang, syzkaller
In-Reply-To: <CAAeHK+zx6W_yxnoEQ2Pc1AT4uLXqYZaoi5oQBeuCntdGS38TQg@mail.gmail.com>
On 4/18/17 2:43 PM, Andrey Konovalov wrote:
> 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: 1 PID: 4035 Comm: a.out Not tainted 4.11.0-rc7+ #250
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> task: ffff880069809600 task.stack: ffff880062dc8000
> RIP: 0010:ip6_rt_cache_alloc+0xa6/0x560 net/ipv6/route.c:975
> RSP: 0018:ffff880062dced30 EFLAGS: 00010206
> RAX: dffffc0000000000 RBX: ffff8800670561c0 RCX: 0000000000000006
> RDX: 0000000000000003 RSI: ffff880062dcfb28 RDI: 0000000000000018
> RBP: ffff880062dced68 R08: 0000000000000001 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> R13: ffff880062dcfb28 R14: dffffc0000000000 R15: 0000000000000000
> FS: 00007feebe37e7c0(0000) GS:ffff88006cb00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00000000205a0fe4 CR3: 000000006b5c9000 CR4: 00000000000006e0
> Call Trace:
> ip6_pol_route+0x1512/0x1f20 net/ipv6/route.c:1128
This one is fixed by:
commit 557c44be917c322860665be3d28376afa84aa936
Author: David Ahern <dsa@cumulusnetworks.com>
Date: Wed Apr 19 14:19:43 2017 -0700
net: ipv6: RTF_PCPU should not be settable from userspace
^ permalink raw reply
* Re: [PATCH net] macsec: avoid heap overflow in skb_to_sgvec on receive
From: Sabrina Dubroca @ 2017-04-25 15:51 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: Netdev
In-Reply-To: <CAHmME9rxdTEe1-5ApgUhdkqiJztSgJb0QuhV2uSemobeXm_oNg@mail.gmail.com>
2017-04-25, 17:39:09 +0200, Jason A. Donenfeld wrote:
> Hi Sabrina,
>
> I think I may have beaten you to the punch here by a few minutes. :)
I said I was going to post a patch.
Mail headers seem to disagree with you ;)
> The difference between our two versions is that you don't re-add the
> FRAGLIST attribute, whereas my patch does, and then it does the
> dynamic allocation. I suspect this might be a bit more robust. It also
> ensures that skb_cow_data is called on both paths. So perhaps let's
> roll with mine?
I don't see the "more robust" argument.
Unless I missed something, encrypt was already handling fragments
correctly. An skb with ->frag_list should have no skb_tailroom, so it
will be linearized skb_copy_expand().
--
Sabrina
^ permalink raw reply
* Re: [PATCH net v1 1/2] tipc: fix socket flow control accounting error at tipc_send_stream
From: David Miller @ 2017-04-25 15:45 UTC (permalink / raw)
To: parthasarathy.bhuvaragan; +Cc: netdev, tipc-discussion
In-Reply-To: <1493038843-30621-1-git-send-email-parthasarathy.bhuvaragan@ericsson.com>
From: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
Date: Mon, 24 Apr 2017 15:00:42 +0200
> Until now in tipc_send_stream(), we return -1 when the socket
> encounters link congestion even if the socket had successfully
> sent partial data. This is incorrect as the application resends
> the same the partial data leading to data corruption at
> receiver's end.
>
> In this commit, we return the partially sent bytes as the return
> value at link congestion.
>
> Fixes: 10724cc7bb78 ("tipc: redesign connection-level flow control")
> Signed-off-by: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
> Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Both patches #1 and #2 applied, thanks.
^ permalink raw reply
* Re: [PATCH v2] net/packet: initialize val in packet_getsockopt()
From: David Miller @ 2017-04-25 15:44 UTC (permalink / raw)
To: glider; +Cc: dvyukov, kcc, edumazet, kuznet, linux-kernel, netdev
In-Reply-To: <20170424125914.43270-1-glider@google.com>
From: Alexander Potapenko <glider@google.com>
Date: Mon, 24 Apr 2017 14:59:14 +0200
> In the case getsockopt() is called with PACKET_HDRLEN and optlen < 4
> |val| remains uninitialized and the syscall may behave differently
> depending on its value. This doesn't have security consequences (as the
> uninit bytes aren't copied back), but it's still cleaner to initialize
> |val| and ensure optlen is not less than sizeof(int).
>
> This bug has been detected with KMSAN.
>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> ---
> v2: - if len < sizeof(int), make it 0
No, you should signal an error if the len is too small.
Returning zero bytes to userspace silently makes the user think that
he got the data he asked for.
^ permalink raw reply
* Re: [PATCH net] ipv6: move stub initialization after ipv6 setup completion
From: David Miller @ 2017-04-25 15:43 UTC (permalink / raw)
To: pabeni; +Cc: netdev, xiyou.wangcong, hannes
In-Reply-To: <fd7c85f650661466a782a71485f227f0b84454a7.1493035843.git.pabeni@redhat.com>
From: Paolo Abeni <pabeni@redhat.com>
Date: Mon, 24 Apr 2017 14:18:28 +0200
> The ipv6 stub pointer is currently initialized before the ipv6
> routing subsystem: a 3rd party can access and use such stub
> before the routing data is ready.
> Moreover, such pointer is not cleared in case of initialization
> error, possibly leading to dangling pointers usage.
>
> This change addresses the above moving the stub initialization
> at the end of ipv6 init code.
>
> Fixes: 5f81bd2e5d80 ("ipv6: export a stub for IPv6 symbols used by vxlan")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 2/2] ARM: dts: Add the ethernet and ethernet PHY to the cygnus core DT.
From: Jon Mason @ 2017-04-25 15:43 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Eric Anholt, Florian Fainelli, Vivien Didelot, Andrew Lunn,
Network Development, Rob Herring, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-arm-kernel, open list, BCM Kernel Feedback, Ray Jui,
Scott Branden, Jon Mason
In-Reply-To: <44d0c48c-d80f-271e-0e19-ac87a53e2e53@cogentembedded.com>
On Tue, Apr 25, 2017 at 11:23 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello!
>
> On 04/25/2017 06:15 PM, Jon Mason wrote:
>
>>>> Cygnus has a single amac controller connected to the B53 switch with 2
>>>> PHYs. On the BCM911360_EP platform, those two PHYs are connected to
>>>> the external ethernet jacks.
>
>
> [...]
>
>>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>>> ---
>>>> arch/arm/boot/dts/bcm-cygnus.dtsi | 60
>>>> ++++++++++++++++++++++++++++++++++
>>>> arch/arm/boot/dts/bcm911360_entphn.dts | 8 +++++
>>>> 2 files changed, 68 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi
>>>> b/arch/arm/boot/dts/bcm-cygnus.dtsi
>>>> index 009f1346b817..318899df9972 100644
>>>> --- a/arch/arm/boot/dts/bcm-cygnus.dtsi
>>>> +++ b/arch/arm/boot/dts/bcm-cygnus.dtsi
>
> [...]
>>>>
>>>> @@ -295,6 +345,16 @@
>>>> status = "disabled";
>>>> };
>>>>
>>>> + eth0: enet@18042000 {
>>>> + compatible = "brcm,amac";
>>>> + reg = <0x18042000 0x1000>,
>>>> + <0x18110000 0x1000>;
>>>> + reg-names = "amac_base", "idm_base";
>>>
>>>
>>>
>>> I don't think "_base" suffixes are necessary here.
>>
>>
>> 100% necessary, per the driver. See
>> drivers/net/ethernet/broadcom/bgmac-platform.c
>
>
> I'd recommend to fix the driver/bindings then...
They're already in use in other device trees. So, we'd need to
support backward compatibility on them, thus removing any real benefit
to changing them.
>
> MBR, Sergei
>
^ permalink raw reply
* Re: [PATCH 1/5] skbuff: return -EMSGSIZE in skb_to_sgvec to prevent overflow
From: Sergei Shtylyov @ 2017-04-25 15:42 UTC (permalink / raw)
To: Jason A. Donenfeld, netdev, linux-kernel, davem, David.Laight,
kernel-hardening
In-Reply-To: <20170425140809.23881-1-Jason@zx2c4.com>
Hello!
On 04/25/2017 05:08 PM, Jason A. Donenfeld wrote:
> This is a defense-in-depth measure in response to bugs like
> 4d6fa57b4dab0d77f4d8e9d9c73d1e63f6fe8fee.
You need to also specify the summary line enclosed in (""). And it's
enough to specify 12 digits of SHA1 ID...
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH net-next 0/2] l2tp: add informations about l2tpeth interfaces in /sys
From: David Miller @ 2017-04-25 15:42 UTC (permalink / raw)
To: g.nault; +Cc: netdev, jchapman
In-Reply-To: <cover.1493035407.git.g.nault@alphalink.fr>
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Mon, 24 Apr 2017 14:16:04 +0200
> Patch #1 lets userspace retrieve the naming scheme of an l2tpeth
> interface, using /sys/class/net/<iface>/name_assign_type.
>
> Patch #2 adds the DEVTYPE field in /sys/class/net/<iface>/uevent so
> that userspace can reliably know if a device is an l2tpeth interface.
Looks great, series applied, thanks!
^ permalink raw reply
* Re: [PATCH] IB/IPoIB: Check the headroom size
From: Doug Ledford @ 2017-04-25 15:40 UTC (permalink / raw)
To: Or Gerlitz, Erez Shitrit, Paolo Abeni
Cc: Honggang LI, Erez Shitrit,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Netdev List, David Miller
In-Reply-To: <CAJ3xEMgwS1Bq8+eZC1iAr6xi8ZPHrchsOJ5r4LNJxR8P+6VipA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, 2017-04-25 at 17:39 +0300, Or Gerlitz wrote:
> On Tue, Apr 25, 2017 at 2:43 PM, Erez Shitrit <erezsh-LDSdmyG8hGUWn1LaSxwUnA@public.gmane.org
> .il> wrote:
> >
> > On Tue, Apr 25, 2017 at 2:14 PM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > wrote:
>
> >
> > >
> > > thanks for the info. Is this bug there since ipoib/bonding day
> > > one (and hence my bug...)
> > > or was indeed introduced later? if later, can you explain how
> > > fc791b633515 introduced that or you only know it by bisection?
>
> >
> > commit "fc791b633515" changes the size of the dev_hardlen to be 24
> > and
> > required 24 extra bytes in the skb, before it was only 4, if skb is
> > aligned to eth "mode" it already has 14 bytes for hard-header.
> > So only after that commit we have the issue.
>
> If got you right, Paolo's commit introduced a regression, so we (I
> guess you and
> Paolo) need to either solve it or we (community) should consider a
> revert, please suggest.
It's a little more complex than that. Paolo's commit *re-introduced* a
regression. If you recall, long ago the IPoIB layer stuck the dgid
into the skb, then pulled it out later. However, we didn't actually
declare things properly back then, but it worked anyway. Then we had
the commit you authored to start using the skb->cb to store the dgid,
and our usage of hard header dropped to only 4 bytes. Paolo's commit
went back to the old way of doing things, but also did the proper
accounting and setup to tell the netstack what we were doing (which the
initial implementation never did IIRC). So, this issue should be
reproducible either after Paolo's commit or with any kernel prior to
your commit to use the skb->cb area to store the DGID, but it probably
requires the specific series of actions in this bug to trigger it. A
normal, clean shutdown of the interface doesn't demonstrate the issue.
> The bug is now in stable and distro kernels, so please act.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox