* 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 net-next v3 00/10] net: qualcomm: rmnet: Enable csum offloads
From: David Miller @ 2018-01-07 1:08 UTC (permalink / raw)
To: subashab; +Cc: netdev, lkp, edumazet
In-Reply-To: <3598b25f3a9138be9b213624b89ce512@codeaurora.org>
From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Fri, 05 Jan 2018 18:05:23 -0700
> I dont see this series in patchwork.
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.
^ permalink raw reply
* Re: [PATCH 0/2] tests: fix issues in autopkgtest environment
From: Luca Boccassi @ 2018-01-06 23:46 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Christian Ehrhardt, Netdev
In-Reply-To: <1514878118-21009-1-git-send-email-christian.ehrhardt@canonical.com>
[-- Attachment #1: Type: text/plain, Size: 1029 bytes --]
On Tue, 2018-01-02 at 08:28 +0100, Christian Ehrhardt wrote:
> Hi,
> while working on Debian bug [1] and the Ubuntu counterpart of it
> I found that the tests can throw a broken pipe warning.
>
> I kept the associated check-and-retry of the length separate to
> discuss the changes individually - feel free to squash them on
> commit if preferred.
>
> [1]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=879854
>
> Christian Ehrhardt (2):
> tests: read limited amount from /dev/urandom
> tests: make sure rand_dev suffix has 6 chars
>
> testsuite/lib/generic.sh | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
Stephen,
The subject of this series is missing the "iproute2" tag so it likely
fell off the back of the wagon, could you have a look please?
Archive link:
https://marc.info/?l=linux-netdev&m=151487814431958&w=2
Patchwork:
https://patchwork.ozlabs.org/patch/854412/
https://patchwork.ozlabs.org/patch/854410/
Thanks
--
Kind regards,
Luca Boccassi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [net-next] netfilter: add segment routing header 'srh' match
From: Pablo Neira Ayuso @ 2018-01-06 23:40 UTC (permalink / raw)
To: Ahmed Abdelsalam
Cc: kadlec, fw, davem, kuznet, yoshfuji, linux-kernel,
netfilter-devel, coreteam, netdev
In-Reply-To: <1514545672-13827-1-git-send-email-amsalam20@gmail.com>
Hi Ahmed,
On Fri, Dec 29, 2017 at 12:07:52PM +0100, Ahmed Abdelsalam wrote:
> It allows matching packets based on Segment Routing Header
> (SRH) information.
> The implementation considers revision 7 of the SRH draft.
> https://tools.ietf.org/html/draft-ietf-6man-segment-routing-header-07
>
> Currently supported match options include:
> (1) Next Header
> (2) Hdr Ext Len
> (3) Segments Left
> (4) Last Entry
> (5) Tag value of SRH
>
> Signed-off-by: Ahmed Abdelsalam <amsalam20@gmail.com>
> ---
> include/uapi/linux/netfilter_ipv6/ip6t_srh.h | 63 ++++++++++
> net/ipv6/netfilter/Kconfig | 9 ++
> net/ipv6/netfilter/Makefile | 1 +
> net/ipv6/netfilter/ip6t_srh.c | 165 +++++++++++++++++++++++++++
> 4 files changed, 238 insertions(+)
> create mode 100644 include/uapi/linux/netfilter_ipv6/ip6t_srh.h
> create mode 100644 net/ipv6/netfilter/ip6t_srh.c
>
> diff --git a/include/uapi/linux/netfilter_ipv6/ip6t_srh.h b/include/uapi/linux/netfilter_ipv6/ip6t_srh.h
> new file mode 100644
> index 0000000..1b5dbd8
> --- /dev/null
> +++ b/include/uapi/linux/netfilter_ipv6/ip6t_srh.h
> @@ -0,0 +1,63 @@
> +/**
> + * Definitions for Segment Routing Header 'srh' match
> + *
> + * Author:
> + * Ahmed Abdelsalam <amsalam20@gmail.com>
> + */
Please, add this in SPDX format instead.
See include/uapi/linux/netfilter/xt_owner.h for instance.
> +#ifndef _IP6T_SRH_H
> +#define _IP6T_SRH_H
> +
> +#include <linux/types.h>
> +#include <linux/netfilter.h>
> +
> +/* Values for "mt_flags" field in struct ip6t_srh */
> +#define IP6T_SRH_NEXTHDR 0x0001
> +#define IP6T_SRH_LEN_EQ 0x0002
> +#define IP6T_SRH_LEN_GT 0x0004
> +#define IP6T_SRH_LEN_LT 0x0008
> +#define IP6T_SRH_SEGS_EQ 0x0010
> +#define IP6T_SRH_SEGS_GT 0x0020
> +#define IP6T_SRH_SEGS_LT 0x0040
> +#define IP6T_SRH_LAST_EQ 0x0080
> +#define IP6T_SRH_LAST_GT 0x0100
> +#define IP6T_SRH_LAST_LT 0x0200
> +#define IP6T_SRH_TAG 0x0400
> +#define IP6T_SRH_MASK 0x07FF
> +
> +/* Values for "mt_invflags" field in struct ip6t_srh */
> +#define IP6T_SRH_INV_NEXTHDR 0x0001
> +#define IP6T_SRH_INV_LEN_EQ 0x0002
> +#define IP6T_SRH_INV_LEN_GT 0x0004
> +#define IP6T_SRH_INV_LEN_LT 0x0008
> +#define IP6T_SRH_INV_SEGS_EQ 0x0010
> +#define IP6T_SRH_INV_SEGS_GT 0x0020
> +#define IP6T_SRH_INV_SEGS_LT 0x0040
> +#define IP6T_SRH_INV_LAST_EQ 0x0080
> +#define IP6T_SRH_INV_LAST_GT 0x0100
> +#define IP6T_SRH_INV_LAST_LT 0x0200
> +#define IP6T_SRH_INV_TAG 0x0400
> +#define IP6T_SRH_INV_MASK 0x07FF
Looking at all these EQ, GT, LT... I think this should be very easy to
implement in nf_tables with no kernel changes.
You only need to add the protocol definition to:
nftables/src/exthdr.c
Would you have a look into this? This would be very much appreciated
to we keep nftables in sync with what we have in iptables.
> +
> +/**
> + * struct ip6t_srh - SRH match options
> + * @ next_hdr: Next header field of SRH
> + * @ hdr_len: Extension header length field of SRH
> + * @ segs_left: Segments left field of SRH
> + * @ last_entry: Last entry field of SRH
> + * @ tag: Tag field of SRH
> + * @ mt_flags: match options
> + * @ mt_invflags: Invert the sense of match options
> + */
> +
> +struct ip6t_srh {
> + __u8 next_hdr;
> + __u8 hdr_len;
> + __u8 segs_left;
> + __u8 last_entry;
> + __u16 tag;
> + __u16 mt_flags;
> + __u16 mt_invflags;
> +};
> +
> +#endif /*_IP6T_SRH_H*/
> diff --git a/net/ipv6/netfilter/Kconfig b/net/ipv6/netfilter/Kconfig
> index 6acb2ee..e1818eb 100644
> --- a/net/ipv6/netfilter/Kconfig
> +++ b/net/ipv6/netfilter/Kconfig
> @@ -232,6 +232,15 @@ config IP6_NF_MATCH_RT
>
> To compile it as a module, choose M here. If unsure, say N.
>
> +config IP6_NF_MATCH_SRH
> + tristate '"srh" Segment Routing header match support'
> + depends on NETFILTER_ADVANCED
> + help
> + srh matching allows you to match packets based on the segment
> + routing header of the packet.
> +
> + To compile it as a module, choose M here. If unsure, say N.
> +
> # The targets
> config IP6_NF_TARGET_HL
> tristate '"HL" hoplimit target support'
> diff --git a/net/ipv6/netfilter/Makefile b/net/ipv6/netfilter/Makefile
> index c6ee0cd..e0d51a9 100644
> --- a/net/ipv6/netfilter/Makefile
> +++ b/net/ipv6/netfilter/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_IP6_NF_MATCH_MH) += ip6t_mh.o
> obj-$(CONFIG_IP6_NF_MATCH_OPTS) += ip6t_hbh.o
> obj-$(CONFIG_IP6_NF_MATCH_RPFILTER) += ip6t_rpfilter.o
> obj-$(CONFIG_IP6_NF_MATCH_RT) += ip6t_rt.o
> +obj-$(CONFIG_IP6_NF_MATCH_SRH) += ip6t_srh.o
>
> # targets
> obj-$(CONFIG_IP6_NF_TARGET_MASQUERADE) += ip6t_MASQUERADE.o
> diff --git a/net/ipv6/netfilter/ip6t_srh.c b/net/ipv6/netfilter/ip6t_srh.c
> new file mode 100644
> index 0000000..75e41dc9
> --- /dev/null
> +++ b/net/ipv6/netfilter/ip6t_srh.c
> @@ -0,0 +1,165 @@
> +/*
> + * Kernel module to match Segment Routing Header (SRH) parameters.
> + *
> + * Author:
> + * Ahmed Abdelsalam <amsalam20@gmail.com>
> + *
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +#include <linux/module.h>
> +#include <linux/skbuff.h>
> +#include <linux/ipv6.h>
> +#include <linux/types.h>
> +#include <net/ipv6.h>
> +#include <net/seg6.h>
> +
> +#include <linux/netfilter/x_tables.h>
> +#include <linux/netfilter_ipv6/ip6t_srh.h>
> +#include <linux/netfilter_ipv6/ip6_tables.h>
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Xtables: IPv6 Segment Routing Header match");
> +MODULE_AUTHOR("Ahmed Abdelsalam <amsalam20@gmail.com>");
Not a deal breaker, but modern code usually places these MODULE_*
lines at the end of the file.
> +
> +/* Test a struct->mt_invflags and a boolean for inequality */
> +#define NF_SRH_INVF(ptr, flag, boolean) \
> + ((boolean) ^ !!((ptr)->mt_invflags & (flag)))
> +
> +static bool srh_mt6(const struct sk_buff *skb, struct xt_action_param *par)
> +{
> + const struct ip6t_srh *srhinfo = par->matchinfo;
> + struct ipv6_sr_hdr *srh;
> + struct ipv6_sr_hdr _srh;
> + int hdrlen, srhoff = 0;
> +
> + if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
> + return false;
> +
> + srh = skb_header_pointer(skb, srhoff, sizeof(_srh), &_srh);
> +
Remove unnecessary line break.
> + if (!srh)
> + return false;
> +
> + hdrlen = ipv6_optlen(srh);
> + if (skb->len - srhoff < hdrlen)
> + return false;
> +
> + if (srh->type != IPV6_SRCRT_TYPE_4)
> + return false;
> +
> + if (srh->segments_left > srh->first_segment)
> + return false;
> +
> + /* Next Header matching */
> + if (srhinfo->mt_flags & IP6T_SRH_NEXTHDR)
> + if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_NEXTHDR,
> + !(srh->nexthdr == srhinfo->next_hdr)))
> + return false;
> +
> + /* Header Extension Length matching */
> + if (srhinfo->mt_flags & IP6T_SRH_LEN_EQ)
> + if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_LEN_EQ,
> + !(srh->hdrlen == srhinfo->hdr_len)))
> + return false;
> +
> + if (srhinfo->mt_flags & IP6T_SRH_LEN_GT)
> + if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_LEN_GT,
> + !(srh->hdrlen > srhinfo->hdr_len)))
> + return false;
> +
> + if (srhinfo->mt_flags & IP6T_SRH_LEN_LT)
> + if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_LEN_LT,
> + !(srh->hdrlen < srhinfo->hdr_len)))
> + return false;
> +
> + /* Segments Left matching */
> + if (srhinfo->mt_flags & IP6T_SRH_SEGS_EQ)
> + if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_SEGS_EQ,
> + !(srh->segments_left == srhinfo->segs_left)))
> + return false;
> +
> + if (srhinfo->mt_flags & IP6T_SRH_SEGS_GT)
> + if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_SEGS_GT,
> + !(srh->segments_left > srhinfo->segs_left)))
> + return false;
> +
> + if (srhinfo->mt_flags & IP6T_SRH_SEGS_LT)
> + if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_SEGS_LT,
> + !(srh->segments_left < srhinfo->segs_left)))
> + return false;
> +
> + /**
> + * Last Entry matching
> + * Last_Entry field was introduced in revision 6 of the SRH draft.
> + * It was called First_Segment in the previous revision
> + */
> + if (srhinfo->mt_flags & IP6T_SRH_LAST_EQ)
> + if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_LAST_EQ,
> + !(srh->first_segment == srhinfo->last_entry)))
> + return false;
> +
> + if (srhinfo->mt_flags & IP6T_SRH_LAST_GT)
> + if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_LAST_GT,
> + !(srh->first_segment > srhinfo->last_entry)))
> + return false;
> +
> + if (srhinfo->mt_flags & IP6T_SRH_LAST_LT)
> + if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_LAST_LT,
> + !(srh->first_segment < srhinfo->last_entry)))
> + return false;
> +
> + /**
> + * Tag matchig
> + * Tag field was introduced in revision 6 of the SRH draft.
> + */
> + if (srhinfo->mt_flags & IP6T_SRH_TAG)
> + if (NF_SRH_INVF(srhinfo, IP6T_SRH_INV_TAG,
> + !(srh->tag == srhinfo->tag)))
> + return false;
> + return true;
> +}
> +
> +static int srh_mt6_check(const struct xt_mtchk_param *par)
> +{
> + const struct ip6t_srh *srhinfo = par->matchinfo;
> +
> + if (srhinfo->mt_flags & ~IP6T_SRH_MASK) {
> + pr_debug("unknown srh match flags %X\n", srhinfo->mt_flags);
Better call pr_err() here. I know we don't do this in other
extensions, but we should not use pr_debug() for this. ip6tables
explicit refers to 'dmesg' when -EINVAL is returned.
Thanks.
^ permalink raw reply
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Dan Williams @ 2018-01-06 23:31 UTC (permalink / raw)
To: Linus Torvalds
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: <CA+55aFzJgyk7peXyuNO8wgm0ZfSY2K-52XA9z=R6JGKcD6hQYw@mail.gmail.com>
On Fri, Jan 5, 2018 at 7:09 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Jan 5, 2018 at 6:52 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> The fact is, we have to stop speculating when access_ok() does *not*
>> fail - because that's when we'll actually do the access. And it's that
>> access that needs to be non-speculative.
>
> I also suspect we should probably do this entirely differently.
>
> Maybe the whole lfence can be part of uaccess_begin() instead (ie
> currently 'stac()'). That would fit the existing structure better, I
> think. And it would avoid any confusion about the whole "when to stop
> speculation".
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?
^ permalink raw reply
* [PATCH] net: ipv4: Remove MTU check in IP_HDRINCL send flow
From: Yonatan Goldschmidt @ 2018-01-06 23:26 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
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>
---
net/ipv4/raw.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 125c1eab3eaa..63167abff05b 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -359,11 +359,6 @@ static int raw_send_hdrinc(struct sock *sk, struct
flowi*,
struct rtable *rt = *rtp;
int hlen, tlen;
- if (length > rt->dst.dev->mtu) {
- ip_local_error(sk, EMSGSIZE, fl4->daddr,
inet->inet_dport,
- rt->dst.dev->mtu);
- return -EMSGSIZE;
- }
if (length < sizeof(struct iphdr))
return -EINVAL;
^ permalink raw reply related
* Re: [bpf-net PATCH v2] bpf: sockmap missing NULL psock check
From: Daniel Borkmann @ 2018-01-06 23:08 UTC (permalink / raw)
To: John Fastabend, borkmann, alexei.starovoitov; +Cc: netdev
In-Reply-To: <20180105040209.11195.85577.stgit@john-Precision-Tower-5810>
On 01/05/2018 05:02 AM, John Fastabend wrote:
> Add psock NULL check to handle a racing sock event that can get the
> sk_callback_lock before this case but after xchg happens causing the
> refcnt to hit zero and sock user data (psock) to be null and queued
> for garbage collection.
>
> Also add a comment in the code because this is a bit subtle and
> not obvious in my opinion.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Applied to bpf, thanks John!
^ permalink raw reply
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Alan Cox @ 2018-01-06 23:05 UTC (permalink / raw)
To: Alexei Starovoitov
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: <20180106211729.cp5oet3at3hyce4o@ast-mbp>
> 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.
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
3. A lot of people outside of the professional security space are
very new to the concept of security economics and risk management as
opposed to seeing the fake binary nice green tick that says their
computers are secure that they can pass to their senior management or
show to audit.
> So how about we do array_access() macro similar to above by default
> with extra CONFIG_ to convert it to lfence ?
We don't have to decide today. Intel's current position is 'lfence'. Over
time we may see vendors provide more sequences. We will see vendors add
new instruction hints and the like (See the ARM whitepaper for example)
and the array access code will change and change again for the better.
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.
> Why default to AND approach instead of lfence ?
> Because the kernel should still be usable. If security
> sacrifices performance so much such security will be turned off.
> Ex: kpti suppose to add 5-30%. If it means 10% on production workload
> and the datacenter capacity cannot grow 10% overnight, kpti will be off.
The differences involved on the "lfence" versus "and" versus before are
not likely to be anywhere in that order of magnitude. As I said I want to
take a hard look at the IPv4/6 ones but most of them are not in places
where you'd expect a lot of data to be in flight in a perf critical path.
Alan
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/2] bpf: implement syscall command BPF_MAP_GET_NEXT_KEY for stacktrace map
From: Daniel Borkmann @ 2018-01-06 23:03 UTC (permalink / raw)
To: Yonghong Song, ast, netdev; +Cc: kernel-team
In-Reply-To: <20180104215504.2013475-1-yhs@fb.com>
On 01/04/2018 10:55 PM, Yonghong Song wrote:
> The patch set implements bpf syscall command BPF_MAP_GET_NEXT_KEY
> for stacktrace map. Patch #1 is the core implementation
> and Patch #2 implements a bpf test at tools/testing/selftests/bpf
> directory. Please see individual patch comments for details.
>
> Changelog:
> v1 -> v2:
> - For invalid key (key pointer is non-NULL), sets next_key to be the first valid key.
Applied to bpf-next, thanks Yonghong!
^ permalink raw reply
* Re: [PATCH] netfilter: fix int overflow in xt_alloc_table_info()
From: Pablo Neira Ayuso @ 2018-01-06 22:52 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: kadlec, fw, davem, netfilter-devel, coreteam, netdev,
linux-kernel
In-Reply-To: <20171228084854.247843-1-dvyukov@google.com>
On Thu, Dec 28, 2017 at 09:48:54AM +0100, Dmitry Vyukov wrote:
> syzkaller triggered OOM kills by passing ipt_replace.size = -1
> to IPT_SO_SET_REPLACE. The root cause is that SMP_ALIGN() in
> xt_alloc_table_info() causes int overflow and the size check passes
> when it should not. SMP_ALIGN() is no longer needed leftover.
>
> Remove SMP_ALIGN() call in xt_alloc_table_info().
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] atm/clip: Use seq_puts() in svc_addr()
From: Stefano Brivio @ 2018-01-06 22:25 UTC (permalink / raw)
To: SF Markus Elfring
Cc: netdev, Bhumika Goyal, David S. Miller, David Windsor,
Elena Reshetova, Hans Liljestrand, Johannes Berg, Kees Cook,
Roopa Prabhu, LKML, kernel-janitors
In-Reply-To: <97636808-1d9f-d196-ebce-fbd2505c50e2@users.sourceforge.net>
On Sat, 6 Jan 2018 22:44:08 +0100
SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 6 Jan 2018 22:34:12 +0100
>
> 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? One shouldn't need to dig into Coccinelle patterns to find
out what you mean, and "strings should be quickly put into a sequence"
isn't terribly helpful.
--
Stefano
^ permalink raw reply
* Re: dvb usb issues since kernel 4.9
From: Alan Stern @ 2018-01-06 21:44 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Josef Griebichler, Greg Kroah-Hartman, linux-usb, Eric Dumazet,
Rik van Riel, Paolo Abeni, Hannes Frederic Sowa,
Jesper Dangaard Brouer, linux-kernel, netdev, Jonathan Corbet,
LMML, Peter Zijlstra, David Miller, torvalds
In-Reply-To: <20180106175420.275e24e7@recife.lan>
On Sat, 6 Jan 2018, Mauro Carvalho Chehab wrote:
> Hi Josef,
>
> Em Sat, 6 Jan 2018 16:04:16 +0100
> "Josef Griebichler" <griebichler.josef@gmx.at> escreveu:
>
> > Hi,
> >
> > the causing commit has been identified.
> > After reverting commit https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4cd13c21b207e80ddb1144c576500098f2d5f882
> > its working again.
>
> Just replying to me won't magically fix this. The ones that were involved on
> this patch should also be c/c, plus USB people. Just added them.
>
> > Please have a look into the thread https://forum.libreelec.tv/thread/4235-dvb-issue-since-le-switched-to-kernel-4-9-x/?pageNo=13
> > here are several users aknowledging the revert solves their issues with usb dvb cards.
>
> I read the entire (long) thread there. In order to make easier for the
> others, from what I understand, the problem happens on both x86 and arm,
> although almost all comments there are mentioning tests with raspbian
> Kernel (with uses a different USB host driver than the upstream one).
>
> It happens when watching digital TV DVB-C channels, with usually means
> a sustained bit rate of 11 MBps to 54 MBps.
>
> The reports mention the dvbsky, with uses USB URB bulk transfers.
> On every several minutes (5 to 10 mins), the stream suffer "glitches"
> caused by frame losses.
>
> The part of the thread that contains the bisect is at:
> https://forum.libreelec.tv/thread/4235-dvb-issue-since-le-switched-to-kernel-4-9-x/?postID=75965#post75965
>
> It indirectly mentions another comment on the thread with points
> to:
> https://github.com/raspberrypi/linux/issues/2134
>
> There, it says that this fix part of the issues:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=34f41c0316ed52b0b44542491d89278efdaa70e4
>
> but it affects URB packet losses on a lesser extend.
>
> The main issue is really the logic changes a the core softirq logic.
>
> Using Kernel 4.14.10 on a Raspberry Pi 3 with 4cd13c2 commit reverted
> fixed the issue.
>
> Joseph, is the above right? Anything else to mention? Does the
> same issue affect also on x86 with vanilla Kernel 4.14.10?
>
> -
>
> It seems that the original patch were designed to solve some IRQ issues
> with network cards with causes data losses on high traffic. However,
> it is also causing bad effects on sustained high bandwidth demands
> required by DVB cards, at least on some USB host drivers.
>
> Alan/Greg/Eric/David:
>
> Any ideas about how to fix it without causing regressions to
> network?
It would be good to know what hardware was involved on the x86 system
and to have some timing data. Can we see the output from lsusb and
usbmon, running on a vanilla kernel that gets plenty of video glitches?
Overall, this may be a very difficult problem to solve. The
4cd13c21b207 commit was intended to improve throughput at the cost of
increased latency. But then what do you do when the latency becomes
too high for the video subsystem to handle?
Alan Stern
^ permalink raw reply
* [PATCH] atm/clip: Use seq_puts() in svc_addr()
From: SF Markus Elfring @ 2018-01-06 21:44 UTC (permalink / raw)
To: netdev, Bhumika Goyal, David S. Miller, David Windsor,
Elena Reshetova, Hans Liljestrand, Johannes Berg, Kees Cook,
Roopa Prabhu
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 6 Jan 2018 22:34:12 +0100
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.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
net/atm/clip.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/atm/clip.c b/net/atm/clip.c
index d4f6029d5109..62a852165b19 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -708,11 +708,11 @@ static void svc_addr(struct seq_file *seq, struct sockaddr_atmsvc *addr)
static int e164[] = { 1, 8, 4, 6, 1, 0 };
if (*addr->sas_addr.pub) {
- seq_printf(seq, "%s", addr->sas_addr.pub);
+ seq_puts(seq, addr->sas_addr.pub);
if (*addr->sas_addr.prv)
seq_putc(seq, '+');
} else if (!*addr->sas_addr.prv) {
- seq_printf(seq, "%s", "(none)");
+ seq_puts(seq, "(none)");
return;
}
if (*addr->sas_addr.prv) {
--
2.15.1
^ permalink raw reply related
* [PATCH] sh_eth: fix TXALCR1 offsets
From: Sergei Shtylyov @ 2018-01-06 21:26 UTC (permalink / raw)
To: netdev, linux-renesas-soc; +Cc: linux-sh, Sergei Shtylyov
[-- Attachment #1: sh_eth-fix-TXALCR1-offsets.patch --]
[-- Type: text/plain, Size: 1091 bytes --]
The TXALCR1 offsets are incorrect in the register offset tables, most
probably due to copy&paste error. Luckily, the driver never uses this
register. :-)
Fixes: 4a55530f38e4 ("net: sh_eth: modify the definitions of register")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: net/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
--- net.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ net/drivers/net/ethernet/renesas/sh_eth.c
@@ -147,7 +147,7 @@ static const u16 sh_eth_offset_gigabit[S
[FWNLCR0] = 0x0090,
[FWALCR0] = 0x0094,
[TXNLCR1] = 0x00a0,
- [TXALCR1] = 0x00a0,
+ [TXALCR1] = 0x00a4,
[RXNLCR1] = 0x00a8,
[RXALCR1] = 0x00ac,
[FWNLCR1] = 0x00b0,
@@ -399,7 +399,7 @@ static const u16 sh_eth_offset_fast_sh3_
[FWNLCR0] = 0x0090,
[FWALCR0] = 0x0094,
[TXNLCR1] = 0x00a0,
- [TXALCR1] = 0x00a0,
+ [TXALCR1] = 0x00a4,
[RXNLCR1] = 0x00a8,
[RXALCR1] = 0x00ac,
[FWNLCR1] = 0x00b0,
^ permalink raw reply
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Thomas Gleixner @ 2018-01-06 21:21 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: <20180106211729.cp5oet3at3hyce4o@ast-mbp>
On Sat, 6 Jan 2018, Alexei Starovoitov wrote:
> So how about we do array_access() macro similar to above by default
> with extra CONFIG_ to convert it to lfence ?
> Why default to AND approach instead of lfence ?
> Because the kernel should still be usable. If security
> sacrifices performance so much such security will be turned off.
> Ex: kpti suppose to add 5-30%. If it means 10% on production workload
> and the datacenter capacity cannot grow 10% overnight, kpti will be off.
That's the decision and responsibility of the person who disables it.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Alexei Starovoitov @ 2018-01-06 21:17 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: <20180106202213.23e553fb@alans-desktop>
On Sat, Jan 06, 2018 at 08:22:13PM +0000, Alan Cox wrote:
> > "Value prediction consists of predicting entire 32- and 64-bit register values
> > based on previously-seen values"
>
> For their implementation yes
>
> >
> > > In other words there are at least two problems with Linus proposal
> > >
> > > 1. The ffff/0000 mask has to be generated and that has to involve
> > > speculative flows.
> >
> > to answer above and Thomas's
> > "For one particular architecture and that's not a solution for generic code."
> >
> > The following:
> > #define array_access(base, idx, max) ({ \
> > union { typeof(base[0]) _val; unsigned long _bit; } __u;\
> > unsigned long _i = (idx); \
> > unsigned long _m = (max); \
> > unsigned long _mask = ~(long)(_m - 1 - _i) >> 63; \
> > __u._val = base[_i & _mask]; \
> > __u._bit &= _mask; \
> > __u._val; })
> >
> > is generic and no speculative flows.
>
> In the value speculation case imagine it's been called 1000 times for
> process A which as a limit of say 16 so that file->priv->max is 16, and
> then is run for process B which is different.
>
> A value speculating processor waiting for file->priv->max which has been
> pushed out of cache by an attacker is at liberty to say 'I've no idea
> what max is but hey it was 16 last time so lets plug 16 in and keep going"
>
> So while the change in the mask computation is clever and (subject to
> compiler cleverness) safe against guesses of which path will be taken I
> don't think it's generically safe.
>
> Unfortunately a lot of things we index are of different sizes as seen by
> different tasks, or when passed different other index values so this does
> matter.
>
> > Even if 'mask' in 'index & mask' example is a stall the educated
> > guess will come from the prior value (according to the quoted paper)
>
> Which might be for a different set of variables when the table is say per
> process like file handles, or the value is different each call.
>
> If we have single array of fixed size then I suspect you are right but
> usually we don't.
Thanks. I see your point. Agree on the above.
The variant 1 exploit does 2000 bytes a second using 64-bit address math.
Things like 'fd' are 32-bit, so it's magnitude higher attack
complexity already (without any kernel changes).
If we do above array_access() the exploit complexity increases even more.
More so the attacker would need to train fdt->max_fds on a known
good fdt with millions of files for 100s of iterations only to do
one speculative access on another fdt with small max_fds
(to exploit value speculation from large max_fds)
while keeping cache line for that speculative out-of-bounds access on
small fdt empty and measuring cache load times on another cpu.
I frankly don't see such attack being able to keep cache lines pristine
for that small fdt speculation doing hundreds of non-speculative
accesses on another fdt. Way too many moving pieces.
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.
So how about we do array_access() macro similar to above by default
with extra CONFIG_ to convert it to lfence ?
Why default to AND approach instead of lfence ?
Because the kernel should still be usable. If security
sacrifices performance so much such security will be turned off.
Ex: kpti suppose to add 5-30%. If it means 10% on production workload
and the datacenter capacity cannot grow 10% overnight, kpti will be off.
^ permalink raw reply
* Re: [PATCH bpf-next v5 2/4] libbpf: add error reporting in XDP
From: Daniel Borkmann @ 2018-01-06 21:16 UTC (permalink / raw)
To: Eric Leblond, Toshiaki Makita, Philippe Ombredanne
Cc: Alexei Starovoitov, netdev, linux-kernel, Thomas Graf
In-Reply-To: <20180104082149.13758-2-eric@regit.org>
On 01/04/2018 09:21 AM, Eric Leblond wrote:
> Parse netlink ext attribute to get the error message returned by
> the card. Code is partially take from libnl.
>
> Signed-off-by: Eric Leblond <eric@regit.org>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> ---
> samples/bpf/Makefile | 2 +-
> tools/lib/bpf/Build | 2 +-
> tools/lib/bpf/bpf.c | 10 ++-
> tools/lib/bpf/nlattr.c | 187 +++++++++++++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/nlattr.h | 70 ++++++++++++++++++
> 5 files changed, 268 insertions(+), 3 deletions(-)
> create mode 100644 tools/lib/bpf/nlattr.c
> create mode 100644 tools/lib/bpf/nlattr.h
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 4fb944a7ecf8..c889ebcba9b3 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -44,7 +44,7 @@ hostprogs-y += xdp_monitor
> hostprogs-y += syscall_tp
>
> # Libbpf dependencies
> -LIBBPF := ../../tools/lib/bpf/bpf.o
> +LIBBPF := ../../tools/lib/bpf/bpf.o ../../tools/lib/bpf/nlattr.o
> CGROUP_HELPERS := ../../tools/testing/selftests/bpf/cgroup_helpers.o
>
> test_lru_dist-objs := test_lru_dist.o $(LIBBPF)
> diff --git a/tools/lib/bpf/Build b/tools/lib/bpf/Build
> index d8749756352d..64c679d67109 100644
> --- a/tools/lib/bpf/Build
> +++ b/tools/lib/bpf/Build
> @@ -1 +1 @@
> -libbpf-y := libbpf.o bpf.o
> +libbpf-y := libbpf.o bpf.o nlattr.o
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index e6c61035b64c..10d71b9fdbd0 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -26,6 +26,7 @@
> #include <linux/bpf.h>
> #include "bpf.h"
> #include "libbpf.h"
> +#include "nlattr.h"
> #include <linux/rtnetlink.h>
> #include <sys/socket.h>
> #include <errno.h>
> @@ -440,6 +441,7 @@ int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
> struct nlmsghdr *nh;
> struct nlmsgerr *err;
> socklen_t addrlen;
> + int one;
Hmm, it's not initialized here to 1 ...
> memset(&sa, 0, sizeof(sa));
> sa.nl_family = AF_NETLINK;
> @@ -449,6 +451,11 @@ int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
> return -errno;
> }
>
> + if (setsockopt(sock, SOL_NETLINK, NETLINK_EXT_ACK,
> + &one, sizeof(one)) < 0) {
... so we turn it on by chance here.
> + fprintf(stderr, "Netlink error reporting not supported\n");
> + }
> +
> if (bind(sock, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
> ret = -errno;
> goto cleanup;
> @@ -524,7 +531,8 @@ int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
> err = (struct nlmsgerr *)NLMSG_DATA(nh);
> if (!err->error)
> continue;
> - ret = err->error;
> + ret = -err->error;
This one looks strange. Your prior patch added the 'ret = err->error'
and this one negates it. Which variant is the correct version? From
digging into the kernel code, my take is that 'ret = err->error' was
the correct variant since it already holds the negative error code.
Could you double check?
> + nla_dump_errormsg(nh);
> goto cleanup;
> case NLMSG_DONE:
> break;
Thanks,
Daniel
^ permalink raw reply
* Aw: Re: dvb usb issues since kernel 4.9
From: Josef Griebichler @ 2018-01-06 21:07 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Greg Kroah-Hartman, Alan Stern, linux-usb, Eric Dumazet,
Rik van Riel, Paolo Abeni, Hannes Frederic Sowa,
Jesper Dangaard Brouer, linux-kernel, netdev, Jonathan Corbet,
LMML, Peter Zijlstra, David Miller, torvalds
In-Reply-To: <20180106175420.275e24e7@recife.lan>
Hi,
thanks for adding the people involved!
Yes arm and x86 are affected.
Bisecting was not done by me on a x86_64 machine on mainline kernel and not raspbian kernel (https://forum.libreelec.tv/thread/4235-dvb-issue-since-le-switched-to-kernel-4-9-x/?postID=75965#post75965). In the mentioned post you also find the bisect log.
I'm using a dvb-s/s2 usb tv card (technotrend s2-4600 with firmware dvb-fe-ds3103.fw, components M88DS3103, M88TS2022), so not only dvb-c is affected.
Yes kernel 4.14.10 with revert of the mentioned commit works as before on kernel 4.8 with rpi3.
I hope this is of some help.
Regards,
Josef
Hi Josef,
Em Sat, 6 Jan 2018 16:04:16 +0100
"Josef Griebichler" <griebichler.josef@gmx.at> escreveu:
> Hi,
>
> the causing commit has been identified.
> After reverting commit https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4cd13c21b207e80ddb1144c576500098f2d5f882
> its working again.
Just replying to me won't magically fix this. The ones that were involved on
this patch should also be c/c, plus USB people. Just added them.
> Please have a look into the thread https://forum.libreelec.tv/thread/4235-dvb-issue-since-le-switched-to-kernel-4-9-x/?pageNo=13[https://forum.libreelec.tv/thread/4235-dvb-issue-since-le-switched-to-kernel-4-9-x/?pageNo=13]
> here are several users aknowledging the revert solves their issues with usb dvb cards.
I read the entire (long) thread there. In order to make easier for the
others, from what I understand, the problem happens on both x86 and arm,
although almost all comments there are mentioning tests with raspbian
Kernel (with uses a different USB host driver than the upstream one).
It happens when watching digital TV DVB-C channels, with usually means
a sustained bit rate of 11 MBps to 54 MBps.
The reports mention the dvbsky, with uses USB URB bulk transfers.
On every several minutes (5 to 10 mins), the stream suffer "glitches"
caused by frame losses.
The part of the thread that contains the bisect is at:
https://forum.libreelec.tv/thread/4235-dvb-issue-since-le-switched-to-kernel-4-9-x/?postID=75965#post75965[https://forum.libreelec.tv/thread/4235-dvb-issue-since-le-switched-to-kernel-4-9-x/?postID=75965#post75965]
It indirectly mentions another comment on the thread with points
to:
https://github.com/raspberrypi/linux/issues/2134[https://github.com/raspberrypi/linux/issues/2134]
There, it says that this fix part of the issues:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=34f41c0316ed52b0b44542491d89278efdaa70e4[https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=34f41c0316ed52b0b44542491d89278efdaa70e4]
but it affects URB packet losses on a lesser extend.
The main issue is really the logic changes a the core softirq logic.
Using Kernel 4.14.10 on a Raspberry Pi 3 with 4cd13c2 commit reverted
fixed the issue.
Joseph, is the above right? Anything else to mention? Does the
same issue affect also on x86 with vanilla Kernel 4.14.10?
-
It seems that the original patch were designed to solve some IRQ issues
with network cards with causes data losses on high traffic. However,
it is also causing bad effects on sustained high bandwidth demands
required by DVB cards, at least on some USB host drivers.
Alan/Greg/Eric/David:
Any ideas about how to fix it without causing regressions to
network?
Regards,
Mauro
> Gesendet: Sonntag, 17. Dezember 2017 um 14:27 Uhr
> Von: "Mauro Carvalho Chehab" <mchehab@s-opensource.com>
> An: "Sean Young" <sean@mess.org>
> Cc: "Josef Griebichler" <griebichler.josef@gmx.at>, lcaumont2@gmail.com, gregkh@linuxfoundation.org, linux-media@vger.kernel.org, linux-usb@vger.kernel.org
> Betreff: Re: dvb usb issues since kernel 4.9
> Em Sun, 17 Dec 2017 12:06:37 +0000
> Sean Young <sean@mess.org> escreveu:
>
> > Hi Josef,
>
> Em Sun, 17 Dec 2017 11:19:38 +0100
> "Josef Griebichler" <griebichler.josef@gmx.at> escreveu:
>
> > > Hello Mr. Caumont,
> > >
> > > since switch to kernel 4.9 there are several users which have issues with their usb dvb cards.
> > > Some get artifacts when watching livetv, I'm getting discontinuity errors in tvheadend when recording.
> > > I'm using latest test build of LibreElec with kernel 4.14.6 but the issues are still there.
> > > There's an librelec forum thread for this topic
> > > https://forum.libreelec.tv/thread/4235-dvb-issue-since-le-switched-to-kernel-4-9-x/[https://forum.libreelec.tv/thread/4235-dvb-issue-since-le-switched-to-kernel-4-9-x/]
> > > and also an open kernel bug
> > > https://bugzilla.kernel.org/show_bug.cgi?id=197835[https://bugzilla.kernel.org/show_bug.cgi?id=197835][https://bugzilla.kernel.org/show_bug.cgi?id=197835[https://bugzilla.kernel.org/show_bug.cgi?id=197835]]
> > >
> > > This is my dmesg http://sprunge.us/WRIE[http://sprunge.us/WRIE][http://sprunge.us/WRIE[http://sprunge.us/WRIE]]
> > > and tvh service log http://sprunge.us/bEiE[http://sprunge.us/bEiE][http://sprunge.us/bEiE[http://sprunge.us/bEiE]]
> > >
> > > I saw in kernel changelog that you made an improvement/change for dvb und usb (commit 9a11204d2b26324636ff54f8d28095ed5dd17e91)
> > >
> > > Is there anything that can be done to improve our situation or are we forced to stay with kernel 4.8?
> > >
> > > Thanks for support!
> > >
> > > Josef
> >
> > Between kernel v4.8 and v4.9 there are many changes, and it is unlikely that
> > commit 9a11204d2b26324636ff54f8d28095ed5dd17e91 is responsible for this.
>
> Let me add linux-media@vger.kernel.org and linux-usb@vger.kernel.org ML.
>
> Josef, Please be sure that your e-mailer won't be sending e-mails with
> HTML tags on it, otherwise the ML server will automatically drop.
>
> > What would be really helpful is if you could find out which commit did
> > cause a regression. This can be done by bisecting the kernel. There are
> > various guides to this:
> >
> > https://wiki.ubuntu.com/Kernel/KernelBisection[https://wiki.ubuntu.com/Kernel/KernelBisection][https://wiki.ubuntu.com/Kernel/KernelBisection[https://wiki.ubuntu.com/Kernel/KernelBisection]]
> > or
> > https://wiki.archlinux.org/index.php/Bisecting_bugs[https://wiki.archlinux.org/index.php/Bisecting_bugs][https://wiki.archlinux.org/index.php/Bisecting_bugs[https://wiki.archlinux.org/index.php/Bisecting_bugs]]
> >
> > Once the commit has been identified we can work together to narrow it down
> > to the exact change, and then work together on a fix.
>
> Yeah, we need more data in order to start tracking it. I suspect,
> however, that a simple git bisect may not work in this case, due to the
> USB changes that forbids DMA on stack that was added to Kernel 4.9, if
> the card Josef is using was affected by such change.
>
> Probably, he'll need to disable CONFIG_VMAP_STACK in the middle
> of bisect (e. g. when the patch that implements it is added),
> or to cherry-pick any needed DMA fixup patch on the top of Kernel
> 4.8 before starting bisect.
>
> It is also worth mentioning what's the USB host controller that
> are used, and what's the media driver, as this could be an issue
> there.
>
> That's said, from the bug report, it seems that this is
> happening on RPi3. Could you please test it also on a PC? That
> will help to identify if the bug is at RPi's host driver or
> on media drivers.
>
> With regards to RPi3, there are actually two different drivers
> for it: one used on Raspbian Kernel, and another one upstream.
> They're completely different ones.
>
> What driver are you using?
>
> Thanks,
> Mauro
Thanks,
Mauro
^ permalink raw reply
* Re: [patch net-next v6 06/11] net: sched: use block index as a handle instead of qdisc when block is shared
From: Jiri Pirko @ 2018-01-06 20:43 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, saeedm, matanb, leonro,
idosch, jakub.kicinski, simon.horman, pieter.jansenvanvuuren,
john.hurley, alexander.h.duyck, ogerlitz, john.fastabend, daniel,
dsahern
In-Reply-To: <20180105230929.5645-7-jiri@resnulli.us>
Sat, Jan 06, 2018 at 12:09:24AM CET, jiri@resnulli.us wrote:
>From: Jiri Pirko <jiri@mellanox.com>
>
>As the tcm_ifindex 0 is invalid ifindex, reuse it to indicate that we
>work with block, instead of qdisc. So if tcm_ifindex is 0, tcm_parent is
>used to carry block_index.
>
>If the block is set to be shared between at least 2 qdiscs, it is
>forbidden to use the qdisc handle to add/delete filters. In that case,
>userspace has to pass block_index.
>
>Also, for dump of the filters, in case the block is shared in between at
>least 2 qdiscs, the each filter is dumped with tcm_ifindex 0 and
>tcm_parent set to block_index. That gives the user clear indication,
>that the filter belongs to a shared block and not only to one qdisc
>under which it is dumped.
>
>Suggested-by: David Ahern <dsahern@gmail.com>
>Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>---
[...]
>@@ -886,8 +887,13 @@ static int tcf_fill_node(struct net *net, struct sk_buff *skb,
> tcm->tcm_family = AF_UNSPEC;
> tcm->tcm__pad1 = 0;
> tcm->tcm__pad2 = 0;
>- tcm->tcm_ifindex = qdisc_dev(q)->ifindex;
>- tcm->tcm_parent = parent;
>+ if (q) {
>+ tcm->tcm_ifindex = qdisc_dev(q)->ifindex;
>+ tcm->tcm_parent = parent;
>+ } else {
>+ tcm->tcm_ifindex = 0; /* block index is stored in parent */
>+ tcm->tcm_parent = block->index;
>+ }
Please guys, please look at this reuse (also on clt side). I would like
you to double-check this reuse of existing API for balock_index carrying
purpose. I believe it's UAPI safe. But please, check it out carefully.
^ permalink raw reply
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Willy Tarreau @ 2018-01-06 20:42 UTC (permalink / raw)
To: Alan Cox
Cc: Alexei Starovoitov, 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: <20180106183859.1ad9ae37@alans-desktop>
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. Especially when it cannot be disabled by
config and people stop updating their systems to stay on the last
"known good" version. Fortunately in Linux we often have the choice so
that users rarely have a valid reason for not upgrading!
Willy
^ permalink raw reply
* Re: [patch net-next v6 00/11] net: sched: allow qdiscs to share filter block instances
From: Jiri Pirko @ 2018-01-06 20:38 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: David Ahern, netdev, davem, xiyou.wangcong, mlxsw, andrew,
vivien.didelot, f.fainelli, michael.chan, ganeshgr, saeedm,
matanb, leonro, idosch, jakub.kicinski, simon.horman,
pieter.jansenvanvuuren, john.hurley, alexander.h.duyck, ogerlitz,
john.fastabend, daniel
In-Reply-To: <571e2252-61e4-11b7-edc1-f82cd7b69cc0@mojatatu.com>
Sat, Jan 06, 2018 at 07:16:10PM CET, jhs@mojatatu.com wrote:
>On 18-01-06 12:41 PM, David Ahern wrote:
>> On 1/6/18 1:07 AM, Jiri Pirko wrote:
>> > Sat, Jan 06, 2018 at 04:57:21AM CET, dsahern@gmail.com wrote:
>> > > On 1/5/18 4:09 PM, Jiri Pirko wrote:
>> > > > From: Jiri Pirko <jiri@mellanox.com>
>> > > >
>
>> > >
>> > > $ tc filter show block 22
>> > > $ echo $?
>> > > 0
>> > > $ tc qdisc show | grep block
>> > > qdisc ingress ffff: dev eth2 parent ffff:fff1 block 42
>> >
>> > Yeah, I will try to fix this. The thing is, this is not error by kernel
>> > but by the userspace. Kernel is perfectly ok with invalid device or
>> > block index, it just does not dump anything and I would leave it like
>> > that. I have to somehow check the validity of block_index in userspace.
>> > Not sure how now.
>>
>> Ok. I saw a response about idr_alloc_ext.
>>
>> Here's another one: adding a filter to an unknown block id:
>>
>> $ tc filter add block 66 ingress protocol ip pref 1 flower dst_ip
>
>
>BTW, above syntax looks redundant because of the ingress parent
>specification. It is possible iproute2/tc is just ignoring it right now
I'm sure that is just a typo on DaveA's side.
>and the parent is not being used.
>i.e Once the block is bound to ingress (aka parent ffff:) via:
>qdisc ingress ffff: dev eth2 parent ffff:fff1 block 42
>then
>it doesnt make sense to specify a parent again because
>it should be possible to bind that block to many parent locations
>eg clsact ingress of dev x and clsact egress of dev y.
>
>what am i missing?
>
>cheers,
>jamal
^ permalink raw reply
* Re: [patch net-next v6 00/11] net: sched: allow qdiscs to share filter block instances
From: Jiri Pirko @ 2018-01-06 20:37 UTC (permalink / raw)
To: David Ahern
Cc: netdev, davem, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, saeedm, matanb, leonro,
idosch, jakub.kicinski, simon.horman, pieter.jansenvanvuuren,
john.hurley, alexander.h.duyck, ogerlitz, john.fastabend, daniel
In-Reply-To: <1b3e06a5-b07f-6de0-db1a-8add5ae8f140@gmail.com>
Sat, Jan 06, 2018 at 06:41:18PM CET, dsahern@gmail.com wrote:
>On 1/6/18 1:07 AM, Jiri Pirko wrote:
>> Sat, Jan 06, 2018 at 04:57:21AM CET, dsahern@gmail.com wrote:
>>> On 1/5/18 4:09 PM, Jiri Pirko wrote:
>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>
>>>> Currently the filters added to qdiscs are independent. So for example if you
>>>> have 2 netdevices and you create ingress qdisc on both and you want to add
>>>> identical filter rules both, you need to add them twice. This patchset
>>>> makes this easier and mainly saves resources allowing to share all filters
>>>> within a qdisc - I call it a "filter block". Also this helps to save
>>>> resources when we do offload to hw for example to expensive TCAM.
>>>>
>>>> So back to the example. First, we create 2 qdiscs. Both will share
>>>> block number 22. "22" is just an identification. If we don't pass any
>>>> block number, a new one will be generated by kernel:
>>>>
>>>> $ tc qdisc add dev ens7 ingress block 22
>>>> ^^^^^^^^
>>>> $ tc qdisc add dev ens8 ingress block 22
>>>> ^^^^^^^^
>>>>
>>>> Now if we list the qdiscs, we will see the block index in the output:
>>>>
>>>> $ tc qdisc
>>>> qdisc ingress ffff: dev ens7 parent ffff:fff1 block 22
>>>> qdisc ingress ffff: dev ens8 parent ffff:fff1 block 22
>>>>
>>>>
>>>> To make is more visual, the situation looks like this:
>>>>
>>>> ens7 ingress qdisc ens7 ingress qdisc
>>>> | |
>>>> | |
>>>> +----------> block 22 <----------+
>>>>
>>>> Unlimited number of qdiscs may share the same block.
>>>>
>>>> Now we can add filter using the block index:
>>>>
>>>> $ tc filter add block 22 protocol ip pref 25 flower dst_ip 192.168.0.0/16 action drop
>>>>
>>>>
>>>> Note we cannot use the qdisc for filter manipulations for shared blocks:
>>>>
>>>> $ tc filter add dev ens8 ingress protocol ip pref 1 flower dst_ip 192.168.100.2 action drop
>>>> Error: Cannot work with shared block, please use block index.
>>>>
>>>>
>>>> We will see the same output if we list filters for ingress qdisc of
>>>> ens7 and ens8, also for the block 22:
>>>>
>>>> $ tc filter show block 22
>>>> filter block 22 protocol ip pref 25 flower chain 0
>>>> filter block 22 protocol ip pref 25 flower chain 0 handle 0x1
>>>> ...
>>>>
>>>> $ tc filter show dev ens7 ingress
>>>> filter block 22 protocol ip pref 25 flower chain 0
>>>> filter block 22 protocol ip pref 25 flower chain 0 handle 0x1
>>>> ...
>>>>
>>>> $ tc filter show dev ens8 ingress
>>>> filter block 22 protocol ip pref 25 flower chain 0
>>>> filter block 22 protocol ip pref 25 flower chain 0 handle 0x1
>>>> ...
>>>
>>> I like the API and output shown here, but I am not getting that with the
>>> patches.
>>>
>>> In this example, I am using 42 for the block id:
>>>
>>> $ tc qdisc show dev eth2
>>> qdisc mq 0: root
>>> qdisc pfifo_fast 0: parent :2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1
>>> 1 1 1
>>> qdisc pfifo_fast 0: parent :1 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1
>>> 1 1 1
>>> qdisc ingress ffff: parent ffff:fff1 block 42
>>>
>>> It allows me to add a filter using the device:
>>> $ tc filter add dev eth2 ingress protocol ip pref 1 flower dst_ip
>>> 192.168.101.2 action drop
>>> $ echo $?
>>> 0
>>
>> Yes, because the block is not shared yet. You have it only for one
>> qdisc. As long as you have that, the "filter add dev" api still works.
>> It stops working when you add another qdisc to that block.
>
>Interesting.
>
>Once I add the block to another qdisc I do get an error:
>
>$ tc filter add dev eth2 ingress protocol ip pref 1 flower dst_ip
>192.168.100.2 action drop
>Error: Cannot work with shared block, please use block index.
>
>Can you change that to something like: "This filter block is shared.
>Please use the block index to make changes."
Ok. Sounds reasonable.
>
>
>>
>>
>>>
>>> And it modifies the shared block:
>>> $ tc filter show block 42
>>> filter pref 1 flower chain 0
>>> filter pref 1 flower chain 0 handle 0x1
>>> eth_type ipv4
>>> dst_ip 192.168.100.2
>>> not_in_hw
>>> action order 1: gact action drop
>>> random type none pass val 0
>>> index 2 ref 1 bind 1
>>>
>>> filter pref 1 flower chain 0 handle 0x2
>>> eth_type ipv4
>>> dst_ip 192.168.101.2
>>> not_in_hw
>>> action order 1: gact action drop
>>> random type none pass val 0
>>> index 3 ref 1 bind 1
>>>
>>> filter pref 25 flower chain 0
>>> filter pref 25 flower chain 0 handle 0x1
>>> eth_type ipv4
>>> dst_ip 192.168.0.0/16
>>> not_in_hw
>>> action order 1: gact action drop
>>> random type none pass val 0
>>> index 1 ref 1 bind 1
>>>
>>> Notice the header does not give the 'filter block N protocol' part. I
>>> don't get that using the device either (tc filter show dev eth2 ingress).
>>
>> That is correct. Check the print_filter function in tc/tc_filter.c. It
>> works with "filter_ifindex" and with my patch with "filter_block_index".
>> That means that if the value for the filter dumped actually differs from
>> what you passed on the command line, it prints it.
>>
>> Once you actually share the block with another qdisc, you will see
>> "block N"
>>
>>
>>>
>>> Something else I noticed is that I do not get an error message if I pass
>>> an invalid block id:
>>>
>>> $ tc filter show block 22
>>> $ echo $?
>>> 0
>>> $ tc qdisc show | grep block
>>> qdisc ingress ffff: dev eth2 parent ffff:fff1 block 42
>>
>> Yeah, I will try to fix this. The thing is, this is not error by kernel
>> but by the userspace. Kernel is perfectly ok with invalid device or
>> block index, it just does not dump anything and I would leave it like
>> that. I have to somehow check the validity of block_index in userspace.
>> Not sure how now.
>
>Ok. I saw a response about idr_alloc_ext.
I have idea having new type of message:
rtnl_register(PF_UNSPEC, RTM_GETBLOCK, tc_getblock_doit,
tc_getblock_dumpit, 0);
Also, block creation would send RTM_NEWBLOCK and block deletion would
send RTM_DELBLOCK. Not needed now, but I guess it would be nice to have
notifications.
>
>Here's another one: adding a filter to an unknown block id:
>
>$ tc filter add block 66 ingress protocol ip pref 1 flower dst_ip
>192.168.100.2 action drop
>RTNETLINK answers: Invalid argument
>We have an error talking to the kernel
>
>
>Can you add a proper extack message for that case?
Yeah, I will. There is couple of other places in the neibour code that
need extact (like qdisq lookup fail). But those I will address in a
separate patchset.
>
>Thanks,
^ permalink raw reply
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Alan Cox @ 2018-01-06 20:22 UTC (permalink / raw)
To: Alexei Starovoitov
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: <20180106200912.zhzdt4qmfrojeeqe@ast-mbp>
> "Value prediction consists of predicting entire 32- and 64-bit register values
> based on previously-seen values"
For their implementation yes
>
> > In other words there are at least two problems with Linus proposal
> >
> > 1. The ffff/0000 mask has to be generated and that has to involve
> > speculative flows.
>
> to answer above and Thomas's
> "For one particular architecture and that's not a solution for generic code."
>
> The following:
> #define array_access(base, idx, max) ({ \
> union { typeof(base[0]) _val; unsigned long _bit; } __u;\
> unsigned long _i = (idx); \
> unsigned long _m = (max); \
> unsigned long _mask = ~(long)(_m - 1 - _i) >> 63; \
> __u._val = base[_i & _mask]; \
> __u._bit &= _mask; \
> __u._val; })
>
> is generic and no speculative flows.
In the value speculation case imagine it's been called 1000 times for
process A which as a limit of say 16 so that file->priv->max is 16, and
then is run for process B which is different.
A value speculating processor waiting for file->priv->max which has been
pushed out of cache by an attacker is at liberty to say 'I've no idea
what max is but hey it was 16 last time so lets plug 16 in and keep going"
So while the change in the mask computation is clever and (subject to
compiler cleverness) safe against guesses of which path will be taken I
don't think it's generically safe.
Unfortunately a lot of things we index are of different sizes as seen by
different tasks, or when passed different other index values so this does
matter.
> Even if 'mask' in 'index & mask' example is a stall the educated
> guess will come from the prior value (according to the quoted paper)
Which might be for a different set of variables when the table is say per
process like file handles, or the value is different each call.
If we have single array of fixed size then I suspect you are right but
usually we don't.
Alan
^ permalink raw reply
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Alexei Starovoitov @ 2018-01-06 20:09 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: <20180106195551.3207f75d@alans-desktop>
On Sat, Jan 06, 2018 at 07:55:51PM +0000, Alan Cox wrote:
> > cpus execute what they see. speculative execution does the same
> > except results are not committed to visible registers and stay
> > in renanmed/shadow set. There is no 'undo' of the speculative execution.
> > The whole issue is that cache and branch predictor don't have
> > a shadow unlike registers.
>
> Can I suggest you read something like "Exploitig Value Locaity to Exceed
> The Dataflow Limit" by Lipasti and Shen 1996.
thanks for the pointer.
A quote from above paper:
"Value prediction consists of predicting entire 32- and 64-bit register values
based on previously-seen values"
> In other words there are at least two problems with Linus proposal
>
> 1. The ffff/0000 mask has to be generated and that has to involve
> speculative flows.
to answer above and Thomas's
"For one particular architecture and that's not a solution for generic code."
The following:
#define array_access(base, idx, max) ({ \
union { typeof(base[0]) _val; unsigned long _bit; } __u;\
unsigned long _i = (idx); \
unsigned long _m = (max); \
unsigned long _mask = ~(long)(_m - 1 - _i) >> 63; \
__u._val = base[_i & _mask]; \
__u._bit &= _mask; \
__u._val; })
is generic and no speculative flows.
> 2. There are processors on the planet that may speculate not just what
> instruction to execute but faced with a stall on an input continue by
> using an educated guess at the value that will appear at the input in
> future.
correct. that's why earlier I mentioned that "if 'mask' cannot
be influenced by attacker".
Even if 'mask' in 'index & mask' example is a stall the educated
guess will come from the prior value (according to the quoted paper)
To be honest I haven't read that particular paper in the past, but
abstracts fits my understanding and this array_access() proposal.
Thanks for the pointer. Will read it fully to make sure
I didn't miss anything.
^ permalink raw reply
* Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
From: Dan Williams @ 2018-01-06 20:07 UTC (permalink / raw)
To: Linux Kernel Mailing List
Cc: Mark Rutland, Peter Zijlstra, Alan Cox, Srinivas Pandruvada,
Will Deacon, Solomon Peachy, H. Peter Anvin, Christian Lamparter,
Elena Reshetova, linux-arch, Andi Kleen, James E.J. Bottomley,
linux-scsi, Jonathan Corbet, X86 ML, Ingo Molnar,
Alexey Kuznetsov, Zhang Rui, Linux-media@vger.kernel.org,
Arnd Bergmann <arn
In-Reply-To: <CAPcyv4gQbo+Bvf89QVL=mJrRy+id=sj3hiNePS=o_aAZv6hu0w@mail.gmail.com>
On Sat, Jan 6, 2018 at 11:37 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Fri, Jan 5, 2018 at 5:09 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>> Quoting Mark's original RFC:
>>
>> "Recently, Google Project Zero discovered several classes of attack
>> against speculative execution. One of these, known as variant-1, allows
>> explicit bounds checks to be bypassed under speculation, providing an
>> arbitrary read gadget. Further details can be found on the GPZ blog [1]
>> and the Documentation patch in this series."
>>
>> This series incorporates Mark Rutland's latest api and adds the x86
>> specific implementation of nospec_barrier. The
>> nospec_{array_ptr,ptr,barrier} helpers are then combined with a kernel
>> wide analysis performed by Elena Reshetova to address static analysis
>> reports where speculative execution on a userspace controlled value
>> could bypass a bounds check. The patches address a precondition for the
>> attack discussed in the Spectre paper [2].
>>
>> A consideration worth noting for reviewing these patches is to weigh the
>> dramatic cost of being wrong about whether a given report is exploitable
>> vs the overhead nospec_{array_ptr,ptr} may introduce. In other words,
>> lets make the bar for applying these patches be "can you prove that the
>> bounds check bypass is *not* exploitable". Consider that the Spectre
>> paper reports one example of a speculation window being ~180 cycles.
>>
>> Note that there is also a proposal from Linus, array_access [3], that
>> attempts to quash speculative execution past a bounds check without
>> introducing an lfence instruction. That may be a future optimization
>> possibility that is compatible with this api, but it would appear to
>> need guarantees from the compiler that it is not clear the kernel can
>> rely on at this point. It is also not clear that it would be a
>> significant performance win vs lfence.
>>
>> These patches also will also be available via the 'nospec' git branch
>> here:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/djbw/linux nospec
>
> It appears that git.kernel.org has not mirrored out the new branch. In
> the meantime here's an alternative location:
>
> https://github.com/djbw/linux.git nospec
>
> If there are updates to these patches they will appear in nospec-v2,
> nospec-v3, etc... branches.
For completeness I appended the bpf fix [1] to the git branch.
https://lwn.net/Articles/743288/
^ 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