Netdev List
 help / color / mirror / Atom feed
* Re: arch: arm: bpf: Converting cBPF to eBPF for arm 32 bit
From: Shubham Bansal @ 2017-05-23  2:58 UTC (permalink / raw)
  To: Kees Cook
  Cc: Daniel Borkmann, David Miller, Mircea Gherzan,
	Network Development, kernel-hardening@lists.openwall.com,
	linux-arm-kernel@lists.infradead.org, ast
In-Reply-To: <CAGXu5jLYunVCJGCfHPebKDaoQ71hdMGq4HhdDxTYpBQw_HXUYQ@mail.gmail.com>

Hi,

On testing the eBPF JIT with CONFIG_FRAME_POINTER I got the following
crash for non jitted testcase.

[   72.032494] test_bpf: #267 BPF_MAXINSNS: Call heavy transformations
jited:0 1112799
[   92.304815] NMI watchdog: BUG: soft lockup - CPU#0 stuck for 22s!
[insmod:104]
[   92.305050] Modules linked in: test_bpf(+)
[   92.305516] CPU: 0 PID: 104 Comm: insmod Not tainted
4.11.0-10603-g13e0988-dirty #21
[   92.305630] Hardware name: ARM-Versatile Express
[   92.305943] task: c75d5280 task.stack: c61b8000
[   92.306383] PC is at __bpf_prog_run+0x818/0x17a8
[   92.306449] LR is at __bpf_prog_run+0xab8/0x17a8
[   92.306510] pc : [<c0407c08>]    lr : [<c0407ea8>]    psr: 20000013
[   92.306510] sp : c61b9a88  ip : c61b9a88  fp : c61b9d4c
[   92.306629] r10: c0404104  r9 : 00000000  r8 : 00000000
[   92.306744] r7 : c0e0b500  r6 : c0c39bb0  r5 : c61b9ad0  r4 : ca314840
[   92.306882] r3 : c0e0b7fc  r2 : 00000000  r1 : c61b9ad8  r0 : 00000000
[   92.307070] Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[   92.307285] Control: 10c5387d  Table: 661e0059  DAC: 00000051
[   92.307503] CPU: 0 PID: 104 Comm: insmod Not tainted
4.11.0-10603-g13e0988-dirty #21
[   92.307575] Hardware name: ARM-Versatile Express
[   92.307651] Backtrace:
[   92.307868] [<c030caec>] (dump_backtrace) from [<c030cda8>]
(show_stack+0x18/0x1c)
[   92.308003]  r7:c1503db8 r6:60000193 r5:00000000 r4:c1570f30
[   92.308085] [<c030cd90>] (show_stack) from [<c064b198>]
(dump_stack+0x90/0xa4)
[   92.308195] [<c064b108>] (dump_stack) from [<c030900c>] (show_regs+0x14/0x18)
[   92.308281]  r7:c1503db8 r6:c14488b8 r5:c16aaea0 r4:c61b8000
[   92.308346] [<c0308ff8>] (show_regs) from [<c03df2a4>]
(watchdog_timer_fn+0x24c/0x2c4)
[   92.308423] [<c03df058>] (watchdog_timer_fn) from [<c03b70d8>]
(__hrtimer_run_queues+0x180/0x318)
[   92.308514]  r10:c03df058 r9:00000003 r8:c1503cbc r7:c7ead580
r6:c7ead5c0 r5:c61b8000
[   92.308578]  r4:c7ead8d8
[   92.308635] [<c03b6f58>] (__hrtimer_run_queues) from [<c03b74e8>]
(hrtimer_interrupt+0xb4/0x204)
[   92.308728]  r10:7fffffff r9:00000003 r8:c7ead5f8 r7:c7ead618
r6:c7ead638 r5:c1448580
[   92.308789]  r4:c7ead580
[   92.308835] [<c03b7434>] (hrtimer_interrupt) from [<c03113fc>]
(twd_handler+0x38/0x48)
[   92.308914]  r10:c0404104 r9:00000010 r8:c1504330 r7:00000001
r6:c701e900 r5:00000000
[   92.308974]  r4:00000001
[   92.309021] [<c03113c4>] (twd_handler) from [<c03a1238>]
(handle_percpu_devid_irq+0x90/0x244)
[   92.309091]  r5:00000000 r4:c7020540
[   92.309165] [<c03a11a8>] (handle_percpu_devid_irq) from
[<c039c148>] (generic_handle_irq+0x2c/0x3c)
[   92.309254]  r10:c0404104 r9:c8803100 r8:c7004a00 r7:00000001
r6:00000000 r5:00000000
[   92.309319]  r4:c1449ed0 r3:c03a11a8
[   92.309369] [<c039c11c>] (generic_handle_irq) from [<c039c6f0>]
(__handle_domain_irq+0x64/0xbc)
[   92.309445] [<c039c68c>] (__handle_domain_irq) from [<c0301808>]
(gic_handle_irq+0x5c/0xa0)
[   92.309525]  r9:c8803100 r8:c8802100 r7:c61b9a38 r6:c880210c
r5:c1571848 r4:c1504330
[   92.309596] [<c03017ac>] (gic_handle_irq) from [<c030d98c>]
(__irq_svc+0x6c/0x90)
[   92.309731] Exception stack(0xc61b9a38 to 0xc61b9a80)
[   92.309943] 9a20:
    00000000 c61b9ad8
[   92.310184] 9a40: 00000000 c0e0b7fc ca314840 c61b9ad0 c0c39bb0
c0e0b500 00000000 00000000
[   92.310377] 9a60: c0404104 c61b9d4c c61b9a88 c61b9a88 c0407ea8
c0407c08 20000013 ffffffff
[   92.310595]  r9:c61b8000 r8:00000000 r7:c61b9a6c r6:ffffffff
r5:20000013 r4:c0407c08
[   92.311103] [<c04073f0>] (__bpf_prog_run) from [<bf15759c>]
(test_bpf_init+0x59c/0x1000 [test_bpf])
[   92.311262]  r10:bf123094 r9:ca2fa020 r8:00000000 r7:bf123128
r6:53edefe8 r5:ca2fa000
[   92.311325]  r4:00000555
[   92.311382] [<bf157000>] (test_bpf_init [test_bpf]) from
[<c0301f7c>] (do_one_initcall+0x4c/0x174)
[   92.311468]  r10:bf154640 r9:c61c2524 r8:39e3db1c r7:00000001
r6:00000000 r5:bf157000
[   92.311529]  r4:ffffe000
[   92.311575] [<c0301f30>] (do_one_initcall) from [<c042a5b0>]
(do_init_module+0x6c/0x1fc)
[   92.311673]  r9:c61c2524 r8:39e3db1c r6:c61c2480 r5:00000001 r4:bf154640
[   92.311744] [<c042a544>] (do_init_module) from [<c03d393c>]
(load_module+0x1f8c/0x2394)
[   92.311815]  r6:c61c2500 r5:00000001 r4:c61b9f34
[   92.311898] [<c03d19b0>] (load_module) from [<c03d3ea0>]
(SyS_init_module+0x15c/0x174)
[   92.311979]  r10:00000051 r9:00000000 r8:00160fda r7:c61b8000
r6:c95a6a18 r5:b6fbca20
[   92.312040]  r4:00006a18
[   92.312087] [<c03d3d44>] (SyS_init_module) from [<c0308260>]
(ret_fast_syscall+0x0/0x3c)
[   92.312196]  r10:00000000 r9:c61b8000 r8:c0308424 r7:00000080
r6:756e694c r5:00156a18
[   92.312277]  r4:00000000
[   93.835343] 1065840 PASS

