* Re: [PATCH] netlink: do not proceed if dump's start() errs
From: Johannes Berg @ 2017-09-27 13:05 UTC (permalink / raw)
To: Jason A. Donenfeld, David Miller, Netdev, LKML; +Cc: stable
In-Reply-To: <CAHmME9pUWenvkMCNkAHa6K71UDdETvriMr52sKPL38q19M3RVw@mail.gmail.com>
On Wed, 2017-09-27 at 14:50 +0200, Jason A. Donenfeld wrote:
> On Wed, Sep 27, 2017 at 2:39 PM, Jason A. Donenfeld <Jason@zx2c4.com>
> wrote:
> > - if (cb->start)
> > - cb->start(cb);
> > + if (cb->start) {
> > + ret = cb->start(cb);
> > + if (ret)
>
> I need to sock_put(sk); before returning. I'll fix this for v2, but
> will for additional comments in case anybody has some.
I guess you could change it to
if (cb->start)
ret = cb->start(cb);
if (!ret)
ret = netlink_dump(sk);
johannes
^ permalink raw reply
* Re: [PATCH v2] tun: bail out from tun_get_user() if the skb is empty
From: Alexander Potapenko @ 2017-09-27 12:58 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Eric Dumazet, Dmitriy Vyukov, syzkaller, Networking,
LKML
In-Reply-To: <1506516344.6617.39.camel@edumazet-glaptop3.roam.corp.google.com>
On Wed, Sep 27, 2017 at 2:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2017-09-27 at 05:42 -0700, Eric Dumazet wrote:
>
>> Or something cleaner to avoid copy/paste and focus on proper
>> skb->data[0] access and meaning.
By the way I'm wondering if this is the only place where skb->data is
being accessed.
Isn't eth_type_trans() under IFF_TAP also touching it? Then we need to
check the size earlier.
>> Thanks.
>>
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index 3c9985f299503ea65dad7eb3b47e2ab3bef87800..8ddb840687c1bdb24e4182612abc9e362624c3e9 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -1496,11 +1496,13 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>> switch (tun->flags & TUN_TYPE_MASK) {
>> case IFF_TUN:
>> if (tun->flags & IFF_NO_PI) {
>> - switch (skb->data[0] & 0xf0) {
>> - case 0x40:
>> + u8 ip_proto = skb->len ? (skb->data[0] >> 4) : 0;
>
> And name this variable ip_version ;)
>
>
>
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply
* Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel options
From: Jiri Pirko @ 2017-09-27 12:56 UTC (permalink / raw)
To: Simon Horman
Cc: David Miller, Jiri Pirko, Jamal Hadi Salim, Cong Wang, netdev,
oss-drivers
In-Reply-To: <20170927125205.GA30000@vergenet.net>
Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote:
>On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote:
>> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:
>> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:
>> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:
>
>...
>
>> >> > enum flow_dissector_key_id {
>> >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
>> >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
>> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {
>> >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */
>> >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */
>> >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */
>> >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */
>> >>
>> >> I don't see the actual dissection implementation. Where is it?
>> >> Did you test the patchset?
>> >
>> >Yes, I did test it. But it is also possible something went astray along the
>> >way and I will retest.
>> >
>> >I think that the code you are looking for is in
>> >fl_classify() in this patch.
>>
>> The dissection should be done in the flow_dissector. That's the whole
>> point in having it generic. You should move it there.
>
>Coming back to this after lunch, I believe what I have done in this patch
>is consistent with handling of other enc fields, which are set in
>fl_classify() rather than the dissector. In particular the ip_tunnel_info,
>which is used by this patch, is already used in fl_classify().
That means the current code is wrong. The dissection should be done in
flow_dissector, not in fl_classify.
>
>Without this patch I see:
>
>
>static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
> struct tcf_result *res)
>{
> ...
> struct ip_tunnel_info *info;
>
> ...
>
> info = skb_tunnel_info(skb);
> if (info) {
> struct ip_tunnel_key *key = &info->key;
>
> switch (ip_tunnel_info_af(info)) {
> case AF_INET:
> skb_key.enc_control.addr_type =
> FLOW_DISSECTOR_KEY_IPV4_ADDRS;
> skb_key.enc_ipv4.src = key->u.ipv4.src;
> skb_key.enc_ipv4.dst = key->u.ipv4.dst;
> break;
> case AF_INET6:
> skb_key.enc_control.addr_type =
> FLOW_DISSECTOR_KEY_IPV6_ADDRS;
> skb_key.enc_ipv6.src = key->u.ipv6.src;
> skb_key.enc_ipv6.dst = key->u.ipv6.dst;
> break;
> }
>
> skb_key.enc_key_id.keyid = tunnel_id_to_key32(key->tun_id);
> skb_key.enc_tp.src = key->tp_src;
> skb_key.enc_tp.dst = key->tp_dst;
> }
>
> ...
>}
>
>This patch adds the following inside the if() clause above:
>
> if (info->options_len) {
> skb_key.enc_opts.len = info->options_len;
> ip_tunnel_info_opts_get(skb_key.enc_opts.data, info);
> }
>
>
^ permalink raw reply
* Re: tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]
From: Jiri Pirko @ 2017-09-27 12:55 UTC (permalink / raw)
To: Paolo Abeni
Cc: Or Gerlitz, Jiri Benc, Simon Horman, David Miller, Jakub Kicinski,
Linux Netdev List, oss-drivers, John Hurley, Paul Blakey,
Jiri Pirko, Roi Dayan
In-Reply-To: <1506515496.6840.6.camel@redhat.com>
Wed, Sep 27, 2017 at 02:31:36PM CEST, pabeni@redhat.com wrote:
>On Wed, 2017-09-27 at 13:11 +0200, Jiri Pirko wrote:
>> Wed, Sep 27, 2017 at 11:46:58AM CEST, pabeni@redhat.com wrote:
>> > On Wed, 2017-09-27 at 11:17 +0200, Jiri Pirko wrote:
>> > > Wed, Sep 27, 2017 at 10:29:35AM CEST, pabeni@redhat.com wrote:
>> > > > So it looks like the H/W offload hook will still be called with the
>> > > > same arguments in both case, and 'bad' rule will still be pushed to the
>> > > > H/W as the driver itself has no way to distinct between the two
>> > > > scenarios.
>> > >
>> > > Why "bad"?
>> >
>> > Such rule is coped differently by the SW and the HW data path.
>> >
>> > a rule like:
>> >
>> > tc filter add dev eth0 protocol ip parent ffff: flower \
>> > enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_hw \
>> > action action mirred redirect eth0_vf_1
>> >
>> > will match 0 packets, while:
>> >
>> > tc filter add dev eth0 protocol ip parent ffff: flower \
>> > enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_sw \
>> > action action mirred redirect eth0_vf_1
>> >
>> > [just flipped 'skip_sw' and 'skip_hw' ]
>> > will match the vxlan-tunneled packets. I understand that one of the
>> > design goal for the h/w offload path is being consistent with the sw
>> > one, but that does not hold in the above scenario.
>>
>> Sure, the consistency is important. Howcome "skip_hw" won't match and
>> "skip_sw" will match? What's different?
>
>For the SW datapath, we need a metadata based/lwt tunnel to collect the
>tunnel information. eth0 is not a such device and does not provide the
>metadata. Any match on tunnel based field will fail - correctly.
So where do you attach the tc filter instead of eth0? vxlan0?
>
>When the HW datapath is used, the underlaying NIC is programmed exactly
>as done when we replace eth0 with vxlan0. The programmed flow matches
>vxlan encapsulated traffic.
Ok, so we should unify the behaviour with sw path and don't allow such
mathing in hw.
>
>Cheers,
>
>Paolo
^ permalink raw reply
* Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel options
From: Simon Horman @ 2017-09-27 12:52 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Miller, Jiri Pirko, Jamal Hadi Salim, Cong Wang, netdev,
oss-drivers
In-Reply-To: <20170927110822.GD1944@nanopsycho.orion>
On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote:
> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:
> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:
> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:
...
> >> > enum flow_dissector_key_id {
> >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
> >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {
> >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */
> >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */
> >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */
> >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */
> >>
> >> I don't see the actual dissection implementation. Where is it?
> >> Did you test the patchset?
> >
> >Yes, I did test it. But it is also possible something went astray along the
> >way and I will retest.
> >
> >I think that the code you are looking for is in
> >fl_classify() in this patch.
>
> The dissection should be done in the flow_dissector. That's the whole
> point in having it generic. You should move it there.
Coming back to this after lunch, I believe what I have done in this patch
is consistent with handling of other enc fields, which are set in
fl_classify() rather than the dissector. In particular the ip_tunnel_info,
which is used by this patch, is already used in fl_classify().
Without this patch I see:
static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
struct tcf_result *res)
{
...
struct ip_tunnel_info *info;
...
info = skb_tunnel_info(skb);
if (info) {
struct ip_tunnel_key *key = &info->key;
switch (ip_tunnel_info_af(info)) {
case AF_INET:
skb_key.enc_control.addr_type =
FLOW_DISSECTOR_KEY_IPV4_ADDRS;
skb_key.enc_ipv4.src = key->u.ipv4.src;
skb_key.enc_ipv4.dst = key->u.ipv4.dst;
break;
case AF_INET6:
skb_key.enc_control.addr_type =
FLOW_DISSECTOR_KEY_IPV6_ADDRS;
skb_key.enc_ipv6.src = key->u.ipv6.src;
skb_key.enc_ipv6.dst = key->u.ipv6.dst;
break;
}
skb_key.enc_key_id.keyid = tunnel_id_to_key32(key->tun_id);
skb_key.enc_tp.src = key->tp_src;
skb_key.enc_tp.dst = key->tp_dst;
}
...
}
This patch adds the following inside the if() clause above:
if (info->options_len) {
skb_key.enc_opts.len = info->options_len;
ip_tunnel_info_opts_get(skb_key.enc_opts.data, info);
}
^ permalink raw reply
* Re: [PATCH] netlink: do not proceed if dump's start() errs
From: Jason A. Donenfeld @ 2017-09-27 12:50 UTC (permalink / raw)
To: David Miller, johannes.berg, Netdev, LKML; +Cc: Jason A. Donenfeld, stable
In-Reply-To: <20170927123915.5779-1-Jason@zx2c4.com>
On Wed, Sep 27, 2017 at 2:39 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> - if (cb->start)
> - cb->start(cb);
> + if (cb->start) {
> + ret = cb->start(cb);
> + if (ret)
I need to sock_put(sk); before returning. I'll fix this for v2, but
will for additional comments in case anybody has some.
> + return ret;
> + }
>
> ret = netlink_dump(sk);
> sock_put(sk);
^ permalink raw reply
* Re: [PATCH v2] tun: bail out from tun_get_user() if the skb is empty
From: Eric Dumazet @ 2017-09-27 12:45 UTC (permalink / raw)
To: Alexander Potapenko
Cc: davem, edumazet, dvyukov, syzkaller, netdev, linux-kernel
In-Reply-To: <1506516168.6617.38.camel@edumazet-glaptop3.roam.corp.google.com>
On Wed, 2017-09-27 at 05:42 -0700, Eric Dumazet wrote:
> Or something cleaner to avoid copy/paste and focus on proper
> skb->data[0] access and meaning.
>
> Thanks.
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 3c9985f299503ea65dad7eb3b47e2ab3bef87800..8ddb840687c1bdb24e4182612abc9e362624c3e9 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1496,11 +1496,13 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> switch (tun->flags & TUN_TYPE_MASK) {
> case IFF_TUN:
> if (tun->flags & IFF_NO_PI) {
> - switch (skb->data[0] & 0xf0) {
> - case 0x40:
> + u8 ip_proto = skb->len ? (skb->data[0] >> 4) : 0;
And name this variable ip_version ;)
^ permalink raw reply
* Re: [PATCH v2] tun: bail out from tun_get_user() if the skb is empty
From: Eric Dumazet @ 2017-09-27 12:42 UTC (permalink / raw)
To: Alexander Potapenko
Cc: davem, edumazet, dvyukov, syzkaller, netdev, linux-kernel
In-Reply-To: <20170927121649.90557-1-glider@google.com>
On Wed, 2017-09-27 at 14:16 +0200, Alexander Potapenko wrote:
> KMSAN (https://github.com/google/kmsan) reported accessing uninitialized
> skb->data[0] in the case the skb is empty (i.e. skb->len is 0):
>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> ---
> v2: free the skb
> ---
> drivers/net/tun.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 3c9985f29950..0d60fd4ada9e 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1496,6 +1496,11 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> switch (tun->flags & TUN_TYPE_MASK) {
> case IFF_TUN:
> if (tun->flags & IFF_NO_PI) {
> + if (!skb->len) {
> + this_cpu_inc(tun->pcpu_stats->rx_dropped);
> + kfree_skb(skb);
> + return -EINVAL;
> + }
> switch (skb->data[0] & 0xf0) {
> case 0x40:
> pi.proto = htons(ETH_P_IP);
Acked-by: Eric Dumazet <edumazet@google.com>
Or something cleaner to avoid copy/paste and focus on proper
skb->data[0] access and meaning.
Thanks.
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 3c9985f299503ea65dad7eb3b47e2ab3bef87800..8ddb840687c1bdb24e4182612abc9e362624c3e9 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1496,11 +1496,13 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
switch (tun->flags & TUN_TYPE_MASK) {
case IFF_TUN:
if (tun->flags & IFF_NO_PI) {
- switch (skb->data[0] & 0xf0) {
- case 0x40:
+ u8 ip_proto = skb->len ? (skb->data[0] >> 4) : 0;
+
+ switch (ip_proto) {
+ case 4:
pi.proto = htons(ETH_P_IP);
break;
- case 0x60:
+ case 6:
pi.proto = htons(ETH_P_IPV6);
break;
default:
^ permalink raw reply related
* Re: [PATCH v4 net-next 00/12] gtp: Additional feature support - Part I
From: Harald Welte @ 2017-09-27 12:24 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, pablo, aschultz, netdev, rohit
In-Reply-To: <20170927045803.2477-1-tom@quantonium.net>
Hi Tom,
thanks for your updated series!
I'll revie as soon as I a, but that may likely not be before next
week. As indicated before, I'm on a motorbike roadtrip on vacation,
with very limited connectivity and even more limited time for any
technical work. Thanks for your understanding.
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* [PATCH] netlink: do not proceed if dump's start() errs
From: Jason A. Donenfeld @ 2017-09-27 12:39 UTC (permalink / raw)
To: davem, johannes.berg, netdev, linux-kernel; +Cc: Jason A. Donenfeld, stable
Drivers that use the start method for netlink dumping rely on dumpit not
being called if start fails. For example, ila_xlat.c allocates memory
and assigns it to cb->args[0] in its start() function. It might fail to
do that and return -ENOMEM instead. However, even when returning an
error, dumpit will be called, which, in the example above, quickly
dereferences the memory in cb->args[0], which will OOPS the kernel. This
is but one example of how this goes wrong.
Since start() has always been a function with an int return type, it
therefore makes sense to use it properly, rather than ignoring it. This
patch thus returns early and does not call dumpit() when start() fails.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: stable@vger.kernel.org
---
net/netlink/af_netlink.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 327807731b44..be179876227d 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2270,8 +2270,11 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
mutex_unlock(nlk->cb_mutex);
- if (cb->start)
- cb->start(cb);
+ if (cb->start) {
+ ret = cb->start(cb);
+ if (ret)
+ return ret;
+ }
ret = netlink_dump(sk);
sock_put(sk);
--
2.14.1
^ permalink raw reply related
* Re: tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]
From: Paolo Abeni @ 2017-09-27 12:31 UTC (permalink / raw)
To: Jiri Pirko
Cc: Or Gerlitz, Jiri Benc, Simon Horman, David Miller, Jakub Kicinski,
Linux Netdev List, oss-drivers, John Hurley, Paul Blakey,
Jiri Pirko, Roi Dayan
In-Reply-To: <20170927111150.GE1944@nanopsycho.orion>
On Wed, 2017-09-27 at 13:11 +0200, Jiri Pirko wrote:
> Wed, Sep 27, 2017 at 11:46:58AM CEST, pabeni@redhat.com wrote:
> > On Wed, 2017-09-27 at 11:17 +0200, Jiri Pirko wrote:
> > > Wed, Sep 27, 2017 at 10:29:35AM CEST, pabeni@redhat.com wrote:
> > > > So it looks like the H/W offload hook will still be called with the
> > > > same arguments in both case, and 'bad' rule will still be pushed to the
> > > > H/W as the driver itself has no way to distinct between the two
> > > > scenarios.
> > >
> > > Why "bad"?
> >
> > Such rule is coped differently by the SW and the HW data path.
> >
> > a rule like:
> >
> > tc filter add dev eth0 protocol ip parent ffff: flower \
> > enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_hw \
> > action action mirred redirect eth0_vf_1
> >
> > will match 0 packets, while:
> >
> > tc filter add dev eth0 protocol ip parent ffff: flower \
> > enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_sw \
> > action action mirred redirect eth0_vf_1
> >
> > [just flipped 'skip_sw' and 'skip_hw' ]
> > will match the vxlan-tunneled packets. I understand that one of the
> > design goal for the h/w offload path is being consistent with the sw
> > one, but that does not hold in the above scenario.
>
> Sure, the consistency is important. Howcome "skip_hw" won't match and
> "skip_sw" will match? What's different?
For the SW datapath, we need a metadata based/lwt tunnel to collect the
tunnel information. eth0 is not a such device and does not provide the
metadata. Any match on tunnel based field will fail - correctly.
When the HW datapath is used, the underlaying NIC is programmed exactly
as done when we replace eth0 with vxlan0. The programmed flow matches
vxlan encapsulated traffic.
Cheers,
Paolo
^ permalink raw reply
* Re: [PATCH] xfrm: don't call xfrm_policy_cache_flush under xfrm_state_lock
From: Florian Westphal @ 2017-09-27 12:31 UTC (permalink / raw)
To: Artem Savkov
Cc: Florian Westphal, Steffen Klassert, Herbert Xu, David S . Miller,
netdev, linux-kernel
In-Reply-To: <20170927122537.14235-1-asavkov@redhat.com>
Artem Savkov <asavkov@redhat.com> wrote:
> I might be wrong but it doesn't look like xfrm_state_lock is required
> for xfrm_policy_cache_flush and calling it under this lock triggers both
> "sleeping function called from invalid context" and "possible circular
> locking dependency detected" warnings on flush.
>
> Fixes: ec30d78c14a8 xfrm: add xdst pcpu cache
> Signed-off-by: Artem Savkov <asavkov@redhat.com>
You're right, its not needed (and wrong).
Acked-by: Florian Westphal <fw@strlen.de>
^ permalink raw reply
* [PATCH] xfrm: don't call xfrm_policy_cache_flush under xfrm_state_lock
From: Artem Savkov @ 2017-09-27 12:25 UTC (permalink / raw)
To: Florian Westphal
Cc: Steffen Klassert, Herbert Xu, David S . Miller, netdev,
linux-kernel, Artem Savkov
I might be wrong but it doesn't look like xfrm_state_lock is required
for xfrm_policy_cache_flush and calling it under this lock triggers both
"sleeping function called from invalid context" and "possible circular
locking dependency detected" warnings on flush.
Fixes: ec30d78c14a8 xfrm: add xdst pcpu cache
Signed-off-by: Artem Savkov <asavkov@redhat.com>
---
net/xfrm/xfrm_state.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 0dab1cd79ce4..12213477cd3a 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -732,12 +732,12 @@ int xfrm_state_flush(struct net *net, u8 proto, bool task_valid)
}
}
}
+out:
+ spin_unlock_bh(&net->xfrm.xfrm_state_lock);
if (cnt) {
err = 0;
xfrm_policy_cache_flush();
}
-out:
- spin_unlock_bh(&net->xfrm.xfrm_state_lock);
return err;
}
EXPORT_SYMBOL(xfrm_state_flush);
--
2.13.5
^ permalink raw reply related
* [PATCH v2] tun: bail out from tun_get_user() if the skb is empty
From: Alexander Potapenko @ 2017-09-27 12:16 UTC (permalink / raw)
To: davem, edumazet; +Cc: dvyukov, syzkaller, netdev, linux-kernel
KMSAN (https://github.com/google/kmsan) reported accessing uninitialized
skb->data[0] in the case the skb is empty (i.e. skb->len is 0):
================================================
BUG: KMSAN: use of uninitialized memory in tun_get_user+0x19ba/0x3770
CPU: 0 PID: 3051 Comm: probe Not tainted 4.13.0+ #3140
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
...
__msan_warning_32+0x66/0xb0 mm/kmsan/kmsan_instr.c:477
tun_get_user+0x19ba/0x3770 drivers/net/tun.c:1301
tun_chr_write_iter+0x19f/0x300 drivers/net/tun.c:1365
call_write_iter ./include/linux/fs.h:1743
new_sync_write fs/read_write.c:457
__vfs_write+0x6c3/0x7f0 fs/read_write.c:470
vfs_write+0x3e4/0x770 fs/read_write.c:518
SYSC_write+0x12f/0x2b0 fs/read_write.c:565
SyS_write+0x55/0x80 fs/read_write.c:557
do_syscall_64+0x242/0x330 arch/x86/entry/common.c:284
entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:245
...
origin:
...
kmsan_poison_shadow+0x6e/0xc0 mm/kmsan/kmsan.c:211
slab_alloc_node mm/slub.c:2732
__kmalloc_node_track_caller+0x351/0x370 mm/slub.c:4351
__kmalloc_reserve net/core/skbuff.c:138
__alloc_skb+0x26a/0x810 net/core/skbuff.c:231
alloc_skb ./include/linux/skbuff.h:903
alloc_skb_with_frags+0x1d7/0xc80 net/core/skbuff.c:4756
sock_alloc_send_pskb+0xabf/0xfe0 net/core/sock.c:2037
tun_alloc_skb drivers/net/tun.c:1144
tun_get_user+0x9a8/0x3770 drivers/net/tun.c:1274
tun_chr_write_iter+0x19f/0x300 drivers/net/tun.c:1365
call_write_iter ./include/linux/fs.h:1743
new_sync_write fs/read_write.c:457
__vfs_write+0x6c3/0x7f0 fs/read_write.c:470
vfs_write+0x3e4/0x770 fs/read_write.c:518
SYSC_write+0x12f/0x2b0 fs/read_write.c:565
SyS_write+0x55/0x80 fs/read_write.c:557
do_syscall_64+0x242/0x330 arch/x86/entry/common.c:284
return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.S:245
================================================
Make sure tun_get_user() doesn't touch skb->data[0] unless there is
actual data.
C reproducer below:
==========================
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#define _GNU_SOURCE
#include <fcntl.h>
#include <linux/if_tun.h>
#include <netinet/ip.h>
#include <net/if.h>
#include <string.h>
#include <sys/ioctl.h>
int main()
{
int sock = socket(PF_INET, SOCK_STREAM, IPPROTO_IP);
int tun_fd = open("/dev/net/tun", O_RDWR);
struct ifreq req;
memset(&req, 0, sizeof(struct ifreq));
strcpy((char*)&req.ifr_name, "gre0");
req.ifr_flags = IFF_UP | IFF_MULTICAST;
ioctl(tun_fd, TUNSETIFF, &req);
ioctl(sock, SIOCSIFFLAGS, "gre0");
write(tun_fd, "hi", 0);
return 0;
}
==========================
Signed-off-by: Alexander Potapenko <glider@google.com>
---
v2: free the skb
---
drivers/net/tun.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 3c9985f29950..0d60fd4ada9e 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1496,6 +1496,11 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
switch (tun->flags & TUN_TYPE_MASK) {
case IFF_TUN:
if (tun->flags & IFF_NO_PI) {
+ if (!skb->len) {
+ this_cpu_inc(tun->pcpu_stats->rx_dropped);
+ kfree_skb(skb);
+ return -EINVAL;
+ }
switch (skb->data[0] & 0xf0) {
case 0x40:
pi.proto = htons(ETH_P_IP);
--
2.14.1.821.g8fa685d3b7-goog
^ permalink raw reply related
* [bug][tipc] Too short struct tipc_subscr passed to tipc_subscrb_rcv_cb()
From: Alexander Potapenko @ 2017-09-27 12:15 UTC (permalink / raw)
To: David Miller, ying.xue, jon.maloy; +Cc: Networking, syzkaller
[-- Attachment #1: Type: text/plain, Size: 3208 bytes --]
Hi all,
I've spotted the following bug in TIPC server code while fuzzing it
with KMSAN and syzkaller:
==================================================================
BUG: KMSAN: use of uninitialized memory in tipc_subscrb_rcv_cb+0x14a/0xc20
CPU: 2 PID: 29 Comm: kworker/u8:1 Not tainted 4.13.0+ #3150
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: tipc_rcv tipc_recv_work
Call Trace:
__dump_stack lib/dump_stack.c:16
dump_stack+0x172/0x1c0 lib/dump_stack.c:52
kmsan_report+0x145/0x3d0 mm/kmsan/kmsan.c:943
__msan_warning_32+0x66/0xb0 mm/kmsan/kmsan_instr.c:477
htohl net/tipc/subscr.c:66
tipc_subscrb_rcv_cb+0x14a/0xc20 net/tipc/subscr.c:333
tipc_receive_from_sock+0x369/0x540 net/tipc/server.c:273
tipc_recv_work+0xbe/0x1c0 net/tipc/server.c:546
process_one_work+0xf9e/0x1ad0 kernel/workqueue.c:2097
worker_thread+0xefd/0x2090 kernel/workqueue.c:2231
kthread+0x499/0x620 kernel/kthread.c:232
ret_from_fork+0x22/0x30 arch/x86/entry/entry_64.S:425
origin:
save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:302
kmsan_internal_poison_shadow+0xb3/0x1b0 mm/kmsan/kmsan.c:198
kmsan_kmalloc+0x80/0xe0 mm/kmsan/kmsan.c:337
kmem_cache_alloc+0x1e2/0x220 mm/slub.c:2756
tipc_receive_from_sock+0xff/0x540 net/tipc/server.c:257
tipc_recv_work+0xbe/0x1c0 net/tipc/server.c:546
process_one_work+0xf9e/0x1ad0 kernel/workqueue.c:2097
worker_thread+0xefd/0x2090 kernel/workqueue.c:2231
kthread+0x499/0x620 kernel/kthread.c:232
ret_from_fork+0x22/0x30 arch/x86/entry/entry_64.S:425
==================================================================
(see the reproducer attached).
Here the user sends 16 bytes via sendmsg(), which the kernel attempts
to treat as a struct tipc_subscr. Because the actual size of a struct
is bigger, we end up touching uninitialized fields.
What is the best way to fix this?
I think it's too late to check for |len| in tipc_subscrb_rcv_cb()
(http://elixir.free-electrons.com/linux/latest/source/net/tipc/subscr.c#L320),
because we'll probably need to call tipc_subscrp_cancel(s, subscriber)
in order to cancel the subscription, which is a bad idea since |s| is
invalid.
We could check that ret >= sizeof(struct tipc_subscr) in
tipc_receive_from_sock()
(http://elixir.free-electrons.com/linux/v4.8/source/net/tipc/server.c#L273),
but this function is agnostic to the type of the data received by the
callback, so we'd better not hardcode the size there.
Shall we add an integer field (or yet another callback) to tipc_server, maybe?
On a related note, something is wrong with the
tipc_receive_from_sock()'s return value.
First, the user appears to receive a positive value (16 in our case)
regardless of what tipc_receive_from_sock() returns.
Second, the branch at
http://elixir.free-electrons.com/linux/v4.8/source/net/tipc/server.c#L288
is never taken.
WBR,
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
[-- Attachment #2: tipc_subscrb_rcv_cb-kmsan.c --]
[-- Type: text/x-csrc, Size: 733 bytes --]
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <arpa/inet.h>
#include <linux/tipc.h>
#include <string.h>
int main()
{
int sock = socket(AF_TIPC, SOCK_SEQPACKET, 0);
struct sockaddr_tipc addr;
memset(&addr, 0, sizeof(addr));
addr.family = AF_TIPC;
addr.addrtype = TIPC_ADDR_NAME;
addr.addr.name.name.type = 1;
addr.addr.name.name.instance = 1;
struct tipc_subscr subscr;
memset(&subscr, 0, sizeof(subscr));
struct iovec iov;
iov.iov_base = &subscr;
iov.iov_len = sizeof(subscr) - 12;
struct msghdr hdr;
memset(&hdr, 0, sizeof(hdr));
hdr.msg_name = &addr;
hdr.msg_namelen = sizeof(addr);
hdr.msg_iov = &iov;
hdr.msg_iovlen = 1;
sendmsg(sock, &hdr, 0);
return 0;
}
^ permalink raw reply
* Re: [RFT] lan78xx: FIX use-after-free in lan78xx_write_reg
From: Andrey Konovalov @ 2017-09-27 12:12 UTC (permalink / raw)
To: Arvind Yadav
Cc: David S. Miller, Woojung Huh, Microchip Linux Driver Support,
netdev, Kostya Serebryany, Dmitry Vyukov, Nisar.Sayed, USB list,
LKML, syzkaller
In-Reply-To: <CAAeHK+y1pc98H4xcVC79yBdfAD3212VxUYzjvFGx9bumwvH_KQ@mail.gmail.com>
On Wed, Sep 27, 2017 at 2:06 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> On Wed, Sep 27, 2017 at 1:18 PM, Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:
>> We are not releasing 'buf' memory on failure or disconnect a device.
>>
>> Adding 'u8 *buf' as part of 'lan78xx_net' structure to make proper
>> handle for 'buf'.
>> Now releasing 'buf' memory on failure. It's allocate first in
>> lan78xx_probe() and it should be freed last in lan78xx_disconnect().
>>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>
> Hi Arvind,
>
> I've tried your patch and still see a crash.
>
> Thanks!
>
> lan78xx 1-1:90.0 (unnamed net_device) (uninitialized): can't register MDIO bus
> lan78xx: probe of 1-1:90.0 failed with error -5
> usb 1-1: USB disconnect, device number 2
> ==================================================================
> BUG: KASAN: use-after-free in lan78xx_write_reg.isra.21+0x1a8/0x1b0
> Read of size 8 at addr ffff880063dfac40 by task kworker/1:1/1152
It seems that the bug is caused by the fact that the
lan78xx_deferred_multicast_write() work is not shut down on
lan78xx_probe() failure.
>
> CPU: 1 PID: 1152 Comm: kworker/1:1 Not tainted
> 4.14.0-rc2-42660-g24b7bd59eec0-dirty #274
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: events lan78xx_deferred_multicast_write
> Call Trace:
> __dump_stack lib/dump_stack.c:16
> dump_stack+0x292/0x395 lib/dump_stack.c:52
> print_address_description+0x78/0x280 mm/kasan/report.c:252
> kasan_report_error mm/kasan/report.c:351
> kasan_report+0x23d/0x350 mm/kasan/report.c:409
> __asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:430
> lan78xx_write_reg.isra.21+0x1a8/0x1b0 drivers/net/usb/lan78xx.c:459
> lan78xx_deferred_multicast_write+0x10d/0x1a0 drivers/net/usb/lan78xx.c:1043
> process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
> worker_thread+0x221/0x1850 kernel/workqueue.c:2253
> kthread+0x3a1/0x470 kernel/kthread.c:231
> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
>
> Allocated by task 1848:
> save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
> save_stack+0x43/0xd0 mm/kasan/kasan.c:447
> set_track mm/kasan/kasan.c:459
> kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
> __kmalloc_node+0x192/0x480 mm/slub.c:3827
> kmalloc_node ./include/linux/slab.h:535
> kvmalloc_node+0x69/0xd0 mm/util.c:397
> kvmalloc ./include/linux/mm.h:529
> kvzalloc ./include/linux/mm.h:537
> alloc_netdev_mqs+0x173/0xea0 net/core/dev.c:8023
> alloc_etherdev_mqs+0x38/0x40 net/ethernet/eth.c:391
> lan78xx_probe+0x13a/0x3020 drivers/net/usb/lan78xx.c:3545
> usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
> generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
> usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
> hub_port_connect drivers/usb/core/hub.c:4903
> hub_port_connect_change drivers/usb/core/hub.c:5009
> port_event drivers/usb/core/hub.c:5115
> hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
> process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
> worker_thread+0x221/0x1850 kernel/workqueue.c:2253
> kthread+0x3a1/0x470 kernel/kthread.c:231
> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
>
> Freed by task 1848:
> save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
> save_stack+0x43/0xd0 mm/kasan/kasan.c:447
> set_track mm/kasan/kasan.c:459
> kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:524
> slab_free_hook mm/slub.c:1390
> slab_free_freelist_hook mm/slub.c:1412
> slab_free mm/slub.c:2988
> kfree+0xf6/0x2f0 mm/slub.c:3919
> kvfree+0x3b/0x60 mm/util.c:416
> netdev_freemem net/core/dev.c:7975
> free_netdev+0x304/0x3c0 net/core/dev.c:8137
> lan78xx_probe+0x21a4/0x3020 drivers/net/usb/lan78xx.c:3649
> usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
> generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
> usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
> hub_port_connect drivers/usb/core/hub.c:4903
> hub_port_connect_change drivers/usb/core/hub.c:5009
> port_event drivers/usb/core/hub.c:5115
> hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
> process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
> worker_thread+0x221/0x1850 kernel/workqueue.c:2253
> kthread+0x3a1/0x470 kernel/kthread.c:231
> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
>
> The buggy address belongs to the object at ffff880063dfa100
> which belongs to the cache kmalloc-8192 of size 8192
> The buggy address is located 2880 bytes inside of
> 8192-byte region [ffff880063dfa100, ffff880063dfc100)
> The buggy address belongs to the page:
> page:ffffea00018f7e00 count:1 mapcount:0 mapping: (null)
> index:0x0 compound_mapcount: 0
> flags: 0x100000000008100(slab|head)
> raw: 0100000000008100 0000000000000000 0000000000000000 0000000100030003
> raw: ffffea0001afd800 0000000300000003 ffff88006c402a80 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff880063dfab00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff880063dfab80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>>ffff880063dfac00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ^
> ffff880063dfac80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff880063dfad00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ==================================================================
>
>> ---
>> drivers/net/usb/lan78xx.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
>> index b99a7fb..e653982 100644
>> --- a/drivers/net/usb/lan78xx.c
>> +++ b/drivers/net/usb/lan78xx.c
>> @@ -402,6 +402,7 @@ struct lan78xx_net {
>> struct statstage stats;
>>
>> struct irq_domain_data domain_data;
>> + u8 *buf;
>> };
>>
>> /* define external phy id */
>> @@ -3470,6 +3471,9 @@ static void lan78xx_disconnect(struct usb_interface *intf)
>>
>> usb_scuttle_anchored_urbs(&dev->deferred);
>>
>> + kfree(dev->buf);
>> + dev->buf = NULL;
>> +
>> lan78xx_unbind(dev, intf);
>>
>> usb_kill_urb(dev->urb_intr);
>> @@ -3520,7 +3524,6 @@ static int lan78xx_probe(struct usb_interface *intf,
>> int ret;
>> unsigned maxp;
>> unsigned period;
>> - u8 *buf = NULL;
>>
>> udev = interface_to_usbdev(intf);
>> udev = usb_get_dev(udev);
>> @@ -3588,16 +3591,15 @@ static int lan78xx_probe(struct usb_interface *intf,
>> period = dev->ep_intr->desc.bInterval;
>>
>> maxp = usb_maxpacket(dev->udev, dev->pipe_intr, 0);
>> - buf = kmalloc(maxp, GFP_KERNEL);
>> - if (buf) {
>> + dev->buf = kmalloc(maxp, GFP_KERNEL);
>> + if (dev->buf) {
>> dev->urb_intr = usb_alloc_urb(0, GFP_KERNEL);
>> if (!dev->urb_intr) {
>> ret = -ENOMEM;
>> - kfree(buf);
>> goto out3;
>> } else {
>> usb_fill_int_urb(dev->urb_intr, dev->udev,
>> - dev->pipe_intr, buf, maxp,
>> + dev->pipe_intr, dev->buf, maxp,
>> intr_complete, dev, period);
>> }
>> }
>> @@ -3626,6 +3628,8 @@ static int lan78xx_probe(struct usb_interface *intf,
>> return 0;
>>
>> out3:
>> + kfree(dev->buf);
>> + dev->buf = NULL;
>> lan78xx_unbind(dev, intf);
>> out2:
>> free_netdev(netdev);
>> --
>> 1.9.1
>>
^ permalink raw reply
* Re: [RFT] lan78xx: FIX use-after-free in lan78xx_write_reg
From: Andrey Konovalov @ 2017-09-27 12:06 UTC (permalink / raw)
To: Arvind Yadav
Cc: David S. Miller, Woojung Huh, Microchip Linux Driver Support,
netdev, Kostya Serebryany, Dmitry Vyukov, Nisar.Sayed, USB list,
LKML, syzkaller
In-Reply-To: <d7710b468c6c1b9cf3b3fe2fc986118a2cd2639a.1506510873.git.arvind.yadav.cs@gmail.com>
On Wed, Sep 27, 2017 at 1:18 PM, Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:
> We are not releasing 'buf' memory on failure or disconnect a device.
>
> Adding 'u8 *buf' as part of 'lan78xx_net' structure to make proper
> handle for 'buf'.
> Now releasing 'buf' memory on failure. It's allocate first in
> lan78xx_probe() and it should be freed last in lan78xx_disconnect().
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
Hi Arvind,
I've tried your patch and still see a crash.
Thanks!
lan78xx 1-1:90.0 (unnamed net_device) (uninitialized): can't register MDIO bus
lan78xx: probe of 1-1:90.0 failed with error -5
usb 1-1: USB disconnect, device number 2
==================================================================
BUG: KASAN: use-after-free in lan78xx_write_reg.isra.21+0x1a8/0x1b0
Read of size 8 at addr ffff880063dfac40 by task kworker/1:1/1152
CPU: 1 PID: 1152 Comm: kworker/1:1 Not tainted
4.14.0-rc2-42660-g24b7bd59eec0-dirty #274
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: events lan78xx_deferred_multicast_write
Call Trace:
__dump_stack lib/dump_stack.c:16
dump_stack+0x292/0x395 lib/dump_stack.c:52
print_address_description+0x78/0x280 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351
kasan_report+0x23d/0x350 mm/kasan/report.c:409
__asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:430
lan78xx_write_reg.isra.21+0x1a8/0x1b0 drivers/net/usb/lan78xx.c:459
lan78xx_deferred_multicast_write+0x10d/0x1a0 drivers/net/usb/lan78xx.c:1043
process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
worker_thread+0x221/0x1850 kernel/workqueue.c:2253
kthread+0x3a1/0x470 kernel/kthread.c:231
ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
Allocated by task 1848:
save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
__kmalloc_node+0x192/0x480 mm/slub.c:3827
kmalloc_node ./include/linux/slab.h:535
kvmalloc_node+0x69/0xd0 mm/util.c:397
kvmalloc ./include/linux/mm.h:529
kvzalloc ./include/linux/mm.h:537
alloc_netdev_mqs+0x173/0xea0 net/core/dev.c:8023
alloc_etherdev_mqs+0x38/0x40 net/ethernet/eth.c:391
lan78xx_probe+0x13a/0x3020 drivers/net/usb/lan78xx.c:3545
usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
really_probe drivers/base/dd.c:413
driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
__device_attach_driver+0x230/0x290 drivers/base/dd.c:653
bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
__device_attach+0x26e/0x3d0 drivers/base/dd.c:710
device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
device_add+0xd0b/0x1660 drivers/base/core.c:1835
usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
really_probe drivers/base/dd.c:413
driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
__device_attach_driver+0x230/0x290 drivers/base/dd.c:653
bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
__device_attach+0x26e/0x3d0 drivers/base/dd.c:710
device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
device_add+0xd0b/0x1660 drivers/base/core.c:1835
usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
hub_port_connect drivers/usb/core/hub.c:4903
hub_port_connect_change drivers/usb/core/hub.c:5009
port_event drivers/usb/core/hub.c:5115
hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
worker_thread+0x221/0x1850 kernel/workqueue.c:2253
kthread+0x3a1/0x470 kernel/kthread.c:231
ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
Freed by task 1848:
save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459
kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:524
slab_free_hook mm/slub.c:1390
slab_free_freelist_hook mm/slub.c:1412
slab_free mm/slub.c:2988
kfree+0xf6/0x2f0 mm/slub.c:3919
kvfree+0x3b/0x60 mm/util.c:416
netdev_freemem net/core/dev.c:7975
free_netdev+0x304/0x3c0 net/core/dev.c:8137
lan78xx_probe+0x21a4/0x3020 drivers/net/usb/lan78xx.c:3649
usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
really_probe drivers/base/dd.c:413
driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
__device_attach_driver+0x230/0x290 drivers/base/dd.c:653
bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
__device_attach+0x26e/0x3d0 drivers/base/dd.c:710
device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
device_add+0xd0b/0x1660 drivers/base/core.c:1835
usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
really_probe drivers/base/dd.c:413
driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
__device_attach_driver+0x230/0x290 drivers/base/dd.c:653
bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
__device_attach+0x26e/0x3d0 drivers/base/dd.c:710
device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
device_add+0xd0b/0x1660 drivers/base/core.c:1835
usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
hub_port_connect drivers/usb/core/hub.c:4903
hub_port_connect_change drivers/usb/core/hub.c:5009
port_event drivers/usb/core/hub.c:5115
hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
worker_thread+0x221/0x1850 kernel/workqueue.c:2253
kthread+0x3a1/0x470 kernel/kthread.c:231
ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
The buggy address belongs to the object at ffff880063dfa100
which belongs to the cache kmalloc-8192 of size 8192
The buggy address is located 2880 bytes inside of
8192-byte region [ffff880063dfa100, ffff880063dfc100)
The buggy address belongs to the page:
page:ffffea00018f7e00 count:1 mapcount:0 mapping: (null)
index:0x0 compound_mapcount: 0
flags: 0x100000000008100(slab|head)
raw: 0100000000008100 0000000000000000 0000000000000000 0000000100030003
raw: ffffea0001afd800 0000000300000003 ffff88006c402a80 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff880063dfab00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff880063dfab80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff880063dfac00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff880063dfac80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff880063dfad00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
> ---
> drivers/net/usb/lan78xx.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index b99a7fb..e653982 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -402,6 +402,7 @@ struct lan78xx_net {
> struct statstage stats;
>
> struct irq_domain_data domain_data;
> + u8 *buf;
> };
>
> /* define external phy id */
> @@ -3470,6 +3471,9 @@ static void lan78xx_disconnect(struct usb_interface *intf)
>
> usb_scuttle_anchored_urbs(&dev->deferred);
>
> + kfree(dev->buf);
> + dev->buf = NULL;
> +
> lan78xx_unbind(dev, intf);
>
> usb_kill_urb(dev->urb_intr);
> @@ -3520,7 +3524,6 @@ static int lan78xx_probe(struct usb_interface *intf,
> int ret;
> unsigned maxp;
> unsigned period;
> - u8 *buf = NULL;
>
> udev = interface_to_usbdev(intf);
> udev = usb_get_dev(udev);
> @@ -3588,16 +3591,15 @@ static int lan78xx_probe(struct usb_interface *intf,
> period = dev->ep_intr->desc.bInterval;
>
> maxp = usb_maxpacket(dev->udev, dev->pipe_intr, 0);
> - buf = kmalloc(maxp, GFP_KERNEL);
> - if (buf) {
> + dev->buf = kmalloc(maxp, GFP_KERNEL);
> + if (dev->buf) {
> dev->urb_intr = usb_alloc_urb(0, GFP_KERNEL);
> if (!dev->urb_intr) {
> ret = -ENOMEM;
> - kfree(buf);
> goto out3;
> } else {
> usb_fill_int_urb(dev->urb_intr, dev->udev,
> - dev->pipe_intr, buf, maxp,
> + dev->pipe_intr, dev->buf, maxp,
> intr_complete, dev, period);
> }
> }
> @@ -3626,6 +3628,8 @@ static int lan78xx_probe(struct usb_interface *intf,
> return 0;
>
> out3:
> + kfree(dev->buf);
> + dev->buf = NULL;
> lan78xx_unbind(dev, intf);
> out2:
> free_netdev(netdev);
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel options
From: Simon Horman @ 2017-09-27 11:54 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Miller, Jiri Pirko, Jamal Hadi Salim, Cong Wang, netdev,
oss-drivers
In-Reply-To: <20170927110822.GD1944@nanopsycho.orion>
On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote:
> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:
> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:
> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:
> >> >Allow matching on options in tunnel headers.
> >> >This makes use of existing tunnel metadata support.
> >> >
> >> >Options are a bytestring of up to 256 bytes.
> >> >Tunnel implementations may support less or more options,
> >> >or no options at all.
> >> >
> >> >e.g.
> >> > # ip link add name geneve0 type geneve dstport 0 external
> >> > # tc qdisc add dev geneve0 ingress
> >> > # tc filter add dev geneve0 protocol ip parent ffff: \
> >> > flower \
> >> > enc_src_ip 10.0.99.192 \
> >> > enc_dst_ip 10.0.99.193 \
> >> > enc_key_id 11 \
> >> > enc_opts 0102800100800020/fffffffffffffff0 \
> >> > ip_proto udp \
> >> > action mirred egress redirect dev eth1
> >> >
> >> >Signed-off-by: Simon Horman <simon.horman@netronome.com>
> >> >Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> >> >
> >> >---
> >> >v2
> >> >* Correct example which was incorrectly described setting rather
> >> > than matching tunnel options
> >> >---
> >> > include/net/flow_dissector.h | 13 +++++++++++++
> >> > include/uapi/linux/pkt_cls.h | 3 +++
> >> > net/sched/cls_flower.c | 35 ++++++++++++++++++++++++++++++++++-
> >> > 3 files changed, 50 insertions(+), 1 deletion(-)
> >> >
> >> >diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> >> >index fc3dce730a6b..43f98bf0b349 100644
> >> >--- a/include/net/flow_dissector.h
> >> >+++ b/include/net/flow_dissector.h
> >> >@@ -183,6 +183,18 @@ struct flow_dissector_key_ip {
> >> > __u8 ttl;
> >> > };
> >> >
> >> >+/**
> >> >+ * struct flow_dissector_key_enc_opts:
> >> >+ * @data: data
> >> >+ * @len: len
> >> >+ */
> >> >+struct flow_dissector_key_enc_opts {
> >> >+ u8 data[256]; /* Using IP_TUNNEL_OPTS_MAX is desired here
> >> >+ * but seems difficult to #include
> >> >+ */
> >> >+ u8 len;
> >> >+};
> >> >+
> >> > enum flow_dissector_key_id {
> >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
> >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {
> >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */
> >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */
> >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */
> >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */
> >>
> >> I don't see the actual dissection implementation. Where is it?
> >> Did you test the patchset?
> >
> >Yes, I did test it. But it is also possible something went astray along the
> >way and I will retest.
> >
> >I think that the code you are looking for is in
> >fl_classify() in this patch.
>
> The dissection should be done in the flow_dissector. That's the whole
> point in having it generic. You should move it there.
>
Thanks, will do.
^ permalink raw reply
* Re: [PATCH net-next 2/2] tools: bpf: add bpftool
From: Markus Heiser @ 2017-09-27 11:49 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Jonathan Corbet
Cc: Jakub Kicinski, Alexei Starovoitov, netdev, daniel, davem, hannes,
dsahern, oss-drivers, Linux Doc Mailing List
In-Reply-To: <20170927131906.286b1e25@redhat.com>
> Am 27.09.2017 um 13:19 schrieb Jesper Dangaard Brouer <brouer@redhat.com>:
[...]
>>> I would prefer adding a README.rst file, in RST-format, as the rest of
>>> the kernel documentation is moving in that direction[1] (your github
>>> version is in README.md format). A man page will always be
>>> out-of-sync, and even out-of-sync on different distros.
>>>
>>> See[1]: https://www.kernel.org/doc/html/latest/
>>>
>>> And then I would find some place in Documentation/admin-guide/ and
>>> include the README.rst file, so it shows up at [1].
>>>
>>> RST have an include method like:
>>>
>>> .. include:: ../../tools/bpf/bpftool/README.rst
>>
>> Can the docs in new format be rendered into a man page? Call me old
>> fashioned but I think we should provide some form of a man page.. :)
>
> Yes, simply create the man page like:
>
> rst2man README.rst README.man
> You can add this to your local makefile.
Hm ... this will work only for simple reST files.
FYI: Sphinx is build up on top of docutils extending the reST markup. Tools
like rst2man are from docutils and do not cover the markup extensions
from Sphinx. Since Sphinx has its own builder for man pages, generating
man pages is not the problem ...
> The standard sphinx build can also generate man-pages, but it have been
> removed from the kernel makefile targets:
>
> Documentation targets:
> Linux kernel internal documentation in different formats from ReST:
> htmldocs - HTML
> latexdocs - LaTeX
> pdfdocs - PDF
> epubdocs - EPUB
> xmldocs - XML
> linkcheckdocs - check for broken external links (will connect to external hosts)
> cleandocs - clean all generated files
The reason why we have no man page target is, that we have not yet
evaluated a concept, how we can manage the 'build man pages' task in
the kernel build.
Beside what you find in the kernel-source tree I'am working on such
a concept (adapted man page builder for the Kernel) [1].
IMO: Even if we merge the builder from [1] or implement something different,
we have to touch the current kernel-doc parser. I guess that and the absence
of a handy concept is the reason why we are dangling with a 'man' target.
Anyway, these are my estimations, it might be better if we hear what
Jon think about a man page builder (concept).
[1] https://return42.github.io/linuxdoc/linuxdoc-howto/man-pages.html
-- Markus --
^ permalink raw reply
* Re: [PATCH net] i40e: Fix limit imprecise of the number of MAC/VLAN that can be added for VFs
From: Sergei Shtylyov @ 2017-09-27 11:33 UTC (permalink / raw)
To: w00273186, davem, jeffrey.t.kirsher; +Cc: netdev, intel-wired-lan, caihe
In-Reply-To: <1506495497-10736-1-git-send-email-wangyunjian@huawei.com>
Hello!
On 9/27/2017 9:58 AM, w00273186 wrote:
> From: Yunjian Wang <wangyunjian@huawei.com>
>
> Now it don't limit the number of MAC/VLAN strictly. When there is more
Doesn't.
> elements in the virtchnl MAC/VLAN list, it can still add successfully.
>
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties
From: Mika Westerberg @ 2017-09-27 11:32 UTC (permalink / raw)
To: David Miller
Cc: gregkh, andreas.noever, michael.jamet, yehezkel.bernat,
amir.jer.levy, Mario.Limonciello, lukas, andriy.shevchenko,
andrew, linux-kernel, netdev
In-Reply-To: <20170926.213354.305351416790211828.davem@davemloft.net>
On Tue, Sep 26, 2017 at 09:33:54PM -0700, David Miller wrote:
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> Date: Mon, 25 Sep 2017 14:07:24 +0300
>
> > +struct tb_property_entry {
> > + u32 key_hi;
> > + u32 key_lo;
> > + u16 length;
> > + u8 reserved;
> > + u8 type;
> > + u32 value;
> > +} __packed;
> > +
> > +struct tb_property_rootdir_entry {
> > + u32 magic;
> > + u32 length;
> > + struct tb_property_entry entries[];
> > +} __packed;
> > +
> > +struct tb_property_dir_entry {
> > + u32 uuid[4];
> > + struct tb_property_entry entries[];
> > +} __packed;
>
> There is no apparent need for __packed here, and __packed should be
> avoided unless absolutely necessary as it pessimizes the code
> significantly on some architectures.
>
> Please remove __packed from these datastructures unless you can
> prove it is absolutely needed and, in such case, please document
> in a comment why that requirement exists. Because from the layout
> of these types, everything will be packed in just fine without
> __packed.
I will thanks.
Just for my education, is there some rule which tells when __packed is
to be used? For example the above structures are all 32-bit aligned but
how about something like:
struct foo {
u32 value1;
u8 value2;
};
If the on-wire format requires such structures I assume __packed
is needed here?
Thanks!
^ permalink raw reply
* Re: [PATCH net-next 2/2] tools: bpf: add bpftool
From: Jesper Dangaard Brouer @ 2017-09-27 11:19 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Alexei Starovoitov, netdev, daniel, davem, hannes, dsahern,
oss-drivers, linux-doc@vger.kernel.org, brouer
In-Reply-To: <20170927035742.5c9ee8c3@cakuba>
On Wed, 27 Sep 2017 03:57:42 -0700
Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> On Wed, 27 Sep 2017 12:45:11 +0200, Jesper Dangaard Brouer wrote:
> > On Wed, 27 Sep 2017 00:02:08 +0100
> > Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> >
> > > On Tue, 26 Sep 2017 15:24:06 -0700, Alexei Starovoitov wrote:
> > > > On Tue, Sep 26, 2017 at 08:35:22AM -0700, Jakub Kicinski wrote:
> > > > > Add a simple tool for querying and updating BPF objects on the system.
> > > > >
> > > > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > > > Reviewed-by: Simon Horman <simon.horman@netronome.com>
> > [...]
> > > > > tools/bpf/Makefile | 18 +-
> > > > > tools/bpf/bpftool/Makefile | 80 +++++
> > > > > tools/bpf/bpftool/common.c | 214 ++++++++++++
> > > > > tools/bpf/bpftool/jit_disasm.c | 83 +++++
> > > > > tools/bpf/bpftool/main.c | 212 ++++++++++++
> > > > > tools/bpf/bpftool/main.h | 99 ++++++
> > > > > tools/bpf/bpftool/map.c | 742 +++++++++++++++++++++++++++++++++++++++++
> > > > > tools/bpf/bpftool/prog.c | 392 ++++++++++++++++++++++
> > > > > 8 files changed, 1837 insertions(+), 3 deletions(-)
> > > > ...
> > > > > +static int do_help(int argc, char **argv)
> > > > > +{
> > > > > + fprintf(stderr,
> > > > > + "Usage: %s %s show [MAP]\n"
> > > > > + " %s %s dump MAP\n"
> > > > > + " %s %s update MAP key BYTES value VALUE [UPDATE_FLAGS]\n"
> > > > > + " %s %s lookup MAP key BYTES\n"
> > > > > + " %s %s getnext MAP [key BYTES]\n"
> > > > > + " %s %s delete MAP key BYTES\n"
> > > > > + " %s %s pin MAP FILE\n"
> > > > > + " %s %s help\n"
> > > > > + "\n"
> > > > > + " MAP := { id MAP_ID | pinned FILE }\n"
> > > > > + " " HELP_SPEC_PROGRAM "\n"
> > > > > + " VALUE := { BYTES | MAP | PROG }\n"
> > > > > + " UPDATE_FLAGS := { any | exist | noexist }\n"
> > > > > + "",
> > > >
> > > > overall looks good to me, but still difficult to grasp how to use it.
> > > > Can you add README with example usage and expected output?
> > >
> > > I have a README on GitHub, but I was thinking about perhaps writing a
> > > proper man page? Do you prefer one over the other?
> >
> > I would prefer adding a README.rst file, in RST-format, as the rest of
> > the kernel documentation is moving in that direction[1] (your github
> > version is in README.md format). A man page will always be
> > out-of-sync, and even out-of-sync on different distros.
> >
> > See[1]: https://www.kernel.org/doc/html/latest/
> >
> > And then I would find some place in Documentation/admin-guide/ and
> > include the README.rst file, so it shows up at [1].
> >
> > RST have an include method like:
> >
> > .. include:: ../../tools/bpf/bpftool/README.rst
>
> Can the docs in new format be rendered into a man page? Call me old
> fashioned but I think we should provide some form of a man page.. :)
Yes, simply create the man page like:
rst2man README.rst README.man
You can add this to your local makefile.
The standard sphinx build can also generate man-pages, but it have been
removed from the kernel makefile targets:
Documentation targets:
Linux kernel internal documentation in different formats from ReST:
htmldocs - HTML
latexdocs - LaTeX
pdfdocs - PDF
epubdocs - EPUB
xmldocs - XML
linkcheckdocs - check for broken external links (will connect to external hosts)
cleandocs - clean all generated files
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [RFT] lan78xx: FIX use-after-free in lan78xx_write_reg
From: Arvind Yadav @ 2017-09-27 11:18 UTC (permalink / raw)
To: davem, woojung.huh, UNGLinuxDriver, netdev, andreyknvl, kcc,
dvyukov, Nisar.Sayed
Cc: linux-usb, linux-kernel, syzkaller
We are not releasing 'buf' memory on failure or disconnect a device.
Adding 'u8 *buf' as part of 'lan78xx_net' structure to make proper
handle for 'buf'.
Now releasing 'buf' memory on failure. It's allocate first in
lan78xx_probe() and it should be freed last in lan78xx_disconnect().
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/net/usb/lan78xx.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index b99a7fb..e653982 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -402,6 +402,7 @@ struct lan78xx_net {
struct statstage stats;
struct irq_domain_data domain_data;
+ u8 *buf;
};
/* define external phy id */
@@ -3470,6 +3471,9 @@ static void lan78xx_disconnect(struct usb_interface *intf)
usb_scuttle_anchored_urbs(&dev->deferred);
+ kfree(dev->buf);
+ dev->buf = NULL;
+
lan78xx_unbind(dev, intf);
usb_kill_urb(dev->urb_intr);
@@ -3520,7 +3524,6 @@ static int lan78xx_probe(struct usb_interface *intf,
int ret;
unsigned maxp;
unsigned period;
- u8 *buf = NULL;
udev = interface_to_usbdev(intf);
udev = usb_get_dev(udev);
@@ -3588,16 +3591,15 @@ static int lan78xx_probe(struct usb_interface *intf,
period = dev->ep_intr->desc.bInterval;
maxp = usb_maxpacket(dev->udev, dev->pipe_intr, 0);
- buf = kmalloc(maxp, GFP_KERNEL);
- if (buf) {
+ dev->buf = kmalloc(maxp, GFP_KERNEL);
+ if (dev->buf) {
dev->urb_intr = usb_alloc_urb(0, GFP_KERNEL);
if (!dev->urb_intr) {
ret = -ENOMEM;
- kfree(buf);
goto out3;
} else {
usb_fill_int_urb(dev->urb_intr, dev->udev,
- dev->pipe_intr, buf, maxp,
+ dev->pipe_intr, dev->buf, maxp,
intr_complete, dev, period);
}
}
@@ -3626,6 +3628,8 @@ static int lan78xx_probe(struct usb_interface *intf,
return 0;
out3:
+ kfree(dev->buf);
+ dev->buf = NULL;
lan78xx_unbind(dev, intf);
out2:
free_netdev(netdev);
--
1.9.1
^ permalink raw reply related
* Re: tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]
From: Jiri Pirko @ 2017-09-27 11:11 UTC (permalink / raw)
To: Paolo Abeni
Cc: Or Gerlitz, Jiri Benc, Simon Horman, David Miller, Jakub Kicinski,
Linux Netdev List, oss-drivers, John Hurley, Paul Blakey,
Jiri Pirko, Roi Dayan
In-Reply-To: <1506505618.2867.34.camel@redhat.com>
Wed, Sep 27, 2017 at 11:46:58AM CEST, pabeni@redhat.com wrote:
>On Wed, 2017-09-27 at 11:17 +0200, Jiri Pirko wrote:
>> Wed, Sep 27, 2017 at 10:29:35AM CEST, pabeni@redhat.com wrote:
>> > So it looks like the H/W offload hook will still be called with the
>> > same arguments in both case, and 'bad' rule will still be pushed to the
>> > H/W as the driver itself has no way to distinct between the two
>> > scenarios.
>>
>> Why "bad"?
>
>Such rule is coped differently by the SW and the HW data path.
>
>a rule like:
>
>tc filter add dev eth0 protocol ip parent ffff: flower \
> enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_hw \
> action action mirred redirect eth0_vf_1
>
>will match 0 packets, while:
>
>tc filter add dev eth0 protocol ip parent ffff: flower \
> enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_sw \
> action action mirred redirect eth0_vf_1
>
>[just flipped 'skip_sw' and 'skip_hw' ]
>will match the vxlan-tunneled packets. I understand that one of the
>design goal for the h/w offload path is being consistent with the sw
>one, but that does not hold in the above scenario.
Sure, the consistency is important. Howcome "skip_hw" won't match and
"skip_sw" will match? What's different?
>
>> Regarding the distinction, driver knows if user add a rule directly to
>> the eth0, or if the eth0 is egress device in the action. Those are 2
>> separete driver entrypoints - of course, talking about code with my
>> changes.
>
>ok, but than each driver should catch the scenario "rule with tunnel
>match over non tunnel device" and cope with them properly - never match
>it - why don't simply avoiding pushing such rules to the H/W ?
>
>Cheers,
>
>Paolo
^ permalink raw reply
* Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel options
From: Jiri Pirko @ 2017-09-27 11:08 UTC (permalink / raw)
To: Simon Horman
Cc: David Miller, Jiri Pirko, Jamal Hadi Salim, Cong Wang, netdev,
oss-drivers
In-Reply-To: <20170927092732.GC25449@vergenet.net>
Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:
>On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:
>> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:
>> >Allow matching on options in tunnel headers.
>> >This makes use of existing tunnel metadata support.
>> >
>> >Options are a bytestring of up to 256 bytes.
>> >Tunnel implementations may support less or more options,
>> >or no options at all.
>> >
>> >e.g.
>> > # ip link add name geneve0 type geneve dstport 0 external
>> > # tc qdisc add dev geneve0 ingress
>> > # tc filter add dev geneve0 protocol ip parent ffff: \
>> > flower \
>> > enc_src_ip 10.0.99.192 \
>> > enc_dst_ip 10.0.99.193 \
>> > enc_key_id 11 \
>> > enc_opts 0102800100800020/fffffffffffffff0 \
>> > ip_proto udp \
>> > action mirred egress redirect dev eth1
>> >
>> >Signed-off-by: Simon Horman <simon.horman@netronome.com>
>> >Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>> >
>> >---
>> >v2
>> >* Correct example which was incorrectly described setting rather
>> > than matching tunnel options
>> >---
>> > include/net/flow_dissector.h | 13 +++++++++++++
>> > include/uapi/linux/pkt_cls.h | 3 +++
>> > net/sched/cls_flower.c | 35 ++++++++++++++++++++++++++++++++++-
>> > 3 files changed, 50 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>> >index fc3dce730a6b..43f98bf0b349 100644
>> >--- a/include/net/flow_dissector.h
>> >+++ b/include/net/flow_dissector.h
>> >@@ -183,6 +183,18 @@ struct flow_dissector_key_ip {
>> > __u8 ttl;
>> > };
>> >
>> >+/**
>> >+ * struct flow_dissector_key_enc_opts:
>> >+ * @data: data
>> >+ * @len: len
>> >+ */
>> >+struct flow_dissector_key_enc_opts {
>> >+ u8 data[256]; /* Using IP_TUNNEL_OPTS_MAX is desired here
>> >+ * but seems difficult to #include
>> >+ */
>> >+ u8 len;
>> >+};
>> >+
>> > enum flow_dissector_key_id {
>> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
>> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
>> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {
>> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */
>> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */
>> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */
>> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */
>>
>> I don't see the actual dissection implementation. Where is it?
>> Did you test the patchset?
>
>Yes, I did test it. But it is also possible something went astray along the
>way and I will retest.
>
>I think that the code you are looking for is in
>fl_classify() in this patch.
The dissection should be done in the flow_dissector. That's the whole
point in having it generic. You should move it there.
^ 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