* Re: [PATCH] net: atheros: atl2: don't return zero on failure path in atl2_probe()
From: David Miller @ 2017-05-22 16:05 UTC (permalink / raw)
To: khoroshilov; +Cc: jcliburn, chris.snook, netdev, linux-kernel, ldv-project
In-Reply-To: <1495234331-27615-1-git-send-email-khoroshilov@ispras.ru>
From: Alexey Khoroshilov <khoroshilov@ispras.ru>
Date: Sat, 20 May 2017 01:52:11 +0300
> If dma mask checks fail in atl2_probe(), it breaks off initialization,
> deallocates all resources, but returns zero.
>
> The patch adds proper error code return value and
> make error code setup unified.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Looks good, applied, thanks.
^ permalink raw reply
* New Coverity defect in BPF verifier
From: Stephen Hemminger @ 2017-05-22 16:05 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Date: Mon, 22 May 2017 04:40:24 -0700
From: scan-admin@coverity.com
To: stephen@networkplumber.org
Subject: New Defects reported by Coverity Scan for Linux
Hi,
Please find the latest report on new defect(s) introduced to Linux found with Coverity Scan.
1 new defect(s) introduced to Linux found with Coverity Scan.
17 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 1 of 1 defect(s)
** CID 1409762: Control flow issues (DEADCODE)
/kernel/bpf/verifier.c: 815 in check_pkt_ptr_alignment()
________________________________________________________________________________________________________
*** CID 1409762: Control flow issues (DEADCODE)
/kernel/bpf/verifier.c: 815 in check_pkt_ptr_alignment()
809 }
810
811 /* skb->data is NET_IP_ALIGN-ed, but for strict alignment checking
812 * we force this to 2 which is universally what architectures use
813 * when they don't set CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.
814 */
>>> CID 1409762: Control flow issues (DEADCODE)
>>> Execution cannot reach the expression "0" inside this statement: "ip_align = (strict ? 2 : 0);".
815 ip_align = strict ? 2 : NET_IP_ALIGN;
816 if ((ip_align + reg_off + off) % size != 0) {
817 verbose("misaligned packet access off %d+%d+%d size %d\n",
818 ip_align, reg_off, off, size);
819 return -EACCES;
820 }
^ permalink raw reply
* Re: [PATCH net-next] macsec: double accounting of dropped rx/tx packets
From: David Miller @ 2017-05-22 15:55 UTC (permalink / raw)
To: girish.moodalbail; +Cc: netdev, sd
In-Reply-To: <1495232744-17075-1-git-send-email-girish.moodalbail@oracle.com>
From: Girish Moodalbail <girish.moodalbail@oracle.com>
Date: Fri, 19 May 2017 15:25:44 -0700
> The macsec implementation shouldn't account for rx/tx packets that are
> dropped in the netdev framework. The netdev framework itself accounts
> for such packets by atomically updating struct net_device`rx_dropped and
> struct net_device`tx_dropped fields. Later on when the stats for macsec
> link is retrieved, the packets dropped in netdev framework will be
> included in dev_get_stats() after calling macsec.c`macsec_get_stats64()
>
> Signed-off-by: Girish Moodalbail <girish.moodalbail@oracle.com>
This looks correct to me.
Applied, thank you.
^ permalink raw reply
* Re: [RFC V1 1/1] net: cdc_ncm: Reduce memory use when kernel memory low
From: David Miller @ 2017-05-22 15:54 UTC (permalink / raw)
To: jim_baxter; +Cc: bjorn, linux-usb, netdev, linux-kernel, oliver
In-Reply-To: <04bc5b49-9282-a6ca-2b95-fb8fc9750555@mentor.com>
From: "Baxter, Jim" <jim_baxter@mentor.com>
Date: Mon, 22 May 2017 16:45:42 +0100
> From: David S. Miller (davem@davemloft.net)
> Sent: Wed, 17 May 2017 14:18:19 -0400
>
>>
>> When there isn't memory pressure this will hurt performance of
>> course.
>>
>> It is a quite common paradigm to back down to 0 order memory requests
>> when higher order ones fail, so this isn't such a bad change from the
>> perspective.
>>
>> However, one negative about it is that when the system is under memory
>> stress it doesn't help at all to keep attemping high order allocations
>> when the system hasn't recovered yet. In fact, this can make it
>> worse.
>>
>
> Do you think the patch should be modified to extend the length of time
> the 0 order memory requests with a time period of 1 minute for example?
>
> Or do you feel the patch is not the correct way this should be performed?
Unfortunately without a real notifier of some sort (there isn't one, and
it isn't actually easy to come up with a clean way to do this which is
probably why it doesn't exist yet in the first place) I really cannot
recommend anything better.
That being said, probably for the time being we should just backoff each
and every request, always trying initially to do the higher order thing.
^ permalink raw reply
* Re: [PATCH net] ipv6: fix out of bound writes in __ip6_append_data()
From: David Miller @ 2017-05-22 15:48 UTC (permalink / raw)
To: eric.dumazet; +Cc: andreyknvl, edumazet, idaifish, netdev
In-Reply-To: <1495228668.6465.44.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 19 May 2017 14:17:48 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> Andrey Konovalov and idaifish@gmail.com reported crashes caused by
> one skb shared_info being overwritten from __ip6_append_data()
>
> Andrey program lead to following state :
>
> copy -4200 datalen 2000 fraglen 2040
> maxfraglen 2040 alloclen 2048 transhdrlen 0 offset 0 fraggap 6200
>
> The skb_copy_and_csum_bits(skb_prev, maxfraglen, data + transhdrlen,
> fraggap, 0); is overwriting skb->head and skb_shared_info
>
> Since we apparently detect this rare condition too late, move the
> code earlier to even avoid allocating skb and risking crashes.
>
> Once again, many thanks to Andrey and syzkaller team.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Tested-by: Andrey Konovalov <andreyknvl@google.com>
> Reported-by: <idaifish@gmail.com>
Looks good, applied and queued up for -stable.
Thanks Eric.
^ permalink raw reply
* Re: [RFC V1 1/1] net: cdc_ncm: Reduce memory use when kernel memory low
From: Baxter, Jim @ 2017-05-22 15:45 UTC (permalink / raw)
To: David Miller, bjorn; +Cc: linux-usb, netdev, linux-kernel, oliver
In-Reply-To: <20170517.141819.1307166900606639947.davem@davemloft.net>
From: David S. Miller (davem@davemloft.net)
Sent: Wed, 17 May 2017 14:18:19 -0400
>
> When there isn't memory pressure this will hurt performance of
> course.
>
> It is a quite common paradigm to back down to 0 order memory requests
> when higher order ones fail, so this isn't such a bad change from the
> perspective.
>
> However, one negative about it is that when the system is under memory
> stress it doesn't help at all to keep attemping high order allocations
> when the system hasn't recovered yet. In fact, this can make it
> worse.
>
Hello David,
Do you think the patch should be modified to extend the length of time
the 0 order memory requests with a time period of 1 minute for example?
Or do you feel the patch is not the correct way this should be performed?
Best regards,
Jim
^ permalink raw reply
* Inconsistency in ipv4/tcp_input regarding sequence number acceptability condition
From: Paul Fiterau Brostean @ 2017-05-22 15:24 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 2050 bytes --]
Hello esteemed Linux Developers,
My name is Paul, I am a PhD student. I sent a different question a while
back to the netdev mailing list, and never got a response. This question
should be a lot easier to address. While doing experiments on the Linux
TCP stack as part of a research project, I have stumbled upon a
potential inconsistency with the RFC specification.
The Linux TCP stack considers a received sequence number as in-window or
acceptable, if the sequence number of the segment is between the
expected sequence number and the expected sequence number plus the size
of the receive window. Naturally, the lower bound is included in the
range. What's a bit weird is that the upper bound of this range is also
considered in-window. This is evident both from my experiments and from
the code
(http://elixir.free-electrons.com/linux/v4.11/source/net/ipv4/tcp_input.c#L3985).
The RFC states that the upper bound shouldn't be considered. If we look
at: https://tools.ietf.org/html/rfc793#page-26 the acceptability
conditions exclude RCV.NXT+RCV.WND (where applicable). Is there a
reason for Linux including the upper bound? Or can this be fixed?
Analyzing FreeBSD, they seem to have adopted the RFC standard, and have
not included the upper bound.
As an aside, my work involves automatic inference of models from
software components. We obtained models for FreeBSD and Linux TCP
clients and comparing them we were able to spot this minor difference.
The models we can obtain automatically abstract away from many
complexities (like timing, re-transmissions), and only consider socket
calls and flags/seq/ack numbers (payloads are 0). I attached a Linux
model to give you an example. If you look at the ESTABLISHED state,
receiving a RST segment (R), prompts the Linux system to respond with a
challenge ACK segment (A) if the sequence number (p1) satisfies the
condition: 'r1+win >=p1 && r1<p1' where 'r1' is the expected sequence
number and 'win' is the receive window size.
Thanks for your attention, Paul.
[-- Attachment #2: linux_model.pdf --]
[-- Type: application/pdf, Size: 216985 bytes --]
^ permalink raw reply
* Re: ray_cs: Avoid reading past end of buffer
From: Kalle Valo @ 2017-05-22 15:27 UTC (permalink / raw)
To: Kees Cook
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Daniel Micay
In-Reply-To: <20170505223841.GA20367@beast>
Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> Using memcpy() from a buffer that is shorter than the length copied means
> the destination buffer is being filled with arbitrary data from the kernel
> rodata segment. In this case, the source was made longer, since it did not
> match the destination structure size. Additionally removes a needless cast.
>
> This was found with the future CONFIG_FORTIFY_SOURCE feature.
>
> Cc: Daniel Micay <danielmicay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Patch applied to wireless-drivers-next.git, thanks.
e48d661eb13f ray_cs: Avoid reading past end of buffer
--
https://patchwork.kernel.org/patch/9714453/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCHv4] wlcore: add wl1285 compatible
From: Kalle Valo @ 2017-05-22 15:26 UTC (permalink / raw)
To: Sebastian Reichel
Cc: David S. Miller, Sebastian Reichel, Tony Lindgren,
Marcel Holtmann, Rob Herring, linux-wireless, linux-omap,
linux-kernel, netdev, Sebastian Reichel
In-Reply-To: <20170505141553.2605-1-sebastian.reichel@collabora.co.uk>
Sebastian Reichel <sebastian.reichel@collabora.co.uk> wrote:
> Motorola Droid 4 uses a WL1285C. With differences between the
> chips not being public let's add explicit binding for wl1285
> instead of relying on wl1283 being very similar.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Acked-by: Kalle Valo <kvalo@codeaurora.org>
> Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
A note to patchwork: this is for 4.12
--
https://patchwork.kernel.org/patch/9713645/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [15/16] hostap: convert hostap_cmd_queue.usecnt from atomic_t to refcount_t
From: Kalle Valo @ 2017-05-22 15:24 UTC (permalink / raw)
To: Elena Reshetova
Cc: gregkh, peterz, matanb, paulus, Elena Reshetova, nbd, linux-rdma,
saeedm, ganeshgr, Hans Liljestrand, David Windsor, keescook, j,
ajk, leonro, matthias.bgg, linux-hams, linux-arm-kernel, blogic,
netdev, yishaih, linux-wireless, linux-kernel, linux-ppp
In-Reply-To: <1490691403-4016-16-git-send-email-elena.reshetova@intel.com>
Elena Reshetova <elena.reshetova@intel.com> wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
>
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>
2 patches applied to wireless-drivers-next.git, thanks.
552aa585faff hostap: convert hostap_cmd_queue.usecnt from atomic_t to refcount_t
0aeffa7041d8 orinoco_usb: convert request_context.refcount from atomic_t to refcount_t
--
https://patchwork.kernel.org/patch/9648427/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [15/16] hostap: convert hostap_cmd_queue.usecnt from atomic_t to refcount_t
From: Kalle Valo @ 2017-05-22 15:24 UTC (permalink / raw)
To: Elena Reshetova
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-hams-u79uwXL29TY76Z2rM5mHXA,
linux-ppp-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
ganeshgr-ut6Up61K2wZBDgjK7y7TUQ, nbd-p3rKhJxN3npAfugRpC6u6w,
blogic-p3rKhJxN3npAfugRpC6u6w,
matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w,
yishaih-VPRAkNaXOzVWk0Htik3J/w, saeedm-VPRAkNaXOzVWk0Htik3J/w,
matanb-VPRAkNaXOzVWk0Htik3J/w, leonro-VPRAkNaXOzVWk0Htik3J/w,
ajk-iz34hMvxm2Hmj42eshorlhS11BummzK+,
paulus-eUNUBHrolfbYtjvyW6yDsg, j, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
keescook-F7+t8E8rja9g9hUCZPvPmw,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, Elena Reshetova,
Hans Liljestrand, David Windsor
In-Reply-To: <1490691403-4016-16-git-send-email-elena.reshetova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Elena Reshetova <elena.reshetova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
>
> Signed-off-by: Elena Reshetova <elena.reshetova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Hans Liljestrand <ishkamiel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: David Windsor <dwindsor-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2 patches applied to wireless-drivers-next.git, thanks.
552aa585faff hostap: convert hostap_cmd_queue.usecnt from atomic_t to refcount_t
0aeffa7041d8 orinoco_usb: convert request_context.refcount from atomic_t to refcount_t
--
https://patchwork.kernel.org/patch/9648427/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
--
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
* Re: [PATCHv4] wlcore: add wl1285 compatible
From: Sebastian Reichel @ 2017-05-22 15:21 UTC (permalink / raw)
To: Kalle Valo
Cc: David Miller, tony-4v6yS6AI5VpBDgjK7y7TUQ,
marcel-kz+m5ild9QBg9hUCZPvPmw, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87wp99t0hj.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2116 bytes --]
Hi,
On Mon, May 22, 2017 at 05:44:24PM +0300, Kalle Valo wrote:
> David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> writes:
> > From: Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > Date: Mon, 22 May 2017 12:28:20 +0300
> >
> >> Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org> writes:
> >>
> >>> Motorola Droid 4 uses a WL1285C. With differences between the
> >>> chips not being public let's add explicit binding for wl1285
> >>> instead of relying on wl1283 being very similar.
> >>>
> >>> Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >>> Acked-by: Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> >>> Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> >>> Signed-off-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
> >>> ---
> >>> Hi Dave,
> >>>
> >>> I previously send this in two patches, but its hard to apply without
> >>> requiring multiple kernel releases (the driver must be updated before
> >>> the DTS change). Since the actual change is not very complex Marcel
> >>> Holtmann & Tony Lindgren suggested, that I send this directly to you
> >>> in a single patch for inclusion into 4.12. This also means, that the
> >>> remaining series can be queued normally for 4.13.
> >>
> >> I noticed that Dave set this patch to Awaiting Upstream state on his
> >> patchwork:
> >>
> >> https://patchwork.ozlabs.org/patch/759042/
> >>
> >> Which makes me suspect that he is waiting me to apply this (as I
> >> normally apply wlcore patches). Dave, should I actually take this patch?
> >> What do you prefer?
> >
> > Anything that touches wireless drivers I defer to you, yes.
>
> Thanks, I'll take it then. Not sure why Sebastian was suggested to
> submit this patch via your tree in the first place.
>
> https://patchwork.kernel.org/patch/9713645/
Thanks. The idea was to get into early 4.12-rc to avoid merge
conflicts in the droid 4 *.dts during 4.13 cycle. This strategy
obviously failed :)
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [v4] brcmfmac: btcoex: replace init_timer with setup_timer
From: Kalle Valo @ 2017-05-22 15:19 UTC (permalink / raw)
To: Xie Qirong
Cc: Arend van Spriel, Franky Lin, Hante Meuleman, linux-wireless,
brcm80211-dev-list.pdl, netdev, linux-kernel, Xie Qirong
In-Reply-To: <20170512093259.28479-1-cheerx1994@gmail.com>
Xie Qirong <cheerx1994@gmail.com> wrote:
> setup_timer.cocci suggested the following improvement:
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/btcoex.c:383:1-11: Use
> setup_timer function for function on line 384.
>
> The combination of init_timer and setting up the data and function field
> manually is equivalent to calling setup_timer(). This is an api
> consolidation only and improves readability.
>
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Xie Qirong <cheerx1994@gmail.com>
Patch applied to wireless-drivers-next.git, thanks.
6572e5f72d01 brcmfmac: btcoex: replace init_timer with setup_timer
--
https://patchwork.kernel.org/patch/9723793/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* [patch net-next RFC] net: sched: cls_api: make reclassify return all the way back to the original tp
From: Jiri Pirko @ 2017-05-22 15:09 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, dsa, edumazet, stephen, daniel,
alexander.h.duyck, simon.horman, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
With the introduction of chain goto action, the reclassification would
cause the re-iteration of the actual chain. But it perhaps makes more
sense to restart the whole thing. Thoughts?
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/cls_api.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 01a8b8b..89fbb35 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -300,7 +300,8 @@ int tcf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
__be16 protocol = tc_skb_protocol(skb);
#ifdef CONFIG_NET_CLS_ACT
const int max_reclassify_loop = 4;
- const struct tcf_proto *old_tp = tp;
+ const struct tcf_proto *orig_tp = tp;
+ const struct tcf_proto *first_tp;
int limit = 0;
reclassify:
@@ -315,9 +316,10 @@ int tcf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
err = tp->classify(skb, tp, res);
#ifdef CONFIG_NET_CLS_ACT
if (unlikely(err == TC_ACT_RECLASSIFY && !compat_mode)) {
+ first_tp = orig_tp;
goto reset;
} else if (unlikely(TC_ACT_EXT_CMP(err, TC_ACT_GOTO_CHAIN))) {
- old_tp = res->goto_tp;
+ first_tp = res->goto_tp;
goto reset;
}
#endif
@@ -335,7 +337,7 @@ int tcf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
return TC_ACT_SHOT;
}
- tp = old_tp;
+ tp = first_tp;
protocol = tc_skb_protocol(skb);
goto reclassify;
#endif
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] kernel: bpf: remove dead code
From: Daniel Borkmann @ 2017-05-22 14:52 UTC (permalink / raw)
To: David Miller, garsilva; +Cc: ast, netdev, linux-kernel
In-Reply-To: <20170522.103800.1354089494827582585.davem@davemloft.net>
On 05/22/2017 04:38 PM, David Miller wrote:
> From: "Gustavo A. R. Silva" <garsilva@embeddedor.com>
> Date: Mon, 22 May 2017 09:07:46 -0500
>
>> Execution cannot reach NET_IP_ALIGN inside the following statement:
>> ip_align = strict ? 2 : NET_IP_ALIGN
>>
>> Addresses-Coverity-ID: 1409762
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> ---
>> NOTE: variable ip_align could also be removed and use value 2 directly.
>
> Incorrect.
>
> Some platforms define NET_IP_ALIGN to zero, so the code must remain
> as is.
In the check_pkt_ptr_alignment(), when !strict you would already
return earlier from that function.
So, above test in ip_align will always give 2, meaning technically
the patch is correct, although hard-coded value less clean.
Perhaps something like the below to keep intentions more clear (and
it will get resolved during compile time anyway ...):
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index a098d95..3cf1d60 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2297,8 +2297,10 @@ static inline int pskb_network_may_pull(struct sk_buff *skb, unsigned int len)
* Since this trade off varies between architectures, we allow NET_IP_ALIGN
* to be overridden.
*/
+#define NET_IP_ALIGN_DEFAULT 2
+
#ifndef NET_IP_ALIGN
-#define NET_IP_ALIGN 2
+#define NET_IP_ALIGN NET_IP_ALIGN_DEFAULT
#endif
/*
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1eddb71..61f6aaa 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -812,7 +812,7 @@ static int check_pkt_ptr_alignment(const struct bpf_reg_state *reg,
* we force this to 2 which is universally what architectures use
* when they don't set CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.
*/
- ip_align = strict ? 2 : NET_IP_ALIGN;
+ ip_align = NET_IP_ALIGN ? : NET_IP_ALIGN_DEFAULT;
if ((ip_align + reg_off + off) % size != 0) {
verbose("misaligned packet access off %d+%d+%d size %d\n",
ip_align, reg_off, off, size);
^ permalink raw reply related
* Re: [PATCH] kernel: bpf: remove dead code
From: Gustavo A. R. Silva @ 2017-05-22 14:51 UTC (permalink / raw)
To: David Miller; +Cc: ast, daniel, netdev, linux-kernel
In-Reply-To: <20170522.103800.1354089494827582585.davem@davemloft.net>
Hi David,
Quoting David Miller <davem@davemloft.net>:
> From: "Gustavo A. R. Silva" <garsilva@embeddedor.com>
> Date: Mon, 22 May 2017 09:07:46 -0500
>
>> Execution cannot reach NET_IP_ALIGN inside the following statement:
>> ip_align = strict ? 2 : NET_IP_ALIGN
>>
>> Addresses-Coverity-ID: 1409762
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> ---
>> NOTE: variable ip_align could also be removed and use value 2 directly.
>
> Incorrect.
>
> Some platforms define NET_IP_ALIGN to zero, so the code must remain
> as is.
The following piece of code at kernel/bpf/verifier.c:798 is preventing
value NET_IP_ALIGN to be stored in variable ip_align when _strict_ is
false:
798 if (!strict || size == 1)
799 return 0;
Thanks
^ permalink raw reply
* Re: [RFC net-next PATCH 3/5] net: introduce XDP driver features interface
From: Jesper Dangaard Brouer @ 2017-05-22 14:49 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev, brouer
In-Reply-To: <5920E62B.3040201@iogearbox.net>
On Sun, 21 May 2017 02:58:19 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 05/20/2017 09:53 AM, Jesper Dangaard Brouer wrote:
> > On Fri, 19 May 2017 19:13:29 +0200
> > Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> >> On 05/18/2017 05:41 PM, Jesper Dangaard Brouer wrote:
> >>> There is a fundamental difference between normal eBPF programs
> >>> and (XDP) eBPF programs getting attached in a driver. For normal
> >>> eBPF programs it is easy to add a new bpf feature, like a bpf
> >>> helper, because is it strongly tied to the feature being
> >>> available in the current core kernel code. When drivers invoke a
> >>> bpf_prog, then it is not sufficient to simply relying on whether
> >>> a bpf_helper exists or not. When a driver haven't implemented a
> >>> given feature yet, then it is possible to expose uninitialized
> >>> parts of xdp_buff. The driver pass in a pointer to xdp_buff,
> >>> usually "allocated" on the stack, which must not be exposed.
> >>
> >> When xdp_buff is being extended, then we should at least zero
> >> initialize all in-tree users that don't support or populate this
> >> field, thus that it's not uninitialized memory. Better would be
> >> to have a way to reject the prog in the first place until it's
> >> implemented (but further comments on feature bits below).
> >
> > Going down a path where we need to zero out the xdp_buff looks a lot
> > like the sk_buff zeroing, which is the top perf cost associated with
> > SKBs see[1]. XDP is is about not repeating the same issue we had with
> > the SKB...
>
> But if we agree on implementing a certain feature that requires to
> add a new member, then basic requirement should be that it needs to
> be implementable by all XDP enabled drivers, so that users eventually
> don't need to worry which driver they run to access a XDP feature.
> In that case, zeroing that member until it gets properly implemented
> is just temporary (and could be an incentive perhaps).
>
> > [1] https://prototype-kernel.readthedocs.io/en/latest/blogposts/xdp25_eval_generic_xdp_tx.html#analyzing-build-skb-and-memset
> >
> >>> Only two user visible NETIF_F_XDP_* net_device feature flags are
> >>> exposed via ethtool (-k) seen as "xdp" and "xdp-partial".
> >>> The "xdp-partial" is detected when there is not feature equality
> >>> between kernel and driver, and a netdev_warn is given.
> >>
> >> I think having something like a NETIF_F_XDP_BIT for ethtool to
> >> indicate support as "xdp" is quite useful. Avoids having to grep
> >> the kernel tree for ndo_xdp callback. ;) A "xdp-partial" would
> >> still be unclear/confusing to the user whether his program loads
> >> or doesn't which is the only thing a user (or some loading infra)
> >> cares about eventually, so one still needs to go trying to load
> >> the XDP code to see whether that fails for the native case.
> >
> > Good that we agree on usefulness of the NETIF_F_XDP_BIT. The
> > "xdp-partial" or "xdp-challenged" is an early indication to the user
> > that they should complain to the vendor. I tried to keep it simple
> > towards the user. Do you think every feature bit should be exposed to
> > userspace?
>
> That would potentially require us to go down that path and expose
> feature bits for everything, even something as silly as new flags
> for a specific helper that requires some sort of additional support.
> We probably rather want to keep such thing in the kernel for now
> and potentially reject loads instead.
>
> >>> The idea is that XDP_DRV_* feature bits define a contract between
> >>> the driver and the kernel, giving a reliable way to know that XDP
> >>> features a driver promised to implement. Thus, knowing what bpf
> >>> side features are safe to allow.
> >>>
> >>> There are 3 levels of features: "required", "devel" and "optional".
> >>>
> >>> The motivation is pushing driver vendors forward to support all
> >>> the new XDP features. Once a given feature bit is moved into
> >>> the "required" features, the kernel will reject loading XDP
> >>> program if feature isn't implemented by driver. Features under
> >>> developement, require help from the bpf infrastrucure to detect
> >>> when a given helper or direct-access is used, using a bpf_prog
> >>> bit to mark a need for the feature, and pulling in this bit in
> >>> the xdp_features_check(). When all drivers have implemented
> >>> a "devel" feature, it can be moved to the "required" feature and
> >>
> >> The problem is that once you add bits markers to bpf_prog like we
> >> used to do in the past, then as you do in patch 4/5 with the
> >> xdp_rxhash_needed bit, they will need to be turned /on/ unconditionally
> >> when a prog has tail calls.
> >
> > Yes, with tail calls, we have to enable all features. But that is a
> > good thing, as it forces vendors to quickly implement all features.
> > And it is no different from moving a feature into the "required" bits,
> > once all drivers implement it. It is only a limitation for tail calls,
> > and something we can fix later (for handling this for tail calls).
>
> But the issue I pointed out with this approach is that XDP programs
> using tail calls will suddenly break and get rejected from one
> version to another.
>
> That's basically breaking the contract we have today where it must
> be guaranteed that older programs keep working on newer kernels,
> exactly the same with uapi. Breaking user programs is a no-go,
> not a good thing at all (it will hurt users and they move on with
> something else). So it's not something we can 'fix' at some later
> point in time; such feature detection would need to consider this
> from day 1.
This is a very good point Daniel, that older program should be able to
run. But this will only happen when tail calls are in-use, and NIC
vendors don't keep their drivers up-to-date. Creating a strong
incentive for drivers implementing all XDP features (which you have
argue for). Still I see your point.
> > BPF have some nice features of evaluating the input program
> > "load-time", which is what I'm taking advantage of as an optimization
> > here (let use this nice bpf property). It is only tail calls that
> > cannot evaluate this "load-time". Thus, if you care about tail calls,
> > supporting intermediate features, we could later fix that by adding a
> > runtime feature check in the case of tail calls.
How do we move forward from here?
Let me try to restate the problem: There is a general disconnect
between bpf and what XDP drivers support.
The bpf model (AFAIK) is that your bpf program will get rejected when
loading your bpf_prog. This happens if you are using a bpf_helper or
direct-ctx-access which is not supported/avail in the kernel. Right??
(This also happens for tail-call programs, right?) (I do like this
model of failing the program when the feature is not avail).
I guess, new bpf features are discovered by looking at uapi/bpf.h and
then try to use the feature, and see if your program loads.
The disconnect is that when writing XDP-bpf programs, then you will
not get any feedback when you are calling a bpf-helper that the XDP
driver doesn't support. The XDP bpf-program loads and can happily run
calling core-kernel XDP helpers. Now, how will the XDP program author
know that his call to the helper is valid?
For rxhash, we could init the fields to zero (in drivers not
supporting it), but how will the XDP bpf_prog author know whether or
not a NIC driver supports XDP-rxhash? (I guess, he could write a XDP
program, that sets the XDP software hash, and then add a match via TC
meta match or another cls_bpf program that read the skb->hash value,
to see if changing the rxhash works... it seem clumpsy)
For xdp_adjust_head, it was even-worse, as a boundry check against
data_end with e.g. zero is no boundry, and would allow unbounded
memory access from the xdp.data_end pointer. We actually have
released kernels with this bug, which can be triggered with a bpf
tail-call.
I just did a quick check on linux-stable.git for v4.10.17 and it looks
like commit c2002f983767 ("bpf: fix checking xdp_adjust_head on tail
calls") is missing, meaning drivers nfp, mlx5, nfp and virtio_net is
likely subject to the adjust_head unbounded memory access bug via tail
calls. I really wish we had a reliable way of avoiding these kind of
bpf vs. XDP-driver mismatches.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
p.s. Remember, to verify if a driver supports adjust_head, grep after
"data_hard_start" in the driver code... which is also quite strange
for users to know.
^ permalink raw reply
* Re: [PATCH net-next 00/20] net: dsa: distribute switch events
From: Andrew Lunn @ 2017-05-22 14:48 UTC (permalink / raw)
To: David Miller; +Cc: vivien.didelot, netdev, linux-kernel, kernel, f.fainelli
In-Reply-To: <20170522.104557.1244582147577060590.davem@davemloft.net>
On Mon, May 22, 2017 at 10:45:57AM -0400, David Miller wrote:
> From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Date: Fri, 19 May 2017 17:00:35 -0400
>
> > DSA is by nature the support for a switch fabric, which can be composed
> > of a single, or multiple interconnected Ethernet switch chips.
> >
> > The current DSA core behavior is to identify the slave port targeted by
> > a request (e.g. adding a VLAN entry), and program the switch chip to
> > which it belongs accordingly.
> >
> > This is problematic in a multi-chip environment, since all chips of a
> > fabric must be aware of most configuration changes. Here are some
> > concrete examples in a 3-chip environment:
> ...
> > This patch series uses the notification chain introduced for bridging,
> > to notify not only bridge, but switchdev attributes and objects events
> > to all switch chips of the fabric.
>
> Andrew or Florian, can I get a review?
Yes, it is on my TODO list for today/tomorrow.
Andrew
^ permalink raw reply
* Re: RDS: TCP: Delete an error message for a failed memory allocation in rds_tcp_init_net()
From: Sowmini Varadhan @ 2017-05-22 14:46 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-rdma, netdev, rds-devel, kernel-janitors, David S. Miller,
Santosh Shilimkar, LKML
In-Reply-To: <1e1a5d08-34a7-1b88-80ac-c650c4ae76cf@users.sourceforge.net>
> Do you find information from a Linux allocation failure report sufficient
> for such an use case?
yes, I suppose that would cover the needed cases.
Your change looks good to me.
Acked-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
^ permalink raw reply
* [patch net-next] net: sched: cls_matchall: fix null pointer dereference
From: Jiri Pirko @ 2017-05-22 14:46 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, mlxsw, colin.king, yotamg
From: Jiri Pirko <jiri@mellanox.com>
Since the head is guaranteed by the check above to be null, the call_rcu
would explode. Remove the previously logically dead code that was made
logically very much alive and kicking.
Fixes: 985538eee06f ("net/sched: remove redundant null check on head")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/cls_matchall.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index dee469f..51859b8 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -203,7 +203,6 @@ static int mall_change(struct net *net, struct sk_buff *in_skb,
*arg = (unsigned long) head;
rcu_assign_pointer(tp->root, new);
- call_rcu(&head->rcu, mall_destroy_rcu);
return 0;
err_replace_hw_filter:
--
2.9.3
^ permalink raw reply related
* Re: [PATCH net-next 00/20] net: dsa: distribute switch events
From: David Miller @ 2017-05-22 14:45 UTC (permalink / raw)
To: vivien.didelot; +Cc: netdev, linux-kernel, kernel, f.fainelli, andrew
In-Reply-To: <20170519210055.9366-1-vivien.didelot@savoirfairelinux.com>
From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Fri, 19 May 2017 17:00:35 -0400
> DSA is by nature the support for a switch fabric, which can be composed
> of a single, or multiple interconnected Ethernet switch chips.
>
> The current DSA core behavior is to identify the slave port targeted by
> a request (e.g. adding a VLAN entry), and program the switch chip to
> which it belongs accordingly.
>
> This is problematic in a multi-chip environment, since all chips of a
> fabric must be aware of most configuration changes. Here are some
> concrete examples in a 3-chip environment:
...
> This patch series uses the notification chain introduced for bridging,
> to notify not only bridge, but switchdev attributes and objects events
> to all switch chips of the fabric.
Andrew or Florian, can I get a review?
I audited the slave-->port transformations and they all look
good.
^ permalink raw reply
* Re: [PATCH v3 net-next 2/5] phy: micrel: add Microchip KSZ 9477 Switch PHY support
From: Andrew Lunn @ 2017-05-22 14:44 UTC (permalink / raw)
To: Woojung.Huh
Cc: f.fainelli, vivien.didelot, sergei.shtylyov, netdev, davem,
UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A80CF8@CHN-SV-EXMX02.mchp-main.com>
On Mon, May 22, 2017 at 02:38:39PM +0000, Woojung.Huh@microchip.com wrote:
> > > Signed-off-by: Woojung Huh <Woojung.Huh@microchip.com>
> > > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> >
> > Signed-off-by? I didn't contribute any code. It probably should be
> >
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Andrew,
>
> Your reply on 5/13 was with "Signed-off" :)
O.K. My error then. Please feel free at ask me, if you think something
i say is wrong...
> Will change to "Reviewed-by" in next patch.
Thanks.
Andrew
^ permalink raw reply
* Re: [PATCHv4] wlcore: add wl1285 compatible
From: Kalle Valo @ 2017-05-22 14:44 UTC (permalink / raw)
To: David Miller
Cc: sebastian.reichel, sre, tony, marcel, robh+dt, linux-wireless,
linux-omap, linux-kernel, netdev
In-Reply-To: <20170522.103010.1779800840743690143.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: Kalle Valo <kvalo@codeaurora.org>
> Date: Mon, 22 May 2017 12:28:20 +0300
>
>> Sebastian Reichel <sebastian.reichel@collabora.co.uk> writes:
>>
>>> Motorola Droid 4 uses a WL1285C. With differences between the
>>> chips not being public let's add explicit binding for wl1285
>>> instead of relying on wl1283 being very similar.
>>>
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>> Acked-by: Kalle Valo <kvalo@codeaurora.org>
>>> Acked-by: Tony Lindgren <tony@atomide.com>
>>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
>>> ---
>>> Hi Dave,
>>>
>>> I previously send this in two patches, but its hard to apply without
>>> requiring multiple kernel releases (the driver must be updated before
>>> the DTS change). Since the actual change is not very complex Marcel
>>> Holtmann & Tony Lindgren suggested, that I send this directly to you
>>> in a single patch for inclusion into 4.12. This also means, that the
>>> remaining series can be queued normally for 4.13.
>>
>> I noticed that Dave set this patch to Awaiting Upstream state on his
>> patchwork:
>>
>> https://patchwork.ozlabs.org/patch/759042/
>>
>> Which makes me suspect that he is waiting me to apply this (as I
>> normally apply wlcore patches). Dave, should I actually take this patch?
>> What do you prefer?
>
> Anything that touches wireless drivers I defer to you, yes.
Thanks, I'll take it then. Not sure why Sebastian was suggested to
submit this patch via your tree in the first place.
https://patchwork.kernel.org/patch/9713645/
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] net: fec: add post PHY reset delay DT property
From: Andrew Lunn @ 2017-05-22 14:43 UTC (permalink / raw)
To: Quentin Schulz
Cc: fugang.duan-3arQi8VN3Tc, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
In-Reply-To: <f667da6e-d300-ec79-cc49-3808d9758a99-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On Mon, May 22, 2017 at 04:00:47PM +0200, Quentin Schulz wrote:
> Hi Andrew
>
> On 22/05/2017 15:57, Andrew Lunn wrote:
> > On Mon, May 22, 2017 at 11:15:17AM +0200, Quentin Schulz wrote:
> >> Some PHY require to wait for a bit after the reset GPIO has been
> >> toggled. This adds support for the DT property `phy-reset-post-delay`
> >> which gives the delay in milliseconds to wait after reset.
> >>
> >> If the DT property is not given, no delay is observed. Post reset delay
> >> greater than 1000ms are invalid and are default to 1ms.
> >
> > Hi Quentin
> >
> > If it is invalid, please return -EINVAL.
> >
>
> Just copying the wording and behavior of phy-reset-duration. Should we
> then change the behavior of phy-reset-duration as well to return -EINVAL
> in that case or I just leave both as is?
No, you cannot change phy-reset-duration. There could be device tree
blobs out in the wild with invalid phy-reset-duration which silently
get converted to 1000ms by the code. You cannot break them.
However, you can prevent new additions which add invalid
phy-reset-post-delay.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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
* RE: [PATCH v3 net-next 2/5] phy: micrel: add Microchip KSZ 9477 Switch PHY support
From: Woojung.Huh @ 2017-05-22 14:38 UTC (permalink / raw)
To: andrew
Cc: f.fainelli, vivien.didelot, sergei.shtylyov, netdev, davem,
UNGLinuxDriver
In-Reply-To: <20170522143249.GD29447@lunn.ch>
> > Signed-off-by: Woojung Huh <Woojung.Huh@microchip.com>
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
>
> Signed-off-by? I didn't contribute any code. It probably should be
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew,
Your reply on 5/13 was with "Signed-off" :)
Will change to "Reviewed-by" in next patch.
Woojung
^ 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