Does this look like a bug? I will send the separate mail if it does.
Let me know.

Best,
Shubham Bansal


On Tue, May 23, 2017 at 1:35 AM, Kees Cook <keescook@chromium.org> wrote:
> On Mon, May 22, 2017 at 10:04 AM, Shubham Bansal
> <illusionist.neo@gmail.com> wrote:
>> These all benchmarks are for ARMv7.
>
> Thanks! In the future, try to avoid the white-space damage
> (line-wrapping). And it looks like you've still got debugging turned
> on in your jit code:
>
> [   56.176033] test_bpf: #21 LD_CPU
> [   56.176329] bpf_jit: *** NOT YET: opcode 85 ***
> [   56.176565] jited:0 2639 702 PASS
>
> That breaks the test report line. After I cleaned these up and parsed
> the results, they look great. Most things are half the speed of the
> interpreter, if not better. Only the LD_ABS suffered, and that's
> mainly the const blinding, I assume.
>
> Please post your current patch. Thanks for this!
>
> -Kees
>
> --
> Kees Cook
> Pixel Security

^ permalink raw reply

* Re: [PATCH net-next] net: rfs: Don't reset RFS entries when nothing changed
From: David Miller @ 2017-05-23  3:02 UTC (permalink / raw)
  To: gfree.wind; +Cc: netdev
In-Reply-To: <1495500311-81095-1-git-send-email-gfree.wind@vip.163.com>

From: gfree.wind@vip.163.com
Date: Tue, 23 May 2017 08:45:11 +0800

> From: Gao Feng <gfree.wind@vip.163.com>
> 
> When the new RFS table size specified by sysctl equals the old one,
> there is nothing changed actually. So it is unnecessary to reset the
> RFS table entris.
> 
> Signed-off-by: Gao Feng <gfree.wind@vip.163.com>

It seems like an intentional feature to be able to reset the
table by simply writing the same value to the sysfs knob.

I'm not applying this, sorry.

^ permalink raw reply

* Re: [PATCH net-next] bpf: update perf event helper function signature and documentation
From: David Miller @ 2017-05-23  3:08 UTC (permalink / raw)
  To: qinteng; +Cc: peterz, bgregg, daniel, netdev, linux-kernel, Kernel-team, ast
In-Reply-To: <E796EF36-C195-4FFA-AAA0-A1F9E7730349@fb.com>

From: Teng Qin <qinteng@fb.com>
Date: Tue, 23 May 2017 00:39:34 +0000

> diff --git a/samples/bpf/bpf_helpers.h b/samples/bpf/bpf_helpers.h
> index 9a9c95f..a94ce42 100644
> --- a/samples/bpf/bpf_helpers.h
> +++ b/samples/bpf/bpf_helpers.h
> @@ -37,9 +37,8 @@ static int (*bpf_clone_redirect)(void *ctx, int ifindex, int flags) =
>  	(void *) BPF_FUNC_clone_redirect;
>  static int (*bpf_redirect)(int ifindex, int flags) =
>  	(void *) BPF_FUNC_redirect;
> -static int (*bpf_perf_event_output)(void *ctx, void *map,
> -				    unsigned long long flags, void *data,
> -				    int size) =
> +static int (*bpf_perf_event_output)(void *ctx, void *map, u64 flags,
> +	                            void *data, int size) =
>  	(void *) BPF_FUNC_perf_event_output;
>  static int (*bpf_get_stackid)(void *ctx, void *map, int flags) =
>  	(void *) BPF_FUNC_get_stackid;

I think we've been intentionally avoiding the use of "u64", "u32",
etc. in this file.

But what do I know.

^ permalink raw reply

* Re:Re: [PATCH net-next] net: rfs: Don't reset RFS entries when nothing changed
From: Gao Feng @ 2017-05-23  3:10 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20170522.230220.402998640470537222.davem@davemloft.net>

At 2017-05-23 11:02:20, "David Miller" <davem@davemloft.net> wrote:
>From: gfree.wind@vip.163.com
>Date: Tue, 23 May 2017 08:45:11 +0800
>
>> From: Gao Feng <gfree.wind@vip.163.com>
>> 
>> When the new RFS table size specified by sysctl equals the old one,
>> there is nothing changed actually. So it is unnecessary to reset the
>> RFS table entris.
>> 
>> Signed-off-by: Gao Feng <gfree.wind@vip.163.com>
>
>It seems like an intentional feature to be able to reset the
>table by simply writing the same value to the sysfs knob.
>
>I'm not applying this, sorry.

It is ok.
I just thought maybe it was used to reset, but I didn't find any comment and tips by google.

Regards
Feng

^ permalink raw reply

* Re: [PATCH net-next] bpf: update perf event helper function signature and documentation
From: Teng Qin @ 2017-05-23  3:17 UTC (permalink / raw)
  To: David Miller
  Cc: peterz@infradead.org, bgregg@netflix.com, daniel@iogearbox.net,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Kernel Team,
	Alexei Starovoitov
In-Reply-To: <20170522.230849.1056416550811494880.davem@davemloft.net>



On 5/22/17, 20:08, "David Miller" <davem@davemloft.net> wrote:

    From: Teng Qin <qinteng@fb.com>
    Date: Tue, 23 May 2017 00:39:34 +0000
    
    > diff --git a/samples/bpf/bpf_helpers.h b/samples/bpf/bpf_helpers.h
    > index 9a9c95f..a94ce42 100644
    > --- a/samples/bpf/bpf_helpers.h
    > +++ b/samples/bpf/bpf_helpers.h
    > @@ -37,9 +37,8 @@ static int (*bpf_clone_redirect)(void *ctx, int ifindex, int flags) =
    >  	(void *) BPF_FUNC_clone_redirect;
    >  static int (*bpf_redirect)(int ifindex, int flags) =
    >  	(void *) BPF_FUNC_redirect;
    > -static int (*bpf_perf_event_output)(void *ctx, void *map,
    > -				    unsigned long long flags, void *data,
    > -				    int size) =
    > +static int (*bpf_perf_event_output)(void *ctx, void *map, u64 flags,
    > +	                            void *data, int size) =
    >  	(void *) BPF_FUNC_perf_event_output;
    >  static int (*bpf_get_stackid)(void *ctx, void *map, int flags) =
    >  	(void *) BPF_FUNC_get_stackid;
    
    I think we've been intentionally avoiding the use of "u64", "u32",
    etc. in this file.
    
    But what do I know.

Alexei said it was due to Clang not taking u64, u32 etc. for compilation.
I didn’t know the context and just used them. But apparently, something
changed and now they build and run OK......


^ permalink raw reply

* Re: Deleting a dynamic mac entry..
From: Toshiaki Makita @ 2017-05-23  3:17 UTC (permalink / raw)
  To: Manohar Kumar; +Cc: netdev, bridge
In-Reply-To: <CA+N+6-xY7+coc6zFqzP665WMhbGFFMHOfre9wNxRAcSF2Bthzw@mail.gmail.com>

