* Re: [PATCH net-next] net: sched: act_ife: disable bh when taking ife_mod_lock
From: David Miller @ 2018-08-13 18:27 UTC (permalink / raw)
To: vladbu; +Cc: netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <vbf4lfygotr.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>
From: Vlad Buslov <vladbu@mellanox.com>
Date: Mon, 13 Aug 2018 20:26:40 +0300
> Is it okay to submit a fix for issue I uncovered when testing actions
> with estimators, or I should resubmit to net when net-next is moved?
Yes, this is fine.
^ permalink raw reply
* Re: [PATCH v1 0/3] WireGuard: Secure Network Tunnel
From: James Bottomley @ 2018-08-13 18:04 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: linux-kernel, netdev, davem, linux-crypto
In-Reply-To: <CAHmME9pn+zm67mmDDrkMLpSAXNEugCT3z2L-QXO2eQw+31V=YQ@mail.gmail.com>
On Mon, 2018-08-13 at 10:55 -0700, Jason A. Donenfeld wrote:
> > but it's very hard for a flow classifier because you have to
>
> The construction and identifier strings might not obviously help with
> the extremely narrow idea you've brought up, but it is very important
> for safely introducing additional versions. Namely, it prevents
> against cross-protocol key reuse attacks and type confusion bugs. So
> don't be too quick to dismiss the importance of these for
> accomplishing what we're after.
I'm not saying a hash check isn't important for safety; I'm saying that
if you only have a hash of a dynamic part plus the protocol identifier
to go on it makes far more work for the flow classifier. You can see
this easily if you contemplate the idea that the hash might be the
algorithm being changed.
> > so lets pick one of the above and try it out.
>
> We have, multiple times, and it's absolutely trivial to do and works
> well. The exact thing you're concerned about has already been
> researched and worked with on live systems quite a bit over the last
> 3 years, and it works in a pretty straight forward way. I'm not sure
> there's much more to add here: the thing you want is already there
> and has been tested extensively. At this point the "pick one and
> let's try it out!" is an old story, and the focus now is on making
> sure the code quality and netdev api usage is correct for merging
Great, thanks, I'll look forward to seeing it in v2 then.
James
^ permalink raw reply
* Re: [PATCH net-next] openvswitch: Derive IP protocol number for IPv6 later frags
From: William Tu @ 2018-08-13 17:48 UTC (permalink / raw)
To: pravin shelar; +Cc: Yi-Hung Wei, Linux Kernel Network Developers
In-Reply-To: <CAOrHB_D7mOjqvghNjBrXF_JnZw6QzzOaZ60LsphC8OacTwYXnA@mail.gmail.com>
On Sun, Aug 12, 2018 at 6:09 PM Pravin Shelar <pshelar@ovn.org> wrote:
>
> On Fri, Aug 10, 2018 at 10:19 AM, Yi-Hung Wei <yihung.wei@gmail.com> wrote:
> > Currently, OVS only parses the IP protocol number for the first
> > IPv6 fragment, but sets the IP protocol number for the later fragments
> > to be NEXTHDF_FRAGMENT. This patch tries to derive the IP protocol
> > number for the IPV6 later frags so that we can match that.
> >
> > Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
> > ---
> > net/openvswitch/flow.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> > index 56b8e7167790..3d654c4f71be 100644
> > --- a/net/openvswitch/flow.c
> > +++ b/net/openvswitch/flow.c
> > @@ -297,7 +297,13 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key)
> >
> > nh_len = payload_ofs - nh_ofs;
> > skb_set_transport_header(skb, nh_ofs + nh_len);
> > - key->ip.proto = nexthdr;
> > + if (key->ip.frag == OVS_FRAG_TYPE_LATER) {
> > + unsigned int offset = 0;
How about we start the 2nd time parsing from
unsigned int offset = payload_ofs;
> > +
> > + key->ip.proto = ipv6_find_hdr(skb, &offset, -1, NULL, NULL);
Then we only find the last header from previous parsed offset.
William
> > + } else {
> > + key->ip.proto = nexthdr;
> > + }
> parsing ipv6 ipv6_skip_exthdr() is called to find fragment hdr and
> then this patch calls ipv6_find_hdr() to find next protocol. I think
> we could call ipv6_find_hdr() to get fragment type and next hdr, that
> would save parsing same packet twice in some cases.
>
> Other option would be calling ipv6_find_hdr() after setting OVS_FRAG_TYPE_LATER.
^ permalink raw reply
* Re: [PATCH net-next] net: sched: act_ife: disable bh when taking ife_mod_lock
From: Vlad Buslov @ 2018-08-13 17:26 UTC (permalink / raw)
To: netdev, David Miller; +Cc: davem, jhs, xiyou.wangcong, jiri
In-Reply-To: <1534180811-10416-1-git-send-email-vladbu@mellanox.com>
Hi David,
Is it okay to submit a fix for issue I uncovered when testing actions
with estimators, or I should resubmit to net when net-next is moved?
Thanks,
Vlad
^ permalink raw reply
* Re: [PATCH net-next] net: sched: act_ife: disable bh when taking ife_mod_lock
From: Jamal Hadi Salim @ 2018-08-13 17:23 UTC (permalink / raw)
To: Vlad Buslov, netdev; +Cc: davem, xiyou.wangcong, jiri
In-Reply-To: <1534180811-10416-1-git-send-email-vladbu@mellanox.com>
On 2018-08-13 1:20 p.m., Vlad Buslov wrote:
> Lockdep reports deadlock for following locking scenario in ife action:
>
> Task one:
> 1) Executes ife action update.
> 2) Takes tcfa_lock.
> 3) Waits on ife_mod_lock which is already taken by task two.
>
> Task two:
>
> 1) Executes any path that obtains ife_mod_lock without disabling bh (any
> path that takes ife_mod_lock while holding tcfa_lock has bh disabled) like
> loading a meta module, or creating new action.
> 2) Takes ife_mod_lock.
> 3) Task is preempted by rate estimator timer.
> 4) Timer callback waits on tcfa_lock which is taken by task one.
>
> In described case tasks deadlock because they take same two locks in
> different order. To prevent potential deadlock reported by lockdep, always
> disable bh when obtaining ife_mod_lock.
>
Looks like your recent changes on net-next exposed this.
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
cheers,
jamal
^ permalink raw reply
* [PATCH net-next] net: sched: act_ife: disable bh when taking ife_mod_lock
From: Vlad Buslov @ 2018-08-13 17:20 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, Vlad Buslov
Lockdep reports deadlock for following locking scenario in ife action:
Task one:
1) Executes ife action update.
2) Takes tcfa_lock.
3) Waits on ife_mod_lock which is already taken by task two.
Task two:
1) Executes any path that obtains ife_mod_lock without disabling bh (any
path that takes ife_mod_lock while holding tcfa_lock has bh disabled) like
loading a meta module, or creating new action.
2) Takes ife_mod_lock.
3) Task is preempted by rate estimator timer.
4) Timer callback waits on tcfa_lock which is taken by task one.
In described case tasks deadlock because they take same two locks in
different order. To prevent potential deadlock reported by lockdep, always
disable bh when obtaining ife_mod_lock.
Lockdep warning:
[ 508.101192] =====================================================
[ 508.107708] WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected
[ 508.114728] 4.18.0-rc8+ #646 Not tainted
[ 508.119050] -----------------------------------------------------
[ 508.125559] tc/5460 [HC0[0]:SC0[2]:HE1:SE0] is trying to acquire:
[ 508.132025] 000000005a938c68 (ife_mod_lock){++++}, at: find_ife_oplist+0x1e/0xc0 [act_ife]
[ 508.140996]
and this task is already holding:
[ 508.147548] 00000000d46f6c56 (&(&p->tcfa_lock)->rlock){+.-.}, at: tcf_ife_init+0x6ae/0xf40 [act_ife]
[ 508.157371] which would create a new lock dependency:
[ 508.162828] (&(&p->tcfa_lock)->rlock){+.-.} -> (ife_mod_lock){++++}
[ 508.169572]
but this new dependency connects a SOFTIRQ-irq-safe lock:
[ 508.178197] (&(&p->tcfa_lock)->rlock){+.-.}
[ 508.178201]
... which became SOFTIRQ-irq-safe at:
[ 508.189771] _raw_spin_lock+0x2c/0x40
[ 508.193906] est_fetch_counters+0x41/0xb0
[ 508.198391] est_timer+0x83/0x3c0
[ 508.202180] call_timer_fn+0x16a/0x5d0
[ 508.206400] run_timer_softirq+0x399/0x920
[ 508.210967] __do_softirq+0x157/0x97d
[ 508.215102] irq_exit+0x152/0x1c0
[ 508.218888] smp_apic_timer_interrupt+0xc0/0x4e0
[ 508.223976] apic_timer_interrupt+0xf/0x20
[ 508.228540] cpuidle_enter_state+0xf8/0x5d0
[ 508.233198] do_idle+0x28a/0x350
[ 508.236881] cpu_startup_entry+0xc7/0xe0
[ 508.241296] start_secondary+0x2e8/0x3f0
[ 508.245678] secondary_startup_64+0xa5/0xb0
[ 508.250347]
to a SOFTIRQ-irq-unsafe lock: (ife_mod_lock){++++}
[ 508.256531]
... which became SOFTIRQ-irq-unsafe at:
[ 508.267279] ...
[ 508.267283] _raw_write_lock+0x2c/0x40
[ 508.273653] register_ife_op+0x118/0x2c0 [act_ife]
[ 508.278926] do_one_initcall+0xf7/0x4d9
[ 508.283214] do_init_module+0x18b/0x44e
[ 508.287521] load_module+0x4167/0x5730
[ 508.291739] __do_sys_finit_module+0x16d/0x1a0
[ 508.296654] do_syscall_64+0x7a/0x3f0
[ 508.300788] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 508.306302]
other info that might help us debug this:
[ 508.315286] Possible interrupt unsafe locking scenario:
[ 508.322771] CPU0 CPU1
[ 508.327681] ---- ----
[ 508.332604] lock(ife_mod_lock);
[ 508.336300] local_irq_disable();
[ 508.342608] lock(&(&p->tcfa_lock)->rlock);
[ 508.349793] lock(ife_mod_lock);
[ 508.355990] <Interrupt>
[ 508.358974] lock(&(&p->tcfa_lock)->rlock);
[ 508.363803]
*** DEADLOCK ***
[ 508.370715] 2 locks held by tc/5460:
[ 508.374680] #0: 00000000e27e4fa4 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x583/0x7b0
[ 508.383366] #1: 00000000d46f6c56 (&(&p->tcfa_lock)->rlock){+.-.}, at: tcf_ife_init+0x6ae/0xf40 [act_ife]
[ 508.393648]
the dependencies between SOFTIRQ-irq-safe lock and the holding lock:
[ 508.403505] -> (&(&p->tcfa_lock)->rlock){+.-.} ops: 1001553 {
[ 508.409646] HARDIRQ-ON-W at:
[ 508.413136] _raw_spin_lock_bh+0x34/0x40
[ 508.419059] gnet_stats_start_copy_compat+0xa2/0x230
[ 508.426021] gnet_stats_start_copy+0x16/0x20
[ 508.432333] tcf_action_copy_stats+0x95/0x1d0
[ 508.438735] tcf_action_dump_1+0xb0/0x4e0
[ 508.444795] tcf_action_dump+0xca/0x200
[ 508.450673] tcf_exts_dump+0xd9/0x320
[ 508.456392] fl_dump+0x1b7/0x4a0 [cls_flower]
[ 508.462798] tcf_fill_node+0x380/0x530
[ 508.468601] tfilter_notify+0xdf/0x1c0
[ 508.474404] tc_new_tfilter+0x84a/0xc90
[ 508.480270] rtnetlink_rcv_msg+0x5bd/0x7b0
[ 508.486419] netlink_rcv_skb+0x184/0x220
[ 508.492394] netlink_unicast+0x31b/0x460
[ 508.507411] netlink_sendmsg+0x3fb/0x840
[ 508.513390] sock_sendmsg+0x7b/0xd0
[ 508.518907] ___sys_sendmsg+0x4c6/0x610
[ 508.524797] __sys_sendmsg+0xd7/0x150
[ 508.530510] do_syscall_64+0x7a/0x3f0
[ 508.536201] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 508.543301] IN-SOFTIRQ-W at:
[ 508.546834] _raw_spin_lock+0x2c/0x40
[ 508.552522] est_fetch_counters+0x41/0xb0
[ 508.558571] est_timer+0x83/0x3c0
[ 508.563912] call_timer_fn+0x16a/0x5d0
[ 508.569699] run_timer_softirq+0x399/0x920
[ 508.575840] __do_softirq+0x157/0x97d
[ 508.581538] irq_exit+0x152/0x1c0
[ 508.586882] smp_apic_timer_interrupt+0xc0/0x4e0
[ 508.593533] apic_timer_interrupt+0xf/0x20
[ 508.599686] cpuidle_enter_state+0xf8/0x5d0
[ 508.605895] do_idle+0x28a/0x350
[ 508.611147] cpu_startup_entry+0xc7/0xe0
[ 508.617097] start_secondary+0x2e8/0x3f0
[ 508.623029] secondary_startup_64+0xa5/0xb0
[ 508.629245] INITIAL USE at:
[ 508.632686] _raw_spin_lock_bh+0x34/0x40
[ 508.638557] gnet_stats_start_copy_compat+0xa2/0x230
[ 508.645491] gnet_stats_start_copy+0x16/0x20
[ 508.651719] tcf_action_copy_stats+0x95/0x1d0
[ 508.657992] tcf_action_dump_1+0xb0/0x4e0
[ 508.663937] tcf_action_dump+0xca/0x200
[ 508.669716] tcf_exts_dump+0xd9/0x320
[ 508.675337] fl_dump+0x1b7/0x4a0 [cls_flower]
[ 508.681650] tcf_fill_node+0x380/0x530
[ 508.687366] tfilter_notify+0xdf/0x1c0
[ 508.693031] tc_new_tfilter+0x84a/0xc90
[ 508.698820] rtnetlink_rcv_msg+0x5bd/0x7b0
[ 508.704869] netlink_rcv_skb+0x184/0x220
[ 508.710758] netlink_unicast+0x31b/0x460
[ 508.716627] netlink_sendmsg+0x3fb/0x840
[ 508.722510] sock_sendmsg+0x7b/0xd0
[ 508.727931] ___sys_sendmsg+0x4c6/0x610
[ 508.733729] __sys_sendmsg+0xd7/0x150
[ 508.739346] do_syscall_64 +0x7a/0x3f0
[ 508.744943] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 508.751930] }
[ 508.753964] ... key at: [<ffffffff916b3e20>] __key.61145+0x0/0x40
[ 508.760946] ... acquired at:
[ 508.764294] _raw_read_lock+0x2f/0x40
[ 508.768513] find_ife_oplist+0x1e/0xc0 [act_ife]
[ 508.773692] tcf_ife_init+0x82f/0xf40 [act_ife]
[ 508.778785] tcf_action_init_1+0x510/0x750
[ 508.783468] tcf_action_init+0x1e8/0x340
[ 508.787938] tcf_action_add+0xc5/0x240
[ 508.792241] tc_ctl_action+0x203/0x2a0
[ 508.796550] rtnetlink_rcv_msg+0x5bd/0x7b0
[ 508.801200] netlink_rcv_skb+0x184/0x220
[ 508.805674] netlink_unicast+0x31b/0x460
[ 508.810129] netlink_sendmsg+0x3fb/0x840
[ 508.814611] sock_sendmsg+0x7b/0xd0
[ 508.818665] ___sys_sendmsg+0x4c6/0x610
[ 508.823029] __sys_sendmsg+0xd7/0x150
[ 508.827246] do_syscall_64+0x7a/0x3f0
[ 508.831483] entry_SYSCALL_64_after_hwframe+0x49/0xbe
the dependencies between the lock to be acquired
[ 508.838945] and SOFTIRQ-irq-unsafe lock:
[ 508.851177] -> (ife_mod_lock){++++} ops: 95 {
[ 508.855920] HARDIRQ-ON-W at:
[ 508.859478] _raw_write_lock+0x2c/0x40
[ 508.865264] register_ife_op+0x118/0x2c0 [act_ife]
[ 508.872071] do_one_initcall+0xf7/0x4d9
[ 508.877947] do_init_module+0x18b/0x44e
[ 508.883819] load_module+0x4167/0x5730
[ 508.889595] __do_sys_finit_module+0x16d/0x1a0
[ 508.896043] do_syscall_64+0x7a/0x3f0
[ 508.901734] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 508.908827] HARDIRQ-ON-R at:
[ 508.912359] _raw_read_lock+0x2f/0x40
[ 508.918043] find_ife_oplist+0x1e/0xc0 [act_ife]
[ 508.924692] tcf_ife_init+0x82f/0xf40 [act_ife]
[ 508.931252] tcf_action_init_1+0x510/0x750
[ 508.937393] tcf_action_init+0x1e8/0x340
[ 508.943366] tcf_action_add+0xc5/0x240
[ 508.949130] tc_ctl_action+0x203/0x2a0
[ 508.954922] rtnetlink_rcv_msg+0x5bd/0x7b0
[ 508.961024] netlink_rcv_skb+0x184/0x220
[ 508.966970] netlink_unicast+0x31b/0x460
[ 508.972915] netlink_sendmsg+0x3fb/0x840
[ 508.978859] sock_sendmsg+0x7b/0xd0
[ 508.984400] ___sys_sendmsg+0x4c6/0x610
[ 508.990264] __sys_sendmsg+0xd7/0x150
[ 508.995952] do_syscall_64+0x7a/0x3f0
[ 509.001643] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 509.008722] SOFTIRQ-ON-W at:\
[ 509.012242] _raw_write_lock+0x2c/0x40
[ 509.018013] register_ife_op+0x118/0x2c0 [act_ife]
[ 509.024841] do_one_initcall+0xf7/0x4d9
[ 509.030720] do_init_module+0x18b/0x44e
[ 509.036604] load_module+0x4167/0x5730
[ 509.042397] __do_sys_finit_module+0x16d/0x1a0
[ 509.048865] do_syscall_64+0x7a/0x3f0
[ 509.054551] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 509.061636] SOFTIRQ-ON-R at:
[ 509.065145] _raw_read_lock+0x2f/0x40
[ 509.070854] find_ife_oplist+0x1e/0xc0 [act_ife]
[ 509.077515] tcf_ife_init+0x82f/0xf40 [act_ife]
[ 509.084051] tcf_action_init_1+0x510/0x750
[ 509.090172] tcf_action_init+0x1e8/0x340
[ 509.096124] tcf_action_add+0xc5/0x240
[ 509.101891] tc_ctl_action+0x203/0x2a0
[ 509.107671] rtnetlink_rcv_msg+0x5bd/0x7b0
[ 509.113811] netlink_rcv_skb+0x184/0x220
[ 509.119768] netlink_unicast+0x31b/0x460
[ 509.125716] netlink_sendmsg+0x3fb/0x840
[ 509.131668] sock_sendmsg+0x7b/0xd0
[ 509.137167] ___sys_sendmsg+0x4c6/0x610
[ 509.143010] __sys_sendmsg+0xd7/0x150
[ 509.148718] do_syscall_64+0x7a/0x3f0
[ 509.154443] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 509.161533] INITIAL USE at:
[ 509.164956] _raw_read_lock+0x2f/0x40
[ 509.170574] find_ife_oplist+0x1e/0xc0 [act_ife]
[ 509.177134] tcf_ife_init+0x82f/0xf40 [act_ife]
[ 509.183619] tcf_action_init_1+0x510/0x750
[ 509.189674] tcf_action_init+0x1e8/0x340
[ 509.195534] tcf_action_add+0xc5/0x240
[ 509.201229] tc_ctl_action+0x203/0x2a0
[ 509.206920] rtnetlink_rcv_msg+0x5bd/0x7b0
[ 509.212936] netlink_rcv_skb+0x184/0x220
[ 509.218818] netlink_unicast+0x31b/0x460
[ 509.224699] netlink_sendmsg+0x3fb/0x840
[ 509.230581] sock_sendmsg+0x7b/0xd0
[ 509.235984] ___sys_sendmsg+0x4c6/0x610
[ 509.241791] __sys_sendmsg+0xd7/0x150
[ 509.247425] do_syscall_64+0x7a/0x3f0
[ 509.253007] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 509.259975] }
[ 509.261998] ... key at: [<ffffffffc1554258>] ife_mod_lock+0x18/0xffffffffffff8dc0 [act_ife]
[ 509.271569] ... acquired at:
[ 509.274912] _raw_read_lock+0x2f/0x40
[ 509.279134] find_ife_oplist+0x1e/0xc0 [act_ife]
[ 509.284324] tcf_ife_init+0x82f/0xf40 [act_ife]
[ 509.289425] tcf_action_init_1+0x510/0x750
[ 509.294068] tcf_action_init+0x1e8/0x340
[ 509.298553] tcf_action_add+0xc5/0x240
[ 509.302854] tc_ctl_action+0x203/0x2a0
[ 509.307153] rtnetlink_rcv_msg+0x5bd/0x7b0
[ 509.311805] netlink_rcv_skb+0x184/0x220
[ 509.316282] netlink_unicast+0x31b/0x460
[ 509.320769] netlink_sendmsg+0x3fb/0x840
[ 509.325248] sock_sendmsg+0x7b/0xd0
[ 509.329290] ___sys_sendmsg+0x4c6/0x610
[ 509.333687] __sys_sendmsg+0xd7/0x150
[ 509.337902] do_syscall_64+0x7a/0x3f0
[ 509.342116] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 509.349601]
stack backtrace:
[ 509.354663] CPU: 6 PID: 5460 Comm: tc Not tainted 4.18.0-rc8+ #646
[ 509.361216] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
Fixes: ef6980b6becb ("introduce IFE action")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_ife.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 5d200495e467..fdb928ca81bb 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -167,16 +167,16 @@ static struct tcf_meta_ops *find_ife_oplist(u16 metaid)
{
struct tcf_meta_ops *o;
- read_lock(&ife_mod_lock);
+ read_lock_bh(&ife_mod_lock);
list_for_each_entry(o, &ifeoplist, list) {
if (o->metaid == metaid) {
if (!try_module_get(o->owner))
o = NULL;
- read_unlock(&ife_mod_lock);
+ read_unlock_bh(&ife_mod_lock);
return o;
}
}
- read_unlock(&ife_mod_lock);
+ read_unlock_bh(&ife_mod_lock);
return NULL;
}
@@ -190,12 +190,12 @@ int register_ife_op(struct tcf_meta_ops *mops)
!mops->get || !mops->alloc)
return -EINVAL;
- write_lock(&ife_mod_lock);
+ write_lock_bh(&ife_mod_lock);
list_for_each_entry(m, &ifeoplist, list) {
if (m->metaid == mops->metaid ||
(strcmp(mops->name, m->name) == 0)) {
- write_unlock(&ife_mod_lock);
+ write_unlock_bh(&ife_mod_lock);
return -EEXIST;
}
}
@@ -204,7 +204,7 @@ int register_ife_op(struct tcf_meta_ops *mops)
mops->release = ife_release_meta_gen;
list_add_tail(&mops->list, &ifeoplist);
- write_unlock(&ife_mod_lock);
+ write_unlock_bh(&ife_mod_lock);
return 0;
}
EXPORT_SYMBOL_GPL(unregister_ife_op);
@@ -214,7 +214,7 @@ int unregister_ife_op(struct tcf_meta_ops *mops)
struct tcf_meta_ops *m;
int err = -ENOENT;
- write_lock(&ife_mod_lock);
+ write_lock_bh(&ife_mod_lock);
list_for_each_entry(m, &ifeoplist, list) {
if (m->metaid == mops->metaid) {
list_del(&mops->list);
@@ -222,7 +222,7 @@ int unregister_ife_op(struct tcf_meta_ops *mops)
break;
}
}
- write_unlock(&ife_mod_lock);
+ write_unlock_bh(&ife_mod_lock);
return err;
}
@@ -343,13 +343,13 @@ static int use_all_metadata(struct tcf_ife_info *ife)
int rc = 0;
int installed = 0;
- read_lock(&ife_mod_lock);
+ read_lock_bh(&ife_mod_lock);
list_for_each_entry(o, &ifeoplist, list) {
rc = add_metainfo(ife, o->metaid, NULL, 0, true);
if (rc == 0)
installed += 1;
}
- read_unlock(&ife_mod_lock);
+ read_unlock_bh(&ife_mod_lock);
if (installed)
return 0;
--
2.7.5
^ permalink raw reply related
* Re: [PATCH 0/3] net, IB/ipoib: Use dev_port to disambiguate
From: Jason Gunthorpe @ 2018-08-13 17:17 UTC (permalink / raw)
To: Arseny Maslennikov; +Cc: linux-rdma, Doug Ledford, netdev
In-Reply-To: <20180813114224.7065-1-ar@cs.msu.ru>
On Mon, Aug 13, 2018 at 02:42:21PM +0300, Arseny Maslennikov wrote:
> Pre-3.15 userspace had trouble distinguishing different ports of a NIC
> on a single PCI bus/device/function. To solve this, a sysfs field `dev_port'
> was introduced quite a while ago (commit v3.14-rc3-739-g3f85944fe207), and
> some relevant device drivers were fixed to use it, but not in case of IPoIB.
>
> The convention for some reason never got documented in the kernel, but
> was immediately adopted by userspace (notably udev[1][2], biosdevname[3])
>
> 3/3 documents the sysfs field — that's why I'm CC-ing netdev.
>
> This series was tested on current LTS and 4.18.
>
> [1] https://lists.freedesktop.org/archives/systemd-devel/2014-June/020788.html
> [2] https://lists.freedesktop.org/archives/systemd-devel/2014-July/020804.html
> [3] https://github.com/CloudAutomationNTools/biosdevname/blob/c795d51dd93a5309652f0d635f12a3ecfabfaa72/src/eths.c#L38
>
> Arseny Maslennikov (3):
> IB/ipoib: Use dev_port to expose network interface port numbers
> IB/ipoib: Stop using dev_id to expose port numbers
> Documentation/ABI: document /sys/class/net/*/dev_port
>
> Documentation/ABI/testing/sysfs-class-net | 10 ++++++++++
> drivers/infiniband/ulp/ipoib/ipoib_main.c | 2 +-
> 2 files changed, 11 insertions(+), 1 deletion(-)
This series doesn't apply to rdma for-next, and it is the merge window
now.
Can you resubmit this aginst 4.19-rc1 in two weeks? Thanks
Jason
^ permalink raw reply
* net-next is CLOSED...
From: David Miller @ 2018-08-13 17:08 UTC (permalink / raw)
To: netdev
Please only submit bug fixes at this time, thank you.
^ permalink raw reply
* Re: pull-request: bpf-next 2018-08-13
From: David Miller @ 2018-08-13 17:08 UTC (permalink / raw)
To: daniel; +Cc: ast, netdev
In-Reply-To: <20180812234922.2704-1-daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Mon, 13 Aug 2018 01:49:22 +0200
> The following pull-request contains BPF updates for your *net-next* tree.
>
> The main changes are:
...
> Please consider pulling these changes from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
Pulled, thanks Daniel.
^ permalink raw reply
* Re: [PATCH bpf-next 0/4] Convert filter.txt to RST
From: Alexei Starovoitov @ 2018-08-13 19:37 UTC (permalink / raw)
To: Tobin C. Harding
Cc: Jonathan Corbet, Daniel Borkmann, Alexei Starovoitov,
David S. Miller, Kees Cook, Andy Lutomirski, Will Drewry,
linux-doc, Network Development, LKML
In-Reply-To: <20180811115058.GN32374@eros>
On Sat, Aug 11, 2018 at 09:50:58PM +1000, Tobin C. Harding wrote:
> On Fri, Aug 10, 2018 at 10:51:28AM -0700, Alexei Starovoitov wrote:
> > On Fri, Aug 10, 2018 at 5:57 AM Jonathan Corbet <corbet@lwn.net> wrote:
> > >
> > > The objective actually is to have SPDX tags in all files in the kernel.
> > > That includes documentation, even though people, as always, care less
> > > about the docs than they do the code.
> >
> > right, but let's do that as a separate patch set.
> > In the current set I'd focus on reviewing the actual doc changes.
> > In particular completely removing
> > Documentation/networking/filter.txt
> > feels wrong, since lots of websites point directly there.
> > Can we have at least few words there pointing to new location?
>
> Something like ...
>
>
> ------------ filter.txt
>
> BPF documentation can now be found in the following places:
>
> - Introduction to BPF (Linux Socket Filter) - Documentation/userspace-api/socket-filter.rst
> - Classic BPF (cBPF) - Documentation/userspace-api/cBPF.rst
> - Internal BPF (eBPF) - Documentation/userspace-api/eBPF.rst
Internal ?
that was the name we used for may be a month many years ago.
Please use 'extended BPF' in new filter.txt and all other places.
since merge window is open the patches would need to wait until
bpf-next opens up in few weeks.
Thanks
^ permalink raw reply
* Re: [PATCH net-next] lan743x: lan743x: Remove duplicated include from lan743x_ptp.c
From: David Miller @ 2018-08-13 16:46 UTC (permalink / raw)
To: yuehaibing; +Cc: bryan.whitehead, UNGLinuxDriver, netdev, kernel-janitors
In-Reply-To: <1534142361-92174-1-git-send-email-yuehaibing@huawei.com>
From: Yue Haibing <yuehaibing@huawei.com>
Date: Mon, 13 Aug 2018 06:39:21 +0000
> Remove duplicated include.
>
> Signed-off-by: Yue Haibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2 iproute2-next] sch_cake: Make gso-splitting configurable
From: Toke Høiland-Jørgensen @ 2018-08-13 16:44 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Dave Taht
In-Reply-To: <f58a05f6-a544-c9b3-7ef7-ae83f37a682e@gmail.com>
David Ahern <dsahern@gmail.com> writes:
> On 8/13/18 5:36 AM, Toke Høiland-Jørgensen wrote:
>> This patch makes sch_cake's gso/gro splitting configurable
>> from userspace.
>>
>> To disable breaking apart superpackets in sch_cake:
>>
>> tc qdisc replace dev whatever root cake no-split-gso
>>
>> to enable:
>>
>> tc qdisc replace dev whatever root cake split-gso
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>> Signed-off-by: Dave Taht <dave.taht@gmail.com>
>> ---
>
> applied to iproute2-next. Thanks
Great, thanks :)
> I think you also need to display it if the attribute is returned.
We already print 'split-gso' when it is set; this was previously decided
by the kernel, this change just makes it user-configurable...
-Toke
^ permalink raw reply
* Re: [PATCH][net-next][v2] packet: switch kvzalloc to allocate memory
From: David Miller @ 2018-08-13 16:37 UTC (permalink / raw)
To: lirongqing; +Cc: netdev
In-Reply-To: <1534128166-11972-1-git-send-email-lirongqing@baidu.com>
From: Li RongQing <lirongqing@baidu.com>
Date: Mon, 13 Aug 2018 10:42:46 +0800
> The patches includes following change:
>
> *Use modern kvzalloc()/kvfree() instead of custom allocations.
>
> *Remove order argument for alloc_pg_vec, it can get from req.
>
> *Remove order argument for free_pg_vec, free_pg_vec now uses
> kvfree which does not need order argument.
>
> *Remove pg_vec_order from struct packet_ring_buffer, no longer
> need to save/restore 'order'
>
> *Remove variable 'order' for packet_set_ring, it is now unused
>
> Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
Applied.
^ permalink raw reply
* Re: [PATCH 2/2] net: socket: Fix potential spectre v1 gadget in sock_is_registered
From: Jeremy Cline @ 2018-08-13 19:03 UTC (permalink / raw)
To: Josh Poimboeuf; +Cc: David S . Miller, netdev, linux-kernel, stable
In-Reply-To: <20180813171642.wlxmnzgsg2rkwe4o@treble>
On 08/13/2018 06:16 PM, Josh Poimboeuf wrote:
> On Sun, Jul 29, 2018 at 11:59:36AM -0400, Jeremy Cline wrote:
>> On 07/29/2018 09:59 AM, Josh Poimboeuf wrote:
>>> On Fri, Jul 27, 2018 at 10:43:02PM +0000, Jeremy Cline wrote:
>>>> 'family' can be a user-controlled value, so sanitize it after the bounds
>>>> check to avoid speculative out-of-bounds access.
>>>>
>>>> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Jeremy Cline <jcline@redhat.com>
>>>> ---
>>>> net/socket.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/socket.c b/net/socket.c
>>>> index f15d5cbb3ba4..608e29ae6baf 100644
>>>> --- a/net/socket.c
>>>> +++ b/net/socket.c
>>>> @@ -2672,7 +2672,8 @@ EXPORT_SYMBOL(sock_unregister);
>>>>
>>>> bool sock_is_registered(int family)
>>>> {
>>>> - return family < NPROTO && rcu_access_pointer(net_families[family]);
>>>> + return family < NPROTO &&
>>>> + rcu_access_pointer(net_families[array_index_nospec(family, NPROTO)]);
>>>> }
>>>>
>>>> static int __init sock_init(void)
>>>
>>> This is another one where I think it would be better to do the nospec
>>> clamp higher up the call chain. The untrusted 'family' value comes from
>>> __sock_diag_cmd():
>>>
>>> __sock_diag_cmd
>>> sock_load_diag_module
>>> sock_is_registered
>>>
>>> That function has a bounds check, and also uses the value in some other
>>> array accesses:
>>>
>>> if (req->sdiag_family >= AF_MAX)
>>> return -EINVAL;
>>>
>>> if (sock_diag_handlers[req->sdiag_family] == NULL)
>>> sock_load_diag_module(req->sdiag_family, 0);
>>>
>>> mutex_lock(&sock_diag_table_mutex);
>>> hndl = sock_diag_handlers[req->sdiag_family];
>>> ...
>>>
>>> So I think clamping 'req->sdiag_family' right after the bounds check
>>> would be the way to go.
>>>
>>
>> Indeed, the clamp there would cover this clamp. I had a scheme that I
>> quickly fix all the gadgets in functions with local comparisons, but
>> clearly that's going to result in call chains with multiple clamps.
>>
>> I can fix this in a follow-up with a clamp here, or respin this patch
>> set, whatever is easier for David.
>
> Hi Jeremy,
>
> Just checking up on this... since this patch was merged, will you be
> doing a followup patch?
>
Yes, apologies, I've been traveling. I'll have a patch tomorrow.
^ permalink raw reply
* Containers and checkpoint/restart micro-conference at LPC2018
From: Stéphane Graber @ 2018-08-13 16:10 UTC (permalink / raw)
To: lxc-devel, lxc-users, containers, linux-security-module,
linux-fsdevel, netdev
[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]
Hello,
This year's edition of the Linux Plumbers Conference will once again
have a containers micro-conference but this time around we'll have twice
the usual amount of time and will include the content that would
traditionally go into the checkpoint/restore micro-conference.
LPC2018 will be held in Vancouver, Canada from the 13th to the 15th of
November, co-located with the Linux Kernel Summit.
We're looking for discussion topics around kernel work related to
containers and namespacing, resource control, access control,
checkpoint/restore of kernel structures, filesystem/mount handling for
containers and any related userspace work.
The format of the event will mostly be discussions where someone
introduces a given topic/problem and it then gets discussed for 20-30min
before moving on to something else. There will also be limited room for
short demos of recent work with shorter 15min slots.
Details can be found here:
https://discuss.linuxcontainers.org/t/containers-micro-conference-at-linux-plumbers-2018/2417
Looking forward to seeing you in Vancouver!
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [net-next,v2] cpumask: make cpumask_next_wrap available without smp
From: Krzysztof Kozlowski @ 2018-08-13 16:01 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, davem, caleb.raitto, lkp, Willem de Bruijn
In-Reply-To: <20180812131403.123852-1-willemdebruijn.kernel@gmail.com>
On Sun, Aug 12, 2018 at 09:14:03AM -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> The kbuild robot shows build failure on machines without CONFIG_SMP:
If this was reported by kbuild robot, then could you credit him with
Reported-by?
>
> drivers/net/virtio_net.c:1916:10: error:
> implicit declaration of function 'cpumask_next_wrap'
>
> cpumask_next_wrap is exported from lib/cpumask.o, which has
>
> lib-$(CONFIG_SMP) += cpumask.o
>
> same as other functions, also define it as static inline in the
> NR_CPUS==1 branch in include/linux/cpumask.h.
>
> If wrap is true and next == start, return nr_cpumask_bits, or 1.
> Else wrap across the range of valid cpus, here [0].
>
> Fixes: 2ca653d607ce ("virtio_net: Stripe queue affinities across cores.")
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> include/linux/cpumask.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
This fixes second build break of virtio driver in recent few days... I
wonder what happened with build-testing of own commits... or at least
reading kbuild robot mails.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH net-next 00/13] net: sched: actions rename for grep-ability and consistency
From: David Miller @ 2018-08-13 16:00 UTC (permalink / raw)
To: jhs; +Cc: xiyou.wangcong, jiri, netdev, kernel
In-Reply-To: <20180812133501.15278-1-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Sun, 12 Aug 2018 09:34:48 -0400
> Having a structure (example tcf_mirred) and a function with the same name is
> not good for readability or grepability.
>
> This long overdue patchset improves it and make sure there is consistency
> across all actions
Series applied.
^ permalink raw reply
* Re: [Query]: DSA Understanding
From: Lad, Prabhakar @ 2018-08-13 15:58 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, netdev
In-Reply-To: <20180813133817.GA18645@lunn.ch>
Hi Andrew/Florain,
On Mon, Aug 13, 2018 at 2:38 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > I agree, this should be padding packets correctly, can you still
> > > instrument cpsw to make sure that what comes to its ndo_start_xmit() is
> > > ETH_ZLEN + tag_len or more?
> > >
> > Yes I can confirm the skb->len is always >= 62 (ETH_ZLEN + 2)
>
> Which switch are you using?
>
> Marvell switches use either 4 or 8 bytes of tag. Broadcom has 4, KSZ
> has 1 for packets going to the switch, lan9303 has 4, mtd uses 4, qca
> has 2.
>
I am using the KSZ switch. for Ingress it has 1 byte and for Egress it
has 2 bytes.
I came across patch [1] and padded 2 more bytes in ksz_xmit() and I was
successfully able to ping from lan4 to PC. Thank you very much for
your guidance/support.
Now I have stumbled into a different issue:
Case 1 Works:
=================
lan0 = 192.168.0.1
PC1 = 192.168.0.10
For the above ping works from both directions.
CASE 2 Doesn’t Work:
=========================
lan0 = 192.168.0.1
PC1 = 192.168.0.10
lan4 = 192.168.0.4
PC2 = 192.168.0.11
Ping from lan0 to PC1 and PC1 to lan0 works
But ping from PC2 to lan4 and lan4 to PC2 fails.
CASE 3 Works:
=========================
lan0 = 192.168.0.1
PC1 = 192.168.0.10
lan4 = 192.168.4.4
PC2 = 192.168.4.11
With the above setup ping works.
[Query] Why does ping fail in case 2. Any thoughts what I am missing here ?
or is it the expected behaviour ?
[1] https://lore.kernel.org/patchwork/patch/851457/
Cheers,
--Prabhakar Lad
^ permalink raw reply
* Re: [PATCH net-next v2] cpumask: make cpumask_next_wrap available without smp
From: David Miller @ 2018-08-13 15:58 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, caleb.raitto, lkp, willemb
In-Reply-To: <20180812131403.123852-1-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Sun, 12 Aug 2018 09:14:03 -0400
> From: Willem de Bruijn <willemb@google.com>
>
> The kbuild robot shows build failure on machines without CONFIG_SMP:
>
> drivers/net/virtio_net.c:1916:10: error:
> implicit declaration of function 'cpumask_next_wrap'
>
> cpumask_next_wrap is exported from lib/cpumask.o, which has
>
> lib-$(CONFIG_SMP) += cpumask.o
>
> same as other functions, also define it as static inline in the
> NR_CPUS==1 branch in include/linux/cpumask.h.
>
> If wrap is true and next == start, return nr_cpumask_bits, or 1.
> Else wrap across the range of valid cpus, here [0].
>
> Fixes: 2ca653d607ce ("virtio_net: Stripe queue affinities across cores.")
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Applied, thanks Willem.
^ permalink raw reply
* Re: [PATCH net] r8169: don't use MSI-X on RTL8168g
From: David Miller @ 2018-08-13 15:58 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev
In-Reply-To: <c2dab5e3-2444-8b34-7433-c7fdcf5f4a44@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sun, 12 Aug 2018 13:26:26 +0200
> There have been two reports that network doesn't come back on resume
> from suspend when using MSI-X. Both cases affect the same chip version
> (RTL8168g - version 40), on different systems. Falling back to MSI
> fixes the issue.
> Even though we don't really have a proof yet that the network chip
> version is to blame, let's disable MSI-X for this version.
>
> Reported-by: Steve Dodd <steved424@gmail.com>
> Reported-by: Lou Reed <gogen@disroot.org>
> Tested-by: Steve Dodd <steved424@gmail.com>
> Tested-by: Lou Reed <gogen@disroot.org>
> Fixes: 6c6aa15fdea5 ("r8169: improve interrupt handling")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net v2] l2tp: use sk_dst_check() to avoid race on sk->sk_dst_cache
From: David Miller @ 2018-08-13 15:49 UTC (permalink / raw)
To: weiwan; +Cc: netdev, kafai, g.nault, dsahern, xiyou.wangcong
In-Reply-To: <20180810181456.76250-1-tracywwnj@gmail.com>
From: Wei Wang <weiwan@google.com>
Date: Fri, 10 Aug 2018 11:14:56 -0700
> From: Wei Wang <weiwan@google.com>
>
> In l2tp code, if it is a L2TP_UDP_ENCAP tunnel, tunnel->sk points to a
> UDP socket. User could call sendmsg() on both this tunnel and the UDP
> socket itself concurrently. As l2tp_xmit_skb() holds socket lock and call
> __sk_dst_check() to refresh sk->sk_dst_cache, while udpv6_sendmsg() is
> lockless and call sk_dst_check() to refresh sk->sk_dst_cache, there
> could be a race and cause the dst cache to be freed multiple times.
> So we fix l2tp side code to always call sk_dst_check() to garantee
> xchg() is called when refreshing sk->sk_dst_cache to avoid race
> conditions.
>
> Syzkaller reported stack trace:
...
>
> Fixes: 71b1391a4128 ("l2tp: ensure sk->dst is still valid")
> Reported-by: syzbot+05f840f3b04f211bad55@syzkaller.appspotmail.com
> Signed-off-by: Wei Wang <weiwan@google.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
...
> ---
> v1->v2: Removed dst_clone() as Guillaume Nault suggested
Applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: [PATCH net-next v3 1/1] net/tls: Combined memory allocation for decryption request
From: David Miller @ 2018-08-13 15:42 UTC (permalink / raw)
To: vakul.garg; +Cc: netdev, borisp, aviadye, davejwatson
In-Reply-To: <20180810151641.14580-2-vakul.garg@nxp.com>
From: Vakul Garg <vakul.garg@nxp.com>
Date: Fri, 10 Aug 2018 20:46:41 +0530
> For preparing decryption request, several memory chunks are required
> (aead_req, sgin, sgout, iv, aad). For submitting the decrypt request to
> an accelerator, it is required that the buffers which are read by the
> accelerator must be dma-able and not come from stack. The buffers for
> aad and iv can be separately kmalloced each, but it is inefficient.
> This patch does a combined allocation for preparing decryption request
> and then segments into aead_req || sgin || sgout || iv || aad.
>
> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
> ---
>
> Changes since v2:
> - Rebased to not require following Doron Roberts-Kedes's patch.
> "net/tls: Calculate nsg for zerocopy path without skb_cow_data."
Applied.
^ permalink raw reply
* Re: [PATCH bpf-next 1/4] bpf: Introduce bpf_skb_ancestor_cgroup_id helper
From: Andrey Ignatov @ 2018-08-13 15:39 UTC (permalink / raw)
To: Tejun Heo; +Cc: netdev, ast, daniel, guro, kernel-team
In-Reply-To: <20180813144635.GI3978217@devbig004.ftw2.facebook.com>
Tejun Heo <tj@kernel.org> [Mon, 2018-08-13 07:47 -0700]:
> Hello, Andrey.
Hey Tejun!
> On Fri, Aug 10, 2018 at 10:35:23PM -0700, Andrey Ignatov wrote:
> > +static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
> > + int ancestor_level)
> > +{
> > + struct cgroup *ptr;
> > +
> > + if (cgrp->level < ancestor_level)
> > + return NULL;
> > +
> > + for (ptr = cgrp;
> > + ptr && ptr->level > ancestor_level;
> > + ptr = cgroup_parent(ptr))
> > + ;
> > +
> > + if (ptr && ptr->level == ancestor_level)
> > + return ptr;
> > +
> > + return NULL;
> > +}
>
> I don't have any objections functionanlity-wise but can we do sth like
> the following instead?
>
> static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
> int ancestor_level)
> {
> if (cgrp->level < ancestor_level)
> return NULL;
>
> while (cgrp->level > ancestor_level)
> cgrp = cgroup_parent(cgrp);
> return cgrp;
> }
Sure, no problem. I'll send follow-up patch to simplify this part when
bpf-next is open again.
>
> Thanks.
>
> --
> tejun
--
Andrey Ignatov
^ permalink raw reply
* Re: [V9fs-developer] [PATCH 2/2] 9p: Add refcount to p9_req_t
From: Tomas Bortoli @ 2018-08-13 18:14 UTC (permalink / raw)
To: Dmitry Vyukov, Dominique Martinet
Cc: piaojun, Eric Van Hensbergen, Ron Minnich, Latchesar Ionkov,
Dominique Martinet, netdev, LKML, syzkaller, v9fs-developer,
David Miller
In-Reply-To: <CACT4Y+YQckNWVcBoW1WtH=5pEU5KXB8J8A=QssWx8nbp5GtNnw@mail.gmail.com>
On 08/13/2018 03:04 PM, Dmitry Vyukov wrote:
> On Mon, Aug 13, 2018 at 3:48 AM, Dominique Martinet
> <asmadeus@codewreck.org> wrote:
>> piaojun wrote on Mon, Aug 13, 2018:
>>> Could you help paste the reason of the crash bug to help others
>>> understand more clearly? And I have another question below.
>>
>> The problem for tcp (but other transports have a similar problem) is
>> that with a malicious server like syzkaller they can try to submit
>> replies before the request came in.
>>
>> This leads in the writer thread trying to write a buffer that has
>> already been freed, and if memory has been reused could potentially leak
>> some information.
>>
>> Now, with the previous patches this is based on this would be a slab and
>> the likeliness of it being sensitive information is rather low (it would
>> likely be some other packet being sent twice, or a mix and match of two
>> packets that would have been sent anyway), but it would nevertheless be
>> a use after free.
>>
>>
>> There is a second advantage to this reference counting, that is now we
>> have this system we will be able to implement flush asynchronously.
>> This will remove the need for the 'goto again' in p9_client_rpc which
>> was making 9p threads unkillable in practice if the server would not
>> reply to the flush requests.
>
>
> Fixing unkillalble task would be nice. Don't know how much they are of
> a problem in real life, but fixing them would allow fuzzer to find
> other, potentially more critical bugs in 9p. These "task hung" crashes
> are quite unpleasant for the fuzzer.
>
> Thanks for all recent 9p work, Tomas!
>
You are welcome, I have to thank Dominique that helped me a lot, I like
to help here, it's educative.
>
>> Even if the server replies I've always found myself needing to hit ^C
>> multiple times to exit a process doing I/Os and I think fixing that
>> behaviour will make 9p more comfortable to use.
>>
>>
>>>> diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
>>>> index 20f46f13fe83..686e24e355d0 100644
>>>> --- a/net/9p/trans_fd.c
>>>> +++ b/net/9p/trans_fd.c
>>>> @@ -132,6 +132,7 @@ struct p9_conn {
>>>> struct list_head req_list;
>>>> struct list_head unsent_req_list;
>>>> struct p9_req_t *req;
>>>> + struct p9_req_t *wreq;
>>>
>>> Why adding a wreq for write work? And I wonder we should rename req to
>>> rreq?
>>
>> We need to store a pointer to the request for the write thread because
>> we need to put the reference to it when we're done writing its content.
>>
>> Previously, the worker would only store the write buffer there but
>> that's not enough to figure what request to dereference.
>>
>>
>> I personally don't think renaming req to rreq would bring much but it
>> could be done in another patch if you think that'd be helpful; I think
>> it shouldn't be done here at least to make the patch more readable.
>>
>> --
>> Dominique
>>
>> --
>> You received this message because you are subscribed to the Google Groups "syzkaller" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH v1 0/3] WireGuard: Secure Network Tunnel
From: Jason A. Donenfeld @ 2018-08-13 17:55 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-kernel, netdev, davem, linux-crypto
In-Reply-To: <1534181830.7872.10.camel@HansenPartnership.com>
> but it's very hard for a flow classifier because you have to
The construction and identifier strings might not obviously help with
the extremely narrow idea you've brought up, but it is very important
for safely introducing additional versions. Namely, it prevents
against cross-protocol key reuse attacks and type confusion bugs. So
don't be too quick to dismiss the importance of these for
accomplishing what we're after.
> so lets pick one of the above and try it out.
We have, multiple times, and it's absolutely trivial to do and works
well. The exact thing you're concerned about has already been
researched and worked with on live systems quite a bit over the last 3
years, and it works in a pretty straight forward way. I'm not sure
there's much more to add here: the thing you want is already there and
has been tested extensively. At this point the "pick one and let's try
it out!" is an old story, and the focus now is on making sure the code
quality and netdev api usage is correct for merging
^ 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