* Re: virtio_net: ethtool supported link modes
From: Michael S. Tsirkin @ 2017-09-01 17:45 UTC (permalink / raw)
To: Radu Rendec; +Cc: virtualization, netdev, linux-kernel, Jason Wang, virtio-dev
In-Reply-To: <1504282793.12952.17.camel@arista.com>
On Fri, Sep 01, 2017 at 05:19:53PM +0100, Radu Rendec wrote:
> On Fri, 2017-09-01 at 18:43 +0300, Michael S. Tsirkin wrote:
> > On Thu, Aug 31, 2017 at 06:04:04PM +0100, Radu Rendec wrote:
> > > Looking at the code in virtnet_set_link_ksettings, it seems the speed
> > > and duplex can be set to any valid value. The driver will "remember"
> > > them and report them back in virtnet_get_link_ksettings.
> > >
> > > However, the supported link modes (link_modes.supported in struct
> > > ethtool_link_ksettings) is always 0, indicating that no speed/duplex
> > > setting is supported.
> > >
> > > Does it make more sense to set (at least a few of) the supported link
> > > modes, such as 10baseT_Half ... 10000baseT_Full?
> > >
> > > I would expect to see consistency between what is reported in
> > > link_modes.supported and what can actually be set. Could you please
> > > share your opinion on this?
> >
> > I would like to know more about why this is desirable.
> >
> > We used not to support the modes at all, but it turned out
> > some tools are confused by this: e.g. people would try to
> > bond virtio with a hardware device, tools would see
> > a mismatch in speed and features between bonded devices
> > and get confused.
> >
> > See
> >
> > commit 16032be56c1f66770da15cb94f0eb366c37aff6e
> > Author: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> > Date: Wed Feb 3 04:04:37 2016 +0100
> >
> > virtio_net: add ethtool support for set and get of settings
> >
> >
> > as well as the discussion around it
> > https://www.spinics.net/lists/netdev/msg362111.html
>
> Thanks for pointing these out. It is much more clear now why modes
> support is implemented the way it is and what the expectations are.
>
> > If you think we need to add more hacks like this, a stronger
> > motivation than "to see consistency" would be needed.
>
> The use case behind my original question is very simple:
> * Net device is queried via ethtool for supported modes.
> * Supported modes are presented to user.
> * User can configure any of the supported modes.
Since this has no effect on virtio, isn't presenting
"no supported modes" to user the right thing to do?
> This is done transparently to the net device type (driver), so it
> actually makes sense for physical NICs.
>
> This alone of course is not a good enough motivation to modify the
> driver. And it can be easily addressed in user-space at the application
> level by testing for the driver.
I think you might want to special-case no supported modes.
Special-casing virtio is probably best avoided.
> I was merely trying to avoid driver-specific workarounds (i.e. keep the
> application driver agnostic)
I think that's the right approach. So if driver does not present
any supported modes this probably means it is not necessary
to display or program any.
> and wondered if "advertising" supported
> modes through ethtool made any sense and/or would be a desirable change
> from the driver perspective. I believe I have my answers now.
>
> Thanks,
> Radu
^ permalink raw reply
* Re: tip -ENOBOOT - bisected to locking/refcounts, x86/asm: Implement fast refcount overflow protection
From: Mike Galbraith @ 2017-09-01 17:52 UTC (permalink / raw)
To: Kees Cook
Cc: David S. Miller, Peter Zijlstra, LKML, Ingo Molnar,
Reshetova, Elena, Network Development
In-Reply-To: <CAGXu5jJ+zq=4Un2JpwWk==sThEYeyqYuvesN6=H-1XeLxW_C0Q@mail.gmail.com>
On Fri, 2017-09-01 at 10:12 -0700, Kees Cook wrote:
> On Fri, Sep 1, 2017 at 6:09 AM, Mike Galbraith <efault@gmx.de> wrote:
> > On Fri, 2017-09-01 at 08:57 +0200, Mike Galbraith wrote:
> >> On Thu, 2017-08-31 at 11:45 -0700, Kees Cook wrote:
> >> > On Thu, Aug 31, 2017 at 10:19 AM, Mike Galbraith <efault@gmx.de> wrote:
> >> > > On Thu, 2017-08-31 at 10:00 -0700, Kees Cook wrote:
> >> > >>
> >> > >> Oh! So it's gcc-version sensitive? That's alarming. Is this mapping correct:
> >> > >>
> >> > >> 4.8.5: WARN, eventual kernel hang
> >> > >> 6.3.1, 7.0.1: WARN, but continues working
> >> > >
> >> > > Yeah, that's correct. I find that troubling, simply because this gcc
> >> > > version has been through one hell of a lot of kernels with me. Yeah, I
> >> > > know, that doesn't exempt it from having bugs, but color me suspicious.
> >> >
> >> > I still can't hit this with a 4.8.5 build. :(
> >> >
> >> > With _RATELIMIT removed, this should, in theory, report whatever goes
> >> > negative first...
> >>
> >> I applied the other patch you posted, and built with gcc-6.3.1 to
> >> remove the gcc-4.8.5 aspect. Look below the resulting splat.
> >
> > Grr, that one has a in6_dev_getx() line missing for the first
> > increment, where things go pear shaped.
> >
> > With that added, looking at counter both before, and after incl, with a
> > trace_printk() in the exception handler showing it doing its saturate
> > thing, irqs disabled across the whole damn refcount_inc(), and even
> > booting box nr_cpus=1 for extra credit...
> >
> > HTH can that first refcount_inc() get there?
> >
> > # tracer: nop
> > #
> > # _-----=> irqs-off
> > # / _----=> need-resched
> > # | / _---=> hardirq/softirq
> > # || / _--=> preempt-depth
> > # ||| / delay
> > # TASK-PID CPU# |||| TIMESTAMP FUNCTION
> > # | | | |||| | |
> > systemd-1 [000] d..1 1.937284: in6_dev_getx: PRE refs.counter:3
> > systemd-1 [000] d..1 1.937295: ex_handler_refcount: *(int *)regs->cx = -1073741824
> > systemd-1 [000] d..1 1.937296: in6_dev_getx: POST refs.counter:-1073741824
>
> O_o
>
> Can you paste the disassembly of in6_dev_getx? I can't understand how
> we're landing in the exception handler.
I was hoping you'd say that.
0xffffffff816b2f72 <+0>: push %rbp
0xffffffff816b2f73 <+1>: mov %rsp,%rbp
0xffffffff816b2f76 <+4>: push %r12
0xffffffff816b2f78 <+6>: push %rbx
0xffffffff816b2f79 <+7>: incl %gs:0x7e95a2d0(%rip) # 0xd250 <__preempt_count>
0xffffffff816b2f80 <+14>: mov 0x308(%rdi),%rbx
0xffffffff816b2f87 <+21>: test %rbx,%rbx
0xffffffff816b2f8a <+24>: je 0xffffffff816b2feb <in6_dev_getx+121>
0xffffffff816b2f8c <+26>: callq *0xffffffff81c35a00
0xffffffff816b2f93 <+33>: mov %rax,%r12
0xffffffff816b2f96 <+36>: callq *0xffffffff81c35a10
0xffffffff816b2f9d <+43>: mov 0x769ad4(%rip),%rsi # 0xffffffff81e1ca78 <trace_printk_fmt.21733>
0xffffffff816b2fa4 <+50>: mov 0xf0(%rbx),%edx
0xffffffff816b2faa <+56>: mov $0xffffffff816b2f8c,%rdi
0xffffffff816b2fb1 <+63>: callq 0xffffffff81171fc0 <__trace_bprintk>
0xffffffff816b2fb6 <+68>: lock incl 0xf0(%rbx)
0xffffffff816b2fbd <+75>: js 0xffffffff816b2fbf <in6_dev_getx+77>
0xffffffff816b2fbf <+77>: lea 0xf0(%rbx),%rcx
0xffffffff816b2fc6 <+84>: (bad)
0xffffffff816b2fc8 <+86>: mov 0x769a99(%rip),%rsi # 0xffffffff81e1ca68 <trace_printk_fmt.21744>
0xffffffff816b2fcf <+93>: mov 0xf0(%rbx),%edx
0xffffffff816b2fd5 <+99>: mov $0xffffffff816b2f8c,%rdi
0xffffffff816b2fdc <+106>: callq 0xffffffff81171fc0 <__trace_bprintk>
0xffffffff816b2fe1 <+111>: mov %r12,%rdi
0xffffffff816b2fe4 <+114>: callq *0xffffffff81c35a08
0xffffffff816b2feb <+121>: decl %gs:0x7e95a25e(%rip) # 0xd250 <__preempt_count>
0xffffffff816b2ff2 <+128>: mov %rbx,%rax
0xffffffff816b2ff5 <+131>: pop %rbx
0xffffffff816b2ff6 <+132>: pop %r12
0xffffffff816b2ff8 <+134>: pop %rbp
0xffffffff816b2ff9 <+135>: retq
I don't get the section business at all, +75 looks to me like we're
gonna trap no matter what.. as we appear to be doing.
> > --- a/arch/x86/include/asm/refcount.h
> > +++ b/arch/x86/include/asm/refcount.h
> > @@ -55,6 +55,20 @@ static __always_inline void refcount_inc
> > : : "cc", "cx");
> > }
> >
> > +static __always_inline void refcount_inc_x(refcount_t *r)
> > +{
> > + unsigned long flags;
> > +
> > + local_irq_save(flags);
> > + trace_printk("PRE refs.counter:%d\n", r->refs.counter);
> > + asm volatile(LOCK_PREFIX "incl %0\n\t"
> > + REFCOUNT_CHECK_LT_ZERO
> > + : [counter] "+m" (r->refs.counter)
> > + : : "cc", "cx");
>
> Does this need an explicit "memory" added to the clobbers line here?
> This isn't present in the atomic_inc() implementation, but maybe
> something confuses gcc in this case into ignoring the "+m" marking?
I thought about adding that (hail mary), but resisted.
> > + trace_printk("POST refs.counter:%d\n", r->refs.counter);
> > + local_irq_restore(flags);
> > +}
> > +
> > static __always_inline void refcount_dec(refcount_t *r)
> > {
> > asm volatile(LOCK_PREFIX "decl %0\n\t"
> > --- a/arch/x86/mm/extable.c
> > +++ b/arch/x86/mm/extable.c
> > @@ -45,6 +45,7 @@ bool ex_handler_refcount(const struct ex
> > {
> > /* First unconditionally saturate the refcount. */
> > *(int *)regs->cx = INT_MIN / 2;
> > + trace_printk("*(int *)regs->cx = %d\n", *(int *)regs->cx);
>
> Just for fun, can you print out *(int *)regs->cx before the assignment too?
Sure, tomorrow.
-Mike
^ permalink raw reply
* Re: [RFC net-next 0/8] net: dsa: Multi-queue awareness
From: Andrew Lunn @ 2017-09-01 17:55 UTC (permalink / raw)
To: Florian Fainelli; +Cc: jiri, jhs, netdev, davem, xiyou.wangcong, vivien.didelot
In-Reply-To: <a0b9ab3a-45d8-3371-901b-2249a26b9192@gmail.com>
Hi Florian
> >> tc bind dev sw0p0 queue 0 dev eth0 queue 16
It this the eth0 i don't like here. Why not in the implementation just
use something like netdev_master_upper_dev_get('sw0p0')? Or does
tc bind dev sw0p0 queue 0 dev lo queue 16
make sense?
Andrew
^ permalink raw reply
* Re: [PATCH 13/31] timer: Remove meaningless .data/.function assignments
From: Krzysztof Halasa @ 2017-09-01 17:59 UTC (permalink / raw)
To: Kees Cook
Cc: Thomas Gleixner, Aditya Shankar, Ganesh Krishna,
Greg Kroah-Hartman, Jens Axboe, netdev, linux-wireless, devel,
linux-kernel
In-Reply-To: <1504222183-61202-14-git-send-email-keescook@chromium.org>
Kees Cook <keescook@chromium.org> writes:
> Several timer users needlessly reset their .function/.data fields during
> their timer callback, but nothing else changes them. Some users do not
> use their .data field at all. Each instance is removed here.
For *wan/hdlc*
Acked-by: Krzysztof Halasa <khc@pm.waw.pl>
> --- a/drivers/net/wan/hdlc_cisco.c
> +++ b/drivers/net/wan/hdlc_cisco.c
> @@ -276,8 +276,6 @@ static void cisco_timer(unsigned long arg)
> spin_unlock(&st->lock);
>
> st->timer.expires = jiffies + st->settings.interval * HZ;
> - st->timer.function = cisco_timer;
> - st->timer.data = arg;
> add_timer(&st->timer);
> }
>
> diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c
> index de42faca076a..7da2424c28a4 100644
> --- a/drivers/net/wan/hdlc_fr.c
> +++ b/drivers/net/wan/hdlc_fr.c
> @@ -644,8 +644,6 @@ static void fr_timer(unsigned long arg)
> state(hdlc)->settings.t391 * HZ;
> }
>
> - state(hdlc)->timer.function = fr_timer;
> - state(hdlc)->timer.data = arg;
> add_timer(&state(hdlc)->timer);
> }
--
Krzysztof Halasa
^ permalink raw reply
* Re: [RFC net-next 0/8] net: dsa: Multi-queue awareness
From: Florian Fainelli @ 2017-09-01 18:27 UTC (permalink / raw)
To: Andrew Lunn, jiri; +Cc: jhs, netdev, davem, xiyou.wangcong, vivien.didelot
In-Reply-To: <20170901175529.GA1249@lunn.ch>
On 09/01/2017 10:55 AM, Andrew Lunn wrote:
> Hi Florian
>
>>>> tc bind dev sw0p0 queue 0 dev eth0 queue 16
>
> It this the eth0 i don't like here. Why not in the implementation just
> use something like netdev_master_upper_dev_get('sw0p0')? Or does
>
> tc bind dev sw0p0 queue 0 dev lo queue 16
>
> make sense?
Last I brought this up with Jiri that we should link DSA network devices
to their master network deviecs with netdev_upper_dev_link() he said
this was not appropriate for DSA slave network devices, but I can't
remember why, I would assume that any stacked device set up would do that.
In any case, we need to establish a mapping so we have to specify at
least the target device's queue number. It is quite similar in premise
to e.g: enslaving a network device to a bridge port:
ip link set dev eth0 master br0
Thanks
--
Florian
^ permalink raw reply
* [net-next PATCH] bpf: sockmap update/simplify memory accounting scheme
From: John Fastabend @ 2017-09-01 18:29 UTC (permalink / raw)
To: davem; +Cc: netdev, daniel, ast
Instead of tracking wmem_queued and sk_mem_charge by incrementing
in the verdict SK_REDIRECT paths and decrementing in the tx work
path use skb_set_owner_w and sock_writeable helpers. This solves
a few issues with the current code. First, in SK_REDIRECT inc on
sk_wmem_queued and sk_mem_charge were being done without the peers
sock lock being held. Under stress this can result in accounting
errors when tx work and/or multiple verdict decisions are working
on the peer psock.
Additionally, this cleans up the code because we can rely on the
default destructor to decrement memory accounting on kfree_skb. Also
this will trigger sk_write_space when space becomes available on
kfree_skb() which wasn't happening before and prevent __sk_free
from being called until all in-flight packets are completed.
Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
kernel/bpf/sockmap.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index db0d99d..f6ffde9 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -111,7 +111,7 @@ static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb)
static void smap_do_verdict(struct smap_psock *psock, struct sk_buff *skb)
{
- struct sock *sock;
+ struct sock *sk;
int rc;
/* Because we use per cpu values to feed input from sock redirect
@@ -123,16 +123,16 @@ static void smap_do_verdict(struct smap_psock *psock, struct sk_buff *skb)
rc = smap_verdict_func(psock, skb);
switch (rc) {
case SK_REDIRECT:
- sock = do_sk_redirect_map();
+ sk = do_sk_redirect_map();
preempt_enable();
- if (likely(sock)) {
- struct smap_psock *peer = smap_psock_sk(sock);
+ if (likely(sk)) {
+ struct smap_psock *peer = smap_psock_sk(sk);
if (likely(peer &&
test_bit(SMAP_TX_RUNNING, &peer->state) &&
- sk_stream_memory_free(peer->sock))) {
- peer->sock->sk_wmem_queued += skb->truesize;
- sk_mem_charge(peer->sock, skb->truesize);
+ !sock_flag(sk, SOCK_DEAD) &&
+ sock_writeable(sk))) {
+ skb_set_owner_w(skb, sk);
skb_queue_tail(&peer->rxqueue, skb);
schedule_work(&peer->tx_work);
break;
@@ -282,16 +282,12 @@ static void smap_tx_work(struct work_struct *w)
/* Hard errors break pipe and stop xmit */
smap_report_sk_error(psock, n ? -n : EPIPE);
clear_bit(SMAP_TX_RUNNING, &psock->state);
- sk_mem_uncharge(psock->sock, skb->truesize);
- psock->sock->sk_wmem_queued -= skb->truesize;
kfree_skb(skb);
goto out;
}
rem -= n;
off += n;
} while (rem);
- sk_mem_uncharge(psock->sock, skb->truesize);
- psock->sock->sk_wmem_queued -= skb->truesize;
kfree_skb(skb);
}
out:
^ permalink raw reply related
* Re: [RFC net-next 0/8] net: dsa: Multi-queue awareness
From: Andrew Lunn @ 2017-09-01 18:50 UTC (permalink / raw)
To: Florian Fainelli; +Cc: jiri, jhs, netdev, davem, xiyou.wangcong, vivien.didelot
In-Reply-To: <abc112cc-7ac5-d65f-d1b2-136cff4de92d@gmail.com>
On Fri, Sep 01, 2017 at 11:27:43AM -0700, Florian Fainelli wrote:
> On 09/01/2017 10:55 AM, Andrew Lunn wrote:
> > Hi Florian
> >
> >>>> tc bind dev sw0p0 queue 0 dev eth0 queue 16
> >
> > It this the eth0 i don't like here. Why not in the implementation just
> > use something like netdev_master_upper_dev_get('sw0p0')? Or does
>
> Last I brought this up with Jiri that we should link DSA network devices
> to their master network deviecs with netdev_upper_dev_link() he said
> this was not appropriate for DSA slave network devices, but I can't
> remember why, I would assume that any stacked device set up would do that.
There is some form a linking going, our device names show that:
9: lan5@eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
link/ether da:87:2a:03:cf:16 brd ff:ff:ff:ff:ff:ff
> In any case, we need to establish a mapping so we have to specify at
> least the target device's queue number. It is quite similar in premise
> to e.g: enslaving a network device to a bridge port:
>
> ip link set dev eth0 master br0
But here br0 is absolutely required, we have to say which bridge the
slave port should be a member of.
But what good is eth0 in
tc bind dev sw0p0 queue 0 dev eth0 queue 16
As i said suggesting, you have to somehow verify that eth0 is the
conduit interface sw0p0 is using. Which makes the parameter pointless.
Determine it from the sw0p0 somehow.
Andrew
^ permalink raw reply
* [PATCH 1/1] bpf: take advantage of stack_depth tracking in powerpc JIT
From: Sandipan Das @ 2017-09-01 18:53 UTC (permalink / raw)
To: mpe; +Cc: naveen.n.rao, daniel, ast, netdev, linuxppc-dev
Take advantage of stack_depth tracking, originally introduced for
x64, in powerpc JIT as well. Round up allocated stack by 16 bytes
to make sure it stays aligned for functions called from JITed bpf
program.
Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
---
arch/powerpc/net/bpf_jit64.h | 7 ++++---
arch/powerpc/net/bpf_jit_comp64.c | 16 ++++++++++------
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/net/bpf_jit64.h b/arch/powerpc/net/bpf_jit64.h
index 62fa7589db2b..8bdef7ed28a8 100644
--- a/arch/powerpc/net/bpf_jit64.h
+++ b/arch/powerpc/net/bpf_jit64.h
@@ -23,7 +23,7 @@
* [ nv gpr save area ] 8*8 |
* [ tail_call_cnt ] 8 |
* [ local_tmp_var ] 8 |
- * fp (r31) --> [ ebpf stack space ] 512 |
+ * fp (r31) --> [ ebpf stack space ] upto 512 |
* [ frame header ] 32/112 |
* sp (r1) ---> [ stack pointer ] --------------
*/
@@ -32,8 +32,8 @@
#define BPF_PPC_STACK_SAVE (8*8)
/* for bpf JIT code internal usage */
#define BPF_PPC_STACK_LOCALS 16
-/* Ensure this is quadword aligned */
-#define BPF_PPC_STACKFRAME (STACK_FRAME_MIN_SIZE + MAX_BPF_STACK + \
+/* stack frame excluding BPF stack, ensure this is quadword aligned */
+#define BPF_PPC_STACKFRAME (STACK_FRAME_MIN_SIZE + \
BPF_PPC_STACK_LOCALS + BPF_PPC_STACK_SAVE)
#ifndef __ASSEMBLY__
@@ -103,6 +103,7 @@ struct codegen_context {
*/
unsigned int seen;
unsigned int idx;
+ unsigned int stack_size;
};
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index 6ba5d253e857..a01362c88f6a 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -69,7 +69,7 @@ static inline bool bpf_has_stack_frame(struct codegen_context *ctx)
static int bpf_jit_stack_local(struct codegen_context *ctx)
{
if (bpf_has_stack_frame(ctx))
- return STACK_FRAME_MIN_SIZE + MAX_BPF_STACK;
+ return STACK_FRAME_MIN_SIZE + ctx->stack_size;
else
return -(BPF_PPC_STACK_SAVE + 16);
}
@@ -82,8 +82,9 @@ static int bpf_jit_stack_tailcallcnt(struct codegen_context *ctx)
static int bpf_jit_stack_offsetof(struct codegen_context *ctx, int reg)
{
if (reg >= BPF_PPC_NVR_MIN && reg < 32)
- return (bpf_has_stack_frame(ctx) ? BPF_PPC_STACKFRAME : 0)
- - (8 * (32 - reg));
+ return (bpf_has_stack_frame(ctx) ?
+ (BPF_PPC_STACKFRAME + ctx->stack_size) : 0)
+ - (8 * (32 - reg));
pr_err("BPF JIT is asking about unknown registers");
BUG();
@@ -134,7 +135,7 @@ static void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
PPC_BPF_STL(0, 1, PPC_LR_STKOFF);
}
- PPC_BPF_STLU(1, 1, -BPF_PPC_STACKFRAME);
+ PPC_BPF_STLU(1, 1, -(BPF_PPC_STACKFRAME + ctx->stack_size));
}
/*
@@ -161,7 +162,7 @@ static void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
/* Setup frame pointer to point to the bpf stack area */
if (bpf_is_seen_register(ctx, BPF_REG_FP))
PPC_ADDI(b2p[BPF_REG_FP], 1,
- STACK_FRAME_MIN_SIZE + MAX_BPF_STACK);
+ STACK_FRAME_MIN_SIZE + ctx->stack_size);
}
static void bpf_jit_emit_common_epilogue(u32 *image, struct codegen_context *ctx)
@@ -183,7 +184,7 @@ static void bpf_jit_emit_common_epilogue(u32 *image, struct codegen_context *ctx
/* Tear down our stack frame */
if (bpf_has_stack_frame(ctx)) {
- PPC_ADDI(1, 1, BPF_PPC_STACKFRAME);
+ PPC_ADDI(1, 1, BPF_PPC_STACKFRAME + ctx->stack_size);
if (ctx->seen & SEEN_FUNC) {
PPC_BPF_LL(0, 1, PPC_LR_STKOFF);
PPC_MTLR(0);
@@ -993,6 +994,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
memset(&cgctx, 0, sizeof(struct codegen_context));
+ /* Make sure that the stack is quadword aligned. */
+ cgctx.stack_size = round_up(fp->aux->stack_depth, 16);
+
/* Scouting faux-generate pass 0 */
if (bpf_jit_build_body(fp, 0, &cgctx, addrs)) {
/* We hit something illegal or unsupported. */
--
2.13.5
^ permalink raw reply related
* Re: tip -ENOBOOT - bisected to locking/refcounts, x86/asm: Implement fast refcount overflow protection
From: Kees Cook @ 2017-09-01 18:58 UTC (permalink / raw)
To: Mike Galbraith
Cc: David S. Miller, Peter Zijlstra, LKML, Ingo Molnar,
Reshetova, Elena, Network Development
In-Reply-To: <1504288357.6035.21.camel@gmx.de>
On Fri, Sep 1, 2017 at 10:52 AM, Mike Galbraith <efault@gmx.de> wrote:
> On Fri, 2017-09-01 at 10:12 -0700, Kees Cook wrote:
>> On Fri, Sep 1, 2017 at 6:09 AM, Mike Galbraith <efault@gmx.de> wrote:
>> > On Fri, 2017-09-01 at 08:57 +0200, Mike Galbraith wrote:
>> >> On Thu, 2017-08-31 at 11:45 -0700, Kees Cook wrote:
>> >> > On Thu, Aug 31, 2017 at 10:19 AM, Mike Galbraith <efault@gmx.de> wrote:
>> >> > > On Thu, 2017-08-31 at 10:00 -0700, Kees Cook wrote:
>> >> > >>
>> >> > >> Oh! So it's gcc-version sensitive? That's alarming. Is this mapping correct:
>> >> > >>
>> >> > >> 4.8.5: WARN, eventual kernel hang
>> >> > >> 6.3.1, 7.0.1: WARN, but continues working
>> >> > >
>> >> > > Yeah, that's correct. I find that troubling, simply because this gcc
>> >> > > version has been through one hell of a lot of kernels with me. Yeah, I
>> >> > > know, that doesn't exempt it from having bugs, but color me suspicious.
>> >> >
>> >> > I still can't hit this with a 4.8.5 build. :(
>> >> >
>> >> > With _RATELIMIT removed, this should, in theory, report whatever goes
>> >> > negative first...
>> >>
>> >> I applied the other patch you posted, and built with gcc-6.3.1 to
>> >> remove the gcc-4.8.5 aspect. Look below the resulting splat.
>> >
>> > Grr, that one has a in6_dev_getx() line missing for the first
>> > increment, where things go pear shaped.
>> >
>> > With that added, looking at counter both before, and after incl, with a
>> > trace_printk() in the exception handler showing it doing its saturate
>> > thing, irqs disabled across the whole damn refcount_inc(), and even
>> > booting box nr_cpus=1 for extra credit...
>> >
>> > HTH can that first refcount_inc() get there?
>> >
>> > # tracer: nop
>> > #
>> > # _-----=> irqs-off
>> > # / _----=> need-resched
>> > # | / _---=> hardirq/softirq
>> > # || / _--=> preempt-depth
>> > # ||| / delay
>> > # TASK-PID CPU# |||| TIMESTAMP FUNCTION
>> > # | | | |||| | |
>> > systemd-1 [000] d..1 1.937284: in6_dev_getx: PRE refs.counter:3
>> > systemd-1 [000] d..1 1.937295: ex_handler_refcount: *(int *)regs->cx = -1073741824
>> > systemd-1 [000] d..1 1.937296: in6_dev_getx: POST refs.counter:-1073741824
>>
>> O_o
>>
>> Can you paste the disassembly of in6_dev_getx? I can't understand how
>> we're landing in the exception handler.
>
> I was hoping you'd say that.
>
> 0xffffffff816b2f72 <+0>: push %rbp
> 0xffffffff816b2f73 <+1>: mov %rsp,%rbp
> 0xffffffff816b2f76 <+4>: push %r12
> 0xffffffff816b2f78 <+6>: push %rbx
> 0xffffffff816b2f79 <+7>: incl %gs:0x7e95a2d0(%rip) # 0xd250 <__preempt_count>
> 0xffffffff816b2f80 <+14>: mov 0x308(%rdi),%rbx
> 0xffffffff816b2f87 <+21>: test %rbx,%rbx
> 0xffffffff816b2f8a <+24>: je 0xffffffff816b2feb <in6_dev_getx+121>
> 0xffffffff816b2f8c <+26>: callq *0xffffffff81c35a00
> 0xffffffff816b2f93 <+33>: mov %rax,%r12
> 0xffffffff816b2f96 <+36>: callq *0xffffffff81c35a10
> 0xffffffff816b2f9d <+43>: mov 0x769ad4(%rip),%rsi # 0xffffffff81e1ca78 <trace_printk_fmt.21733>
> 0xffffffff816b2fa4 <+50>: mov 0xf0(%rbx),%edx
> 0xffffffff816b2faa <+56>: mov $0xffffffff816b2f8c,%rdi
> 0xffffffff816b2fb1 <+63>: callq 0xffffffff81171fc0 <__trace_bprintk>
> 0xffffffff816b2fb6 <+68>: lock incl 0xf0(%rbx)
> 0xffffffff816b2fbd <+75>: js 0xffffffff816b2fbf <in6_dev_getx+77>
> 0xffffffff816b2fbf <+77>: lea 0xf0(%rbx),%rcx
> 0xffffffff816b2fc6 <+84>: (bad)
> 0xffffffff816b2fc8 <+86>: mov 0x769a99(%rip),%rsi # 0xffffffff81e1ca68 <trace_printk_fmt.21744>
> 0xffffffff816b2fcf <+93>: mov 0xf0(%rbx),%edx
> 0xffffffff816b2fd5 <+99>: mov $0xffffffff816b2f8c,%rdi
> 0xffffffff816b2fdc <+106>: callq 0xffffffff81171fc0 <__trace_bprintk>
> 0xffffffff816b2fe1 <+111>: mov %r12,%rdi
> 0xffffffff816b2fe4 <+114>: callq *0xffffffff81c35a08
> 0xffffffff816b2feb <+121>: decl %gs:0x7e95a25e(%rip) # 0xd250 <__preempt_count>
> 0xffffffff816b2ff2 <+128>: mov %rbx,%rax
> 0xffffffff816b2ff5 <+131>: pop %rbx
> 0xffffffff816b2ff6 <+132>: pop %r12
> 0xffffffff816b2ff8 <+134>: pop %rbp
> 0xffffffff816b2ff9 <+135>: retq
>
> I don't get the section business at all, +75 looks to me like we're
> gonna trap no matter what.. as we appear to be doing.
The section stuff is supposed to be a trick to push the error case off
into the .text.unlikely area to avoid needing a jmp over the handler
and with possibly some redundancy removal done by the compiler (though
this appears to be rather limited) if it notices a bunch of error
paths are the same. However, in your disassembly, it's inline (!!) in
the code, as if "pushsection" and "popsection" were entirely ignored.
And when I make my own in6_dev_getx(), I see the same disassembly:
0xffffffff818a757b <+181>: lock incl 0x1e0(%rbx)
0xffffffff818a7582 <+188>: js 0xffffffff818a7584 <in6_dev_getx+190>
0xffffffff818a7584 <+190>: lea 0x1e0(%rbx),%rcx
0xffffffff818a758b <+197>: (bad)
Which is VERY different from how it looks in other places!
e.g. from lkdtm_REFCOUNT_INC_SATURATED:
0xffffffff815657df <+47>: lock incl -0xc(%rbp)
0xffffffff815657e3 <+51>: js 0xffffffff81565cac
...
0xffffffff81565cac: lea -0xc(%rbp),%rcx
0xffffffff81565cb0: (bad)
So, at least I can reproduce this in the build now. I must not be
exercising these paths. FWIW, this is with Ubuntu's 6.3.0 gcc.
I'll try to figure out what's going on here...
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [PATCH 1/1] bpf: take advantage of stack_depth tracking in powerpc JIT
From: Naveen N. Rao @ 2017-09-01 19:03 UTC (permalink / raw)
To: Sandipan Das; +Cc: mpe, daniel, ast, netdev, linuxppc-dev
In-Reply-To: <20170901185301.17978-1-sandipan@linux.vnet.ibm.com>
On 2017/09/02 12:23AM, Sandipan Das wrote:
> Take advantage of stack_depth tracking, originally introduced for
> x64, in powerpc JIT as well. Round up allocated stack by 16 bytes
> to make sure it stays aligned for functions called from JITed bpf
> program.
>
> Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
> ---
LGTM, thanks!
Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Michael,
Seeing as this is powerpc specific, can you please take this through
your tree?
Thanks,
Naveen
> arch/powerpc/net/bpf_jit64.h | 7 ++++---
> arch/powerpc/net/bpf_jit_comp64.c | 16 ++++++++++------
> 2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/net/bpf_jit64.h b/arch/powerpc/net/bpf_jit64.h
> index 62fa7589db2b..8bdef7ed28a8 100644
> --- a/arch/powerpc/net/bpf_jit64.h
> +++ b/arch/powerpc/net/bpf_jit64.h
> @@ -23,7 +23,7 @@
> * [ nv gpr save area ] 8*8 |
> * [ tail_call_cnt ] 8 |
> * [ local_tmp_var ] 8 |
> - * fp (r31) --> [ ebpf stack space ] 512 |
> + * fp (r31) --> [ ebpf stack space ] upto 512 |
> * [ frame header ] 32/112 |
> * sp (r1) ---> [ stack pointer ] --------------
> */
> @@ -32,8 +32,8 @@
> #define BPF_PPC_STACK_SAVE (8*8)
> /* for bpf JIT code internal usage */
> #define BPF_PPC_STACK_LOCALS 16
> -/* Ensure this is quadword aligned */
> -#define BPF_PPC_STACKFRAME (STACK_FRAME_MIN_SIZE + MAX_BPF_STACK + \
> +/* stack frame excluding BPF stack, ensure this is quadword aligned */
> +#define BPF_PPC_STACKFRAME (STACK_FRAME_MIN_SIZE + \
> BPF_PPC_STACK_LOCALS + BPF_PPC_STACK_SAVE)
>
> #ifndef __ASSEMBLY__
> @@ -103,6 +103,7 @@ struct codegen_context {
> */
> unsigned int seen;
> unsigned int idx;
> + unsigned int stack_size;
> };
>
> #endif /* !__ASSEMBLY__ */
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> index 6ba5d253e857..a01362c88f6a 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -69,7 +69,7 @@ static inline bool bpf_has_stack_frame(struct codegen_context *ctx)
> static int bpf_jit_stack_local(struct codegen_context *ctx)
> {
> if (bpf_has_stack_frame(ctx))
> - return STACK_FRAME_MIN_SIZE + MAX_BPF_STACK;
> + return STACK_FRAME_MIN_SIZE + ctx->stack_size;
> else
> return -(BPF_PPC_STACK_SAVE + 16);
> }
> @@ -82,8 +82,9 @@ static int bpf_jit_stack_tailcallcnt(struct codegen_context *ctx)
> static int bpf_jit_stack_offsetof(struct codegen_context *ctx, int reg)
> {
> if (reg >= BPF_PPC_NVR_MIN && reg < 32)
> - return (bpf_has_stack_frame(ctx) ? BPF_PPC_STACKFRAME : 0)
> - - (8 * (32 - reg));
> + return (bpf_has_stack_frame(ctx) ?
> + (BPF_PPC_STACKFRAME + ctx->stack_size) : 0)
> + - (8 * (32 - reg));
>
> pr_err("BPF JIT is asking about unknown registers");
> BUG();
> @@ -134,7 +135,7 @@ static void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
> PPC_BPF_STL(0, 1, PPC_LR_STKOFF);
> }
>
> - PPC_BPF_STLU(1, 1, -BPF_PPC_STACKFRAME);
> + PPC_BPF_STLU(1, 1, -(BPF_PPC_STACKFRAME + ctx->stack_size));
> }
>
> /*
> @@ -161,7 +162,7 @@ static void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
> /* Setup frame pointer to point to the bpf stack area */
> if (bpf_is_seen_register(ctx, BPF_REG_FP))
> PPC_ADDI(b2p[BPF_REG_FP], 1,
> - STACK_FRAME_MIN_SIZE + MAX_BPF_STACK);
> + STACK_FRAME_MIN_SIZE + ctx->stack_size);
> }
>
> static void bpf_jit_emit_common_epilogue(u32 *image, struct codegen_context *ctx)
> @@ -183,7 +184,7 @@ static void bpf_jit_emit_common_epilogue(u32 *image, struct codegen_context *ctx
>
> /* Tear down our stack frame */
> if (bpf_has_stack_frame(ctx)) {
> - PPC_ADDI(1, 1, BPF_PPC_STACKFRAME);
> + PPC_ADDI(1, 1, BPF_PPC_STACKFRAME + ctx->stack_size);
> if (ctx->seen & SEEN_FUNC) {
> PPC_BPF_LL(0, 1, PPC_LR_STKOFF);
> PPC_MTLR(0);
> @@ -993,6 +994,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>
> memset(&cgctx, 0, sizeof(struct codegen_context));
>
> + /* Make sure that the stack is quadword aligned. */
> + cgctx.stack_size = round_up(fp->aux->stack_depth, 16);
> +
> /* Scouting faux-generate pass 0 */
> if (bpf_jit_build_body(fp, 0, &cgctx, addrs)) {
> /* We hit something illegal or unsupported. */
> --
> 2.13.5
>
^ permalink raw reply
* [GIT] Networking
From: David Miller @ 2017-09-01 19:06 UTC (permalink / raw)
To: torvalds; +Cc: akpm, netdev, linux-kernel
1) Fix handling of pinned BPF map nodes in hash of maps, from Daniel
Borkmann.
2) IPSEC ESP error paths leak memory, from Steffen Klassert.
3) We need an RCU grace period before freeing fib6_node objects,
from Wei Wang.
4) Must check skb_put_padto() return value in HSR driver, from
FLorian Fainelli.
5) Fix oops on PHY probe failure in ftgmac100 driver, from Andrew
Jeffery.
6) Fix infinite loop in UDP queue when using SO_PEEK_OFF, from
Eric Dumazet.
7) Use after free when tcf_chain_destroy() called multiple times,
from Jiri Pirko.
8) Fix KSZ DSA tag layer multiple free of SKBS, from Florian
Fainelli.
9) Fix leak of uninitialized memory in sctp_get_sctp_info(),
inet_diag_msg_sctpladdrs_fill() and inet_diag_msg_sctpaddrs_fill().
From Stefano Brivio.
10) L2TP tunnel refcount fixes from Guillaume Nault.
11) Don't leak UDP secpath in udp_set_dev_scratch(), from Yossi
Kauperman.
12) Revert a PHY layer change wrt. handling of PHY_HALTED state
in phy_stop_machine(), it causes regressions for multiple
people. From Florian Fainelli.
13) When packets are sent out of br0 we have to clear the offload_fwdq_mark
value.
14) Several NULL pointer deref fixes in packet schedulers when their
->init() routine fails. From Nikolay Aleksandrov.
15) Aquantium devices cannot checksum offload correctly when the packet
is <= 60 bytes. From Pavel Belous.
16) Fix vnet header access past end of buffer in AF_PACKET, from Benjamin
Poirier.
17) Double free in probe error paths of nfp driver, from Dan Carpenter.
18) QOS capability not checked properly in DCB init paths of mlx5 driver,
from Huy Nguyen.
19) Fix conflicts between firmware load failure and health_care timer
in mlx5, also from Huy Nguyen.
20) Fix dangling page pointer when DMA mapping errors occur in mlx5,
from Eran Ben ELisha.
21) ->ndo_setup_tc() in bnxt_en driver doesn't count rings properly, from
Michael Chan.
22) Missing MSIX vector free in bnxt_en, also from Michael Chan.
23) Refcount leak in xfrm layer when using sk_policy, from Lorenzo
Colitti.
24) Fix copy of uninitialized data in qlge driver, from Arnd Bergmann.
25) bpf_setsockopts() erroneously always returns -EINVAL even on
success. Fix from Yuchung Cheng.
26) tipc_rcv() needs to linearize the SKB before parsing the inner
headers, from Parthasarathy Bhuvaragan.
27) Fix deadlock between link status updates and link removal in netvsc
driver, from Stephen Hemminger.
28) Missed locking of page fragment handling in ESP output, from
Steffen Klassert.
29) Fix refcnt leak in ebpf congestion control code, from Sabrina
Dubroca.
30) sxgbe_probe_config_dt() doesn't check devm_kzalloc()'s return
value, from Christophe Jaillet.
31) Fix missing ipv6 rx_dst_cookie update when rx_dst is updated
during early demux, from Paolo Abeni.
32) Several info leaks in xfrm_user layer, from Mathias Krause.
33) Fix out of bounds read in cxgb4 driver, from Stefano Brivio.
34) Properly propagate obsolete state of route upwards in ipv6
so that upper holders like xfrm can see it. From Xin Long.
Please pull, thanks a lot!
The following changes since commit 6470812e22261d2342ef1597be62e63a0423d691:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc (2017-08-21 14:07:48 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
for you to fetch changes up to e8a732d1bc3ac313e22249c13a153c3fe54aa577:
udp: fix secpath leak (2017-09-01 10:29:34 -0700)
----------------------------------------------------------------
Aleksander Morgado (1):
cdc_ncm: flag the u-blox TOBY-L4 as wwan
Andrew Jeffery (1):
net: ftgmac100: Fix oops in probe on failure to find associated PHY
Antoine Tenart (1):
net: mvpp2: fix the mac address used when using PPv2.2
Arnd Bergmann (1):
qlge: avoid memcpy buffer overflow
Benjamin Poirier (1):
packet: Don't write vnet header beyond end of buffer
Bob Peterson (1):
tipc: Fix tipc_sk_reinit handling of -EAGAIN
Christophe Jaillet (1):
net: sxgbe: check memory allocation failure
Cong Wang (1):
wl1251: add a missing spin_lock_init()
Dan Carpenter (1):
nfp: double free on error in probe
Daniel Borkmann (1):
bpf: fix map value attribute for hash of maps
David S. Miller (16):
Merge branch 'master' of git://git.kernel.org/.../klassert/ipsec
Merge branch 'tipc-topology-server-fixes'
Merge branch 'net-sched-couple-of-chain-fixes'
Merge branch 'dst-tag-ksz-fix'
Merge branch 'nfp-fixes'
Merge branch 'bnxt_en-bug-fixes'
Merge git://git.kernel.org/.../pablo/nf
Merge branch 'tipc-buffer-reassignment-fixes'
Merge branch 'r8169-Be-drop-monitor-friendly'
Merge tag 'wireless-drivers-for-davem-2017-08-25' of git://git.kernel.org/.../kvalo/wireless-drivers
Merge branch 'l2tp-tunnel-refs'
Merge branch 'nfp-flow-dissector-layer'
Merge branch 'master' of git://git.kernel.org/.../klassert/ipsec
Merge branch 'aquantia-fixes'
Merge branch 'net-sched-init-failure-fixes'
Merge tag 'mlx5-fixes-2017-08-30' of git://git.kernel.org/.../saeed/linux
Eran Ben Elisha (1):
net/mlx5e: Fix dangling page pointer on DMA mapping error
Eric Dumazet (5):
udp: on peeking bad csum, drop packets even if not at head
net: dsa: use consume_skb()
virtio_net: be drop monitor friendly
net_sched: fix a refcount_t issue with noop_qdisc
kcm: do not attach PF_KCM sockets to avoid deadlock
Florian Fainelli (13):
net/hsr: Check skb_put_padto() return value
net: phy: Deal with unbound PHY driver in phy_attached_print()
fsl/man: Inherit parent device and of_node
net: core: Specify skb_pad()/skb_put_padto() SKB freeing
net: dsa: skb_put_padto() already frees nskb
net: systemport: Be drop monitor friendly
net: bcmgenet: Be drop monitor friendly
net: systemport: Free DMA coherent descriptors on errors
r8169: Do not increment tx_dropped in TX ring cleaning
r8169: Be drop monitor friendly
net: dsa: Don't dereference dst->cpu_dp->netdev
net: dsa: bcm_sf2: Fix number of CFP entries for BCM7278
Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"
Guillaume Nault (6):
l2tp: initialise session's refcount before making it reachable
l2tp: hold tunnel while looking up sessions in l2tp_netlink
l2tp: hold tunnel while processing genl delete command
l2tp: hold tunnel while handling genl tunnel updates
l2tp: hold tunnel while handling genl TUNNEL_GET commands
l2tp: hold tunnel used while creating sessions with netlink
Huy Nguyen (4):
net/mlx5e: Check for qos capability in dcbnl_initialize
net/mlx5e: Fix DCB_CAP_ATTR_DCBX capability for DCBNL getcap.
net/mlx5: Skip mlx5_unload_one if mlx5_load_one fails
net/mlx5: Remove the flag MLX5_INTERFACE_STATE_SHUTDOWN
Ido Schimmel (2):
mlxsw: spectrum: Forbid linking to devices that have uppers
bridge: switchdev: Clear forward mark when transmitting packet
Igor Russkikh (1):
net:ethernet:aquantia: Fix for multicast filter handling.
Inbar Karmy (1):
net/mlx5e: Don't override user RSS upon set channels
Jakub Kicinski (4):
nfp: don't hold PF lock while enabling SR-IOV
nfp: make sure representors are destroyed before their lower netdev
nfp: avoid buffer leak when representor is missing
nfp: TX time stamp packets before HW doorbell is rung
Jesper Dangaard Brouer (1):
net: missing call of trace_napi_poll in busy_poll_stop
Jiri Pirko (2):
net: sched: fix use after free when tcf_chain_destroy is called multiple times
net: sched: don't do tcf_chain_flush from tcf_chain_destroy
Koichiro Den (1):
xfrm: fix null pointer dereference on state and tmpl sort
Lorenzo Colitti (1):
net: xfrm: don't double-hold dst when sk_policy in use.
Luca Coelho (1):
iwlwifi: pcie: move rx workqueue initialization to iwl_trans_pcie_alloc()
Mathias Krause (4):
xfrm_user: fix info leak in copy_user_offload()
xfrm_user: fix info leak in xfrm_notify_sa()
xfrm_user: fix info leak in build_expire()
xfrm_user: fix info leak in build_aevent()
Maxime Ripard (1):
net: stmmac: sun8i: Remove the compatibles
Michael Chan (3):
bnxt_en: Fix .ndo_setup_tc() to include XDP rings.
bnxt_en: Free MSIX vectors when unregistering the device from bnxt_re.
bnxt_en: Do not setup MAC address in bnxt_hwrm_func_qcaps().
Moshe Shemesh (1):
net/mlx5e: Fix inline header size for small packets
Nikolay Aleksandrov (9):
sch_htb: fix crash on init failure
sch_multiq: fix double free on init failure
sch_hhf: fix null pointer dereference on init failure
sch_hfsc: fix null pointer deref and double free on init failure
sch_cbq: fix null pointer dereferences on init failure
sch_fq_codel: avoid double free on init failure
sch_netem: avoid null pointer deref on init failure
sch_sfq: fix null pointer dereference on init failure
sch_tbf: fix two null pointer dereferences on init failure
Noa Osherovich (1):
net/mlx5: Fix arm SRQ command for ISSI version 0
Nogah Frankel (1):
mlxsw: spectrum_switchdev: Fix mrouter flag update
Pablo Neira Ayuso (1):
netfilter: nft_compat: check extension hook mask only if set
Paolo Abeni (1):
udp6: set rx_dst_cookie on rx_dst updates
Parthasarathy Bhuvaragan (5):
tipc: remove subscription references only for pending timers
tipc: perform skb_linearize() before parsing the inner header
tipc: reassign pointers after skb reallocation / linearization
tipc: context imbalance at node read unlock
tipc: permit bond slave as bearer
Paul Blakey (1):
net/mlx5e: Properly resolve TC offloaded ipv6 vxlan tunnel source address
Pavel Belous (5):
net:ethernet:aquantia: Extra spinlocks removed.
net:ethernet:aquantia: Fix for number of RSS queues.
net:ethernet:aquantia: Workaround for HW checksum bug.
net:ethernet:aquantia: Fix for incorrect speed index.
net:ethernet:aquantia: Show info message if bad firmware version detected.
Pieter Jansen van Vuuren (3):
nfp: fix unchecked flow dissector use
nfp: fix supported key layers calculation
nfp: remove incorrect mask check for vlan matching
Quan Nguyen (1):
drivers: net: xgene: Correct probe sequence handling
Roopa Prabhu (1):
bridge: check for null fdb->dst before notifying switchdev drivers
Sabrina Dubroca (3):
netfilter: ipt_CLUSTERIP: fix use-after-free of proc entry
macsec: add genl family module alias
tcp: fix refcnt leak with ebpf congestion control
Sekhar Nori (1):
net: ti: cpsw-common: dont print error if ti_cm_get_macid() fails
Shahar Klein (1):
net/mlx5: E-Switch, Unload the representors in the correct order
Stefano Brivio (3):
ipv6: accept 64k - 1 packet length in ip6_find_1stfragopt()
sctp: Avoid out-of-bounds reads from address storage
cxgb4: Fix stack out-of-bounds read due to wrong size to t4_record_mbox()
Steffen Klassert (5):
esp: Fix memleaks on error paths.
esp: Fix error handling on layer 2 xmit.
esp: Fix locking on page fragment allocation
esp: Fix skb tailroom calculation
ipv6: Fix may be used uninitialized warning in rt6_check
Stephan Gatzka (1):
net: stmmac: socfgpa: Ensure emac bit set in sys manager for MII/GMII/SGMII.
Taehee Yoo (1):
netfilter: x_tables: Fix use-after-free in ipt_do_table.
Tal Gilboa (1):
net/mlx5e: Fix CQ moderation mode not set properly
Vladis Dronov (1):
xfrm: policy: check policy direction value
Wei Wang (2):
ipv6: add rcu grace period before freeing fib6_node
ipv6: fix sparse warning on rt6i_node
Xin Long (3):
netfilter: check for seqadj ext existence before adding it in nf_nat_setup_info
ipv6: set dst.obsolete when a cached route has expired
ipv6: do not set sk_destruct in IPV6_ADDRFORM sockopt
Ying Xue (1):
tipc: fix a race condition of releasing subscriber object
Yossi Kuperman (1):
udp: fix secpath leak
Yuchung Cheng (1):
bpf: fix bpf_setsockopts return value
andy zhou (1):
netfilter: nf_tables: Fix nft limit burst handling
stephen hemminger (1):
netvsc: fix deadlock betwen link status and removal
drivers/net/dsa/bcm_sf2.c | 4 +++
drivers/net/dsa/bcm_sf2.h | 1 +
drivers/net/dsa/bcm_sf2_cfp.c | 8 ++---
drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 27 ++++++++--------
drivers/net/ethernet/aquantia/atlantic/aq_hw.h | 3 +-
drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 92 ++++++++++++++++++++++++++-----------------------------
drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 1 -
drivers/net/ethernet/aquantia/atlantic/aq_utils.h | 1 -
drivers/net/ethernet/aquantia/atlantic/aq_vec.c | 11 ++-----
drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c | 6 ++++
drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c | 6 ++++
drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c | 10 ++++--
drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.h | 3 +-
drivers/net/ethernet/broadcom/bcmsysport.c | 4 ++-
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 41 +++++++++++++++++--------
drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 2 ++
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 6 ++--
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 6 ++--
drivers/net/ethernet/faraday/ftgmac100.c | 1 -
drivers/net/ethernet/freescale/fman/mac.c | 2 ++
drivers/net/ethernet/marvell/mvpp2.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en.h | 1 +
drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c | 24 +++++++++------
drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 6 ++--
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 1 +
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 8 ++---
drivers/net/ethernet/mellanox/mlx5/core/en_tx.c | 17 ++++++-----
drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/main.c | 6 +---
drivers/net/ethernet/mellanox/mlx5/core/srq.c | 12 ++++----
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 6 ++++
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 15 ++++++---
drivers/net/ethernet/netronome/nfp/flower/match.c | 139 ++++++++++++++++++++++++++++++++++++++++++-----------------------------------------
drivers/net/ethernet/netronome/nfp/flower/offload.c | 60 +++++++++++++++++++++++++-----------
drivers/net/ethernet/netronome/nfp/nfp_main.c | 16 +++++-----
drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 14 ++++-----
drivers/net/ethernet/netronome/nfp/nfp_net_main.c | 26 +++++++++-------
drivers/net/ethernet/qlogic/qlge/qlge_dbg.c | 2 +-
drivers/net/ethernet/realtek/r8169.c | 5 ++-
drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c | 2 ++
drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 5 ++-
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 8 -----
drivers/net/ethernet/ti/cpsw-common.c | 2 +-
drivers/net/hyperv/netvsc_drv.c | 7 ++++-
drivers/net/macsec.c | 1 +
drivers/net/phy/phy.c | 3 --
drivers/net/phy/phy_device.c | 6 ++--
drivers/net/usb/cdc_ncm.c | 7 +++++
drivers/net/virtio_net.c | 2 +-
drivers/net/wireless/intel/iwlwifi/pcie/internal.h | 2 ++
drivers/net/wireless/intel/iwlwifi/pcie/rx.c | 10 +-----
drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 9 ++++++
drivers/net/wireless/ti/wl1251/main.c | 1 +
include/linux/mlx5/driver.h | 4 +--
include/linux/netdevice.h | 2 ++
include/linux/skbuff.h | 41 ++++++++++++++++++++++---
include/net/ip6_fib.h | 32 +++++++++++++++++--
include/net/sch_generic.h | 7 +++++
include/net/tcp.h | 4 +--
include/net/udp.h | 2 +-
kernel/bpf/hashtab.c | 30 ++++++++++--------
net/bridge/br_device.c | 3 ++
net/bridge/br_switchdev.c | 2 +-
net/core/datagram.c | 2 +-
net/core/dev.c | 4 ++-
net/core/filter.c | 8 ++---
net/core/skbuff.c | 13 +++++---
net/dsa/dsa2.c | 2 +-
net/dsa/tag_ksz.c | 12 +++++---
net/dsa/tag_trailer.c | 2 +-
net/hsr/hsr_device.c | 3 +-
net/ipv4/esp4.c | 20 +++++++-----
net/ipv4/esp4_offload.c | 2 +-
net/ipv4/netfilter/arp_tables.c | 10 +++---
net/ipv4/netfilter/ip_tables.c | 9 +++---
net/ipv4/netfilter/ipt_CLUSTERIP.c | 4 ++-
net/ipv4/tcp.c | 2 +-
net/ipv4/tcp_cong.c | 19 +++++++++---
net/ipv4/udp.c | 6 ++--
net/ipv6/addrconf.c | 2 +-
net/ipv6/esp6.c | 16 +++++-----
net/ipv6/esp6_offload.c | 2 +-
net/ipv6/ip6_fib.c | 35 +++++++++++++++------
net/ipv6/ipv6_sockglue.c | 1 -
net/ipv6/output_core.c | 6 ++--
net/ipv6/route.c | 20 +++++++++---
net/ipv6/udp.c | 11 ++++++-
net/kcm/kcmsock.c | 4 +++
net/l2tp/l2tp_core.c | 72 ++++++++++++++-----------------------------
net/l2tp/l2tp_core.h | 13 ++++++++
net/l2tp/l2tp_netlink.c | 66 +++++++++++++++++++++++----------------
net/netfilter/nf_nat_core.c | 2 +-
net/netfilter/nft_compat.c | 4 +--
net/netfilter/nft_limit.c | 25 ++++++++-------
net/packet/af_packet.c | 12 ++++++--
net/sched/cls_api.c | 16 +++++++---
net/sched/sch_api.c | 6 ++--
net/sched/sch_cbq.c | 10 ++++--
net/sched/sch_fq_codel.c | 4 +--
net/sched/sch_generic.c | 2 +-
net/sched/sch_hfsc.c | 10 ++----
net/sched/sch_hhf.c | 3 ++
net/sched/sch_htb.c | 5 +--
net/sched/sch_multiq.c | 7 +----
net/sched/sch_netem.c | 4 +--
net/sched/sch_sfq.c | 6 ++--
net/sched/sch_tbf.c | 5 +--
net/sctp/sctp_diag.c | 7 +++--
net/sctp/socket.c | 3 +-
net/tipc/bearer.c | 26 +++++++---------
net/tipc/bearer.h | 2 ++
net/tipc/msg.c | 7 +++--
net/tipc/node.c | 4 ++-
net/tipc/socket.c | 6 ++--
net/tipc/subscr.c | 21 ++++++-------
net/xfrm/xfrm_policy.c | 7 ++++-
net/xfrm/xfrm_state.c | 8 +++++
net/xfrm/xfrm_user.c | 6 +++-
119 files changed, 816 insertions(+), 559 deletions(-)
^ permalink raw reply
* Re: [net-next PATCH] ixgbe: add counter for times rx pages gets allocated, not recycled
From: Jeff Kirsher @ 2017-09-01 19:09 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev; +Cc: Alexander Duyck
In-Reply-To: <150426329512.26774.12697329010993174947.stgit@firesoul>
[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]
On Fri, 2017-09-01 at 12:54 +0200, Jesper Dangaard Brouer wrote:
> The ixgbe driver have page recycle scheme based around the RX-ring
> queue, where a RX page is shared between two packets. Based on the
> refcnt, the driver can determine if the RX-page is currently only
> used
> by a single packet, if so it can then directly refill/recycle the
> RX-slot by with the opposite "side" of the page.
>
> While this is a clever trick, it is hard to determine when this
> recycling is successful and when it fails. Adding a counter, which
> is
> available via ethtool --statistics as 'alloc_rx_page'. Which counts
> the number of times the recycle fails and the real page allocator is
> invoked. When interpreting the stats, do remember that every alloc
> will serve two packets.
>
> The counter is collected per rx_ring, but is summed and ethtool
> exported as 'alloc_rx_page'. It would be relevant to know what
> rx_ring that cannot keep up, but that can be exported later if
> someone experience a need for this.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Since Alex has a suggested change for this patch, when you resubmit v2,
can you make sure you CC intel-wired-lan mailing list, so that my
patchwork project picks up this patch? Thanks in advance Jesper.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [iproute PATCH 0/2] Fix and enhance link_gre6
From: Stephen Hemminger @ 2017-09-01 19:10 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
In-Reply-To: <20170901140809.13230-1-phil@nwl.cc>
On Fri, 1 Sep 2017 16:08:07 +0200
Phil Sutter <phil@nwl.cc> wrote:
> Changing a tunnel's flowlabel value was broken if it was set to a
> non-zero value before. Since the same problem existed for tclass, patch
> 1 fixes both instances at once.
>
> Patch 2 enhances 'ip link show' to also print the tclass value. This
> change was necessary to properly test the first patch's result.
>
> Phil Sutter (2):
> link_gre6: Fix for changing tclass/flowlabel
> link_gre6: Print the tunnel's tclass setting
>
> ip/link_gre6.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
Applied, thanks.
^ permalink raw reply
* Re: [iproute PATCH 0/6] strlcpy() and strlcat() for iproute2
From: Stephen Hemminger @ 2017-09-01 19:12 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
In-Reply-To: <20170901165256.21459-1-phil@nwl.cc>
On Fri, 1 Sep 2017 18:52:50 +0200
Phil Sutter <phil@nwl.cc> wrote:
> The following series adds my own implementations of strlcpy() and
> strlcat() in patch 1 and changes the code to make use of them in the
> following patches but the last two: Patch 5 just eliminates a line of
> useless code I found while searching for potential users of the
> introduced functions, patch 6 sanitizes a call to strncpy() in
> misc/lnstat_util.c without using strlcpy() since lnstat is not being
> linked against libutil.
>
> I implemented both functions solely based on information in libbsd's man
> pages, so they are safe to be released under the GPL.
>
> Phil Sutter (6):
> utils: Implement strlcpy() and strlcat()
> Convert the obvious cases to strlcpy()
> Convert harmful calls to strncpy() to strlcpy()
> ipxfrm: Replace STRBUF_CAT macro with strlcat()
> tc_util: No need to terminate an snprintf'ed buffer
> lnstat_util: Make sure buffer is NUL-terminated
>
> genl/ctrl.c | 2 +-
> include/utils.h | 3 +++
> ip/ipnetns.c | 3 +--
> ip/iproute_lwtunnel.c | 3 +--
> ip/ipvrf.c | 5 ++---
> ip/ipxfrm.c | 21 +++++----------------
> ip/xfrm_state.c | 2 +-
> lib/bpf.c | 3 +--
> lib/fs.c | 3 +--
> lib/inet_proto.c | 3 +--
> lib/utils.c | 19 +++++++++++++++++++
> misc/lnstat_util.c | 3 ++-
> misc/ss.c | 3 +--
> tc/em_ipset.c | 3 +--
> tc/tc_util.c | 1 -
> 15 files changed, 40 insertions(+), 37 deletions(-)
>
Applied, thanks.
^ permalink raw reply
* Re: [iproute PATCH] lib/bpf: Fix bytecode-file parsing
From: Daniel Borkmann @ 2017-09-01 19:13 UTC (permalink / raw)
To: Phil Sutter, Stephen Hemminger, netdev
In-Reply-To: <20170830141155.GH20614@orbyte.nwl.cc>
On 08/30/2017 04:11 PM, Phil Sutter wrote:
> On Wed, Aug 30, 2017 at 03:53:59PM +0200, Daniel Borkmann wrote:
>> On 08/29/2017 05:09 PM, Phil Sutter wrote:
[...]
>>
>> I don't really have a strong opinion on this, but the logic for
>> normalizing here is getting a bit convoluted. Is your use case
>> for making the parser more robust mainly so you can just use the
>> -ddd output from tcpdump for cBPF w/o piping through tr? But even
>> that shouldn't give multiple empty lines afaik, no?
>
> Well, using tcpdump output was functional before already. I just noticed
> that if I add an empty line to the end of bytecode-file, it will fail
> and I didn't like that. Then while searching for the EOF issue, I
> noticed that the parser logic above is a bit faulty in that it will
> treat different characters equally but doesn't make sure c_prev will be
> assigned only one of them. So apart from the added robustness, it really
> fixes an inconsistency in the parsing logic.
Ok, fine by me.
^ permalink raw reply
* Re: [iproute PATCH 0/2] Fix and enhance link_gre6
From: Stephen Hemminger @ 2017-09-01 19:13 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
In-Reply-To: <20170901140809.13230-1-phil@nwl.cc>
On Fri, 1 Sep 2017 16:08:07 +0200
Phil Sutter <phil@nwl.cc> wrote:
> Changing a tunnel's flowlabel value was broken if it was set to a
> non-zero value before. Since the same problem existed for tclass, patch
> 1 fixes both instances at once.
>
> Patch 2 enhances 'ip link show' to also print the tclass value. This
> change was necessary to properly test the first patch's result.
>
> Phil Sutter (2):
> link_gre6: Fix for changing tclass/flowlabel
> link_gre6: Print the tunnel's tclass setting
>
> ip/link_gre6.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
This doesn't work with net-next where json has been added.
I fixing it now
^ permalink raw reply
* Re: [iproute PATCH 2/6] Convert the obvious cases to strlcpy()
From: Daniel Borkmann @ 2017-09-01 19:13 UTC (permalink / raw)
To: Phil Sutter, Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170901165256.21459-3-phil@nwl.cc>
On 09/01/2017 06:52 PM, Phil Sutter wrote:
> This converts the typical idiom of manually terminating the buffer after
> a call to strncpy().
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
For BPF loader bits:
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH 1/1] bpf: take advantage of stack_depth tracking in powerpc JIT
From: Daniel Borkmann @ 2017-09-01 19:15 UTC (permalink / raw)
To: Sandipan Das, mpe; +Cc: naveen.n.rao, ast, netdev, linuxppc-dev
In-Reply-To: <20170901185301.17978-1-sandipan@linux.vnet.ibm.com>
On 09/01/2017 08:53 PM, Sandipan Das wrote:
> Take advantage of stack_depth tracking, originally introduced for
> x64, in powerpc JIT as well. Round up allocated stack by 16 bytes
> to make sure it stays aligned for functions called from JITed bpf
> program.
>
> Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
Awesome, thanks for following up! :)
^ permalink raw reply
* RE: [PATCH 0/2] i40e: fix firmware update
From: Keller, Jacob E @ 2017-09-01 19:19 UTC (permalink / raw)
To: Stefan Assmann, intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org, davem@davemloft.net, Kirsher, Jeffrey T
In-Reply-To: <20170901140234.7840-1-sassmann@kpanic.de>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Stefan Assmann
> Sent: Friday, September 01, 2017 7:03 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; davem@davemloft.net; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>; sassmann@kpanic.de
> Subject: [PATCH 0/2] i40e: fix firmware update
>
> The first patch fixes the firmware update which is currently broken and
> results in a bad flash (corrupt firmware). Recovery is possible with a
> fixed driver.
> The second patch reverts a commit that causes the firmware checksum
> verification to fail right after a successful flash. This is related to
> a recent workqueue change. Haven't gotten to the bottom of this yet, but
> for the sake of a smooth firmware update experience let's revert the
> commit for now.
Hi Stefan,
Thanks for these patches, I apologize for the time it took for us to respond to this.
The first patch is functionally correct, and I'm surprised we missed sending an equivalent ourselves. It looks like some related changes occurred around this code, and we failed to submit the patch.
I think Jeff would prefer if we send the version based directly on the out-of-tree code, which I will be reviving and submitting shortly.
The second issue I believe is not fixed correctly by the patch, I'm unsure why exactly changing the WQ would cause this but I believe that a similar patch which creates a non-locked version of i40e_nvm_read_buffer() will resolve this, and I will be sending that patch as well, which I believe is the real fix versus halting the work queue.
Thanks,
Jake
>
> Stefan Assmann (2):
> i40e: use non-locking i40e_read_nvm_word() function during nvmupdate
> Revert "i40e: remove WQ_UNBOUND and the task limit of our workqueue"
>
> drivers/net/ethernet/intel/i40e/i40e_main.c | 12 +++++-------
> drivers/net/ethernet/intel/i40e/i40e_nvm.c | 24 ++++++++++++++++++++++--
> 2 files changed, 27 insertions(+), 9 deletions(-)
>
> --
> 2.13.5
^ permalink raw reply
* Re: [RFC net-next 0/8] net: dsa: Multi-queue awareness
From: Florian Fainelli @ 2017-09-01 19:21 UTC (permalink / raw)
To: Andrew Lunn; +Cc: jiri, jhs, netdev, davem, xiyou.wangcong, vivien.didelot
In-Reply-To: <20170901185012.GA8122@lunn.ch>
On 09/01/2017 11:50 AM, Andrew Lunn wrote:
> On Fri, Sep 01, 2017 at 11:27:43AM -0700, Florian Fainelli wrote:
>> On 09/01/2017 10:55 AM, Andrew Lunn wrote:
>>> Hi Florian
>>>
>>>>>> tc bind dev sw0p0 queue 0 dev eth0 queue 16
>>>
>>> It this the eth0 i don't like here. Why not in the implementation just
>>> use something like netdev_master_upper_dev_get('sw0p0')? Or does
>>
>> Last I brought this up with Jiri that we should link DSA network devices
>> to their master network deviecs with netdev_upper_dev_link() he said
>> this was not appropriate for DSA slave network devices, but I can't
>> remember why, I would assume that any stacked device set up would do that.
>
> There is some form a linking going, our device names show that:
>
> 9: lan5@eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
> link/ether da:87:2a:03:cf:16 brd ff:ff:ff:ff:ff:ff
This is because iproute2 is linking the devices based on what
ndo_get_iflink() returns.
>
>> In any case, we need to establish a mapping so we have to specify at
>> least the target device's queue number. It is quite similar in premise
>> to e.g: enslaving a network device to a bridge port:
>>
>> ip link set dev eth0 master br0
>
> But here br0 is absolutely required, we have to say which bridge the
> slave port should be a member of.
Right,
>
> But what good is eth0 in
>
> tc bind dev sw0p0 queue 0 dev eth0 queue 16
>
> As i said suggesting, you have to somehow verify that eth0 is the
> conduit interface sw0p0 is using. Which makes the parameter pointless.
> Determine it from the sw0p0 somehow.
I see what you mean, so something along the lines of just:
tc bind dev swp0p0 queue 0 master queue 16
without having to specify the master network device since it's implicit,
I kind of like that.
--
Florian
^ permalink raw reply
* Re: tip -ENOBOOT - bisected to locking/refcounts, x86/asm: Implement fast refcount overflow protection
From: Mike Galbraith @ 2017-09-01 19:24 UTC (permalink / raw)
To: Kees Cook
Cc: David S. Miller, Peter Zijlstra, LKML, Ingo Molnar,
Reshetova, Elena, Network Development
In-Reply-To: <CAGXu5jKiGFAp8Lp2Nt1yHg_nYsY_rfrUjMgOPTNxMLkbQ8V-ng@mail.gmail.com>
On Fri, 2017-09-01 at 11:58 -0700, Kees Cook wrote:
>
> The section stuff is supposed to be a trick to push the error case off
> into the .text.unlikely area to avoid needing a jmp over the handler
> and with possibly some redundancy removal done by the compiler (though
> this appears to be rather limited) if it notices a bunch of error
> paths are the same. However, in your disassembly, it's inline (!!) in
> the code, as if "pushsection" and "popsection" were entirely ignored.
>
> And when I make my own in6_dev_getx(), I see the same disassembly:
>
> 0xffffffff818a757b <+181>: lock incl 0x1e0(%rbx)
> 0xffffffff818a7582 <+188>: js 0xffffffff818a7584 <in6_dev_getx+190>
> 0xffffffff818a7584 <+190>: lea 0x1e0(%rbx),%rcx
> 0xffffffff818a758b <+197>: (bad)
>
> Which is VERY different from how it looks in other places!
>
> e.g. from lkdtm_REFCOUNT_INC_SATURATED:
>
> 0xffffffff815657df <+47>: lock incl -0xc(%rbp)
> 0xffffffff815657e3 <+51>: js 0xffffffff81565cac
> ...
> 0xffffffff81565cac: lea -0xc(%rbp),%rcx
> 0xffffffff81565cb0: (bad)
>
> So, at least I can reproduce this in the build now. I must not be
> exercising these paths. FWIW, this is with Ubuntu's 6.3.0 gcc.
>
> I'll try to figure out what's going on here...
Heh, make in6_dev_getx() __always_inline.
swapper/0-1 [000] d..1 1.438587: ip6_route_init_special_entries: PRE refs.counter:3
swapper/0-1 [000] d..1 1.438590: ip6_route_init_special_entries: POST refs.counter:4
swapper/0-1 [000] d..1 1.438591: ip6_route_init_special_entries: PRE refs.counter:4
swapper/0-1 [000] d..1 1.438592: ip6_route_init_special_entries: POST refs.counter:5
swapper/0-1 [000] d..1 1.438592: ip6_route_init_special_entries: PRE refs.counter:5
swapper/0-1 [000] d..1 1.438593: ip6_route_init_special_entries: POST refs.counter:6
-Mike
^ permalink raw reply
* Re: [net-next PATCH] bpf: sockmap update/simplify memory accounting scheme
From: Alexei Starovoitov @ 2017-09-01 19:25 UTC (permalink / raw)
To: John Fastabend, davem; +Cc: netdev, daniel
In-Reply-To: <20170901182926.8981.77450.stgit@john-Precision-Tower-5810>
On 9/1/17 11:29 AM, John Fastabend wrote:
> Instead of tracking wmem_queued and sk_mem_charge by incrementing
> in the verdict SK_REDIRECT paths and decrementing in the tx work
> path use skb_set_owner_w and sock_writeable helpers. This solves
> a few issues with the current code. First, in SK_REDIRECT inc on
> sk_wmem_queued and sk_mem_charge were being done without the peers
> sock lock being held. Under stress this can result in accounting
> errors when tx work and/or multiple verdict decisions are working
> on the peer psock.
>
> Additionally, this cleans up the code because we can rely on the
> default destructor to decrement memory accounting on kfree_skb. Also
> this will trigger sk_write_space when space becomes available on
> kfree_skb() which wasn't happening before and prevent __sk_free
> from being called until all in-flight packets are completed.
>
> Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
thanks. it's cleaner indeed.
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [RESEND PATCH] Allow passing tid or pid in SCM_CREDENTIALS without CAP_SYS_ADMIN
From: Eric W. Biederman @ 2017-09-01 19:29 UTC (permalink / raw)
To: Prakash Sangappa; +Cc: David Miller, linux-kernel, netdev, drepper
In-Reply-To: <ca086e10-764c-4389-808e-43fd1ace4e0c@oracle.com>
Prakash Sangappa <prakash.sangappa@oracle.com> writes:
> On 8/30/17 10:41 AM, ebiederm@xmission.com wrote:
>> Prakash Sangappa <prakash.sangappa@oracle.com> writes:
>>
>>
>>> With regards to security, the question basically is what is the consequence
>>> of passing the wrong id. As I understand it, Interpreting the id to be pid
>>> or tid, the effective uid and gid will be the same. It would be a problem
>>> only if the incorrect interpretation of the id would refer a different process.
>>> But that cannot happen as the the global tid(gettid() of a thread is
>>> unique.
>> There is also the issue that the receiving process could look, not see
>> the pid in proc and assume the sending process is dead. That I suspect
>> is the larger danger.
>>
>
> Will this not be a bug in the application, if it is sending the wrong
> id?
No. It could be deliberate and malicious.
>>> As long as the thread is alive, that id cannot reference another process / thread.
>>> Unless the thread were to exit and the id gets recycled and got used for another
>>> thread or process. This would be no different from a process exiting and its
>>> pid getting recycled which is the case now.
>> Largely I agree.
>>
>> If all you want are pid translations I suspect the are far easier ways
>> thant updating the SCM_CREDENTIALS code.
>
> What would be an another easier & efficient way of doing pid translation?
>
> Should a new API/mechanism be considered mainly for pid translation purpose
> for use with pid namespaces, say based on 'pipe' something similar to
> I_SENDFD?
There are proc files that provide all of the pids of a process you can
read those.
Other possibilities exist if you want to go that fast.
Eric
^ permalink raw reply
* Re: tip -ENOBOOT - bisected to locking/refcounts, x86/asm: Implement fast refcount overflow protection
From: Kees Cook @ 2017-09-01 19:40 UTC (permalink / raw)
To: Mike Galbraith
Cc: David S. Miller, Peter Zijlstra, LKML, Ingo Molnar,
Reshetova, Elena, Network Development
In-Reply-To: <CAGXu5jKiGFAp8Lp2Nt1yHg_nYsY_rfrUjMgOPTNxMLkbQ8V-ng@mail.gmail.com>
On Fri, Sep 1, 2017 at 11:58 AM, Kees Cook <keescook@chromium.org> wrote:
> On Fri, Sep 1, 2017 at 10:52 AM, Mike Galbraith <efault@gmx.de> wrote:
>> On Fri, 2017-09-01 at 10:12 -0700, Kees Cook wrote:
>>> On Fri, Sep 1, 2017 at 6:09 AM, Mike Galbraith <efault@gmx.de> wrote:
>>> > On Fri, 2017-09-01 at 08:57 +0200, Mike Galbraith wrote:
>>> >> On Thu, 2017-08-31 at 11:45 -0700, Kees Cook wrote:
>>> >> > On Thu, Aug 31, 2017 at 10:19 AM, Mike Galbraith <efault@gmx.de> wrote:
>>> >> > > On Thu, 2017-08-31 at 10:00 -0700, Kees Cook wrote:
>>> >> > >>
>>> >> > >> Oh! So it's gcc-version sensitive? That's alarming. Is this mapping correct:
>>> >> > >>
>>> >> > >> 4.8.5: WARN, eventual kernel hang
>>> >> > >> 6.3.1, 7.0.1: WARN, but continues working
>>> >> > >
>>> >> > > Yeah, that's correct. I find that troubling, simply because this gcc
>>> >> > > version has been through one hell of a lot of kernels with me. Yeah, I
>>> >> > > know, that doesn't exempt it from having bugs, but color me suspicious.
>>> >> >
>>> >> > I still can't hit this with a 4.8.5 build. :(
>>> >> >
>>> >> > With _RATELIMIT removed, this should, in theory, report whatever goes
>>> >> > negative first...
>>> >>
>>> >> I applied the other patch you posted, and built with gcc-6.3.1 to
>>> >> remove the gcc-4.8.5 aspect. Look below the resulting splat.
>>> >
>>> > Grr, that one has a in6_dev_getx() line missing for the first
>>> > increment, where things go pear shaped.
>>> >
>>> > With that added, looking at counter both before, and after incl, with a
>>> > trace_printk() in the exception handler showing it doing its saturate
>>> > thing, irqs disabled across the whole damn refcount_inc(), and even
>>> > booting box nr_cpus=1 for extra credit...
>>> >
>>> > HTH can that first refcount_inc() get there?
>>> >
>>> > # tracer: nop
>>> > #
>>> > # _-----=> irqs-off
>>> > # / _----=> need-resched
>>> > # | / _---=> hardirq/softirq
>>> > # || / _--=> preempt-depth
>>> > # ||| / delay
>>> > # TASK-PID CPU# |||| TIMESTAMP FUNCTION
>>> > # | | | |||| | |
>>> > systemd-1 [000] d..1 1.937284: in6_dev_getx: PRE refs.counter:3
>>> > systemd-1 [000] d..1 1.937295: ex_handler_refcount: *(int *)regs->cx = -1073741824
>>> > systemd-1 [000] d..1 1.937296: in6_dev_getx: POST refs.counter:-1073741824
>>>
>>> O_o
>>>
>>> Can you paste the disassembly of in6_dev_getx? I can't understand how
>>> we're landing in the exception handler.
>>
>> I was hoping you'd say that.
>>
>> 0xffffffff816b2f72 <+0>: push %rbp
>> 0xffffffff816b2f73 <+1>: mov %rsp,%rbp
>> 0xffffffff816b2f76 <+4>: push %r12
>> 0xffffffff816b2f78 <+6>: push %rbx
>> 0xffffffff816b2f79 <+7>: incl %gs:0x7e95a2d0(%rip) # 0xd250 <__preempt_count>
>> 0xffffffff816b2f80 <+14>: mov 0x308(%rdi),%rbx
>> 0xffffffff816b2f87 <+21>: test %rbx,%rbx
>> 0xffffffff816b2f8a <+24>: je 0xffffffff816b2feb <in6_dev_getx+121>
>> 0xffffffff816b2f8c <+26>: callq *0xffffffff81c35a00
>> 0xffffffff816b2f93 <+33>: mov %rax,%r12
>> 0xffffffff816b2f96 <+36>: callq *0xffffffff81c35a10
>> 0xffffffff816b2f9d <+43>: mov 0x769ad4(%rip),%rsi # 0xffffffff81e1ca78 <trace_printk_fmt.21733>
>> 0xffffffff816b2fa4 <+50>: mov 0xf0(%rbx),%edx
>> 0xffffffff816b2faa <+56>: mov $0xffffffff816b2f8c,%rdi
>> 0xffffffff816b2fb1 <+63>: callq 0xffffffff81171fc0 <__trace_bprintk>
>> 0xffffffff816b2fb6 <+68>: lock incl 0xf0(%rbx)
>> 0xffffffff816b2fbd <+75>: js 0xffffffff816b2fbf <in6_dev_getx+77>
>> 0xffffffff816b2fbf <+77>: lea 0xf0(%rbx),%rcx
>> 0xffffffff816b2fc6 <+84>: (bad)
>> 0xffffffff816b2fc8 <+86>: mov 0x769a99(%rip),%rsi # 0xffffffff81e1ca68 <trace_printk_fmt.21744>
>> 0xffffffff816b2fcf <+93>: mov 0xf0(%rbx),%edx
>> 0xffffffff816b2fd5 <+99>: mov $0xffffffff816b2f8c,%rdi
>> 0xffffffff816b2fdc <+106>: callq 0xffffffff81171fc0 <__trace_bprintk>
>> 0xffffffff816b2fe1 <+111>: mov %r12,%rdi
>> 0xffffffff816b2fe4 <+114>: callq *0xffffffff81c35a08
>> 0xffffffff816b2feb <+121>: decl %gs:0x7e95a25e(%rip) # 0xd250 <__preempt_count>
>> 0xffffffff816b2ff2 <+128>: mov %rbx,%rax
>> 0xffffffff816b2ff5 <+131>: pop %rbx
>> 0xffffffff816b2ff6 <+132>: pop %r12
>> 0xffffffff816b2ff8 <+134>: pop %rbp
>> 0xffffffff816b2ff9 <+135>: retq
>>
>> I don't get the section business at all, +75 looks to me like we're
>> gonna trap no matter what.. as we appear to be doing.
>
> The section stuff is supposed to be a trick to push the error case off
> into the .text.unlikely area to avoid needing a jmp over the handler
> and with possibly some redundancy removal done by the compiler (though
> this appears to be rather limited) if it notices a bunch of error
> paths are the same. However, in your disassembly, it's inline (!!) in
> the code, as if "pushsection" and "popsection" were entirely ignored.
>
> And when I make my own in6_dev_getx(), I see the same disassembly:
>
> 0xffffffff818a757b <+181>: lock incl 0x1e0(%rbx)
> 0xffffffff818a7582 <+188>: js 0xffffffff818a7584 <in6_dev_getx+190>
> 0xffffffff818a7584 <+190>: lea 0x1e0(%rbx),%rcx
> 0xffffffff818a758b <+197>: (bad)
>
> Which is VERY different from how it looks in other places!
Found it.
If the compiler already pushed the entire function into
.text.unlikely, x86-refcount's .pushsection doesn't do any good
(obviously). Durrr.
.section .text.unlikely,"ax",@progbits
.type in6_dev_getx, @function
in6_dev_getx:
.LFB4673:
.loc 2 4128 0
.cfi_startproc
...
lock; incl 480(%rbx)
js 111f
.pushsection .text.unlikely
111: lea 480(%rbx), %rcx
112: .byte 0x0f, 0xff
.popsection
113:
I will get this fixed. Thank you again for helping track this down!
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [RFC net-next 0/8] net: dsa: Multi-queue awareness
From: Andrew Lunn @ 2017-09-01 19:44 UTC (permalink / raw)
To: Florian Fainelli; +Cc: jiri, jhs, netdev, davem, xiyou.wangcong, vivien.didelot
In-Reply-To: <5a3fa91f-fb1f-e5ea-987d-973dee3bf8eb@gmail.com>
> I see what you mean, so something along the lines of just:
>
> tc bind dev swp0p0 queue 0 master queue 16
>
> without having to specify the master network device since it's implicit,
> I kind of like that.
Yes, that is better.
Andrew
^ 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