On 2017/05/21 11:28, Manohar Kumar wrote:
> Hello,
> 
> In 3.19 the following bridge fdb command to delete a dynamically
> learned entry fails..
> 
> root@net-3:~# bridge fdb show | grep 02:42:0a:ff:00:06
> 02:42:0a:ff:00:06 dev vxlan0 master br0
> root@net-3:~# bridge fdb del 02:42:0a:ff:00:06 dev vxlan0 master
> RTNETLINK answers: No such file or directory
> 
> It works in 4.4.
> 
> Can someone please point to the patch that made this change ?

25d3b493a52d ("bridge: Fix inability to add non-vlan fdb entry") might
be what you are looking for, but you might want to do git-bisect to
track down any regression or fix.

> In kernels without this patch is there an alternative to delete
> (actually I want to do it programmatically) dynamic mac entries ?

If 25d3b493a52d is causing your problem, set default_pvid to 0 in order
to disable default_pvid, and delete any vlans which is already
configured in bridge's vlan_filtering. Then, delete the fdb entry.

Toshiaki Makita

^ permalink raw reply

* Re: arch: arm: bpf: Converting cBPF to eBPF for arm 32 bit
From: Shubham Bansal @ 2017-05-23  3:34 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Kees Cook, Daniel Borkmann, kernel-hardening@lists.openwall.com,
	Network Development, ast, Mircea Gherzan, David Miller,
	linux-arm-kernel@lists.infradead.org, nschichan, andrew
In-Reply-To: <8c53012b-cb81-5228-de79-f55457d1cf69@gmail.com>

Hi Florian,

>> I think it is fine to only target ARMv7. It is harder and harder to
>> find devices on v5 or v6 CPUs that would want to be using BPF JIT,
>> IMO.
>
> There are still a ton of Marvell-based routers out there (e.g: Kirkwood,
> Orion5x) that are ARMv5 and that prompted Nicholas (hey there) to fix
> the cBPF JIT a while ago. I don't think you can just ignore those, it's
> fine not to target them initially, but arguably, QEMU has decent support
> for some ARMv5 platforms that could be used for testing as well
> (realview-eb, versatileab/pbm.

I am using busybox to get the rootfs. Here is what I am doing :-

1. ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make -j4 (for kernel
build as well as busybox build)
2. qemu-system-arm -M vexpress-a9 -dtb
./linux/arch/arm/boot/dts/versatile-ab.dts -kernel ./linux/arch/a
rm/boot/zImage -append "root=/dev/mmcblk0 console=ttyAMA0" -sd
./a9rootfs.ext3 --nographic

Can you help me with running qemu for ARMv5 and ARMv6 ?

>> When they "disappear", it's because there isn't a prerequisite met. I
>> either read the Kconfig files or use "make menuconfig" and "search" to
>> tell me where a config is defined and what is needed to meet the
>> prerequisites.
>>
>> In the case of CPU_BIG_ENDIAN, you need ARCH_SUPPORTS_BIG_ENDIAN,
>> which appears to be only ARCH_IXP4XX. I don't think you're going to
>> find an emulator that will handle this, so I'd suggest ignoring this
>> config for now unless you can find someone with that hardware that you
>> can work with to test it.
>>
>> In the case of CONFIG_FRAME_POINTER, I assume you built a
>> THUMB2_KERNEL? I'd read the notes in arch/arm/Kconfig.debug for
>> 'config FRAME_POINTER'.
>
> It sounds like we are at the point where Shubham's patches should be
> posted so people could test/fix on earlier ARM devices for instance.
>
I would post them as soon as I test them on ARMv5 and ARMv6. If you
can help me with that, please let me know.

> Thanks
> --
> Florian

-Shubham

^ permalink raw reply

* Re: [PATCH 00/12] Netfilter/IPVS fixes for net
From: David Miller @ 2017-05-23  4:02 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <20170522.195444.1331021340581014126.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Mon, 22 May 2017 19:54:44 -0400 (EDT)

> From: Pablo Neira Ayuso <pablo@netfilter.org>
> Date: Mon, 22 May 2017 00:25:38 +0200
> 
>> Could you merge net into net-next as well? I have several patches for
>> net-next that need to apply on these fixes. No rush BTW.
> 
> Sure, no problem.
> 
> As soon as Linus takes in my pull request from today, I will do this
> and let you know.

This is now done.

^ permalink raw reply

* Re: arch: arm: bpf: Converting cBPF to eBPF for arm 32 bit
From: Kees Cook @ 2017-05-23  4:22 UTC (permalink / raw)
  To: Shubham Bansal
  Cc: Florian Fainelli, Daniel Borkmann,
	kernel-hardening@lists.openwall.com, Network Development, ast,
	Mircea Gherzan, David Miller,
	linux-arm-kernel@lists.infradead.org, Nicolas Schichan, andrew
In-Reply-To: <CAHgaXdL2qReJF1Aw08k3C+OiC-3AOy3VeYmQ1Hs7vKcrfGQnvA@mail.gmail.com>

On Mon, May 22, 2017 at 8:34 PM, Shubham Bansal
<illusionist.neo@gmail.com> wrote:
> I would post them as soon as I test them on ARMv5 and ARMv6. If you
> can help me with that, please let me know.

Please post what you have: it would be better to see what you've got
now in case additional changes are needed so you don't have to do it
again on v5 and v6. Also, it means other people with real v5 and v6
hardware could test for you if they were so inclined, and you won't
need to be blocked on doing the tests in qemu.

You can send it as an "RFC" in the subject, just to make sure people
know it's not considered fully done. :)

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply

* Re: arch: arm: bpf: Converting cBPF to eBPF for arm 32 bit
From: Kees Cook @ 2017-05-23  4:27 UTC (permalink / raw)
  To: Shubham Bansal
  Cc: Daniel Borkmann, David Miller, Mircea Gherzan,
	Network Development, kernel-hardening@lists.openwall.com,
	linux-arm-kernel@lists.infradead.org, ast
In-Reply-To: <CAHgaXd+5h7aMxF83EEkD3iRyeZ1JxAX2oFYJdy3GtcNOWRsBGw@mail.gmail.com>

On Mon, May 22, 2017 at 7:58 PM, Shubham Bansal
<illusionist.neo@gmail.com> wrote:
> On testing the eBPF JIT with CONFIG_FRAME_POINTER I got the following
> crash for non jitted testcase.

It's just a softlockup WARN, not a crash, and I think it'd to be
expected given the large runtime test_bpf reports:

> [   72.032494] test_bpf: #267 BPF_MAXINSNS: Call heavy transformations
> jited:0 1112799
> [   92.304815] NMI watchdog: BUG: soft lockup - CPU#0 stuck for 22s!
> [insmod:104]
> ...
> [   93.835343] 1065840 PASS

https://www.kernel.org/doc/Documentation/lockup-watchdogs.txt

You can raise the softlockup time-out by changing the number of
seconds here: /proc/sys/kernel/watchdog_thresh I think the softlockup
is counting the entire runtime of the bpf_tests run, so if it takes 30
seconds to run, put at least 15 into /proc/sys/kernel/watchdog_thresh

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply

* Re: [PATCH 0/5] atm: Adjustments for some function implementations
From: Kees Cook @ 2017-05-23  4:32 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Network Development, Augusto Mecking Caringi, David S. Miller,
	Jarod Wilson, Javier Martinez Canillas, LKML, kernel-janitors
In-Reply-To: <49543220-93e4-781c-877b-381277837152@users.sourceforge.net>

On Sun, May 21, 2017 at 1:12 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 21 May 2017 22:09:11 +0200
>
> A few update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (5):
>   Improve a size determination in four functions
>   Delete an error message for a failed memory allocation in make_entry()
>   Adjust 19 checks for null pointers
>   Use seq_puts() in lec_info()
>   Use seq_putc() in lec_info()
>
>  net/atm/lec.c | 55 +++++++++++++++++++++++++++----------------------------
>  1 file changed, 27 insertions(+), 28 deletions(-)

These all look fine to me. Thanks!

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply

* [PATCH net] ip6_tunnel, ip6_gre: fix setting of DSCP on encapsulated packets
From: Peter Dawson @ 2017-05-23  4:36 UTC (permalink / raw)
  To: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, stephen, netdev, linux-kernel

This fix addresses two problems in the way the DSCP field is formulated
 on the encapsulating header of IPv6 tunnels.

1) The IPv6 tunneling code was manipulating the DSCP field of the
 encapsulating packet using the 32b flowlabel. Since the flowlabel is
 only the lower 20b it was incorrect to assume that the upper 12b
 containing the DSCP and ECN fields would remain intact when formulating
 the encapsulating header. This fix handles the 'inherit' and
 'fixed-value' DSCP cases explicitly using the extant dsfield u8 variable.

