* Re: [Patch net] act_sample: get rid of tcf_sample_cleanup_rcu()
From: Cong Wang @ 2017-11-29 18:34 UTC (permalink / raw)
To: Linux Kernel Network Developers
Cc: Eric Dumazet, Cong Wang, Jamal Hadi Salim, Jiri Pirko, Yotam Gigi
In-Reply-To: <20171129183234.2541-1-xiyou.wangcong@gmail.com>
On Wed, Nov 29, 2017 at 10:32 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> Similar to commit d7fb60b9cafb ("net_sched: get rid of tcfa_rcu"),
> TC actions don't need to respect RCU grace period, because it
> is either just detached from tc filter (standalone case) or
> it is removed together with tc filter (bound case) in which case
> RCU grace period is already respected at filter layer.
The rcu_barrier() now can be removed too...
I will send v2 shortly.
^ permalink raw reply
* [Patch net] act_sample: get rid of tcf_sample_cleanup_rcu()
From: Cong Wang @ 2017-11-29 18:32 UTC (permalink / raw)
To: netdev; +Cc: eric.dumazet, Cong Wang, Jamal Hadi Salim, Jiri Pirko, Yotam Gigi
Similar to commit d7fb60b9cafb ("net_sched: get rid of tcfa_rcu"),
TC actions don't need to respect RCU grace period, because it
is either just detached from tc filter (standalone case) or
it is removed together with tc filter (bound case) in which case
RCU grace period is already respected at filter layer.
Fixes: 5c5670fae430 ("net/sched: Introduce sample tc action")
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: Yotam Gigi <yotamg@mellanox.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/net/tc_act/tc_sample.h | 1 -
net/sched/act_sample.c | 13 +++----------
2 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/include/net/tc_act/tc_sample.h b/include/net/tc_act/tc_sample.h
index 524cee4f4c81..01dbfea32672 100644
--- a/include/net/tc_act/tc_sample.h
+++ b/include/net/tc_act/tc_sample.h
@@ -14,7 +14,6 @@ struct tcf_sample {
struct psample_group __rcu *psample_group;
u32 psample_group_num;
struct list_head tcfm_list;
- struct rcu_head rcu;
};
#define to_sample(a) ((struct tcf_sample *)a)
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index 8b5abcd2f32f..83402e0434b2 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -96,23 +96,16 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
return ret;
}
-static void tcf_sample_cleanup_rcu(struct rcu_head *rcu)
+static void tcf_sample_cleanup(struct tc_action *a, int bind)
{
- struct tcf_sample *s = container_of(rcu, struct tcf_sample, rcu);
+ struct tcf_sample *s = to_sample(a);
struct psample_group *psample_group;
- psample_group = rcu_dereference_protected(s->psample_group, 1);
+ psample_group = rtnl_dereference(s->psample_group);
RCU_INIT_POINTER(s->psample_group, NULL);
psample_group_put(psample_group);
}
-static void tcf_sample_cleanup(struct tc_action *a, int bind)
-{
- struct tcf_sample *s = to_sample(a);
-
- call_rcu(&s->rcu, tcf_sample_cleanup_rcu);
-}
-
static bool tcf_sample_dev_ok_push(struct net_device *dev)
{
switch (dev->type) {
--
2.13.0
^ permalink raw reply related
* Fw: [Bug 198025] New: <unknown>: hw csum failure
From: Stephen Hemminger @ 2017-11-29 18:25 UTC (permalink / raw)
To: netdev
Looks like a virtio bug
Begin forwarded message:
Date: Wed, 29 Nov 2017 08:16:45 +0000
From: bugzilla-daemon@bugzilla.kernel.org
To: stephen@networkplumber.org
Subject: [Bug 198025] New: <unknown>: hw csum failure
https://bugzilla.kernel.org/show_bug.cgi?id=198025
Bug ID: 198025
Summary: <unknown>: hw csum failure
Product: Networking
Version: 2.5
Kernel Version: 4.8.13-1.el6.elrepo.x86_64
Hardware: x86-64
OS: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Other
Assignee: stephen@networkplumber.org
Reporter: svimik@gmail.com
Regression: No
Created attachment 260919
--> https://bugzilla.kernel.org/attachment.cgi?id=260919&action=edit
oops message
<unknown>: hw csum failure
CPU: 3 PID: 2741 Comm: openvpn Not tainted 4.8.13-1.el6.elrepo.x86_64 #1
Hardware name: /DH61WW, BIOS BEH6110H.86A.0034.2011.1018.1937
10/18/2011
0000000000000000 ffff880119a2faa8 ffffffff8134d2ef 0000000000000001
000000029083b90d 0000000000000000 ffff88008e629400 ffff880119a2fac8
ffffffff816602f2 ffff880119a2fdc8 0000000000000000 ffff880119a2fb08
Call Trace:
[<ffffffff8134d2ef>] dump_stack+0x67/0x98
[<ffffffff816602f2>] netdev_rx_csum_fault+0x42/0x50
[<ffffffff816563d4>] skb_copy_and_csum_datagram_msg+0xc4/0xf0
[<ffffffff816dd186>] udp_recvmsg+0x286/0x510
[<ffffffff816e67e1>] inet_recvmsg+0x51/0x90
[<ffffffff8164642a>] sock_recvmsg+0x4a/0x60
[<ffffffff8164700b>] ___sys_recvmsg+0xeb/0x210
[<ffffffff810af434>] ? try_to_wake_up+0x54/0x260
[<ffffffff811079a1>] ? get_futex_key+0x251/0x3f0
[<ffffffff81240cec>] ? __fget_light+0x2c/0x70
----cut----
A full message of this oops can be found in attachment. Message was received
using netconsole.
--
You are receiving this mail because:
You are the assignee for the bug.
^ permalink raw reply
* Re: [BUG] net/sched: use after free
From: Cong Wang @ 2017-11-29 18:05 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1511976722.16595.22.camel@gmail.com>
On Wed, Nov 29, 2017 at 9:32 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Hi
>
> Running the following script twice gave me a KASAN splat on net tree,
> (this is on commit f6454f80e8a965fca203dab28723f68ec78db608 )
>
>
> ETH=eth0
>
> tc qdisc del dev $ETH ingress 2>/dev/null
> tc qdisc add dev $ETH ingress
>
> tc filter add dev eth0 parent ffff: \
> matchall action sample rate 12 group 4
I blame tcf_sample_cleanup_rcu(). And clearly it is not netns cleanup
path this time. Let me see how we can fix this.
Thanks for the report!
^ permalink raw reply
* Re: [PATCH net-next 0/3] rxrpc: Fixes
From: David Miller @ 2017-11-29 18:09 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
In-Reply-To: <21463.1511976341@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Wed, 29 Nov 2017 17:25:41 +0000
> David Miller <davem@davemloft.net> wrote:
>
>> This email says "net-next", yet your patches say "net".
>
> Sorry about that - it should be 'net'. I copied an old cover note. All the
> patches have a macro substitution, but the cover note does not. Do you want
> me to repost?
It is not necessary to repost, thanks.
^ permalink raw reply
* Re: [PATCH 4/4] selinux: Add SCTP support
From: kbuild test robot @ 2017-11-29 18:08 UTC (permalink / raw)
To: Richard Haines
Cc: kbuild-all, selinux, netdev, linux-sctp, linux-security-module,
paul, vyasevich, nhorman, sds, eparis, marcelo.leitner,
Richard Haines
In-Reply-To: <20171127193217.2768-1-richard_c_haines@btinternet.com>
[-- Attachment #1: Type: text/plain, Size: 2099 bytes --]
Hi Richard,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on security/next]
[cannot apply to net-next/master net/master v4.15-rc1 next-20171129]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Richard-Haines/Add-SELinux-SCTP-protocol-support/20171129-222900
base: https://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next
config: x86_64-allyesdebian (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
security/selinux/hooks.c: In function 'selinux_sctp_sk_clone':
>> security/selinux/hooks.c:5185:10: error: 'struct sk_security_struct' has no member named 'nlbl_state'
newsksec->nlbl_state = sksec->nlbl_state;
^~
security/selinux/hooks.c:5185:30: error: 'struct sk_security_struct' has no member named 'nlbl_state'
newsksec->nlbl_state = sksec->nlbl_state;
^~
vim +5185 security/selinux/hooks.c
5168
5169 /* Called whenever a new socket is created by accept(2) or sctp_peeloff(3). */
5170 static void selinux_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
5171 struct sock *newsk)
5172 {
5173 struct sk_security_struct *sksec = sk->sk_security;
5174 struct sk_security_struct *newsksec = newsk->sk_security;
5175
5176 /* If policy does not support SECCLASS_SCTP_SOCKET then call
5177 * the non-sctp clone version.
5178 */
5179 if (!selinux_policycap_extsockclass)
5180 return selinux_sk_clone_security(sk, newsk);
5181
5182 newsksec->sid = ep->secid;
5183 newsksec->peer_sid = ep->peer_secid;
5184 newsksec->sclass = sksec->sclass;
> 5185 newsksec->nlbl_state = sksec->nlbl_state;
5186 }
5187
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39266 bytes --]
^ permalink raw reply
* Re: [PATCHv2 2/2] userns: control capabilities of some user namespaces
From: Serge E. Hallyn @ 2017-11-29 17:57 UTC (permalink / raw)
To: Mahesh Bandewar (महेश बंडेवार)
Cc: Serge E. Hallyn, Mahesh Bandewar, LKML, Netdev, Kernel-hardening,
Linux API, Kees Cook, Eric W . Biederman, Eric Dumazet,
David Miller
In-Reply-To: <CAF2d9jhCDasnf2tbaGA04MV5ygZ9o1FaVRCEU2TkHjZR7d1ifw@mail.gmail.com>
Quoting Mahesh Bandewar (महेश बंडेवार) (maheshb@google.com):
> On Tue, Nov 28, 2017 at 3:04 PM, Serge E. Hallyn <serge@hallyn.com> wrote:
> > Quoting Mahesh Bandewar (महेश बंडेवार) (maheshb@google.com):
> > ...
> >> >> diff --git a/security/commoncap.c b/security/commoncap.c
> >> >> index fc46f5b85251..89103f16ac37 100644
> >> >> --- a/security/commoncap.c
> >> >> +++ b/security/commoncap.c
> >> >> @@ -73,6 +73,14 @@ int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
> >> >> {
> >> >> struct user_namespace *ns = targ_ns;
> >> >>
> >> >> + /* If the capability is controlled and user-ns that process
> >> >> + * belongs-to is 'controlled' then return EPERM and no need
> >> >> + * to check the user-ns hierarchy.
> >> >> + */
> >> >> + if (is_user_ns_controlled(cred->user_ns) &&
> >> >> + is_capability_controlled(cap))
> >> >> + return -EPERM;
> >> >
> >> > I'd be curious to see the performance impact on this on a regular
> >> > workload (kernel build?) in a controlled ns.
> >> >
> >> Should it affect? If at all, it should be +ve since, the recursive
> >> user-ns hierarchy lookup is avoided with the above check if the
> >> capability is controlled.
> >
> > Yes but I expect that to be the rare case for normal lxc installs
> > (which are of course what I am interested in)
> >
> >> The additional cost otherwise is this check
> >> per cap_capable() call.
> >
> > And pipeline refetching?
> >
> > Capability calls also shouldn't be all that frequent, but still I'm
> > left wondering...
>
> Correct, and capability checks are part of the control-path and not
> the data-path so shouldn't matter but I guess it doesn't hurt to
> find-out the number. Do you have any workload in mind, that we can use
> for this test/benchmark?
I suppose if you did both (a) a kernel build and (b) a webserver
like https://github.com/m3ng9i/ran , being hit for a minute by a
heavy load of requests, those two together would be re-assuring.
thanks,
-serge
^ permalink raw reply
* Re: [BUG] kernel stack corruption during/after Netlabel error
From: Eric Dumazet @ 2017-11-29 17:34 UTC (permalink / raw)
To: Stephen Smalley; +Cc: James Morris, Paul Moore, netdev, selinux
In-Reply-To: <1511976706.10464.19.camel@tycho.nsa.gov>
On Wed, Nov 29, 2017 at 9:31 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Wed, 2017-11-29 at 21:26 +1100, James Morris wrote:
>> I'm seeing a kernel stack corruption bug (detected via gcc) when
>> running
>> the SELinux testsuite on a 4.15-rc1 kernel, in the 2nd inet_socket
>> test:
>>
>> https://github.com/SELinuxProject/selinux-testsuite/blob/master/tests
>> /inet_socket/test
>>
>> # Verify that unauthorized client cannot communicate with the
>> server.
>> $result = system
>> "runcon -t test_inet_bad_client_t -- $basedir/client stream
>> 127.0.0.1 65535 2>&1";
>>
>> This correctlly causes an access control error in the Netlabel code,
>> and
>> the bug seems to be triggered during the ICMP send:
>>
>> [ 339.806024] SELinux: failure in selinux_parse_skb(), unable to
>> parse packet
>> [ 339.822505] Kernel panic - not syncing: stack-protector: Kernel
>> stack is corrupted in: ffffffff81745af5
>> [ 339.822505]
>> [ 339.852250] CPU: 4 PID: 3642 Comm: client Not tainted 4.15.0-rc1-
>> test #15
>> [ 339.868498] Hardware name: LENOVO 10FGS0VA1L/30BC, BIOS
>> FWKT68A 01/19/2017
>> [ 339.885060] Call Trace:
>> [ 339.896875] <IRQ>
>> [ 339.908103] dump_stack+0x63/0x87
>> [ 339.920645] panic+0xe8/0x248
>> [ 339.932668] ? ip_push_pending_frames+0x33/0x40
>> [ 339.946328] ? icmp_send+0x525/0x530
>> [ 339.958861] ? kfree_skbmem+0x60/0x70
>> [ 339.971431] __stack_chk_fail+0x1b/0x20
>> [ 339.984049] icmp_send+0x525/0x530
>> [ 339.996205] ? netlbl_skbuff_err+0x36/0x40
>> [ 340.008997] ? selinux_netlbl_err+0x11/0x20
>> [ 340.021816] ? selinux_socket_sock_rcv_skb+0x211/0x230
>> [ 340.035529] ? security_sock_rcv_skb+0x3b/0x50
>> [ 340.048471] ? sk_filter_trim_cap+0x44/0x1c0
>> [ 340.061246] ? tcp_v4_inbound_md5_hash+0x69/0x1b0
>> [ 340.074562] ? tcp_filter+0x2c/0x40
>> [ 340.086400] ? tcp_v4_rcv+0x820/0xa20
>> [ 340.098329] ? ip_local_deliver_finish+0x71/0x1a0
>> [ 340.111279] ? ip_local_deliver+0x6f/0xe0
>> [ 340.123535] ? ip_rcv_finish+0x3a0/0x3a0
>> [ 340.135523] ? ip_rcv_finish+0xdb/0x3a0
>> [ 340.147442] ? ip_rcv+0x27c/0x3c0
>> [ 340.158668] ? inet_del_offload+0x40/0x40
>> [ 340.170580] ? __netif_receive_skb_core+0x4ac/0x900
>> [ 340.183285] ? rcu_accelerate_cbs+0x5b/0x80
>> [ 340.195282] ? __netif_receive_skb+0x18/0x60
>> [ 340.207288] ? process_backlog+0x95/0x140
>> [ 340.218948] ? net_rx_action+0x26c/0x3b0
>> [ 340.230416] ? __do_softirq+0xc9/0x26a
>> [ 340.241625] ? do_softirq_own_stack+0x2a/0x40
>> [ 340.253368] </IRQ>
>> [ 340.262673] ? do_softirq+0x50/0x60
>> [ 340.273450] ? __local_bh_enable_ip+0x57/0x60
>> [ 340.285045] ? ip_finish_output2+0x175/0x350
>> [ 340.296403] ? ip_finish_output+0x127/0x1d0
>> [ 340.307665] ? nf_hook_slow+0x3c/0xb0
>> [ 340.318230] ? ip_output+0x72/0xe0
>> [ 340.328524] ? ip_fragment.constprop.54+0x80/0x80
>> [ 340.340070] ? ip_local_out+0x35/0x40
>> [ 340.350497] ? ip_queue_xmit+0x15c/0x3f0
>> [ 340.361060] ? __kmalloc_reserve.isra.40+0x31/0x90
>> [ 340.372484] ? __skb_clone+0x2e/0x130
>> [ 340.382633] ? tcp_transmit_skb+0x558/0xa10
>> [ 340.393262] ? tcp_connect+0x938/0xad0
>> [ 340.403370] ? ktime_get_with_offset+0x4c/0xb0
>> [ 340.414206] ? tcp_v4_connect+0x457/0x4e0
>> [ 340.424471] ? __inet_stream_connect+0xb3/0x300
>> [ 340.435195] ? inet_stream_connect+0x3b/0x60
>> [ 340.445607] ? SYSC_connect+0xd9/0x110
>> [ 340.455455] ? __audit_syscall_entry+0xaf/0x100
>> [ 340.466112] ? syscall_trace_enter+0x1d0/0x2b0
>> [ 340.476636] ? __audit_syscall_exit+0x209/0x290
>> [ 340.487151] ? SyS_connect+0xe/0x10
>> [ 340.496453] ? do_syscall_64+0x67/0x1b0
>> [ 340.506078] ? entry_SYSCALL64_slow_path+0x25/0x25
>> [ 340.516693] Kernel Offset: disabled
>> [ 340.526393] Rebooting in 11 seconds..
>>
>> This is mostly reliable, and I'm only seeing it on bare metal (not in
>> a
>> virtualbox vm).
>>
>> The SELinux skb parse error at the start only sometimes appears, and
>> looking at the code, I suspect some kind of memory corruption being
>> the
>> cause at that point (basic packet header checks).
>>
>> I bisected the bug down to the following change:
>>
>> commit bffa72cf7f9df842f0016ba03586039296b4caaf
>> Author: Eric Dumazet <edumazet@google.com>
>> Date: Tue Sep 19 05:14:24 2017 -0700
>>
>> net: sk_buff rbnode reorg
>> ...
>>
>>
>> Anyone else able to reproduce this, or have any ideas on what's
>> happening?
>
> So far I haven't been able to reproduce with 4.15-rc1 or -linus.
>
You might try adding KASAN in the picture ? ( CONFIG_KASAN=y )
Thanks.
^ permalink raw reply
* [iproute PATCH] man: tc-csum.8: Fix inconsistency in example description
From: Phil Sutter @ 2017-11-29 17:34 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Commit 6bbe5e6290db5 ("man: tc-csum.8: Fix example") changed both source
and destination IP addresses in example code but missed to update the
example's description accordingly.
Fixes: 6bbe5e6290db5 ("man: tc-csum.8: Fix example")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
man/man8/tc-csum.8 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/man/man8/tc-csum.8 b/man/man8/tc-csum.8
index 409ab71791cce..65724b88d0b68 100644
--- a/man/man8/tc-csum.8
+++ b/man/man8/tc-csum.8
@@ -53,8 +53,8 @@ SCTP header
.B SWEETS
These are merely syntactic sugar and ignored internally.
.SH EXAMPLES
-The following performs stateless NAT for incoming packets from 192.168.1.100 to
-new destination 18.52.86.120 (0x12345678 in hex). Assuming these are UDP
+The following performs stateless NAT for incoming packets from 192.0.2.100 to
+new destination 198.51.100.1. Assuming these are UDP
packets, both IP and UDP checksums have to be recalculated:
.RS
--
2.13.1
^ permalink raw reply related
* [BUG] net/sched: use after free
From: Eric Dumazet @ 2017-11-29 17:32 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev
Hi
Running the following script twice gave me a KASAN splat on net tree,
(this is on commit f6454f80e8a965fca203dab28723f68ec78db608 )
ETH=eth0
tc qdisc del dev $ETH ingress 2>/dev/null
tc qdisc add dev $ETH ingress
tc filter add dev eth0 parent ffff: \
matchall action sample rate 12 group 4
[ 118.517005] ==================================================================
[ 118.524253] BUG: KASAN: use-after-free in rcu_cblist_dequeue+0xb8/0xe0
[ 118.530779] Read of size 8 at addr ffff881056a38f60 by task swapper/29/0
[ 118.538996] CPU: 29 PID: 0 Comm: swapper/29 Not tainted 4.14.0-smp-DEV #2
[ 118.538997] Hardware name: Intel RML,PCH/Ibis_QC_18, BIOS 2.40.0 06/22/2016
[ 118.538998] Call Trace:
[ 118.539000] <IRQ>
[ 118.539006] dump_stack+0x46/0x67
[ 118.539010] print_address_description+0x6a/0x260
[ 118.539012] ? rcu_cblist_dequeue+0xb8/0xe0
[ 118.539014] kasan_report+0x228/0x340
[ 118.539016] rcu_cblist_dequeue+0xb8/0xe0
[ 118.539018] rcu_process_callbacks+0x2d8/0x1430
[ 118.539020] ? __hrtimer_run_queues+0x2c3/0x620
[ 118.539022] ? note_gp_changes+0x190/0x190
[ 118.539025] __do_softirq+0x1d7/0x5c8
[ 118.539028] irq_exit+0x14e/0x180
[ 118.539030] smp_apic_timer_interrupt+0xbc/0x310
[ 118.539032] apic_timer_interrupt+0x89/0x90
[ 118.539041] </IRQ>
[ 118.539044] RIP: 0010:cpuidle_enter_state+0xfa/0x740
[ 118.539045] RSP: 0018:ffff88105aa07eb0 EFLAGS: 00000212 ORIG_RAX: ffffffffffffff11
[ 118.539048] RAX: dffffc0000000000 RBX: ffffe8ffffd553d0 RCX: 000000000000001f
[ 118.539049] RDX: 1ffff1020ffac84d RSI: fff3b038d885714d RDI: ffff88107fd64268
[ 118.539050] RBP: 00000000000f27d5 R08: ffff88107fd60704 R09: ffff88107fd606a0
[ 118.539051] R10: 071c71c71c71c71c R11: ffff88107fd606e4 R12: 0000000000000005
[ 118.539052] R13: 0000000000000005 R14: 0000000000000005 R15: 0000001b981aba1d
[ 118.539055] ? rcu_eqs_exit_common.isra.51+0x26/0x140
[ 118.539058] do_idle+0x226/0x2a0
[ 118.539059] cpu_startup_entry+0x19/0x20
[ 118.539061] secondary_startup_64+0xa5/0xb0
[ 118.540556] Allocated by task 10820:
[ 118.544135] kasan_kmalloc+0xa0/0xd0
[ 118.544137] __kmalloc+0xd4/0x570
[ 118.544139] tcf_idr_create+0xa2/0x6e0
[ 118.544142] tcf_sample_init+0x4bc/0x830 [act_sample]
[ 118.544144] tcf_action_init_1+0x5c6/0x8f0
[ 118.544145] tcf_action_init+0x1ff/0x3e0
[ 118.544147] tcf_exts_validate+0x27c/0x500
[ 118.544149] mall_change+0x3c5/0xcbf [cls_matchall]
[ 118.544150] tc_ctl_tfilter+0x1104/0x1c70
[ 118.544152] rtnetlink_rcv_msg+0x35a/0x680
[ 118.544153] netlink_rcv_skb+0x2a5/0x3d0
[ 118.544155] netlink_unicast+0x3f4/0x580
[ 118.544156] netlink_sendmsg+0x738/0xae0
[ 118.544159] sock_sendmsg+0xb1/0xe0
[ 118.544160] ___sys_sendmsg+0x658/0x870
[ 118.544162] __sys_sendmsg+0xb0/0x130
[ 118.544163] entry_SYSCALL_64_fastpath+0x13/0x6c
[ 118.545658] Freed by task 5:
[ 118.548537] kasan_slab_free+0x71/0xc0
[ 118.548538] kfree+0x75/0xe0
[ 118.548540] __tcf_idr_release+0x16e/0x220
[ 118.548541] tcf_action_destroy+0xcb/0x160
[ 118.548543] tcf_exts_destroy+0x177/0x350
[ 118.548544] __mall_destroy+0xe/0x60 [cls_matchall]
[ 118.548546] mall_destroy_work+0x17/0x20 [cls_matchall]
[ 118.548548] process_one_work+0x785/0x11c0
[ 118.548549] worker_thread+0xdf/0x12f0
[ 118.548551] kthread+0x29e/0x390
[ 118.548552] ret_from_fork+0x1f/0x30
[ 118.550044] The buggy address belongs to the object at ffff881056a38e80
which belongs to the cache kmalloc-256 of size 256
[ 118.562564] The buggy address is located 224 bytes inside of
256-byte region [ffff881056a38e80, ffff881056a38f80)
[ 118.574281] The buggy address belongs to the page:
[ 118.579074] page:ffffea00415a8e00 count:1 mapcount:0 mapping:ffff881056a380c0 index:0x0
[ 118.587075] flags: 0x2800000000000100(slab)
[ 118.591262] raw: 2800000000000100 ffff881056a380c0 0000000000000000 000000010000000c
[ 118.599000] raw: ffffea00414151e0 ffffea004126a520 ffff88081f400680 0000000000000000
[ 118.606737] page dumped because: kasan: bad access detected
[ 118.613793] Memory state around the buggy address:
[ 118.618575] ffff881056a38e00: 00 00 00 00 00 00 00 05 fc fc fc fc fc fc fc fc
[ 118.625793] ffff881056a38e80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 118.633006] >ffff881056a38f00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 118.640225] ^
[ 118.646578] ffff881056a38f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 118.653808] ffff881056a39000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 118.661035] ==================================================================
[ 118.668254] Disabling lock debugging due to kernel taint
^ permalink raw reply
* Re: [BUG] kernel stack corruption during/after Netlabel error
From: Stephen Smalley @ 2017-11-29 17:31 UTC (permalink / raw)
To: James Morris, Paul Moore, Eric Dumazet; +Cc: netdev, selinux
In-Reply-To: <alpine.LFD.2.20.1711292113350.7808@localhost>
On Wed, 2017-11-29 at 21:26 +1100, James Morris wrote:
> I'm seeing a kernel stack corruption bug (detected via gcc) when
> running
> the SELinux testsuite on a 4.15-rc1 kernel, in the 2nd inet_socket
> test:
>
> https://github.com/SELinuxProject/selinux-testsuite/blob/master/tests
> /inet_socket/test
>
> # Verify that unauthorized client cannot communicate with the
> server.
> $result = system
> "runcon -t test_inet_bad_client_t -- $basedir/client stream
> 127.0.0.1 65535 2>&1";
>
> This correctlly causes an access control error in the Netlabel code,
> and
> the bug seems to be triggered during the ICMP send:
>
> [ 339.806024] SELinux: failure in selinux_parse_skb(), unable to
> parse packet
> [ 339.822505] Kernel panic - not syncing: stack-protector: Kernel
> stack is corrupted in: ffffffff81745af5
> [ 339.822505]
> [ 339.852250] CPU: 4 PID: 3642 Comm: client Not tainted 4.15.0-rc1-
> test #15
> [ 339.868498] Hardware name: LENOVO 10FGS0VA1L/30BC, BIOS
> FWKT68A 01/19/2017
> [ 339.885060] Call Trace:
> [ 339.896875] <IRQ>
> [ 339.908103] dump_stack+0x63/0x87
> [ 339.920645] panic+0xe8/0x248
> [ 339.932668] ? ip_push_pending_frames+0x33/0x40
> [ 339.946328] ? icmp_send+0x525/0x530
> [ 339.958861] ? kfree_skbmem+0x60/0x70
> [ 339.971431] __stack_chk_fail+0x1b/0x20
> [ 339.984049] icmp_send+0x525/0x530
> [ 339.996205] ? netlbl_skbuff_err+0x36/0x40
> [ 340.008997] ? selinux_netlbl_err+0x11/0x20
> [ 340.021816] ? selinux_socket_sock_rcv_skb+0x211/0x230
> [ 340.035529] ? security_sock_rcv_skb+0x3b/0x50
> [ 340.048471] ? sk_filter_trim_cap+0x44/0x1c0
> [ 340.061246] ? tcp_v4_inbound_md5_hash+0x69/0x1b0
> [ 340.074562] ? tcp_filter+0x2c/0x40
> [ 340.086400] ? tcp_v4_rcv+0x820/0xa20
> [ 340.098329] ? ip_local_deliver_finish+0x71/0x1a0
> [ 340.111279] ? ip_local_deliver+0x6f/0xe0
> [ 340.123535] ? ip_rcv_finish+0x3a0/0x3a0
> [ 340.135523] ? ip_rcv_finish+0xdb/0x3a0
> [ 340.147442] ? ip_rcv+0x27c/0x3c0
> [ 340.158668] ? inet_del_offload+0x40/0x40
> [ 340.170580] ? __netif_receive_skb_core+0x4ac/0x900
> [ 340.183285] ? rcu_accelerate_cbs+0x5b/0x80
> [ 340.195282] ? __netif_receive_skb+0x18/0x60
> [ 340.207288] ? process_backlog+0x95/0x140
> [ 340.218948] ? net_rx_action+0x26c/0x3b0
> [ 340.230416] ? __do_softirq+0xc9/0x26a
> [ 340.241625] ? do_softirq_own_stack+0x2a/0x40
> [ 340.253368] </IRQ>
> [ 340.262673] ? do_softirq+0x50/0x60
> [ 340.273450] ? __local_bh_enable_ip+0x57/0x60
> [ 340.285045] ? ip_finish_output2+0x175/0x350
> [ 340.296403] ? ip_finish_output+0x127/0x1d0
> [ 340.307665] ? nf_hook_slow+0x3c/0xb0
> [ 340.318230] ? ip_output+0x72/0xe0
> [ 340.328524] ? ip_fragment.constprop.54+0x80/0x80
> [ 340.340070] ? ip_local_out+0x35/0x40
> [ 340.350497] ? ip_queue_xmit+0x15c/0x3f0
> [ 340.361060] ? __kmalloc_reserve.isra.40+0x31/0x90
> [ 340.372484] ? __skb_clone+0x2e/0x130
> [ 340.382633] ? tcp_transmit_skb+0x558/0xa10
> [ 340.393262] ? tcp_connect+0x938/0xad0
> [ 340.403370] ? ktime_get_with_offset+0x4c/0xb0
> [ 340.414206] ? tcp_v4_connect+0x457/0x4e0
> [ 340.424471] ? __inet_stream_connect+0xb3/0x300
> [ 340.435195] ? inet_stream_connect+0x3b/0x60
> [ 340.445607] ? SYSC_connect+0xd9/0x110
> [ 340.455455] ? __audit_syscall_entry+0xaf/0x100
> [ 340.466112] ? syscall_trace_enter+0x1d0/0x2b0
> [ 340.476636] ? __audit_syscall_exit+0x209/0x290
> [ 340.487151] ? SyS_connect+0xe/0x10
> [ 340.496453] ? do_syscall_64+0x67/0x1b0
> [ 340.506078] ? entry_SYSCALL64_slow_path+0x25/0x25
> [ 340.516693] Kernel Offset: disabled
> [ 340.526393] Rebooting in 11 seconds..
>
> This is mostly reliable, and I'm only seeing it on bare metal (not in
> a
> virtualbox vm).
>
> The SELinux skb parse error at the start only sometimes appears, and
> looking at the code, I suspect some kind of memory corruption being
> the
> cause at that point (basic packet header checks).
>
> I bisected the bug down to the following change:
>
> commit bffa72cf7f9df842f0016ba03586039296b4caaf
> Author: Eric Dumazet <edumazet@google.com>
> Date: Tue Sep 19 05:14:24 2017 -0700
>
> net: sk_buff rbnode reorg
> ...
>
>
> Anyone else able to reproduce this, or have any ideas on what's
> happening?
So far I haven't been able to reproduce with 4.15-rc1 or -linus.
^ permalink raw reply
* Re: [PATCH v5 next 1/5] modules:capabilities: add request_module_cap()
From: Serge E. Hallyn @ 2017-11-29 17:28 UTC (permalink / raw)
To: Theodore Ts'o, David Miller, gnomes, keescook, mcgrof, tixxdz,
luto, akpm, james.l.morris, ben.hutchings, solar, serge, jeyu,
rusty, linux-kernel, linux-security-module, kernel-hardening,
corbet, mingo, netdev, peterz, torvalds
In-Reply-To: <20171129155406.i2lyclquj75lvtn4@thunk.org>
Quoting Theodore Ts'o (tytso@mit.edu):
> Half the problem here is that with containers, people are changing the
> security model, because they want to let untrusted users have "root",
> without really having "root". Part of the fundamental problem is that
> there are some well-meaning, but fundamentally misguided people, who
> have been asserting: "Containers are just as secure as VM's".
>
> Well, they are not. And the sooner people get past this, the better
> off they'll be....
Just to be clear, module loading requires - and must always continue to
require - CAP_SYS_MODULE against the initial user namespace. Containers
in user namespaces do not have that.
I don't believe anyone has ever claimed that containers which are not in
a user namespace are in any way secure.
(And as for the other claim, I'd prefer to stick to "VMs are in most
cases as insecure as properly configured containers" :)
-serge
^ permalink raw reply
* Re: [PATCH net-next 0/3] rxrpc: Fixes
From: David Howells @ 2017-11-29 17:25 UTC (permalink / raw)
To: David Miller; +Cc: dhowells, netdev, linux-afs, linux-kernel
In-Reply-To: <20171129.103909.54399347722993610.davem@davemloft.net>
David Miller <davem@davemloft.net> wrote:
> This email says "net-next", yet your patches say "net".
Sorry about that - it should be 'net'. I copied an old cover note. All the
patches have a macro substitution, but the cover note does not. Do you want
me to repost?
David
^ permalink raw reply
* [PATCH ipsec] xfrm: fix XFRMA_OUTPUT_MARK policy entry
From: Michal Kubecek @ 2017-11-29 17:23 UTC (permalink / raw)
To: Steffen Klassert
Cc: Herbert Xu, David S. Miller, Lorenzo Colitti, netdev,
linux-kernel
This seems to be an obvious typo, NLA_U32 is type of the attribute, not its
(minimal) length.
Fixes: 077fbac405bf ("net: xfrm: support setting an output mark.")
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
net/xfrm/xfrm_user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index c2cfcc6fdb34..ff58c37469d6 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2485,7 +2485,7 @@ static const struct nla_policy xfrma_policy[XFRMA_MAX+1] = {
[XFRMA_PROTO] = { .type = NLA_U8 },
[XFRMA_ADDRESS_FILTER] = { .len = sizeof(struct xfrm_address_filter) },
[XFRMA_OFFLOAD_DEV] = { .len = sizeof(struct xfrm_user_offload) },
- [XFRMA_OUTPUT_MARK] = { .len = NLA_U32 },
+ [XFRMA_OUTPUT_MARK] = { .type = NLA_U32 },
};
static const struct nla_policy xfrma_spd_policy[XFRMA_SPD_MAX+1] = {
--
2.15.0
^ permalink raw reply related
* Re: [PATCH net v2 00/10] bpf: offload: check netdev pointer in the drivers and namespace trouble
From: Daniel Borkmann @ 2017-11-29 17:12 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Jakub Kicinski, netdev, oss-drivers, alexei.starovoitov
In-Reply-To: <20171129085246.GC1975@nanopsycho.orion>
On 11/29/2017 09:52 AM, Jiri Pirko wrote:
> Wed, Nov 29, 2017 at 09:18:29AM CET, jiri@resnulli.us wrote:
>> Tue, Nov 21, 2017 at 01:25:56AM CET, daniel@iogearbox.net wrote:
>>> On 11/21/2017 12:21 AM, Jakub Kicinski wrote:
>>>> Hi!
>>>>
>>>> This series addresses some late comments and moves checking if program
>>>> has been loaded for the correct device to the drivers. There are also
>>>> some problems with net namespaces which I didn't take into consideration.
>>>> On the kernel side we will now simply ignore namespace moves. Since the
>>>> user space API is not reporting any namespace identification we have to
>>>> remove the ifindex until a correct way of reporting is agreed upon.
>>>>
>>>> v2:
>>>> - fix ext ack reporting for XDP (David A);
>>>> - add Jiri's Ack.
>>>
>>> Series applied to bpf tree, thanks Jakub!
>>
>> Is it this git:
>> https://git.kernel.org/pub/scm/linux/kernel/git/dborkman/bpf.git/
>> ?
>> I don't see the patchset there :( When do you plan to send it to DaveM?
>
> Ha, it is already in net-next. I have to get used to this new flows :)
> Sorry for the fuzz.
Heh, yeah, we'll add a FAQ similar to the netdev-FAQ but for BPF
specifically in order to clarify. ETA at latest end of week.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH v7 1/5] add infrastructure for tagging functions as error injectable
From: Daniel Borkmann @ 2017-11-29 16:59 UTC (permalink / raw)
To: Josef Bacik, Jonathan Corbet
Cc: rostedt, mingo, davem, netdev, linux-kernel, ast, kernel-team,
linux-btrfs, Josef Bacik
In-Reply-To: <20171128200241.tr5d6a2obvxl6inb@destiny>
On 11/28/2017 09:02 PM, Josef Bacik wrote:
> On Tue, Nov 28, 2017 at 11:58:41AM -0700, Jonathan Corbet wrote:
>> On Wed, 22 Nov 2017 16:23:30 -0500
>> Josef Bacik <josef@toxicpanda.com> wrote:
>>> From: Josef Bacik <jbacik@fb.com>
>>>
>>> Using BPF we can override kprob'ed functions and return arbitrary
>>> values. Obviously this can be a bit unsafe, so make this feature opt-in
>>> for functions. Simply tag a function with KPROBE_ERROR_INJECT_SYMBOL in
>>> order to give BPF access to that function for error injection purposes.
>>>
>>> Signed-off-by: Josef Bacik <jbacik@fb.com>
>>> Acked-by: Ingo Molnar <mingo@kernel.org>
>>> ---
>>> arch/x86/include/asm/asm.h | 6 ++
>>> include/asm-generic/vmlinux.lds.h | 10 +++
>>> include/linux/bpf.h | 11 +++
>>> include/linux/kprobes.h | 1 +
>>> include/linux/module.h | 5 ++
>>> kernel/kprobes.c | 163 ++++++++++++++++++++++++++++++++++++++
>>> kernel/module.c | 6 +-
>>> 7 files changed, 201 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
>>> index b0dc91f4bedc..340f4cc43255 100644
>>> --- a/arch/x86/include/asm/asm.h
>>> +++ b/arch/x86/include/asm/asm.h
>>> @@ -85,6 +85,12 @@
>>> _ASM_PTR (entry); \
>>> .popsection
>>>
>>> +# define _ASM_KPROBE_ERROR_INJECT(entry) \
>>> + .pushsection "_kprobe_error_inject_list","aw" ; \
>>> + _ASM_ALIGN ; \
>>> + _ASM_PTR (entry); \
>>> + .popseciton
>>
>> So this stuff is not my area of greatest expertise, but I do have to wonder
>> how ".popseciton" can work ... ?
>
> Well fuck, do you want me to send a increment Daniel/Alexei or resend this patch
> fixed? Thanks,
Sorry for late reply, please rebase + respin the whole series with
this fixed. There were also few typos in the cover letter / commit
messages that would be good to get fixed along the way.
Also, could you debug why this wasn't caught at compile/runtime during
testing?
Thanks a lot,
Daniel
^ permalink raw reply
* Re: [PATCH] [RFC v3] packet: experimental support for 64-bit timestamps
From: Willem de Bruijn @ 2017-11-29 16:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S. Miller, Willem de Bruijn, Björn Töpel,
Richard Cochran, Thomas Gleixner, Mike Maloney, Eric Dumazet,
Kees Cook, Hans Liljestrand, Andrey Konovalov, Rosen, Rami,
Reshetova, Elena, Sowmini Varadhan, Network Development, LKML
In-Reply-To: <CAK8P3a2w=JYfptNdKho5EdriRMHH60AA9MLTVHGf9QVf3jgDxg@mail.gmail.com>
> Thanks for the review! Any suggestions for how to do the testing? If you have
> existing test cases, could you give my next version a test run to see if there
> are any regressions and if the timestamps work as expected?
>
> I see that there are test cases in tools/testing/selftests/net/, but none
> of them seem to use the time stamps so far, and I'm not overly familiar
> with how it works in the details to extend it in a meaningful way.
I could not find any good tests for this interface, either. The only
user of the interface I found was a little tool I wrote a few years
ago that compares timestamps at multiple points in the transmit
path for latency measurement [1]. But it may be easier to just write
a new test under tools/testing/selftests/net for this purpose. I can
help with that, too, if you want.
[1] https://github.com/wdebruij/kerneltools/blob/master/tools/tcplate/tcplate.c
^ permalink raw reply
* Re: [PATCH 11/17] cls_bpf: Convert to use idr_alloc_u32
From: David Miller @ 2017-11-29 16:35 UTC (permalink / raw)
To: willy
Cc: kubakici, mawilcox, chrism, jiri, xiyou.wangcong, jhs, daniel,
ebiggers, laijs, tj, aquannie, netdev, linux-kernel
In-Reply-To: <20171129162916.GA18402@bombadil.infradead.org>
From: Matthew Wilcox <willy@infradead.org>
Date: Wed, 29 Nov 2017 08:29:16 -0800
> On Tue, Nov 28, 2017 at 05:08:40PM -0800, Jakub Kicinski wrote:
>> On Tue, 28 Nov 2017 13:33:06 -0800, Matthew Wilcox wrote:
>> > + ret = idr_alloc_u32(&head->handle_idr, prog, &handle,
>> > + INT_MAX, GFP_KERNEL);
>> > + } else if (!oldprog) {
>> > + ret = idr_alloc_u32(&head->handle_idr, prog, &handle,
>> > + handle, GFP_KERNEL);
>>
>> nit: in many places you seem to not align the second line with opening
>> parenthesis. Is that intentional?
>
> It's more that I don't care. I press 'enter', which indents the arguments
> by a certain amount, then press the 'tab' key until it looks aesthetically
> pleasing.
>
>> FWIW there may be a small merge conflict with net on cls_bpf in patch
>> 5, some of the code has been removed.
>
> Thanks. Dave, do you want to take the IDR patches through your tree to
> save conflict resolution?
Sure, after you fix the indentation problems Jakub pointed out please
resubmit them separately to netdev.
^ permalink raw reply
* Re: [PATCH v5 next 1/5] modules:capabilities: add request_module_cap()
From: Theodore Ts'o @ 2017-11-29 16:29 UTC (permalink / raw)
To: David Miller
Cc: gnomes, keescook, mcgrof, tixxdz, luto, akpm, james.l.morris,
ben.hutchings, solar, serge, jeyu, rusty, linux-kernel,
linux-security-module, kernel-hardening, corbet, mingo, netdev,
peterz, torvalds
In-Reply-To: <20171129.105816.1108278026315471171.davem@davemloft.net>
On Wed, Nov 29, 2017 at 10:58:16AM -0500, David Miller wrote:
> That's not what we're talking about.
>
> We're talking about making sure that loading "ppp.ko" really gets
> ppp.ko rather than some_other_module.ko renamed to ppp.ko via some
> other mechanism.
Right, and the best solution to this problem is to include in the
signature the name of the module. (One way of doing this is adding
the module name into the cryptographic checksum which is then
digitally signed by the kernel module key; we would have to bump the
version number of the signature format, obviously.) This binds the
name of the module into the digital signature so that it can't be
renamed.
- Ted
^ permalink raw reply
* Re: [PATCH 11/17] cls_bpf: Convert to use idr_alloc_u32
From: Matthew Wilcox @ 2017-11-29 16:29 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Matthew Wilcox, Chris Mi, Jiri Pirko, David S . Miller, Cong Wang,
Jamal Hadi Salim, Daniel Borkmann, Eric Biggers, Lai Jiangshan,
Tejun Heo, Rehas Sachdeva, netdev, linux-kernel
In-Reply-To: <20171128170817.1c23d420@cakuba.netronome.com>
On Tue, Nov 28, 2017 at 05:08:40PM -0800, Jakub Kicinski wrote:
> On Tue, 28 Nov 2017 13:33:06 -0800, Matthew Wilcox wrote:
> > + ret = idr_alloc_u32(&head->handle_idr, prog, &handle,
> > + INT_MAX, GFP_KERNEL);
> > + } else if (!oldprog) {
> > + ret = idr_alloc_u32(&head->handle_idr, prog, &handle,
> > + handle, GFP_KERNEL);
>
> nit: in many places you seem to not align the second line with opening
> parenthesis. Is that intentional?
It's more that I don't care. I press 'enter', which indents the arguments
by a certain amount, then press the 'tab' key until it looks aesthetically
pleasing.
> FWIW there may be a small merge conflict with net on cls_bpf in patch
> 5, some of the code has been removed.
Thanks. Dave, do you want to take the IDR patches through your tree to
save conflict resolution?
^ permalink raw reply
* Re: [RFC PATCH] net_sched: bulk free tcf_block
From: Alexander Duyck @ 2017-11-29 16:14 UTC (permalink / raw)
To: Paolo Abeni
Cc: Netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller
In-Reply-To: <9b2f686e8a08de829d5be050770d16e93ba5d797.1511965365.git.pabeni@redhat.com>
On Wed, Nov 29, 2017 at 6:25 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> Currently deleting qdisc with a large number of children and filters
> can take a lot of time:
>
> tc qdisc add dev lo root htb
> for I in `seq 1 1000`; do
> tc class add dev lo parent 1: classid 1:$I htb rate 100kbit
> tc qdisc add dev lo parent 1:$I handle $((I + 1)): htb
> for J in `seq 1 10`; do
> tc filter add dev lo parent $((I + 1)): u32 match ip src 1.1.1.$J
> done
> done
> time tc qdisc del dev root
>
> real 0m54.764s
> user 0m0.023s
> sys 0m0.000s
>
> This is due to the multiple rcu_barrier() calls, one for each tcf_block
> freed, invoked with the rtnl lock held. Most other network related
> tasks will block in this timespan.
>
> This change implements bulk free of tcf_block() at destroy() time:
> when deleting a qdisc hierarchy, the to-be-deleted blocks are queued
> in a rtnl_lock protected list, and a single rcu_barrier is invoked
> for each burst.
>
> The burst is flushed after the deletion of the topmost qdisc of the
> destroyed hierarchy and all the queued blocks are deleted with a
> single delayed work call.
>
> After this change, the previous example gives:
>
> real 0m0.193s
> user 0m0.000s
> sys 0m0.016s
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
While I agree there is an issue I don't think this is being approached
quite the right way. The question I have is why aren't we using the
standard RCU approach for this and simply using either call_rcu or
kfree_rcu to free the object?
> ---
> This patch adds 2 new list_head fields to tcf_block, that could be
> replaced with a single pointer, open coding single linked list
> manipulation in cls_api.c, if a lower memory impact is required.
> ---
> include/net/pkt_cls.h | 1 +
> include/net/sch_generic.h | 5 +++
> net/sched/cls_api.c | 78 +++++++++++++++++++++++++++++++++++------------
> net/sched/sch_api.c | 1 +
> net/sched/sch_generic.c | 17 +++++++++++
> net/sched/sch_ingress.c | 1 +
> 6 files changed, 83 insertions(+), 20 deletions(-)
>
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index 0105445cab83..12777cfae77c 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -45,6 +45,7 @@ int tcf_block_get_ext(struct tcf_block **p_block, struct Qdisc *q,
> void tcf_block_put(struct tcf_block *block);
> void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q,
> struct tcf_block_ext_info *ei);
> +void tcf_flush_blocks(void);
>
> static inline struct Qdisc *tcf_block_q(struct tcf_block *block)
> {
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index 65d0d25f2648..99846ee644a8 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -71,6 +71,7 @@ struct Qdisc {
> * qdisc_tree_decrease_qlen() should stop.
> */
> #define TCQ_F_INVISIBLE 0x80 /* invisible by default in dump */
> +#define TCQ_F_DELETING 0x100
> u32 limit;
> const struct Qdisc_ops *ops;
> struct qdisc_size_table __rcu *stab;
> @@ -279,6 +280,10 @@ struct tcf_block {
> struct Qdisc *q;
> struct list_head cb_list;
> struct work_struct work;
> +
> + /* TODO: use a single list, do avoid wasting too much memory */
> + struct list_head del_list;
> + struct list_head del_head;
> };
>
This is just adding yet another layer of deferred freeing. We already
have RCU why don't we just use that?
> static inline void qdisc_cb_private_validate(const struct sk_buff *skb, int sz)
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 7d97f612c9b9..446b16c1f532 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -37,6 +37,61 @@ static LIST_HEAD(tcf_proto_base);
> /* Protects list of registered TC modules. It is pure SMP lock. */
> static DEFINE_RWLOCK(cls_mod_lock);
>
> +/* List of tcf_blocks queued for deletion. Bulk freeing them we avoid the
> + * rcu_barrier() storm at root_qdisc->destroy() time
> + */
> +static LIST_HEAD(queued_blocks);
> +
> +static void queue_for_deletion(struct tcf_block *block)
> +{
> + if (WARN_ON(!list_empty(&block->del_list)))
> + return;
> +
> + ASSERT_RTNL();
> + list_add(&block->del_list, &queued_blocks);
> +}
> +
> +static void flush_blocks(struct work_struct *work)
> +{
> + struct tcf_block *block, *tmp_block;
> + struct tcf_chain *chain, *tmp;
> + struct list_head *head;
> +
> + head = &container_of(work, struct tcf_block, work)->del_head;
> + rtnl_lock();
> + list_for_each_entry(block, head, del_list)
> + /* Only chain 0 should be still here. */
> + list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
> + tcf_chain_put(chain);
> + rtnl_unlock();
> +
> + list_for_each_entry_safe(block, tmp_block, head, del_list)
> + kfree(block);
> +}
> +
> +void tcf_flush_blocks(void)
> +{
> + struct tcf_block *head;
> + LIST_HEAD(flush_burst);
> +
> + ASSERT_RTNL();
> + if (list_empty(&queued_blocks))
> + return;
> +
> + head = list_first_entry(&queued_blocks, struct tcf_block, del_list);
> + INIT_LIST_HEAD(&head->del_head);
> + list_splice_init(&queued_blocks, &head->del_head);
> + INIT_WORK(&head->work, flush_blocks);
> +
> + /* Wait for existing RCU callbacks to cool down, make sure their works
> + * have been queued before this. We can not flush pending works here
> + * because we are holding the RTNL lock.
> + */
> + rcu_barrier();
> + schedule_work(&head->work);
> +}
> +EXPORT_SYMBOL_GPL(tcf_flush_blocks);
> +
> /* Find classifier type by string name */
>
> static const struct tcf_proto_ops *tcf_proto_lookup_ops(const char *kind)
> @@ -288,6 +343,7 @@ int tcf_block_get_ext(struct tcf_block **p_block, struct Qdisc *q,
> return -ENOMEM;
> INIT_LIST_HEAD(&block->chain_list);
> INIT_LIST_HEAD(&block->cb_list);
> + INIT_LIST_HEAD(&block->del_list);
>
> /* Create chain 0 by default, it has to be always present. */
> chain = tcf_chain_create(block, 0);
> @@ -330,19 +386,6 @@ int tcf_block_get(struct tcf_block **p_block,
> }
> EXPORT_SYMBOL(tcf_block_get);
>
> -static void tcf_block_put_final(struct work_struct *work)
> -{
> - struct tcf_block *block = container_of(work, struct tcf_block, work);
> - struct tcf_chain *chain, *tmp;
> -
> - rtnl_lock();
> - /* Only chain 0 should be still here. */
> - list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
> - tcf_chain_put(chain);
So it seems like the heart of the problem is right here. The
tcf_chain_put call is updating the reference count and I would assume
that is the only bit that really needs you to still be holding onto
the rtnl_lock.
The question I would have is if there is anything accessing the
reference count or manipulating the list itself without holding the
rtnl lock? If not you could look at converting this whole thing from
using a list to an rculist and it seems like it would save you a lot
of trouble. As far as I can tell the only thing you would then really
have to worry about would be the freeing of the chain itself which
would happen in an rcu callback instead of with the rtnl lock held.
> - rtnl_unlock();
> - kfree(block);
> -}
> -
> /* XXX: Standalone actions are not allowed to jump to any chain, and bound
> * actions should be all removed after flushing. However, filters are now
> * destroyed in tc filter workqueue with RTNL lock, they can not race here.
> @@ -357,13 +400,7 @@ void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q,
>
> tcf_block_offload_unbind(block, q, ei);
>
> - INIT_WORK(&block->work, tcf_block_put_final);
> - /* Wait for existing RCU callbacks to cool down, make sure their works
> - * have been queued before this. We can not flush pending works here
> - * because we are holding the RTNL lock.
> - */
> - rcu_barrier();
> - tcf_queue_work(&block->work);
> + queue_for_deletion(block);
> }
> EXPORT_SYMBOL(tcf_block_put_ext);
>
> @@ -920,6 +957,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
> if (tp_created)
> tcf_proto_destroy(tp);
> }
> + tcf_flush_blocks();
>
> errout:
> if (chain)
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index b6c4f536876b..5fe2dcb73bfd 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1645,6 +1645,7 @@ static int tclass_del_notify(struct net *net,
> kfree_skb(skb);
> return err;
> }
> + tcf_flush_blocks();
>
> return rtnetlink_send(skb, net, portid, RTNLGRP_TC,
> n->nlmsg_flags & NLM_F_ECHO);
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index 3839cbbdc32b..299c5d33916a 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -28,6 +28,7 @@
> #include <linux/if_vlan.h>
> #include <net/sch_generic.h>
> #include <net/pkt_sched.h>
> +#include <net/pkt_cls.h>
> #include <net/dst.h>
> #include <trace/events/qdisc.h>
>
> @@ -708,11 +709,25 @@ static void qdisc_free(struct Qdisc *qdisc)
> void qdisc_destroy(struct Qdisc *qdisc)
> {
> const struct Qdisc_ops *ops = qdisc->ops;
> + struct Qdisc *p;
> + bool flush;
>
> if (qdisc->flags & TCQ_F_BUILTIN ||
> !refcount_dec_and_test(&qdisc->refcnt))
> return;
>
> + /* we can avoid flush the pending blocks if this qdisc is a child
> + * deleted a recursive destroy() call and the parent qdisc is already
> + * removed.
> + */
> + qdisc->flags |= TCQ_F_DELETING;
> + if (qdisc->parent != TC_H_ROOT) {
> + p = qdisc_lookup(qdisc_dev(qdisc), TC_H_MAJ(qdisc->parent));
> + flush = p && !(p->flags & TCQ_F_DELETING);
> + } else {
> + flush = true;
> + }
> +
> #ifdef CONFIG_NET_SCHED
> qdisc_hash_del(qdisc);
>
> @@ -723,6 +738,8 @@ void qdisc_destroy(struct Qdisc *qdisc)
> ops->reset(qdisc);
> if (ops->destroy)
> ops->destroy(qdisc);
> + if (flush)
> + tcf_flush_blocks();
>
> module_put(ops->owner);
> dev_put(qdisc_dev(qdisc));
> diff --git a/net/sched/sch_ingress.c b/net/sched/sch_ingress.c
> index 5ecc38f35d47..7329b8f55160 100644
> --- a/net/sched/sch_ingress.c
> +++ b/net/sched/sch_ingress.c
> @@ -201,6 +201,7 @@ static int clsact_init(struct Qdisc *sch, struct nlattr *opt)
>
> err_egress_block_get:
> tcf_block_put_ext(q->ingress_block, sch, &q->ingress_block_info);
> + tcf_flush_blocks();
> return err;
> }
>
> --
> 2.13.6
>
^ permalink raw reply
* Re: fec driver and two micrel phys
From: Andrew Lunn @ 2017-11-29 16:12 UTC (permalink / raw)
To: netdev
In-Reply-To: <20171129090822.GD32345@graute-opti>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/314706.html
>
> &mac0 {
> phy-mode = "rmii";
> phy-handle = <ðphy0>;
> pinctrl-names = "default";
> pinctrl-0 = <&mac0_pins_a>;
> phy-reset-gpios = <&gpio4 13 0>;
> phy-reset-duration = <100>;
> status = "okay";
>
> mdio {
> #address-cells = <1>;
> #size-cells = <0>;
>
> ethphy0: ethernet-phy@1 {
> reg = <1>;
> };
>
> ethphy1: ethernet-phy@3 {
> reg = <3>;
> };
> };
> };
>
> &mac1 {
> phy-mode = "rmii";
> phy-handle = <ðphy1>;
> pinctrl-names = "default";
> pinctrl-0 = <&mac1_pins_a>;
> status = "okay";
>
> };
>
> Thx for stub me in right direction.
Hi Oliver
This looks a lot better.
Andrew
^ permalink raw reply
* Re: [PATCH v4 7/8] netdev: octeon-ethernet: Add Cavium Octeon III support.
From: Souptick Joarder @ 2017-11-29 16:07 UTC (permalink / raw)
To: David Daney
Cc: Mark Rutland, linux-mips, devel, devicetree, netdev, linux-kernel,
ralf, Carlos Munoz, Rob Herring, Andrew Lunn, Steven J. Hill,
Greg Kroah-Hartman, Florian Fainelli, James Hogan,
David S. Miller
In-Reply-To: <CAFqt6zabdQhyjUc4WsjzJ6CxMr70H3V_JdipJVwRi8LuOG54tA@mail.gmail.com>
On Wed, Nov 29, 2017 at 4:00 PM, Souptick Joarder <jrdr.linux@gmail.com> wrote:
> On Wed, Nov 29, 2017 at 6:25 AM, David Daney <david.daney@cavium.com> wrote:
>> From: Carlos Munoz <cmunoz@cavium.com>
>>
>> The Cavium OCTEON cn78xx and cn73xx SoCs have network packet I/O
>> hardware that is significantly different from previous generations of
>> the family.
>> diff --git a/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c b/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
>> new file mode 100644
>> index 000000000000..4dad35fa4270
>> --- /dev/null
>> +++ b/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
>> @@ -0,0 +1,2033 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2017 Cavium, Inc.
>> + *
>> + * This file is subject to the terms and conditions of the GNU General Public
>> + * License. See the file "COPYING" in the main directory of this archive
>> + * for more details.
>> + */
>> +#include <linux/platform_device.h>
>> +#include <linux/netdevice.h>
>> +#include <linux/etherdevice.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_mdio.h>
>> +#include <linux/of_net.h>
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/list.h>
>> +
>> +static void bgx_port_sgmii_set_link_down(struct bgx_port_priv *priv)
>> +{
>> + u64 data;
>> + data = oct_csr_read(BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx, priv->index));
>> + data |= BIT(11);
>> + oct_csr_write(data, BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx, priv->index));
>> + data = oct_csr_read(BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx, priv->index));
>
> Any particular reason to read immediately after write ?
>> +static int bgx_port_sgmii_set_link_speed(struct bgx_port_priv *priv, struct port_status status)
>> +{
>> + u64 data;
>> + u64 prtx;
>> + u64 miscx;
>> + int timeout;
>> +
>> +
>> + switch (status.speed) {
>> + case 10:
>
> In my opinion, instead of hard coding the value, is it fine to use ENUM ?
Similar comments applicable in other places where hard coded values are used.
>> +static int bgx_port_gser_27882(struct bgx_port_priv *priv)
>> +{
>> + u64 data;
>> + u64 addr;
>
>> + int timeout = 200;
>> +
>> + // timeout = 200;
Better to initialize the timeout value
>> +static int bgx_port_qlm_rx_equalization(struct bgx_port_priv *priv, int qlm, int lane)
>> +{
>> + lmode = oct_csr_read(GSER_LANE_MODE(priv->node, qlm));
>> + lmode &= 0xf;
>> + addr = GSER_LANE_P_MODE_1(priv->node, qlm, lmode);
>> + data = oct_csr_read(addr);
>> + /* Don't complete rx equalization if in VMA manual mode */
>> + if (data & BIT(14))
>> + return 0;
>> +
>> + /* Apply rx equalization for speed > 6250 */
>> + if (bgx_port_get_qlm_speed(priv, qlm) < 6250)
>> + return 0;
>> +
>> + /* Wait until rx data is valid (CDRLOCK) */
>> + timeout = 500;
>
> 500 us is the min required value or it can be further reduced ?
>> +static int bgx_port_init_xaui_link(struct bgx_port_priv *priv)
>> +{
>> +
>> + if (use_ber) {
>> + timeout = 10000;
>> + do {
>> + data =
>> + oct_csr_read(BGX_SPU_BR_STATUS1(priv->node, priv->bgx, priv->index));
>> + if (data & BIT(0))
>> + break;
>> + timeout--;
>> + udelay(1);
>> + } while (timeout);
>
> In my opinion, it's better to implement similar kind of loops inside macros.
>
>> + if (!timeout) {
>> + pr_debug("BGX%d:%d:%d: BLK_LOCK timeout\n",
>> + priv->bgx, priv->index, priv->node);
>> + return -1;
>> + }
>> + } else {
>> + timeout = 10000;
>> + do {
>> + data =
>> + oct_csr_read(BGX_SPU_BX_STATUS(priv->node, priv->bgx, priv->index));
>> + if (data & BIT(12))
>> + break;
>> + timeout--;
>> + udelay(1);
>> + } while (timeout);
> same here
^ permalink raw reply
* Re: [PATCH v5 next 1/5] modules:capabilities: add request_module_cap()
From: David Miller @ 2017-11-29 15:58 UTC (permalink / raw)
To: tytso
Cc: gnomes, keescook, mcgrof, tixxdz, luto, akpm, james.l.morris,
ben.hutchings, solar, serge, jeyu, rusty, linux-kernel,
linux-security-module, kernel-hardening, corbet, mingo, netdev,
peterz, torvalds
In-Reply-To: <20171129155406.i2lyclquj75lvtn4@thunk.org>
From: Theodore Ts'o <tytso@mit.edu>
Date: Wed, 29 Nov 2017 10:54:06 -0500
> On Wed, Nov 29, 2017 at 09:50:14AM -0500, David Miller wrote:
>> From: Alan Cox <gnomes@lxorguk.ukuu.org.uk>
>> Date: Wed, 29 Nov 2017 13:46:12 +0000
>>
>> > I really don't care what the module loading rules end up with and
>> > whether we add CAP_SYS_YET_ANOTHER_MEANINGLESS_FLAG but what is
>> > actually needed is to properly incorporate it into securiy ruiles
>> > for whatever LSM you are using.
>>
>> I'm surprised we're not using the SHA1 hashes or whatever we compute
>> for the modules to make sure we are loading the foo.ko that we expect
>> to be.
>
> We do have signed modules. But this won't help us if the user is
> using a distro kernel which has compiled some module which is known to
> be unmaintained which everyone in the know *expects* to have 0-day
> bugs, such as DCCP. That's because the DCCP module is signed.
That's not what we're talking about.
We're talking about making sure that loading "ppp.ko" really gets
ppp.ko rather than some_other_module.ko renamed to ppp.ko via some
other mechanism.
Both modules have legitimate signatures so the kernel will happily
load both.
^ permalink raw reply
* Re: [PATCH 3/4] RFC: net: dsa: Add bindings for Realtek SMI DSAs
From: Andrew Lunn @ 2017-11-29 15:56 UTC (permalink / raw)
To: Linus Walleij
Cc: Vivien Didelot, Florian Fainelli, netdev, Antti Seppälä,
Roman Yeryomin, Colin Leitner, Gabor Juhos, devicetree
In-Reply-To: <CACRpkdbzqnbF49yN-SAqLkhufk=R2C=jpdLo-QWmL0zR_+Vj=w@mail.gmail.com>
> I have the phy-handle in the ethernet controller. This RTL8366RB
> thing is just one big PHY as far as I know.
Hi Linus
We don't model switches as PHYs. They are their own device type. And
the internal or external PHYs are just normal PHYs in the linux
model. Meaning their interrupt properties goes in the PHY node in
device tree, as documented in the phy.txt binding documentation.
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