* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Linus Torvalds @ 2018-01-07 1:20 UTC (permalink / raw)
To: Dan Williams
Cc: Linux Kernel Mailing List, linux-arch, Andi Kleen, Arnd Bergmann,
Greg Kroah-Hartman, Peter Zijlstra, Network Development,
the arch/x86 maintainers, Ingo Molnar, H. Peter Anvin,
Thomas Gleixner, Alan Cox
In-Reply-To: <CAPcyv4gx+2Dh6HS8e+TKNKP00+rYUkzGnEwdpZ_P6hFiOHKHNg@mail.gmail.com>
On Sat, Jan 6, 2018 at 3:31 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>
> I assume if we put this in uaccess_begin() we also need audit for
> paths that use access_ok but don't do on to call uaccess_begin()? A
> quick glance shows a few places where we are open coding the stac().
> Perhaps land the lfence in stac() directly?
Yeah, we should put it in uaccess_begin(), and in the actual user
accessor helpers that do stac. Some of them probably should be changed
to use uaccess_begin() instead while at it.
One question for the CPU people: do we actually care and need to do
this for things that might *write* to something? The speculative write
obviously is killed, but does it perhaps bring in a cacheline even
when killed?
Because maybe we don't need the lfence in put_user(), only in get_user()?
Linus
^ permalink raw reply
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: David Miller @ 2018-01-07 1:36 UTC (permalink / raw)
To: w
Cc: gnomes, alexei.starovoitov, torvalds, dan.j.williams,
linux-kernel, linux-arch, ak, arnd, gregkh, peterz, netdev, mingo,
hpa, tglx
In-Reply-To: <20180106204229.GD9075@1wt.eu>
From: Willy Tarreau <w@1wt.eu>
Date: Sat, 6 Jan 2018 21:42:29 +0100
> On Sat, Jan 06, 2018 at 06:38:59PM +0000, Alan Cox wrote:
>> Normally people who propose security fixes don't have to argue about the
>> fact they added 30 clocks to avoid your box being 0wned.
>
> In fact it depends, because if a fix makes the system unusable for its
> initial purpose, this fix will simply not be deployed at all, which is
> the worst that can happen.
+1
I completely agree with Willy and Alexei.
And the scale isn't even accurate, we're talking about at least
hundreds upon hundreds of clocks, not 30, if we add an operation whose
side effect is to wait for all pending loads to complete. So yeah
this is going to be heavily scrutinized.
^ permalink raw reply
* Re: [PATCH net] ipv6: remove null_entry before adding default route
From: Wei Wang @ 2018-01-07 1:41 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: David Miller, Linux Kernel Network Developers, Eric Dumazet
In-Reply-To: <20180106074239.uakjomkb3ijlzx5m@kafai-mbp>
On Fri, Jan 5, 2018 at 11:42 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> On Fri, Jan 05, 2018 at 05:38:35PM -0800, Wei Wang wrote:
>> From: Wei Wang <weiwan@google.com>
>>
>> In the current code, when creating a new fib6 table, tb6_root.leaf gets
>> initialized to net->ipv6.ip6_null_entry.
>> If a default route is being added with rt->rt6i_metric = 0xffffffff,
>> fib6_add() will add this route after net->ipv6.ip6_null_entry. As
>> null_entry is shared, it could cause problem.
>>
>> In order to fix it, set fn->leaf to NULL before calling
>> fib6_add_rt2node() when trying to add the first default route.
>> And reset fn->leaf to null_entry when adding fails or when deleting the
>> last default route.
>>
>> syzkaller reported the following issue which is fixed by this commit:
>> =============================
>> WARNING: suspicious RCU usage
>> 4.15.0-rc5+ #171 Not tainted
>> -----------------------------
>> net/ipv6/ip6_fib.c:1702 suspicious rcu_dereference_protected() usage!
>>
>> other info that might help us debug this:
>>
>> rcu_scheduler_active = 2, debug_locks = 1
>> 4 locks held by swapper/0/0:
>> #0: ((&net->ipv6.ip6_fib_timer)){+.-.}, at: [<00000000d43f631b>] lockdep_copy_map include/linux/lockdep.h:178 [inline]
>> #0: ((&net->ipv6.ip6_fib_timer)){+.-.}, at: [<00000000d43f631b>] call_timer_fn+0x1c6/0x820 kernel/time/timer.c:1310
>> #1: (&(&net->ipv6.fib6_gc_lock)->rlock){+.-.}, at: [<000000002ff9d65c>] spin_lock_bh include/linux/spinlock.h:315 [inline]
>> #1: (&(&net->ipv6.fib6_gc_lock)->rlock){+.-.}, at: [<000000002ff9d65c>] fib6_run_gc+0x9d/0x3c0 net/ipv6/ip6_fib.c:2007
>> #2: (rcu_read_lock){....}, at: [<0000000091db762d>] __fib6_clean_all+0x0/0x3a0 net/ipv6/ip6_fib.c:1560
>> #3: (&(&tb->tb6_lock)->rlock){+.-.}, at: [<000000009e503581>] spin_lock_bh include/linux/spinlock.h:315 [inline]
>> #3: (&(&tb->tb6_lock)->rlock){+.-.}, at: [<000000009e503581>] __fib6_clean_all+0x1d0/0x3a0 net/ipv6/ip6_fib.c:1948
>>
>> stack backtrace:
>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.15.0-rc5+ #171
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>> Call Trace:
>> <IRQ>
>> __dump_stack lib/dump_stack.c:17 [inline]
>> dump_stack+0x194/0x257 lib/dump_stack.c:53
>> lockdep_rcu_suspicious+0x123/0x170 kernel/locking/lockdep.c:4585
>> fib6_del+0xcaa/0x11b0 net/ipv6/ip6_fib.c:1701
>> fib6_clean_node+0x3aa/0x4f0 net/ipv6/ip6_fib.c:1892
>> fib6_walk_continue+0x46c/0x8a0 net/ipv6/ip6_fib.c:1815
>> fib6_walk+0x91/0xf0 net/ipv6/ip6_fib.c:1863
>> fib6_clean_tree+0x1e6/0x340 net/ipv6/ip6_fib.c:1933
>> __fib6_clean_all+0x1f4/0x3a0 net/ipv6/ip6_fib.c:1949
>> fib6_clean_all net/ipv6/ip6_fib.c:1960 [inline]
>> fib6_run_gc+0x16b/0x3c0 net/ipv6/ip6_fib.c:2016
>> fib6_gc_timer_cb+0x20/0x30 net/ipv6/ip6_fib.c:2033
>> call_timer_fn+0x228/0x820 kernel/time/timer.c:1320
>> expire_timers kernel/time/timer.c:1357 [inline]
>> __run_timers+0x7ee/0xb70 kernel/time/timer.c:1660
>> run_timer_softirq+0x4c/0xb0 kernel/time/timer.c:1686
>> __do_softirq+0x2d7/0xb85 kernel/softirq.c:285
>> invoke_softirq kernel/softirq.c:365 [inline]
>> irq_exit+0x1cc/0x200 kernel/softirq.c:405
>> exiting_irq arch/x86/include/asm/apic.h:540 [inline]
>> smp_apic_timer_interrupt+0x16b/0x700 arch/x86/kernel/apic/apic.c:1052
>> apic_timer_interrupt+0xa9/0xb0 arch/x86/entry/entry_64.S:904
>> </IRQ>
>>
>> Reported-by: syzbot <syzkaller@googlegroups.com>
>> Fixes: 66f5d6ce53e6 ("ipv6: replace rwlock with rcu and spinlock in fib6_table")
>> Signed-off-by: Wei Wang <weiwan@google.com>
>> ---
>> net/ipv6/ip6_fib.c | 45 +++++++++++++++++++++++++++++++++++----------
>> 1 file changed, 35 insertions(+), 10 deletions(-)
>>
>> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
>> index d11a5578e4f8..37cb4ad1ea29 100644
>> --- a/net/ipv6/ip6_fib.c
>> +++ b/net/ipv6/ip6_fib.c
>> @@ -640,6 +640,11 @@ static struct fib6_node *fib6_add_1(struct net *net,
>> if (!(fn->fn_flags & RTN_RTINFO)) {
>> RCU_INIT_POINTER(fn->leaf, NULL);
>> rt6_release(leaf);
>> + /* remove null_entry in the root node */
>> + } else if (fn->fn_flags & RTN_TL_ROOT &&
>> + rcu_access_pointer(fn->leaf) ==
>> + net->ipv6.ip6_null_entry) {
>> + RCU_INIT_POINTER(fn->leaf, NULL);
> It seems the reader side could see tb6_root.leaf == NULL after
> this change and I think it should be fine? If it is, instead
> of switching betwen NULL and ip6_null_entry, would it be simpler
> to always set tb6_root.leaf to NULL until a legit default route
> is added?
>
Agree that reader side could see tb6_root.leaf == NULL and it should
be fine cause the lookup code path takes care of NULL case.
If we init tb6_root to NULL, it does seem simpler. The only place we
need to take care is that fib6_del() should not try to expunge fn if
is root node if it does not contain any route. But does it seem weird
that tb6_root.fn_flags is marked with RTN_RTINFO without containing
any route?
>> }
>>
>> return fn;
>> @@ -1270,14 +1275,27 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
>> return err;
>>
>> failure:
>> - /* fn->leaf could be NULL if fn is an intermediate node and we
>> - * failed to add the new route to it in both subtree creation
>> - * failure and fib6_add_rt2node() failure case.
>> - * In both cases, fib6_repair_tree() should be called to fix
>> + /* fn->leaf could be NULL if:
>> + * 1. fn is the root node in the table and we fail to add the default
>> + * route to it.
>> + * In this case, we put fn->leaf back to net->ipv6.ip6_null_entry as
>> + * the way the table was created.
>> + * 2. fn is an intermediate node and we failed to add the new
>> + * route to it in both subtree creation failure and fib6_add_rt2node()
>> + * failure case.
>> + * In this case, fib6_repair_tree() should be called to fix
>> * fn->leaf.
>> */
>> - if (fn && !(fn->fn_flags & (RTN_RTINFO|RTN_ROOT)))
>> - fib6_repair_tree(info->nl_net, table, fn);
>> + if (fn) {
>> + if (fn->fn_flags & RTN_TL_ROOT) {
>> + if (!rcu_access_pointer(fn->leaf)) {
>> + rcu_assign_pointer(fn->leaf,
>> + info->nl_net->ipv6.ip6_null_entry);
>> + }
>> + } else if (!(fn->fn_flags & (RTN_RTINFO|RTN_ROOT))) {
>> + fib6_repair_tree(info->nl_net, table, fn);
>> + }
>> + }
>> /* Always release dst as dst->__refcnt is guaranteed
>> * to be taken before entering this function
>> */
>> @@ -1685,11 +1703,18 @@ static void fib6_del_route(struct fib6_table *table, struct fib6_node *fn,
>> }
>> read_unlock(&net->ipv6.fib6_walker_lock);
>>
>> - /* If it was last route, expunge its radix tree node */
>> + /* If it was last route:
>> + * 1. For root node, put back null_entry as how the table was created.
>> + * 2. For other nodes, expunge its radix tree node.
>> + */
>> if (!rcu_access_pointer(fn->leaf)) {
>> - fn->fn_flags &= ~RTN_RTINFO;
>> - net->ipv6.rt6_stats->fib_route_nodes--;
>> - fn = fib6_repair_tree(net, table, fn);
>> + if (fn->fn_flags & RTN_TL_ROOT) {
>> + rcu_assign_pointer(fn->leaf, net->ipv6.ip6_null_entry);
>> + } else {
>> + fn->fn_flags &= ~RTN_RTINFO;
>> + net->ipv6.rt6_stats->fib_route_nodes--;
>> + fn = fib6_repair_tree(net, table, fn);
>> + }
>> }
>>
>> fib6_purge_rt(rt, fn, net);
>> --
>> 2.16.0.rc0.223.g4a4ac83678-goog
>>
^ permalink raw reply
* Re: [PATCH] net: ipv4: Remove MTU check in IP_HDRINCL send flow
From: David Miller @ 2018-01-07 1:41 UTC (permalink / raw)
To: dev.jongy; +Cc: netdev
In-Reply-To: <20180106232647.GA8203@jong.localdomain>
From: Yonatan Goldschmidt <dev.jongy@gmail.com>
Date: Sun, 7 Jan 2018 01:26:48 +0200
> IP fragmentation can be performed as expected down the stack, without touching
> irrelevant fields in the included header besides fragment offset, setting
> IP_MF and header checksum.
> If the included header has IP_DF set, EMSGSIZE is returned.
>
> This allows users of IP_HDRINCL to have the kernel perform fragmentation.
>
> Signed-off-by: Yonatan Goldschmidt <dev.jongy@gmail.com>
This has been brought up before and we decided that for RAW sockets
the user is responsible for fragmentation, and the kernel should not
"manage" the packet given to it in any way whatsoever.
^ permalink raw reply
* [RFC] memdup_user() and friends
From: Al Viro @ 2018-01-07 2:16 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel, netdev
After reviewing memdup_user() callers, I've found several places
where it got completely unbounded values passed for size (up to 2Gb),
as well as some bounded by ridiculously high values - e.g.
if (size > 1024 * 128) /* sane value */
return -EINVAL;
container = memdup_user(buf, size);
if (IS_ERR(container))
return PTR_ERR(container);
in sound/core/control.c:replace_user_tlv(). To add insult to injury, that
particular caller uses the result only for memcmp() and copy_to_user(), so
there's no point whatsoever to try and keep the copy physically contiguous.
IOW, it would be just fine with vmalloc()-based analogue of memdup_user().
There is a bunch of places where a kvmalloc-based analogue is
open-coded and they deserve a primitive of their own. We can't convert
memdup_user() itself to that - it would have to be paired with kvfree()
instead of kfree() and we *do* have callers that need kmalloc() - e.g.
when the copy is passed to usb_bulk_msg(), etc. So it needs to be
a separate primitive.
However, memdup_user() itself has another fun issue:
/*
* Always use GFP_KERNEL, since copy_from_user() can sleep and
* cause pagefault, which makes it pointless to use GFP_NOFS
* or GFP_ATOMIC.
*/
p = kmalloc_track_caller(len, GFP_KERNEL);
if (!p)
return ERR_PTR(-ENOMEM);
Sure, GFP_ATOMIC and GFP_NOFS would be pointless. However, GFP_USER and,
possibly, __GFP_NOWARN would not. As the matter of fact, of 200-odd callers
only for two I couldn't prove that they'd be fine with GFP_USER:
arch/x86/kvm/x86.c:2076: page = memdup_user(blob_addr + (page_num * PAGE_SIZE), PAGE_SIZE);
include/rdma/ib_verbs.h:2437: buf = memdup_user(p, len);
The former is weird xen crap used by one place in kvm_set_msr_common() and
I've given up trying to sort the call chains out; it might very well be fine
with GFP_USER, actually. The latter is a flaming atrocity from the
bowels of infiniband. Might or might not be fine with GFP_USER, but in
any case that code is utter crap. What it tries to do is verifying that given
range of userland memory contains only zeroes. Callers are also not pretty and
hard to track, as well.
Everything else is definitely fine with GFP_USER - it's stuff like "copy of ioctl
arguments in an ioctl never issued by the kernel code, must have come straight from
ioctl(2)" and things like that. IMO we should simply switch memdup_user() to
GFP_USER and be done with that. Limiting the size ought to be done by callers and
IMO there's no point in __GFP_NOWARN there.
What I propose is
* switch memdup_user() to GFP_USER
* add vmemdup_user(), using kvmalloc() instead of kmalloc() (also with
GFP_USER)
* switch open-coded instances of the latter to calling it
* switch some of the memdup_user() callers to vmemdup_user() - the ones that
don't need physically contiguous copy and might be larger than a couple of pages.
* add apriori bounds on size in the call sites that do not have those yet -
that'll require comments from maintainers of the code in question in some cases.
Objections?
^ permalink raw reply
* Re: [PATCH net-next v3 00/10] net: qualcomm: rmnet: Enable csum offloads
From: Subash Abhinov Kasiviswanathan @ 2018-01-07 2:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev, lkp, edumazet
In-Reply-To: <20180106.200846.2293975263975831555.davem@davemloft.net>
> It's there, in state "Changes Requested" because you were given
> feedback on your patch series and you must address it.
>
> "Changes Requested" state patches do not show up in the default
> view, you must explicitly adjust the search criteria to see
> patches which are not in state which qualifies as "Action Required"
> on my part which is "New" and "Under Review".
>
>> Do I need to resubmit this?
>
> Yes, but only after you have addressed the feedback you were
> given.
Hi David
I believe the comments were on the v2 of this patch series. I have fixed
them in the v3 submission.
I still dont see the v3 submission in patchwork if I query it.
https://patchwork.ozlabs.org/project/netdev/list/?submitter=65547&state=%2A&archive=both
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Alexei Starovoitov @ 2018-01-07 3:38 UTC (permalink / raw)
To: Alan Cox
Cc: Linus Torvalds, Dan Williams, linux-kernel, linux-arch,
Andi Kleen, Arnd Bergmann, Greg Kroah-Hartman, Peter Zijlstra,
netdev, Ingo Molnar, H. Peter Anvin, Thomas Gleixner
In-Reply-To: <20180106230507.3547c9a0@alans-desktop>
On Sat, Jan 06, 2018 at 11:05:07PM +0000, Alan Cox wrote:
> > Even if it would be practical the speed probably going to be in bytes per second,
> > so to read anything meaningful an attack detection techniques (that people
> > are actively working on) will be able to catch it.
> > At the end security cannot be absolute.
> > The current level of paranoia shouldn't force us to make hastily decisions.
>
> I think there are at least three overlapping problem spaces here
>
> 1. This is a new field. That could mean that it turns out to be
> really hard and everyone discovers that eBPF was pretty much the only
> interesting attack. It could also mean we are going to see several years
> or refinement by evil geniuses all over the world and what we see now is
> tip of iceberg in cleverness.
yep. plenty of unknowns and what's happening now is an overreaction.
> 2. It is very very complicated to answer a question like "is
> sequence x safe on all of vendor's microprocessors" even for the vendor
so far "is sequence x safe" was viewed by cpu vendors as
"is sequence x going to stop speculative execution".
The AND approach wasn't even considered and I suspect there may be
other ways to avoid the side channel attack, but vendors are too
focused on stopping speculation. Why? The vulnerability already
disclosed in detail. Public clouds already patched their kernels.
What is the rush to push half baked patches into upstream
that don't even address the variant 1 ?
> Intel's current position is 'lfence'.
Let's look at the facts. From GPZ blog:
"In simplified terms, this program is used to determine the address
of prog_map by guessing the offset from prog_map to a userspace
address and tail-calling through prog_map at the guessed offsets."
which clearly states that bpf_tail_call() was used in the attack.
Yet none of the intel nor arm patches address speculation in
this bpf helper!
It means that:
- gpz didn't share neither exploit nor the detailed description
of the POC with cpu vendors until now
- coverity rules used to find all these places in the kernel
failed to find bpf_tail_call
- cpu vendors were speculating what variant 1 can actually do
Now the attack is well described, yet cpu vendors still pushing
for lfence patches that don't make sense. Why?
Furthermore GPZ blog states:
"To cause the branch prediction to predict that the offset is below
the length of prog_map, tail calls to an in-bounds index are
performed in between."
which means that attack is 'in-bound * largeN + out-of-bound * 1'.
Going back to our discussion few emails ago about 'mask' as a variable
it means that value predictor when speculating this last out of
bound access will correctly predict exact 'mask', so 'index & mask'
fix will certainly defeat this poc.
More from the blog:
"To increase the mis-speculation window, the cache line containing
the length of prog_map is bounced to another core."
and in the kernel we have:
struct bpf_map {
atomic_t refcnt;
enum bpf_map_type map_type;
u32 key_size;
u32 value_size;
u32 max_entries;
...
note that only 'refcnt' could be used for bouncing,
so if we place max_entries and refcnt into different cache
lines this poc will likely fail as well.
More from the blog:
"At this point, a second eBPF program can be used to actually leak data.
In pseudocode, this program looks as follows:
uint64_t progmap_index = (((secret_data & bitmask) >> bitshift_selector) << 7) + prog_array_base_offset;
bpf_tail_call(prog_map, progmap_index);
"
which means that POC is relying 64-bit address speculation.
In the places coverity found the user supplied value is 32-bit,
so none of them are suitable for this POC.
We cannot rule out that the attack cannot be crafted to work
with 32-bit, but it means there is no urgency to lfence everything.
> The important thing is that there is something clean, all architecture
> that can be used today that doesn't keep forcing everyone to change
> drivers when new/better ways to do the speculation management appear.
What I think is important is to understand vulnerability first.
I don't think it was done.
> The differences involved on the "lfence" versus "and" versus before are
> not likely to be anywhere in that order of magnitude.
we clearly disagree here. Both intel and arm patches proposed
to add lfence in bpf_map_lookup() which is the hottest function
we have and we do run it at 40+Gbps speeds where every nanosecond
counts, so no, lfence is not a solution.
^ permalink raw reply
* Re: [PATCH net-next] net: tracepoint: adding new tracepoint arguments in inet_sock_set_state
From: Yafang Shao @ 2018-01-07 5:38 UTC (permalink / raw)
To: Song Liu
Cc: David Miller, brendan.d.gregg@gmail.com,
marcelo.leitner@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <544ED2FF-A44E-493D-B84E-46A58EEAB201@fb.com>
On Sun, Jan 7, 2018 at 1:46 AM, Song Liu <songliubraving@fb.com> wrote:
>
>> On Jan 5, 2018, at 12:09 AM, Yafang Shao <laoar.shao@gmail.com> wrote:
>>
>> On Fri, Jan 5, 2018 at 3:21 PM, Song Liu <songliubraving@fb.com> wrote:
>>>
>>>> On Jan 4, 2018, at 10:42 PM, Yafang Shao <laoar.shao@gmail.com> wrote:
>>>>
>>>> sk->sk_protocol and sk->sk_family are exposed as tracepoint arguments.
>>>> Then we can conveniently use these two arguments to do the filter.
>>>>
>>>> Suggested-by: Brendan Gregg <brendan.d.gregg@gmail.com>
>>>> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
>>>> ---
>>>> include/trace/events/sock.h | 24 ++++++++++++++++++------
>>>> net/ipv4/af_inet.c | 6 ++++--
>>>> 2 files changed, 22 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
>>>> index 3537c5f..c7df70f 100644
>>>> --- a/include/trace/events/sock.h
>>>> +++ b/include/trace/events/sock.h
>>>> @@ -11,7 +11,11 @@
>>>> #include <linux/ipv6.h>
>>>> #include <linux/tcp.h>
>>>>
>>>> -/* The protocol traced by sock_set_state */
>>>> +#define family_names \
>>>> + EM(AF_INET) \
>>>> + EMe(AF_INET6)
>>>> +
>>>> +/* The protocol traced by inet_sock_set_state */
>>>> #define inet_protocol_names \
>>>> EM(IPPROTO_TCP) \
>>>> EM(IPPROTO_DCCP) \
>>>> @@ -37,6 +41,7 @@
>>>> #define EM(a) TRACE_DEFINE_ENUM(a);
>>>> #define EMe(a) TRACE_DEFINE_ENUM(a);
>>>>
>>>> +family_names
>>>> inet_protocol_names
>>>> tcp_state_names
>>>>
>>>> @@ -45,6 +50,9 @@
>>>> #define EM(a) { a, #a },
>>>> #define EMe(a) { a, #a }
>>>>
>>>> +#define show_family_name(val) \
>>>> + __print_symbolic(val, family_names)
>>>> +
>>>> #define show_inet_protocol_name(val) \
>>>> __print_symbolic(val, inet_protocol_names)
>>>>
>>>> @@ -108,9 +116,10 @@
>>>>
>>>> TRACE_EVENT(inet_sock_set_state,
>>>>
>>>> - TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
>>>> + TP_PROTO(const struct sock *sk, const int family, const int protocol,
>>>> + const int oldstate, const int newstate),
>>>
>>> Are there cases we need protocol and/or family that is different to
>>> sk->sk_protocol/sk_family? If not, I think we don't need to change the
>>> TP_PROTO.
>>>
>>> Thanks,
>>> Song
>>>
>>
>> As of now, there're two sk_family, which are AF_INET and AF_INET6, and
>> three sk_protocol which are IPPROTO_TCP, IPPROTO_SCTP and
>> IPPROTO_DCCP.
>>
>> Thanks
>> Yafang
>
> How about we do not change the TP_PROTO line? The patch will be like:
>
> @@ -118,6 +127,7 @@
> __field(int, newstate)
> __field(__u16, sport)
> __field(__u16, dport)
> + __field(__u16, family)
> __field(__u8, protocol)
> __array(__u8, saddr, 4)
> __array(__u8, daddr, 4)
> @@ -133,8 +143,9 @@
> __entry->skaddr = sk;
> __entry->oldstate = oldstate;
> __entry->newstate = newstate;
> + __entry->family = sk->sk_family;
> __entry->protocol = sk->sk_protocol;
> __entry->sport = ntohs(inet->inet_sport);
> __entry->dport = ntohs(inet->inet_dport);
> @@ xxxxx
> - TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
> + TP_printk("family=%s protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
> + show_family_name(__entry->family),
>
Agreed.
IMO that's a better solution, because we make littler change and could
use the printk message to do the filter as well.
I will submit a new patch.
Thanks
Yafang
>
>>>>
>>>> - TP_ARGS(sk, oldstate, newstate),
>>>> + TP_ARGS(sk, family, protocol, oldstate, newstate),
>>>>
>>>> TP_STRUCT__entry(
>>>> __field(const void *, skaddr)
>>>> @@ -118,6 +127,7 @@
>>>> __field(int, newstate)
>>>> __field(__u16, sport)
>>>> __field(__u16, dport)
>>>> + __field(__u16, family)
>>>> __field(__u8, protocol)
>>>> __array(__u8, saddr, 4)
>>>> __array(__u8, daddr, 4)
>>>> @@ -133,8 +143,9 @@
>>>> __entry->skaddr = sk;
>>>> __entry->oldstate = oldstate;
>>>> __entry->newstate = newstate;
>>>> + __entry->family = family;
>>>> + __entry->protocol = protocol;
>>>>
>>>> - __entry->protocol = sk->sk_protocol;
>>>> __entry->sport = ntohs(inet->inet_sport);
>>>> __entry->dport = ntohs(inet->inet_dport);
>>>>
>>>> @@ -145,7 +156,7 @@
>>>> *p32 = inet->inet_daddr;
>>>>
>>>> #if IS_ENABLED(CONFIG_IPV6)
>>>> - if (sk->sk_family == AF_INET6) {
>>>> + if (family == AF_INET6) {
>>>> pin6 = (struct in6_addr *)__entry->saddr_v6;
>>>> *pin6 = sk->sk_v6_rcv_saddr;
>>>> pin6 = (struct in6_addr *)__entry->daddr_v6;
>>>> @@ -160,7 +171,8 @@
>>>> }
>>>> ),
>>>>
>>>> - TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
>>>> + TP_printk("family=%s protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
>>>> + show_family_name(__entry->family),
>>>> show_inet_protocol_name(__entry->protocol),
>>>> __entry->sport, __entry->dport,
>>>> __entry->saddr, __entry->daddr,
>>>> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
>>>> index bab98a4..1d52796 100644
>>>> --- a/net/ipv4/af_inet.c
>>>> +++ b/net/ipv4/af_inet.c
>>>> @@ -1223,14 +1223,16 @@ int inet_sk_rebuild_header(struct sock *sk)
>>>>
>>>> void inet_sk_set_state(struct sock *sk, int state)
>>>> {
>>>> - trace_inet_sock_set_state(sk, sk->sk_state, state);
>>>> + trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol,
>>>> + sk->sk_state, state);
>>>> sk->sk_state = state;
>>>> }
>>>> EXPORT_SYMBOL(inet_sk_set_state);
>>>>
>>>> void inet_sk_state_store(struct sock *sk, int newstate)
>>>> {
>>>> - trace_inet_sock_set_state(sk, sk->sk_state, newstate);
>>>> + trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol,
>>>> + sk->sk_state, newstate);
>>>> smp_store_release(&sk->sk_state, newstate);
>>>> }
>>>>
>>>> --
>>>> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH net] ipv6: remove null_entry before adding default route
From: Martin KaFai Lau @ 2018-01-07 6:16 UTC (permalink / raw)
To: Wei Wang; +Cc: David Miller, Linux Kernel Network Developers, Eric Dumazet
In-Reply-To: <CAEA6p_DJXZ9Or7-CF8VC8yHEUHrSA48ty6U5ssA4WVoUSH-5Kg@mail.gmail.com>
On Sat, Jan 06, 2018 at 05:41:28PM -0800, Wei Wang wrote:
> On Fri, Jan 5, 2018 at 11:42 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> > On Fri, Jan 05, 2018 at 05:38:35PM -0800, Wei Wang wrote:
> >> From: Wei Wang <weiwan@google.com>
> >>
> >> In the current code, when creating a new fib6 table, tb6_root.leaf gets
> >> initialized to net->ipv6.ip6_null_entry.
> >> If a default route is being added with rt->rt6i_metric = 0xffffffff,
> >> fib6_add() will add this route after net->ipv6.ip6_null_entry. As
> >> null_entry is shared, it could cause problem.
> >>
> >> In order to fix it, set fn->leaf to NULL before calling
> >> fib6_add_rt2node() when trying to add the first default route.
> >> And reset fn->leaf to null_entry when adding fails or when deleting the
> >> last default route.
> >>
> >> syzkaller reported the following issue which is fixed by this commit:
> >> =============================
> >> WARNING: suspicious RCU usage
> >> 4.15.0-rc5+ #171 Not tainted
> >> -----------------------------
> >> net/ipv6/ip6_fib.c:1702 suspicious rcu_dereference_protected() usage!
> >>
> >> other info that might help us debug this:
> >>
> >> rcu_scheduler_active = 2, debug_locks = 1
> >> 4 locks held by swapper/0/0:
> >> #0: ((&net->ipv6.ip6_fib_timer)){+.-.}, at: [<00000000d43f631b>] lockdep_copy_map include/linux/lockdep.h:178 [inline]
> >> #0: ((&net->ipv6.ip6_fib_timer)){+.-.}, at: [<00000000d43f631b>] call_timer_fn+0x1c6/0x820 kernel/time/timer.c:1310
> >> #1: (&(&net->ipv6.fib6_gc_lock)->rlock){+.-.}, at: [<000000002ff9d65c>] spin_lock_bh include/linux/spinlock.h:315 [inline]
> >> #1: (&(&net->ipv6.fib6_gc_lock)->rlock){+.-.}, at: [<000000002ff9d65c>] fib6_run_gc+0x9d/0x3c0 net/ipv6/ip6_fib.c:2007
> >> #2: (rcu_read_lock){....}, at: [<0000000091db762d>] __fib6_clean_all+0x0/0x3a0 net/ipv6/ip6_fib.c:1560
> >> #3: (&(&tb->tb6_lock)->rlock){+.-.}, at: [<000000009e503581>] spin_lock_bh include/linux/spinlock.h:315 [inline]
> >> #3: (&(&tb->tb6_lock)->rlock){+.-.}, at: [<000000009e503581>] __fib6_clean_all+0x1d0/0x3a0 net/ipv6/ip6_fib.c:1948
> >>
> >> stack backtrace:
> >> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.15.0-rc5+ #171
> >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> >> Call Trace:
> >> <IRQ>
> >> __dump_stack lib/dump_stack.c:17 [inline]
> >> dump_stack+0x194/0x257 lib/dump_stack.c:53
> >> lockdep_rcu_suspicious+0x123/0x170 kernel/locking/lockdep.c:4585
> >> fib6_del+0xcaa/0x11b0 net/ipv6/ip6_fib.c:1701
> >> fib6_clean_node+0x3aa/0x4f0 net/ipv6/ip6_fib.c:1892
> >> fib6_walk_continue+0x46c/0x8a0 net/ipv6/ip6_fib.c:1815
> >> fib6_walk+0x91/0xf0 net/ipv6/ip6_fib.c:1863
> >> fib6_clean_tree+0x1e6/0x340 net/ipv6/ip6_fib.c:1933
> >> __fib6_clean_all+0x1f4/0x3a0 net/ipv6/ip6_fib.c:1949
> >> fib6_clean_all net/ipv6/ip6_fib.c:1960 [inline]
> >> fib6_run_gc+0x16b/0x3c0 net/ipv6/ip6_fib.c:2016
> >> fib6_gc_timer_cb+0x20/0x30 net/ipv6/ip6_fib.c:2033
> >> call_timer_fn+0x228/0x820 kernel/time/timer.c:1320
> >> expire_timers kernel/time/timer.c:1357 [inline]
> >> __run_timers+0x7ee/0xb70 kernel/time/timer.c:1660
> >> run_timer_softirq+0x4c/0xb0 kernel/time/timer.c:1686
> >> __do_softirq+0x2d7/0xb85 kernel/softirq.c:285
> >> invoke_softirq kernel/softirq.c:365 [inline]
> >> irq_exit+0x1cc/0x200 kernel/softirq.c:405
> >> exiting_irq arch/x86/include/asm/apic.h:540 [inline]
> >> smp_apic_timer_interrupt+0x16b/0x700 arch/x86/kernel/apic/apic.c:1052
> >> apic_timer_interrupt+0xa9/0xb0 arch/x86/entry/entry_64.S:904
> >> </IRQ>
> >>
> >> Reported-by: syzbot <syzkaller@googlegroups.com>
> >> Fixes: 66f5d6ce53e6 ("ipv6: replace rwlock with rcu and spinlock in fib6_table")
> >> Signed-off-by: Wei Wang <weiwan@google.com>
> >> ---
> >> net/ipv6/ip6_fib.c | 45 +++++++++++++++++++++++++++++++++++----------
> >> 1 file changed, 35 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> >> index d11a5578e4f8..37cb4ad1ea29 100644
> >> --- a/net/ipv6/ip6_fib.c
> >> +++ b/net/ipv6/ip6_fib.c
> >> @@ -640,6 +640,11 @@ static struct fib6_node *fib6_add_1(struct net *net,
> >> if (!(fn->fn_flags & RTN_RTINFO)) {
> >> RCU_INIT_POINTER(fn->leaf, NULL);
> >> rt6_release(leaf);
> >> + /* remove null_entry in the root node */
> >> + } else if (fn->fn_flags & RTN_TL_ROOT &&
> >> + rcu_access_pointer(fn->leaf) ==
> >> + net->ipv6.ip6_null_entry) {
> >> + RCU_INIT_POINTER(fn->leaf, NULL);
> > It seems the reader side could see tb6_root.leaf == NULL after
> > this change and I think it should be fine? If it is, instead
> > of switching betwen NULL and ip6_null_entry, would it be simpler
> > to always set tb6_root.leaf to NULL until a legit default route
> > is added?
> >
>
> Agree that reader side could see tb6_root.leaf == NULL and it should
> be fine cause the lookup code path takes care of NULL case.
> If we init tb6_root to NULL, it does seem simpler. The only place we
> need to take care is that fib6_del() should not try to expunge fn if
> is root node if it does not contain any route. But does it seem weird
> that tb6_root.fn_flags is marked with RTN_RTINFO without containing
> any route?
Good point on the writer's side. I think we also need to modify
fib6_add_1() to expect fn->leaf == NULL. Also, agree that
RTN_RTINFO needs to be set/clear accordingly. We also need to
make changes to the CONFIG_IPV6_SUBTREES......
It should be doable but requires more changes to break the writer's side
assumption. net-next may be a better fit for those changes.
For this patch, I have one inline comment below:
>
> >> }
> >>
> >> return fn;
> >> @@ -1270,14 +1275,27 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
> >> return err;
> >>
> >> failure:
> >> - /* fn->leaf could be NULL if fn is an intermediate node and we
> >> - * failed to add the new route to it in both subtree creation
> >> - * failure and fib6_add_rt2node() failure case.
> >> - * In both cases, fib6_repair_tree() should be called to fix
> >> + /* fn->leaf could be NULL if:
> >> + * 1. fn is the root node in the table and we fail to add the default
> >> + * route to it.
> >> + * In this case, we put fn->leaf back to net->ipv6.ip6_null_entry as
> >> + * the way the table was created.
> >> + * 2. fn is an intermediate node and we failed to add the new
> >> + * route to it in both subtree creation failure and fib6_add_rt2node()
> >> + * failure case.
> >> + * In this case, fib6_repair_tree() should be called to fix
> >> * fn->leaf.
> >> */
> >> - if (fn && !(fn->fn_flags & (RTN_RTINFO|RTN_ROOT)))
> >> - fib6_repair_tree(info->nl_net, table, fn);
> >> + if (fn) {
> >> + if (fn->fn_flags & RTN_TL_ROOT) {
> >> + if (!rcu_access_pointer(fn->leaf)) {
> >> + rcu_assign_pointer(fn->leaf,
> >> + info->nl_net->ipv6.ip6_null_entry);
> >> + }
> >> + } else if (!(fn->fn_flags & (RTN_RTINFO|RTN_ROOT))) {
> >> + fib6_repair_tree(info->nl_net, table, fn);
fib6_repair_tree() is responsible to ensure intermediate fn->leaf is
not NULL (or free the fn if possible). In this case, we want tb6_root.leaf
to be not NULL. Can we consolidate the tb6_root.leaf fixing codes to
fib6_repair_tree() also such that all tree repairing logic is
in one place?
> >> + }
> >> + }
> >> /* Always release dst as dst->__refcnt is guaranteed
> >> * to be taken before entering this function
> >> */
> >> @@ -1685,11 +1703,18 @@ static void fib6_del_route(struct fib6_table *table, struct fib6_node *fn,
> >> }
> >> read_unlock(&net->ipv6.fib6_walker_lock);
> >>
> >> - /* If it was last route, expunge its radix tree node */
> >> + /* If it was last route:
> >> + * 1. For root node, put back null_entry as how the table was created.
> >> + * 2. For other nodes, expunge its radix tree node.
> >> + */
> >> if (!rcu_access_pointer(fn->leaf)) {
> >> - fn->fn_flags &= ~RTN_RTINFO;
> >> - net->ipv6.rt6_stats->fib_route_nodes--;
> >> - fn = fib6_repair_tree(net, table, fn);
> >> + if (fn->fn_flags & RTN_TL_ROOT) {
> >> + rcu_assign_pointer(fn->leaf, net->ipv6.ip6_null_entry);
> >> + } else {
> >> + fn->fn_flags &= ~RTN_RTINFO;
> >> + net->ipv6.rt6_stats->fib_route_nodes--;
> >> + fn = fib6_repair_tree(net, table, fn);
> >> + }
> >> }
> >>
> >> fib6_purge_rt(rt, fn, net);
> >> --
> >> 2.16.0.rc0.223.g4a4ac83678-goog
> >>
^ permalink raw reply
* [PATCH v2 net-next] net: tracepoint: exposing sk_faimily in tracepoint inet_sock_set_state
From: Yafang Shao @ 2018-01-07 6:31 UTC (permalink / raw)
To: songliubraving, davem, brendan.d.gregg
Cc: marcelo.leitner, netdev, linux-kernel, Yafang Shao
As of now, there're two sk_family are traced with sock:inet_sock_set_state,
which are AF_INET and AF_INET6.
So the sk_family are exposed as well.
Then we can conveniently use it to do the filter.
Both sk_family and sk_protocol are showed in the printk message, so we need
not expose them as tracepoint arguments.
Suggested-by: Brendan Gregg <brendan.d.gregg@gmail.com>
Suggested-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
include/trace/events/sock.h | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
index 3537c5f..3176a39 100644
--- a/include/trace/events/sock.h
+++ b/include/trace/events/sock.h
@@ -11,7 +11,11 @@
#include <linux/ipv6.h>
#include <linux/tcp.h>
-/* The protocol traced by sock_set_state */
+#define family_names \
+ EM(AF_INET) \
+ EMe(AF_INET6)
+
+/* The protocol traced by inet_sock_set_state */
#define inet_protocol_names \
EM(IPPROTO_TCP) \
EM(IPPROTO_DCCP) \
@@ -37,6 +41,7 @@
#define EM(a) TRACE_DEFINE_ENUM(a);
#define EMe(a) TRACE_DEFINE_ENUM(a);
+family_names
inet_protocol_names
tcp_state_names
@@ -45,6 +50,9 @@
#define EM(a) { a, #a },
#define EMe(a) { a, #a }
+#define show_family_name(val) \
+ __print_symbolic(val, family_names)
+
#define show_inet_protocol_name(val) \
__print_symbolic(val, inet_protocol_names)
@@ -118,6 +126,7 @@
__field(int, newstate)
__field(__u16, sport)
__field(__u16, dport)
+ __field(__u16, family)
__field(__u8, protocol)
__array(__u8, saddr, 4)
__array(__u8, daddr, 4)
@@ -134,6 +143,7 @@
__entry->oldstate = oldstate;
__entry->newstate = newstate;
+ __entry->family = sk->sk_family;
__entry->protocol = sk->sk_protocol;
__entry->sport = ntohs(inet->inet_sport);
__entry->dport = ntohs(inet->inet_dport);
@@ -160,7 +170,8 @@
}
),
- TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
+ TP_printk("family=%s protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
+ show_family_name(__entry->family),
show_inet_protocol_name(__entry->protocol),
__entry->sport, __entry->dport,
__entry->saddr, __entry->daddr,
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Willy Tarreau @ 2018-01-07 6:33 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alan Cox, Linus Torvalds, Dan Williams, linux-kernel, linux-arch,
Andi Kleen, Arnd Bergmann, Greg Kroah-Hartman, Peter Zijlstra,
netdev, Ingo Molnar, H. Peter Anvin, Thomas Gleixner
In-Reply-To: <20180107033812.awq3vz4gdkps7tix@ast-mbp>
On Sat, Jan 06, 2018 at 07:38:14PM -0800, Alexei Starovoitov wrote:
> yep. plenty of unknowns and what's happening now is an overreaction.
To be fair there's overreaction on both sides. The vast majority of
users need to get a 100% safe system and will never notice any
difference. A few of us are more concerned by the risk of performance
loss brought by these fixes. We do all need to run tests on the
patchsets to bring numbers on the table.
> What is the rush to push half baked patches into upstream
> that don't even address the variant 1 ?
You probably need to trust the CPU vendors a bit for having had all
the details about the risks for a few months now and accept that if
they're destroying their product's performance compared to their main
competitor, they probably studied a number of other alternatives first.
It doesn't mean they thought about everything of course, and maybe they
need to study your proposal as a better solution to reduce criticism.
> which clearly states that bpf_tail_call() was used in the attack.
> Yet none of the intel nor arm patches address speculation in
> this bpf helper!
> It means that:
> - gpz didn't share neither exploit nor the detailed description
> of the POC with cpu vendors until now
Or it was considered less urgent to fix compared to the rest. It's
also possible that the scariest details were not written in the GPZ
article.
> Now the attack is well described, yet cpu vendors still pushing
> for lfence patches that don't make sense. Why?
Imagine if you were in their position and were pushing a solution which
would later be found to be inefficient and to be vulnerable again. Your
name would appear twice in the press in a few months, this would be
terrible. It makes sense in their position to find the safest fix first
given that those like you or me really concerned about the performance
impact know how to add an option to a boot loader or revert a patch that
causes trouble.
> What I think is important is to understand vulnerability first.
> I don't think it was done.
I suspect it was clearly done by those how had no other option but
working on this painful series over the last few months :-/
> > The differences involved on the "lfence" versus "and" versus before are
> > not likely to be anywhere in that order of magnitude.
>
> we clearly disagree here. Both intel and arm patches proposed
> to add lfence in bpf_map_lookup() which is the hottest function
> we have and we do run it at 40+Gbps speeds where every nanosecond
> counts, so no, lfence is not a solution.
Please help here by testing the patch series and reporting numbers
before, with the fix, and with your proposal. That's the best way to
catch their attention and to get your proposal considered as a viable
alternative (or as a partial one for certain environments). I did the
same when I believed it could be sufficient to add noise to RDTSC and
found it totally inefficient after testing. But it's better for
everyone that research and testing is done rather than criticizing the
proposed fixes (which is not fair to the people who work hard on them
for everyone else).
Cheers,
Willy
^ permalink raw reply
* Re: [PATCH iproute2-next v1 3/9] rdma: Add filtering infrastructure
From: Leon Romanovsky @ 2018-01-07 7:44 UTC (permalink / raw)
To: David Ahern
Cc: Doug Ledford, Jason Gunthorpe, RDMA mailing list, netdev,
Stephen Hemminger
In-Reply-To: <b7ad60c5-a1c5-ff79-113d-d6c57baf1c40@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]
On Thu, Jan 04, 2018 at 08:29:31PM -0700, David Ahern wrote:
> On 1/4/18 12:01 AM, Leon Romanovsky wrote:
> > diff --git a/rdma/utils.c b/rdma/utils.c
> > index af2b374d..446c23da 100644
> > --- a/rdma/utils.c
> > +++ b/rdma/utils.c
> > @@ -114,6 +114,225 @@ static void dev_map_cleanup(struct rd *rd)
> > }
> > }
> >
> > +static int add_filter(struct rd *rd, char *key, char *value,
> > + const char * const valid_filters[])
> > +{
> > + char cset[] = "1234567890,-";
> > + struct filter_entry *fe;
> > + bool key_found = false;
> > + int idx = 0;
> > + int ret;
> > +
> > + fe = calloc(1, sizeof(*fe));
> > + if (!fe)
> > + return -ENOMEM;
> > +
> > + while (valid_filters[idx]) {
> > + if (!strcmpx(key, valid_filters[idx])) {
> > + key_found = true;
> > + break;
> > + }
> > + idx++;
> > + }
> > + if (!key_found) {
> > + pr_err("Unsupported filter option: %s\n", key);
> > + ret = -EINVAL;
> > + goto err;
> > + }
> > +
> > + /*
> > + * Check the filter validity, not optimal, but works
> > + *
> > + * Actually, there are three types of filters
> > + * numeric - for example PID or QPN
> > + * string - for example states, currently only "display"
> > + * is from this type
> > + * link - user requested to filter on specific link
> > + * e.g. mlx5_1/1, mlx5_1/-, mlx5_1 ...
> > + */
> > + if (strcmpx(key, "link") && strcmpx(key, "display") &&
> > + strspn(value, cset) != strlen(value)) {
> > + pr_err("%s filter accepts \"%s\" characters only\n", key, cset);
> > + ret = -EINVAL;
> > + goto err;
> > + }
> > +
> > + fe->key = strdup(key);
> > + fe->value = strdup(value);
> > +
>
> Missed this the other day. 2 more strdup's that should be checked.
Thanks, David,
I'll fix and send new version once the RDMA kernel part will be accepted.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: atm/clip: Use seq_puts() in svc_addr()
From: SF Markus Elfring @ 2018-01-07 8:19 UTC (permalink / raw)
To: Stefano Brivio, netdev
Cc: Bhumika Goyal, David S. Miller, David Windsor, Elena Reshetova,
Hans Liljestrand, Johannes Berg, Kees Cook, Roopa Prabhu, LKML,
kernel-janitors
In-Reply-To: <20180106232539.5d6bb620@elisabeth>
>> Two strings should be quickly put into a sequence by two function calls.
>> Thus use the function "seq_puts" instead of "seq_printf".
>>
>> This issue was detected by using the Coccinelle software.
>
> Can you please explain what the issue really is and what you're trying
> to do here?
Is the function "seq_puts" a bit more efficient for the desired output
of a single string in comparison to calling the function "seq_printf"
for this purpose?
> One shouldn't need to dig into Coccinelle patterns to find
> out what you mean,
Why did an attribution for a software tool confuse you?
> and "strings should be quickly put into a sequence"
> isn't terribly helpful.
Which wording would you find more appropriate for the suggested
adjustment of these function calls?
Regards,
Markus
^ permalink raw reply
* Dear Talented
From: Kim Sharma @ 2018-01-07 15:32 UTC (permalink / raw)
To: Recipients
Dear Talented,
I am Talent Scout For BLUE SKY FILM STUDIO, Present Blue sky Studio a
Film Corporation Located in the United State, is Soliciting for the
Right to use Your Photo/Face and Personality as One of the Semi -Major
Role/ Character in our Upcoming ANIMATED Stereoscope 3D Movie-The Story
of Anubis (Anubis 2018) The Movie is Currently Filming (In
Production) Please Note That There Will Be No Auditions, Traveling or
Any Special / Professional Acting Skills, Since the Production of This
Movie Will Be Done with our State of Art Computer -Generating Imagery
Equipment. We Are Prepared to Pay the Total Sum of $620,000.00 USD. For
More Information/Understanding, Please Write us on the E-Mail Below.
CONTACT EMAIL: blueskyfilms.studio1@usa.com
All Reply to: blueskyfilms.studio1@usa.com
Note: Only the Response send to this mail will be Given a Prior
Consideration.
Talent Scout
Kim Sharma
^ permalink raw reply
* Re: [PATCH 07/18] [media] uvcvideo: prevent bounds-check bypass via speculative execution
From: Greg KH @ 2018-01-07 9:09 UTC (permalink / raw)
To: Dan Williams
Cc: Linux Kernel Mailing List, linux-arch, Alan Cox, Peter Zijlstra,
Netdev, Laurent Pinchart, Thomas Gleixner, Mauro Carvalho Chehab,
Linus Torvalds, Elena Reshetova, Linux-media@vger.kernel.org, dsj
In-Reply-To: <CAPcyv4je-agqvmNSJf7v-1VBOrfhOvcs_qASNPJiBzgTt70dPA@mail.gmail.com>
On Sat, Jan 06, 2018 at 09:41:17AM -0800, Dan Williams wrote:
> On Sat, Jan 6, 2018 at 1:40 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Sat, Jan 06, 2018 at 10:09:07AM +0100, Greg KH wrote:
> >> On Fri, Jan 05, 2018 at 05:10:32PM -0800, Dan Williams wrote:
> >> > Static analysis reports that 'index' may be a user controlled value that
> >> > is used as a data dependency to read 'pin' from the
> >> > 'selector->baSourceID' array. In order to avoid potential leaks of
> >> > kernel memory values, block speculative execution of the instruction
> >> > stream that could issue reads based on an invalid value of 'pin'.
> >> >
> >> > Based on an original patch by Elena Reshetova.
> >> >
> >> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> >> > Cc: linux-media@vger.kernel.org
> >> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> >> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> >> > ---
> >> > drivers/media/usb/uvc/uvc_v4l2.c | 7 +++++--
> >> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> >> > index 3e7e283a44a8..7442626dc20e 100644
> >> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> >> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> >> > @@ -22,6 +22,7 @@
> >> > #include <linux/mm.h>
> >> > #include <linux/wait.h>
> >> > #include <linux/atomic.h>
> >> > +#include <linux/compiler.h>
> >> >
> >> > #include <media/v4l2-common.h>
> >> > #include <media/v4l2-ctrls.h>
> >> > @@ -810,6 +811,7 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
> >> > struct uvc_entity *iterm = NULL;
> >> > u32 index = input->index;
> >> > int pin = 0;
> >> > + __u8 *elem;
> >> >
> >> > if (selector == NULL ||
> >> > (chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
> >> > @@ -820,8 +822,9 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
> >> > break;
> >> > }
> >> > pin = iterm->id;
> >> > - } else if (index < selector->bNrInPins) {
> >> > - pin = selector->baSourceID[index];
> >> > + } else if ((elem = nospec_array_ptr(selector->baSourceID, index,
> >> > + selector->bNrInPins))) {
> >> > + pin = *elem;
> >>
> >> I dug through this before, and I couldn't find where index came from
> >> userspace, I think seeing the coverity rule would be nice.
> >
> > Ok, I take it back, this looks correct. Ugh, the v4l ioctl api is
> > crazy complex (rightfully so), it's amazing that coverity could navigate
> > that whole thing :)
> >
> > While I'm all for fixing this type of thing, I feel like we need to do
> > something "else" for this as playing whack-a-mole for this pattern is
> > going to be a never-ending battle for all drivers for forever. Either
> > we need some way to mark this data path to make it easy for tools like
> > sparse to flag easily, or we need to catch the issue in the driver
> > subsystems, which unfortunatly, would harm the drivers that don't have
> > this type of issue (like here.)
> >
> > I'm guessing that other operating systems, which don't have the luxury
> > of auditing all of their drivers are going for the "big hammer in the
> > subsystem" type of fix, right?
> >
> > I don't have a good answer for this, but if there was some better way to
> > rewrite these types of patterns to just prevent the need for the
> > nospec_array_ptr() type thing, that might be the best overall for
> > everyone. Much like ebpf did with their changes. That way a simple
> > coccinelle rule would be able to catch the pattern and rewrite it.
> >
> > Or am I just dreaming?
>
> At least on the coccinelle front you're dreaming. Julia already took a
> look and said:
>
> "I don't think Coccinelle would be good for doing this (ie
> implementing taint analysis) because the dataflow is too complicated."
Sorry for the confusion, no, I don't mean the "taint tracking", I mean
the generic pattern of "speculative out of bounds access" that we are
fixing here.
Yes, as you mentioned before, there are tons of false-positives in the
tree, as to find the real problems you have to show that userspace
controls the access index. But if we have a generic pattern that can
rewrite that type of logic into one where it does not matter at all
(i.e. like the ebpf proposed changes), then it would not be an issue if
they are false or not, we just rewrite them all to be safe.
We need to find some way not only to fix these issues now (like you are
doing with this series), but to prevent them from every coming back into
the codebase again. It's that second part that we need to keep in the
back of our minds here, while doing the first portion of this work.
> Perhaps the Coverity instance Dave mentioned at Ksummit 2012 has a
> role to play here?
We have a coverity instance that all kernel developers have access to
(just sign up and we grant it.) We have at least one person working
full time on fixing up errors that this instance reports. So if we
could get those rules added (which is why I asked for them), it would be
a great first line of defense to prevent the "adding new problems" issue
from happening right now for the 4.16-rc1 merge window.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net-next 1/6] net: Fix netdev_WARN_ONCE macro
From: Gal Pressman @ 2018-01-07 9:31 UTC (permalink / raw)
To: David Miller; +Cc: netdev, tariqt, saeedm
In-Reply-To: <20180105.133519.1602365963790420581.davem@davemloft.net>
On 05-Jan-18 20:35, David Miller wrote:
> From: Gal Pressman <galp@mellanox.com>
> Date: Thu, 4 Jan 2018 16:55:58 +0200
>
>> netdev_WARN_ONCE is broken (whoops..), fix it.
>
> You don't say how it is broken and how you are fixing that
> brokenness, and you must do this.
>
> Yes, if I stare at your patch for a few minutes I can see
> the missing "," but you could have saved everyone reviewing
> this patch (now and in the future) by writing a proper and
> complete commit message.
>
Thank you for reviewing,
I will fix the commit message and submit v2.
^ permalink raw reply
* Re: [PATCH net-next v2 06/10] net/mlx5e: Change Mellanox references in DIM code
From: Tal Gilboa @ 2018-01-07 9:44 UTC (permalink / raw)
To: Andy Gospodarek, netdev; +Cc: mchan, ogerlitz, Andy Gospodarek
In-Reply-To: <1515193094-9554-7-git-send-email-andy@greyhouse.net>
On 1/6/2018 12:58 AM, Andy Gospodarek wrote:
> From: Andy Gospodarek <gospo@broadcom.com>
>
> Change all appropriate mlx5_am* and MLX5_AM* references to net_dim and
> NET_DIM, respectively, in code that handles dynamic interrupt
> moderation. Also change all references from 'am' to 'dim' when used as
> local variables.
>
> Signed-off-by: Andy Gospodarek <gospo@broadcom.com>
> Acked-by: Tal Gilboa <talgi@mellanox.com>
> Acked-by: Saeed Mahameed <saeedm@mellanox.com>
>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 12 +-
> drivers/net/ethernet/mellanox/mlx5/core/en_dim.c | 14 +-
> .../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 12 +-
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 52 ++--
> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 4 +-
> drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 2 +-
> drivers/net/ethernet/mellanox/mlx5/core/net_dim.c | 284 ++++++++++-----------
> drivers/net/ethernet/mellanox/mlx5/core/net_dim.h | 63 +++--
> 8 files changed, 232 insertions(+), 211 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index 121f280..732f275 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -115,6 +115,9 @@
> #define MLX5E_PARAMS_DEFAULT_MIN_RX_WQES 0x80
> #define MLX5E_PARAMS_DEFAULT_MIN_RX_WQES_MPW 0x2
>
> +#define MLX5E_CQ_PERIOD_MODE_START_FROM_EQE 0x0
> +#define MLX5E_CQ_PERIOD_MODE_START_FROM_CQE 0x1
> +
This enum should be defined under include/linux/mlx5/mlx5_ifc.h:
enum {
MLX5_CQ_PERIOD_MODE_START_FROM_EQE = 0x0,
MLX5_CQ_PERIOD_MODE_START_FROM_CQE = 0x1,
MLX5_CQ_PERIOD_NUM_MODES
};
We already include this in the relevant files. This was changed in patch
01/10, please revert.
^ permalink raw reply
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Thomas Gleixner @ 2018-01-07 10:08 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alan Cox, Linus Torvalds, Dan Williams, linux-kernel, linux-arch,
Andi Kleen, Arnd Bergmann, Greg Kroah-Hartman, Peter Zijlstra,
netdev, Ingo Molnar, H. Peter Anvin
In-Reply-To: <20180107033812.awq3vz4gdkps7tix@ast-mbp>
On Sat, 6 Jan 2018, Alexei Starovoitov wrote:
> which clearly states that bpf_tail_call() was used in the attack.
> Yet none of the intel nor arm patches address speculation in
> this bpf helper!
> It means that:
> - gpz didn't share neither exploit nor the detailed description
> of the POC with cpu vendors until now
> - coverity rules used to find all these places in the kernel
> failed to find bpf_tail_call
> - cpu vendors were speculating what variant 1 can actually do
You forgot to mention that there might be other attacks than the public POC
which are not covered by a simple AND ....
Thanks,
tglx
^ permalink raw reply
* [PATCH net-next v2 0/6] Replace WARN_ONCE usages with netdev_WARN_ONCE
From: Gal Pressman @ 2018-01-07 10:08 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Tariq Toukan, Saeed Mahameed, Gal Pressman
Hi,
This series will fix an issue in netdev_WARN_ONCE, improve its formatting and
replace drivers' usage of WARN_ONCE to netdev_WARN_ONCE.
Driver specific patches were compilation tested, in addition, functional tested
on Mellanox NIC.
v1->v2:
- Addressed commit message comments in patch #1
Gal Pressman (6):
net: Fix netdev_WARN_ONCE macro
net: No line break on netdev_WARN* formatting
net/mlx5e: Replace WARN_ONCE with netdev_WARN_ONCE
e1000: Replace WARN_ONCE with netdev_WARN_ONCE
bnx2x: Replace WARN_ONCE with netdev_WARN_ONCE
8139cp: Replace WARN_ONCE with netdev_WARN_ONCE
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 5 +++--
drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 4 ++--
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 9 ++++-----
drivers/net/ethernet/realtek/8139cp.c | 4 ++--
include/linux/netdevice.h | 6 +++---
5 files changed, 14 insertions(+), 14 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH net-next v2 1/6] net: Fix netdev_WARN_ONCE macro
From: Gal Pressman @ 2018-01-07 10:08 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Tariq Toukan, Saeed Mahameed, Gal Pressman
In-Reply-To: <1515319720-18428-1-git-send-email-galp@mellanox.com>
netdev_WARN_ONCE is broken (whoops..), this fix will remove the
unnecessary "condition" parameter, add the missing comma and change
"arg" to "args".
Fixes: 375ef2b1f0d0 ("net: Introduce netdev_*_once functions")
Signed-off-by: Gal Pressman <galp@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
---
include/linux/netdevice.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 352066e..5ff1ef9 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4407,8 +4407,8 @@ do { \
WARN(1, "netdevice: %s%s\n" format, netdev_name(dev), \
netdev_reg_state(dev), ##args)
-#define netdev_WARN_ONCE(dev, condition, format, arg...) \
- WARN_ONCE(1, "netdevice: %s%s\n" format, netdev_name(dev) \
+#define netdev_WARN_ONCE(dev, format, args...) \
+ WARN_ONCE(1, "netdevice: %s%s\n" format, netdev_name(dev), \
netdev_reg_state(dev), ##args)
/* netif printk helpers, similar to netdev_printk */
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 3/6] net/mlx5e: Replace WARN_ONCE with netdev_WARN_ONCE
From: Gal Pressman @ 2018-01-07 10:08 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Tariq Toukan, Saeed Mahameed, Gal Pressman
In-Reply-To: <1515319720-18428-1-git-send-email-galp@mellanox.com>
Use the more appropriate netdev_WARN_ONCE instead of WARN_ONCE macro.
Signed-off-by: Gal Pressman <galp@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 5334b2d..38803e3 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -495,8 +495,8 @@ static inline void mlx5e_poll_ico_single_cqe(struct mlx5e_cq *cq,
mlx5_cqwq_pop(&cq->wq);
if (unlikely((cqe->op_own >> 4) != MLX5_CQE_REQ)) {
- WARN_ONCE(true, "mlx5e: Bad OP in ICOSQ CQE: 0x%x\n",
- cqe->op_own);
+ netdev_WARN_ONCE(cq->channel->netdev,
+ "Bad OP in ICOSQ CQE: 0x%x\n", cqe->op_own);
return;
}
@@ -506,9 +506,8 @@ static inline void mlx5e_poll_ico_single_cqe(struct mlx5e_cq *cq,
}
if (unlikely(icowi->opcode != MLX5_OPCODE_NOP))
- WARN_ONCE(true,
- "mlx5e: Bad OPCODE in ICOSQ WQE info: 0x%x\n",
- icowi->opcode);
+ netdev_WARN_ONCE(cq->channel->netdev,
+ "Bad OPCODE in ICOSQ WQE info: 0x%x\n", icowi->opcode);
}
static void mlx5e_poll_ico_cq(struct mlx5e_cq *cq, struct mlx5e_rq *rq)
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 2/6] net: No line break on netdev_WARN* formatting
From: Gal Pressman @ 2018-01-07 10:08 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Tariq Toukan, Saeed Mahameed, Gal Pressman
In-Reply-To: <1515319720-18428-1-git-send-email-galp@mellanox.com>
Remove the unnecessary line break between the netdev name and reg state
to the actual message that should be printed.
For example, this:
[86730.307236] ------------[ cut here ]------------
[86730.313496] netdevice: enp27s0f0
Message from the driver
[...]
Will be replaced with:
[86770.259289] ------------[ cut here ]------------
[86770.265191] netdevice: enp27s0f0: Message from the driver
[...]
Signed-off-by: Gal Pressman <galp@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
---
include/linux/netdevice.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5ff1ef9..87211c4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4404,11 +4404,11 @@ do { \
* file/line information and a backtrace.
*/
#define netdev_WARN(dev, format, args...) \
- WARN(1, "netdevice: %s%s\n" format, netdev_name(dev), \
+ WARN(1, "netdevice: %s%s: " format, netdev_name(dev), \
netdev_reg_state(dev), ##args)
#define netdev_WARN_ONCE(dev, format, args...) \
- WARN_ONCE(1, "netdevice: %s%s\n" format, netdev_name(dev), \
+ WARN_ONCE(1, "netdevice: %s%s: " format, netdev_name(dev), \
netdev_reg_state(dev), ##args)
/* netif printk helpers, similar to netdev_printk */
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 4/6] e1000: Replace WARN_ONCE with netdev_WARN_ONCE
From: Gal Pressman @ 2018-01-07 10:08 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Tariq Toukan, Saeed Mahameed, Gal Pressman, Jeff Kirsher
In-Reply-To: <1515319720-18428-1-git-send-email-galp@mellanox.com>
Use the more appropriate netdev_WARN_ONCE instead of WARN_ONCE macro.
Signed-off-by: Gal Pressman <galp@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index 3b3983a..dc71e87 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -1838,8 +1838,8 @@ static void e1000_get_ethtool_stats(struct net_device *netdev,
p = (char *)adapter + stat->stat_offset;
break;
default:
- WARN_ONCE(1, "Invalid E1000 stat type: %u index %d\n",
- stat->type, i);
+ netdev_WARN_ONCE(netdev, "Invalid E1000 stat type: %u index %d\n",
+ stat->type, i);
continue;
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 5/6] bnx2x: Replace WARN_ONCE with netdev_WARN_ONCE
From: Gal Pressman @ 2018-01-07 10:08 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Tariq Toukan, Saeed Mahameed, Gal Pressman, Ariel Elior
In-Reply-To: <1515319720-18428-1-git-send-email-galp@mellanox.com>
Use the more appropriate netdev_WARN_ONCE instead of WARN_ONCE macro.
Signed-off-by: Gal Pressman <galp@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
Cc: Ariel Elior <ariel.elior@cavium.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 57eb26d..d7c98e8 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -738,8 +738,9 @@ static void bnx2x_gro_receive(struct bnx2x *bp, struct bnx2x_fastpath *fp,
bnx2x_gro_csum(bp, skb, bnx2x_gro_ipv6_csum);
break;
default:
- WARN_ONCE(1, "Error: FW GRO supports only IPv4/IPv6, not 0x%04x\n",
- be16_to_cpu(skb->protocol));
+ netdev_WARN_ONCE(bp->dev,
+ "Error: FW GRO supports only IPv4/IPv6, not 0x%04x\n",
+ be16_to_cpu(skb->protocol));
}
}
#endif
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 6/6] 8139cp: Replace WARN_ONCE with netdev_WARN_ONCE
From: Gal Pressman @ 2018-01-07 10:08 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Tariq Toukan, Saeed Mahameed, Gal Pressman,
Realtek linux nic maintainers
In-Reply-To: <1515319720-18428-1-git-send-email-galp@mellanox.com>
Use the more appropriate netdev_WARN_ONCE instead of WARN_ONCE macro.
Signed-off-by: Gal Pressman <galp@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
Cc: Realtek linux nic maintainers <nic_swsd@realtek.com>
---
drivers/net/ethernet/realtek/8139cp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index e7ab23e..81045df 100644
--- a/drivers/net/ethernet/realtek/8139cp.c
+++ b/drivers/net/ethernet/realtek/8139cp.c
@@ -748,8 +748,8 @@ static netdev_tx_t cp_start_xmit (struct sk_buff *skb,
mss = skb_shinfo(skb)->gso_size;
if (mss > MSSMask) {
- WARN_ONCE(1, "Net bug: GSO size %d too large for 8139CP\n",
- mss);
+ netdev_WARN_ONCE(dev, "Net bug: GSO size %d too large for 8139CP\n",
+ mss);
goto out_dma_error;
}
--
2.7.4
^ permalink raw reply related
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