2) The use of INET_ECN_encapsulate(0, dsfield) in ip6_tnl_xmit was
 incorrect and resulted in the DSCP value always being set to 0.

Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=195661
Signed-off-by: Peter Dawson <peter.a.dawson@boeing.com>
---
 net/ipv6/ip6_gre.c    | 13 +++++++------
 net/ipv6/ip6_tunnel.c | 21 +++++++++++++--------
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 8d128ba..0c5b4caa 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -537,11 +537,10 @@ static inline int ip6gre_xmit_ipv4(struct sk_buff *skb, struct net_device *dev)
 
 	memcpy(&fl6, &t->fl.u.ip6, sizeof(fl6));
 
-	dsfield = ipv4_get_dsfield(iph);
-
 	if (t->parms.flags & IP6_TNL_F_USE_ORIG_TCLASS)
-		fl6.flowlabel |= htonl((__u32)iph->tos << IPV6_TCLASS_SHIFT)
-					  & IPV6_TCLASS_MASK;
+		dsfield = ipv4_get_dsfield(iph);
+	else
+		dsfield = ip6_tclass(t->parms.flowinfo);
 	if (t->parms.flags & IP6_TNL_F_USE_ORIG_FWMARK)
 		fl6.flowi6_mark = skb->mark;
 	else
@@ -598,9 +597,11 @@ static inline int ip6gre_xmit_ipv6(struct sk_buff *skb, struct net_device *dev)
 
 	memcpy(&fl6, &t->fl.u.ip6, sizeof(fl6));
 
-	dsfield = ipv6_get_dsfield(ipv6h);
 	if (t->parms.flags & IP6_TNL_F_USE_ORIG_TCLASS)
-		fl6.flowlabel |= (*(__be32 *) ipv6h & IPV6_TCLASS_MASK);
+		dsfield = ipv6_get_dsfield(ipv6h);
+	else
+		dsfield = ip6_tclass(t->parms.flowinfo);
+
 	if (t->parms.flags & IP6_TNL_F_USE_ORIG_FLOWLABEL)
 		fl6.flowlabel |= ip6_flowlabel(ipv6h);
 	if (t->parms.flags & IP6_TNL_F_USE_ORIG_FWMARK)
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 6eb2ae5..7ae6c50 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1196,7 +1196,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield,
 	skb_push(skb, sizeof(struct ipv6hdr));
 	skb_reset_network_header(skb);
 	ipv6h = ipv6_hdr(skb);
-	ip6_flow_hdr(ipv6h, INET_ECN_encapsulate(0, dsfield),
+	ip6_flow_hdr(ipv6h, dsfield,
 		     ip6_make_flowlabel(net, skb, fl6->flowlabel, true, fl6));
 	ipv6h->hop_limit = hop_limit;
 	ipv6h->nexthdr = proto;
@@ -1231,8 +1231,6 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (tproto != IPPROTO_IPIP && tproto != 0)
 		return -1;
 
-	dsfield = ipv4_get_dsfield(iph);
-
 	if (t->parms.collect_md) {
 		struct ip_tunnel_info *tun_info;
 		const struct ip_tunnel_key *key;
@@ -1246,6 +1244,7 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 		fl6.flowi6_proto = IPPROTO_IPIP;
 		fl6.daddr = key->u.ipv6.dst;
 		fl6.flowlabel = key->label;
+		dsfield = ip6_tclass(key->label);
 	} else {
 		if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT))
 			encap_limit = t->parms.encap_limit;
@@ -1254,8 +1253,9 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 		fl6.flowi6_proto = IPPROTO_IPIP;
 
 		if (t->parms.flags & IP6_TNL_F_USE_ORIG_TCLASS)
-			fl6.flowlabel |= htonl((__u32)iph->tos << IPV6_TCLASS_SHIFT)
-					 & IPV6_TCLASS_MASK;
+			dsfield = ipv4_get_dsfield(iph);
+		else
+			dsfield = ip6_tclass(t->parms.flowinfo);
 		if (t->parms.flags & IP6_TNL_F_USE_ORIG_FWMARK)
 			fl6.flowi6_mark = skb->mark;
 		else
@@ -1267,6 +1267,8 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP6))
 		return -1;
 
+	dsfield = INET_ECN_encapsulate(dsfield, ipv4_get_dsfield(iph));
+
 	skb_set_inner_ipproto(skb, IPPROTO_IPIP);
 
 	err = ip6_tnl_xmit(skb, dev, dsfield, &fl6, encap_limit, &mtu,
@@ -1300,8 +1302,6 @@ ip6ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 	    ip6_tnl_addr_conflict(t, ipv6h))
 		return -1;
 
-	dsfield = ipv6_get_dsfield(ipv6h);
-
 	if (t->parms.collect_md) {
 		struct ip_tunnel_info *tun_info;
 		const struct ip_tunnel_key *key;
@@ -1315,6 +1315,7 @@ ip6ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 		fl6.flowi6_proto = IPPROTO_IPV6;
 		fl6.daddr = key->u.ipv6.dst;
 		fl6.flowlabel = key->label;
+		dsfield = ip6_tclass(key->label);
 	} else {
 		offset = ip6_tnl_parse_tlv_enc_lim(skb, skb_network_header(skb));
 		/* ip6_tnl_parse_tlv_enc_lim() might have reallocated skb->head */
@@ -1337,7 +1338,9 @@ ip6ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 		fl6.flowi6_proto = IPPROTO_IPV6;
 
 		if (t->parms.flags & IP6_TNL_F_USE_ORIG_TCLASS)
-			fl6.flowlabel |= (*(__be32 *)ipv6h & IPV6_TCLASS_MASK);
+			dsfield = ipv6_get_dsfield(ipv6h);
+		else
+			dsfield = ip6_tclass(t->parms.flowinfo);
 		if (t->parms.flags & IP6_TNL_F_USE_ORIG_FLOWLABEL)
 			fl6.flowlabel |= ip6_flowlabel(ipv6h);
 		if (t->parms.flags & IP6_TNL_F_USE_ORIG_FWMARK)
@@ -1351,6 +1354,8 @@ ip6ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP6))
 		return -1;
 
+	dsfield = INET_ECN_encapsulate(dsfield, ipv6_get_dsfield(ipv6h));
+
 	skb_set_inner_ipproto(skb, IPPROTO_IPV6);
 
 	err = ip6_tnl_xmit(skb, dev, dsfield, &fl6, encap_limit, &mtu,
-- 
2.7.4

^ permalink raw reply related

* Re: arch: arm: bpf: Converting cBPF to eBPF for arm 32 bit
From: Shubham Bansal @ 2017-05-23  5:03 UTC (permalink / raw)
  To: Kees Cook
  Cc: Florian Fainelli, Daniel Borkmann,
	kernel-hardening@lists.openwall.com, Network Development, ast,
	Mircea Gherzan, David Miller,
	linux-arm-kernel@lists.infradead.org, Nicolas Schichan, andrew
In-Reply-To: <CAGXu5jKwTDYeNQggezzSKkFzZQkroC+6SdBB0qUKh5crfNk8PA@mail.gmail.com>

Hi Kees,

I already have ARMv5 and ARMv6 code written. I just haven't tested it
yet. Should i send the patch with those as well ?

Best,
Shubham Bansal


On Tue, May 23, 2017 at 9:52 AM, Kees Cook <keescook@chromium.org> wrote:
> On Mon, May 22, 2017 at 8:34 PM, Shubham Bansal
> <illusionist.neo@gmail.com> wrote:
>> I would post them as soon as I test them on ARMv5 and ARMv6. If you
>> can help me with that, please let me know.
>
> Please post what you have: it would be better to see what you've got
> now in case additional changes are needed so you don't have to do it
> again on v5 and v6. Also, it means other people with real v5 and v6
> hardware could test for you if they were so inclined, and you won't
> need to be blocked on doing the tests in qemu.
>
> You can send it as an "RFC" in the subject, just to make sure people
> know it's not considered fully done. :)
>
> -Kees
>
> --
> Kees Cook
> Pixel Security

^ permalink raw reply

* Re: [patch net-next RFC] net: sched: cls_api: make reclassify return all the way back to the original tp
From: Jiri Pirko @ 2017-05-23  5:15 UTC (permalink / raw)
  To: Cong Wang
  Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
	David Ahern, Eric Dumazet, Stephen Hemminger, Daniel Borkmann,
	Alexander Duyck, Simon Horman, mlxsw
In-Reply-To: <CAM_iQpUUfrCvaLUeEOyzCYg9_9sZ3URZeVaP13R7OTX9tLOQuQ@mail.gmail.com>

Tue, May 23, 2017 at 01:57:47AM CEST, xiyou.wangcong@gmail.com wrote:
>On Mon, May 22, 2017 at 8:09 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> With the introduction of chain goto action, the reclassification would
>> cause the re-iteration of the actual chain. But it perhaps makes more
>> sense to restart the whole thing. Thoughts?
>
>I think reclassification is meant to restart the whole logic rather
>than just one chain. So your patch makes sense to me, but not
>sure if there is any corner case I miss.

Agreed. Will send v1. Thanks

^ permalink raw reply

* Re: [patch net-next 2/2] net/sched: fix filter flushing
From: Jiri Pirko @ 2017-05-23  5:17 UTC (permalink / raw)
  To: Cong Wang
  Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
	Eric Dumazet, Daniel Borkmann, Simon Horman, mlxsw, Colin King
In-Reply-To: <CAM_iQpVvsMhTVh7P5qL19dhbWitPPwN0FcFKW9F0eDJAkr5ViQ@mail.gmail.com>

Mon, May 22, 2017 at 11:04:58PM CEST, xiyou.wangcong@gmail.com wrote:
>On Mon, May 22, 2017 at 1:54 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Sun, May 21, 2017 at 12:19 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>>>You can't claim you really delete it as long as actions can still
>>>>see it and dump it.
>>>
>>> No, user just wants to delete all the filters. That is done. User does
>>> not care if the actual chain structure is there or not.
>>>
>>
>> Hmm, so users see a chain with no filters... Fair enough.
>
>But since you remove the chain from the chain_list, it means
>users could not add new filters to this chain after flushing? And

No, in flush, I don't remove it from the list. That is not in the
patch. Why would you think so?


>users could create a new chain with the same index??
>
>If so, you should instead keep it in the chain_list, although empty.

^ permalink raw reply

* [PATCH net v2 1/2] net: ieee802154: remove explicit set skb->sk
From: Lin Zhang @ 2017-05-23  5:21 UTC (permalink / raw)
  To: aar, stefan, davem; +Cc: linux-wpan, netdev, linux-kernel, Lin Zhang

Explicit set skb->sk is needless, sock_alloc_send_skb is already set it.

Signed-off-by: Lin Zhang <xiaolou4617@gmail.com>
Acked-by: Stefan Schmidt <stefan@osg.samsung.com>
---
changelog:

v1 -> v2:
        * split v1 into two patches, per Stefan Schmidt.

Thanks to Stefan Schmidt for reviewing !
---
 net/ieee802154/socket.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
index eedba76..b01a1f0 100644
--- a/net/ieee802154/socket.c
+++ b/net/ieee802154/socket.c
@@ -301,7 +301,6 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 		goto out_skb;
 
 	skb->dev = dev;
-	skb->sk  = sk;
 	skb->protocol = htons(ETH_P_IEEE802154);
 
 	dev_put(dev);
@@ -690,7 +689,6 @@ static int dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 		goto out_skb;
 
 	skb->dev = dev;
-	skb->sk  = sk;
 	skb->protocol = htons(ETH_P_IEEE802154);
 
 	dev_put(dev);
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net 0/2] sctp: a bunch of fixes for processing dupcookie
From: Xin Long @ 2017-05-23  5:28 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman

After introducing transport hashtable and per stream info into sctp,
some regressions were caused when processing dupcookie, this patchset
is to fix them.

Xin Long (2):
  sctp: fix stream update when processing dupcookie
  sctp: set new_asoc temp when processing dupcookie

 net/sctp/associola.c     |  4 +++-
 net/sctp/sm_make_chunk.c | 13 ++++---------
 net/sctp/sm_statefuns.c  |  3 +++
 3 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.1.0

^ permalink raw reply

* [PATCH net 1/2] sctp: fix stream update when processing dupcookie
From: Xin Long @ 2017-05-23  5:28 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman
In-Reply-To: <cover.1495517205.git.lucien.xin@gmail.com>

Since commit 3dbcc105d556 ("sctp: alloc stream info when initializing
asoc"), stream and stream.out info are always alloced when creating
an asoc.

So it's not correct to check !asoc->stream before updating stream
info when processing dupcookie, but would be better to check asoc
state instead.

Fixes: 3dbcc105d556 ("sctp: alloc stream info when initializing asoc")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/associola.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index a9708da..9523828 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1176,7 +1176,9 @@ void sctp_assoc_update(struct sctp_association *asoc,
 
 		asoc->ctsn_ack_point = asoc->next_tsn - 1;
 		asoc->adv_peer_ack_point = asoc->ctsn_ack_point;
-		if (!asoc->stream) {
+
+		if (sctp_state(asoc, COOKIE_WAIT)) {
+			sctp_stream_free(asoc->stream);
 			asoc->stream = new->stream;
 			new->stream = NULL;
 		}
-- 
2.1.0

^ permalink raw reply related

* [PATCH net 2/2] sctp: set new_asoc temp when processing dupcookie
From: Xin Long @ 2017-05-23  5:28 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman
In-Reply-To: <cover.1495517205.git.lucien.xin@gmail.com>

After sctp changed to use transport hashtable, a transport would be
added into global hashtable when adding the peer to an asoc, then
the asoc can be got by searching the transport in the hashtbale.

The problem is when processing dupcookie in sctp_sf_do_5_2_4_dupcook,
a new asoc would be created. A peer with the same addr and port as
the one in the old asoc might be added into the new asoc, but fail
to be added into the hashtable, as they also belong to the same sk.

It causes that sctp's dupcookie processing can not really work.

Since the new asoc will be freed after copying it's information to
the old asoc, it's more like a temp asoc. So this patch is to fix
it by setting it as a temp asoc to avoid adding it's any transport
into the hashtable and also avoid allocing assoc_id.

An extra thing it has to do is to also alloc stream info for any
temp asoc, as sctp dupcookie process needs it to update old asoc.
But I don't think it would hurt something, as a temp asoc would
always be freed after finishing processing cookie echo packet.

Reported-by: Jianwen Ji <jiji@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/sm_make_chunk.c | 13 ++++---------
 net/sctp/sm_statefuns.c  |  3 +++
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 8a08f13..92e332e 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -2454,16 +2454,11 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
 	 * stream sequence number shall be set to 0.
 	 */
 
-	/* Allocate storage for the negotiated streams if it is not a temporary
-	 * association.
-	 */
-	if (!asoc->temp) {
-		if (sctp_stream_init(asoc, gfp))
-			goto clean_up;
+	if (sctp_stream_init(asoc, gfp))
+		goto clean_up;
 
-		if (sctp_assoc_set_id(asoc, gfp))
-			goto clean_up;
-	}
+	if (!asoc->temp && sctp_assoc_set_id(asoc, gfp))
+		goto clean_up;
 
 	/* ADDIP Section 4.1 ASCONF Chunk Procedures
 	 *
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 4f5e6cf..f863b55 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -2088,6 +2088,9 @@ sctp_disposition_t sctp_sf_do_5_2_4_dupcook(struct net *net,
 		}
 	}
 
+	/* Set temp so that it won't be added into hashtable */
+	new_asoc->temp = 1;
+
 	/* Compare the tie_tag in cookie with the verification tag of
 	 * current association.
 	 */
-- 
2.1.0

^ permalink raw reply related

* [PATCH net v2 2/2] net: ieee802154: fix net_device reference release too early
From: Lin Zhang @ 2017-05-23  5:29 UTC (permalink / raw)
  To: aar, stefan, davem; +Cc: linux-wpan, netdev, linux-kernel, Lin Zhang

This patch fixes the kernel oops when release net_device reference in
advance. In function raw_sendmsg(i think the dgram_sendmsg has the same
problem), there is a race condition between dev_put and dev_queue_xmit
when the device is gong that maybe lead to dev_queue_ximt to see
an illegal net_device pointer.

My test kernel is 3.13.0-32 and because i am not have a real 802154 
device, so i change lowpan_newlink function to this:

        /* find and hold real wpan device */
        real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
        if (!real_dev)
                return -ENODEV;
//      if (real_dev->type != ARPHRD_IEEE802154) {
//              dev_put(real_dev);
//              return -EINVAL;
//      }
        lowpan_dev_info(dev)->real_dev = real_dev;
        lowpan_dev_info(dev)->fragment_tag = 0;
        mutex_init(&lowpan_dev_info(dev)->dev_list_mtx);

Also, in order to simulate preempt, i change the raw_sendmsg function 
to this:

        skb->dev = dev;
        skb->sk  = sk;
        skb->protocol = htons(ETH_P_IEEE802154);
        dev_put(dev);
        //simulate preempt
        schedule_timeout_uninterruptible(30 * HZ);
        err = dev_queue_xmit(skb);
        if (err > 0)
                err = net_xmit_errno(err);

and this is my userspace test code named test_send_data:

int main(int argc, char **argv)
{
        char buf[127];
        int sockfd;
        sockfd = socket(AF_IEEE802154, SOCK_RAW, 0);
        if (sockfd < 0) {
                printf("create sockfd error: %s\n", strerror(errno));
                return -1;
        }
        send(sockfd, buf, sizeof(buf), 0);
        return 0;
}


This is my test case:

root@zhanglin-x-computer:~/develop/802154# uname -a
Linux zhanglin-x-computer 3.13.0-32-generic #57-Ubuntu SMP Tue Jul 15
03:51:08 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
root@zhanglin-x-computer:~/develop/802154# ip link add link eth0 name
lowpan0 type lowpan
root@zhanglin-x-computer:~/develop/802154#
//keep the lowpan0 device down
root@zhanglin-x-computer:~/develop/802154# ./test_send_data &
//wait a while
root@zhanglin-x-computer:~/develop/802154# ip link del link dev lowpan0
//the device is gone
//oops
[381.303307] general protection fault: 0000 [#1]SMP
[381.303407] Modules linked in: af_802154 6lowpan bnep rfcomm
bluetooth nls_iso8859_1 snd_hda_codec_hdmi snd_hda_codec_realtek
rts5139(C) snd_hda_intel
snd_had_codec snd_hwdep snd_pcm snd_page_alloc snd_seq_midi
snd_seq_midi_event snd_rawmidi snd_req intel_rapl snd_seq_device
coretemp i915 kvm_intel
kvm snd_timer snd crct10dif_pclmul crc32_pclmul ghash_clmulni_intel
cypted drm_kms_helper drm i2c_algo_bit soundcore video mac_hid
parport_pc ppdev ip parport hid_generic
usbhid hid ahci r8169 mii libahdi
[381.304286] CPU:1 PID: 2524 Commm: 1 Tainted: G C 0 3.13.0-32-generic
[381.304409] Hardware name: Haier Haier DT Computer/Haier DT Codputer,
BIOS FIBT19H02_X64 06/09/2014
[381.304546] tasks: ffff000096965fc0 ti: ffffB0013779c000 task.ti:
ffffB8013779c000
[381.304659] RIP: 0010:[<ffffffff01621fe1>] [<ffffffff81621fe1>]
__dev_queue_ximt+0x61/0x500
[381.304798] RSP: 0018:ffffB8013779dca0 EFLAGS: 00010202
[381.304880] RAX: 272b031d57565351 RBX: 0000000000000000 RCX: ffff8800968f1a00
[381.304987] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8800968f1a00
[381.305095] RBP: ffff8e013773dce0 R08: 0000000000000266 R09: 0000000000000004
[381.305202] R10: 0000000000000004 R11: 0000000000000005 R12: ffff88013902e000
[381.305310] R13: 000000000000007f R14: 000000000000007f R15: ffff8800968f1a00
[381.305418] FS:  00007fc57f50f740(0000) GS: ffff88013fc80000(0000)
knlGS: 0000000000000000
[381.305540] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[381.305627] CR2: 00007fad0841c000 CR3: 00000001368dd000 CR4: 00000000001007e0
[361.905734] Stack:
[381.305768]  00000000002052d0 000000003facb30a ffff88013779dcc0
ffff880137764000
[381.305898]  ffff88013779de70 000000000000007f 000000000000007f
ffff88013902e000
[381.306026]  ffff88013779dcf0 ffffffff81622490 ffff88013779dd39
ffffffffa03af9f1
[381.306155] Call Trace:
[381.306202]  [<ffffffff81622490>] dev_queue_xmit+0x10/0x20
[381.306294]  [<ffffffffa03af9f1>] raw_sendmsg+0x1b1/0x270 [af_802154]
[381.306396]  [<ffffffffa03af054>] ieee802154_sock_sendmsg+0x14/0x20 [af_802154]
[381.306512]  [<ffffffff816079eb>] sock_sendmsg+0x8b/0xc0
[381.306600]  [<ffffffff811d52a5>] ? __d_alloc+0x25/0x180
[381.306687]  [<ffffffff811a1f56>] ? kmem_cache_alloc_trace+0x1c6/0x1f0
[381.306791]  [<ffffffff81607b91>] SYSC_sendto+0x121/0x1c0
[381.306878]  [<ffffffff8109ddf4>] ? vtime_account_user+x54/0x60
[381.306975]  [<ffffffff81020d45>] ? syscall_trace_enter+0x145/0x250
[381.307073]  [<ffffffff816086ae>] SyS_sendto+0xe/0x10
[381.307156]  [<ffffffff8172c87f>] tracesys+0xe1/0xe6
[381.307233] Code: c6 a1 a4 ff 41 8b 57 78 49 8b 47 20 85 d2 48 8b 80
78 07 00 00 75 21 49 8b 57 18 48 85 d2 74 18 48 85 c0 74 13 8b 92 ac
01 00 00 <3b> 50 10 73 08 8b 44 90 14 41 89 47 78 41 f6 84 24 d5 00 00
00
[381.307801] RIP [<ffffffff81621fe1>] _dev_queue_xmit+0x61/0x500
[381.307901]  RSP <ffff88013779dca0>
[381.347512] Kernel panic - not syncing: Fatal exception in interrupt
[381.347747] drm_kms_helper: panic occurred, switching back to text console

In my opinion, there is always exist a chance that the device is gong
before call dev_queue_xmit.

I think the latest kernel is have the same problem and that 
dev_put should be behind of the dev_queue_xmit.

Signed-off-by: Lin Zhang <xiaolou4617@gmail.com>
Acked-by: Stefan Schmidt <stefan@osg.samsung.com>
---
changelog:

v1 -> v2:
        * split v1 into two patches, per Stefan Schmidt.

Hello, Stefan:
	If you have a real 802154 device, maybe use the test case as above, thanks.

Thanks to Stefan Schmidt for reviewing !
---
 net/ieee802154/socket.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
index b01a1f0..a60658c 100644
--- a/net/ieee802154/socket.c
+++ b/net/ieee802154/socket.c
@@ -303,12 +303,12 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 	skb->dev = dev;
 	skb->protocol = htons(ETH_P_IEEE802154);
 
-	dev_put(dev);
-
 	err = dev_queue_xmit(skb);
 	if (err > 0)
 		err = net_xmit_errno(err);
 
+	dev_put(dev);
+
 	return err ?: size;
 
 out_skb:
@@ -691,12 +691,12 @@ static int dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 	skb->dev = dev;
 	skb->protocol = htons(ETH_P_IEEE802154);
 
-	dev_put(dev);
-
 	err = dev_queue_xmit(skb);
 	if (err > 0)
 		err = net_xmit_errno(err);
 
+	dev_put(dev);
+
 	return err ?: size;
 
 out_skb:
-- 
1.8.3.1

^ permalink raw reply related

* pull request (net): ipsec 2017-05-23
From: Steffen Klassert @ 2017-05-23  5:33 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

1) Fix wrong header offset for esp4 udpencap packets.

2) Fix a stack access out of bounds when creating a bundle
   with sub policies. From Sabrina Dubroca.

3) Fix slab-out-of-bounds in pfkey due to an incorrect
   sadb_x_sec_len calculation.

4) We checked the wrong feature flags when taking down
   an interface with IPsec offload enabled.
   Fix from Ilan Tayari.

5) Copy the anti replay sequence numbers when doing a state
   migration, otherwise we get out of sync with the sequence
   numbers. Fix from Antony Antony.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit f411af6822182f84834c4881b825dd40534e7fe8:

  Merge branch 'ibmvnic-Updated-reset-handler-andcode-fixes' (2017-05-03 11:33:06 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master

for you to fetch changes up to a486cd23661c9387fb076c3f6ae8b2aa9d20d54a:

  xfrm: fix state migration copy replay sequence numbers (2017-05-19 12:49:13 +0200)

----------------------------------------------------------------
Antony Antony (1):
      xfrm: fix state migration copy replay sequence numbers

Ilan Tayari (1):
      xfrm: Fix NETDEV_DOWN with IPSec offload

Sabrina Dubroca (1):
      xfrm: fix stack access out of bounds with CONFIG_XFRM_SUB_POLICY

Steffen Klassert (2):
      esp4: Fix udpencap for local TCP packets.
      af_key: Fix slab-out-of-bounds in pfkey_compile_policy.

 include/net/xfrm.h     | 10 ----------
 net/ipv4/esp4.c        |  5 ++++-
 net/key/af_key.c       |  2 +-
 net/xfrm/xfrm_device.c |  2 +-
 net/xfrm/xfrm_policy.c | 47 -----------------------------------------------
 net/xfrm/xfrm_state.c  |  2 ++
 6 files changed, 8 insertions(+), 60 deletions(-)

^ permalink raw reply

* [PATCH 1/5] esp4: Fix udpencap for local TCP packets.
From: Steffen Klassert @ 2017-05-23  5:33 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1495517624-27096-1-git-send-email-steffen.klassert@secunet.com>

Locally generated TCP packets are usually cloned, so we
do skb_cow_data() on this packets. After that we need to
reload the pointer to the esp header. On udpencap this
header has an offset to skb_transport_header, so take this
offset into account.

Fixes: 67d349ed603 ("net/esp4: Fix invalid esph pointer crash")
Fixes: fca11ebde3f0 ("esp4: Reorganize esp_output")
Reported-by: Don Bowman <db@donbowman.ca>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/esp4.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 65cc02b..93322f8 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -248,6 +248,7 @@ int esp_output_head(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
 	u8 *tail;
 	u8 *vaddr;
 	int nfrags;
+	int esph_offset;
 	struct page *page;
 	struct sk_buff *trailer;
 	int tailen = esp->tailen;
@@ -313,11 +314,13 @@ int esp_output_head(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
 	}
 
 cow:
+	esph_offset = (unsigned char *)esp->esph - skb_transport_header(skb);
+
 	nfrags = skb_cow_data(skb, tailen, &trailer);
 	if (nfrags < 0)
 		goto out;
 	tail = skb_tail_pointer(trailer);
-	esp->esph = ip_esp_hdr(skb);
+	esp->esph = (struct ip_esp_hdr *)(skb_transport_header(skb) + esph_offset);
 
 skip_cow:
 	esp_output_fill_trailer(tail, esp->tfclen, esp->plen, esp->proto);
-- 
2.7.4

^ permalink raw reply related

* [PATCH 3/5] af_key: Fix slab-out-of-bounds in pfkey_compile_policy.
From: Steffen Klassert @ 2017-05-23  5:33 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1495517624-27096-1-git-send-email-steffen.klassert@secunet.com>

The sadb_x_sec_len is stored in the unit 'byte divided by eight'.
So we have to multiply this value by eight before we can do
size checks. Otherwise we may get a slab-out-of-bounds when
we memcpy the user sec_ctx.

Fixes: df71837d502 ("[LSM-IPSec]: Security association restriction.")
Reported-by: Andrey Konovalov <andreyknvl@google.com>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/key/af_key.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index c1950bb..512dc43 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -3285,7 +3285,7 @@ static struct xfrm_policy *pfkey_compile_policy(struct sock *sk, int opt,
 		p += pol->sadb_x_policy_len*8;
 		sec_ctx = (struct sadb_x_sec_ctx *)p;
 		if (len < pol->sadb_x_policy_len*8 +
-		    sec_ctx->sadb_x_sec_len) {
+		    sec_ctx->sadb_x_sec_len*8) {
 			*dir = -EINVAL;
 			goto out;
 		}
-- 
2.7.4

^ permalink raw reply related

* [PATCH 4/5] xfrm: Fix NETDEV_DOWN with IPSec offload
From: Steffen Klassert @ 2017-05-23  5:33 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1495517624-27096-1-git-send-email-steffen.klassert@secunet.com>

From: Ilan Tayari <ilant@mellanox.com>

Upon NETDEV_DOWN event, all xfrm_state objects which are bound to
the device are flushed.

The condition for this is wrong, though, testing dev->hw_features
instead of dev->features. If a device has non-user-modifiable
NETIF_F_HW_ESP, then its xfrm_state objects are not flushed,
causing a crash later on after the device is deleted.

Check dev->features instead of dev->hw_features.

Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API")
Signed-off-by: Ilan Tayari <ilant@mellanox.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index 8ec8a3f..574e6f3 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -170,7 +170,7 @@ static int xfrm_dev_feat_change(struct net_device *dev)
 
 static int xfrm_dev_down(struct net_device *dev)
 {
-	if (dev->hw_features & NETIF_F_HW_ESP)
+	if (dev->features & NETIF_F_HW_ESP)
 		xfrm_dev_state_flush(dev_net(dev), dev, true);
 
 	xfrm_garbage_collect(dev_net(dev));
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/5] xfrm: fix stack access out of bounds with CONFIG_XFRM_SUB_POLICY
From: Steffen Klassert @ 2017-05-23  5:33 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1495517624-27096-1-git-send-email-steffen.klassert@secunet.com>

From: Sabrina Dubroca <sd@queasysnail.net>

When CONFIG_XFRM_SUB_POLICY=y, xfrm_dst stores a copy of the flowi for
that dst. Unfortunately, the code that allocates and fills this copy
doesn't care about what type of flowi (flowi, flowi4, flowi6) gets
passed. In multiple code paths (from raw_sendmsg, from TCP when
replying to a FIN, in vxlan, geneve, and gre), the flowi that gets
passed to xfrm is actually an on-stack flowi4, so we end up reading
stuff from the stack past the end of the flowi4 struct.

Since xfrm_dst->origin isn't used anywhere following commit
ca116922afa8 ("xfrm: Eliminate "fl" and "pol" args to
xfrm_bundle_ok()."), just get rid of it.  xfrm_dst->partner isn't used
either, so get rid of that too.

Fixes: 9d6ec938019c ("ipv4: Use flowi4 in public route lookup interfaces.")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/net/xfrm.h     | 10 ----------
 net/xfrm/xfrm_policy.c | 47 -----------------------------------------------
 2 files changed, 57 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 6793a30c..7e7e2b0 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -979,10 +979,6 @@ struct xfrm_dst {
 	struct flow_cache_object flo;
 	struct xfrm_policy *pols[XFRM_POLICY_TYPE_MAX];
 	int num_pols, num_xfrms;
-#ifdef CONFIG_XFRM_SUB_POLICY
-	struct flowi *origin;
-	struct xfrm_selector *partner;
-#endif
 	u32 xfrm_genid;
 	u32 policy_genid;
 	u32 route_mtu_cached;
@@ -998,12 +994,6 @@ static inline void xfrm_dst_destroy(struct xfrm_dst *xdst)
 	dst_release(xdst->route);
 	if (likely(xdst->u.dst.xfrm))
 		xfrm_state_put(xdst->u.dst.xfrm);
-#ifdef CONFIG_XFRM_SUB_POLICY
-	kfree(xdst->origin);
-	xdst->origin = NULL;
-	kfree(xdst->partner);
-	xdst->partner = NULL;
-#endif
 }
 #endif
 
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index b00a1d5..ed4e52d 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1797,43 +1797,6 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
 	goto out;
 }
 
-#ifdef CONFIG_XFRM_SUB_POLICY
-static int xfrm_dst_alloc_copy(void **target, const void *src, int size)
-{
-	if (!*target) {
-		*target = kmalloc(size, GFP_ATOMIC);
-		if (!*target)
-			return -ENOMEM;
-	}
-
-	memcpy(*target, src, size);
-	return 0;
-}
-#endif
-
-static int xfrm_dst_update_parent(struct dst_entry *dst,
-				  const struct xfrm_selector *sel)
-{
-#ifdef CONFIG_XFRM_SUB_POLICY
-	struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
-	return xfrm_dst_alloc_copy((void **)&(xdst->partner),
-				   sel, sizeof(*sel));
-#else
-	return 0;
-#endif
-}
-
-static int xfrm_dst_update_origin(struct dst_entry *dst,
-				  const struct flowi *fl)
-{
-#ifdef CONFIG_XFRM_SUB_POLICY
-	struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
-	return xfrm_dst_alloc_copy((void **)&(xdst->origin), fl, sizeof(*fl));
-#else
-	return 0;
-#endif
-}
-
 static int xfrm_expand_policies(const struct flowi *fl, u16 family,
 				struct xfrm_policy **pols,
 				int *num_pols, int *num_xfrms)
@@ -1905,16 +1868,6 @@ xfrm_resolve_and_create_bundle(struct xfrm_policy **pols, int num_pols,
 
 	xdst = (struct xfrm_dst *)dst;
 	xdst->num_xfrms = err;
-	if (num_pols > 1)
-		err = xfrm_dst_update_parent(dst, &pols[1]->selector);
-	else
-		err = xfrm_dst_update_origin(dst, fl);
-	if (unlikely(err)) {
-		dst_free(dst);
-		XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTBUNDLECHECKERROR);
-		return ERR_PTR(err);
-	}
-
 	xdst->num_pols = num_pols;
 	memcpy(xdst->pols, pols, sizeof(struct xfrm_policy *) * num_pols);
 	xdst->policy_genid = atomic_read(&pols[0]->genid);
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox