netdev - RE: [patch net v2 1/4] net/sched: Change tc_action refcnt and bindcnt to atomic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <VI1PR0501MB21439CB952C540F903969648AB4C0@VI1PR0501MB2143.eurprd05.prod.outlook.com>
Date:   Tue, 17 Oct 2017 01:14:32 +0000
From:   Chris Mi <chrism@...lanox.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
CC:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Lucas Bates <lucasb@...atatu.com>,
        Jiri Pirko <jiri@...nulli.us>,
        David Miller <davem@...emloft.net>
Subject: RE: [patch net v2 1/4] net/sched: Change tc_action refcnt and bindcnt
 to atomic



> -----Original Message-----
> From: Cong Wang [mailto:xiyou.wangcong@...il.com]
> Sent: Tuesday, October 17, 2017 1:06 AM
> To: Chris Mi <chrism@...lanox.com>
> Cc: Linux Kernel Network Developers <netdev@...r.kernel.org>; Jamal Hadi
> Salim <jhs@...atatu.com>; Lucas Bates <lucasb@...atatu.com>; Jiri Pirko
> <jiri@...nulli.us>; David Miller <davem@...emloft.net>
> Subject: Re: [patch net v2 1/4] net/sched: Change tc_action refcnt and
> bindcnt to atomic
> 
> On Mon, Oct 16, 2017 at 4:18 AM, Chris Mi <chrism@...lanox.com> wrote:
> > If many filters share the same action. That action's refcnt and
> > bindcnt could be manipulated by many RCU callbacks at the same time.
> > This patch makes these operations atomic.
> 
> Actually I have been thinking about removing these RCU callbacks, they are
> not necessary AFAIK, callers hold RTNL lock so they are allowed to block. The
> only drawback is that adding a synchronize_rcu(), but these are slow paths
> anyway...
> 
> I am not sure, it is arguable anyway, essentially it is:
> 
> synchronize_rcu() in slow path vs.  multiple RCU callback races
> 
> 
> >
> > Fixes commit in pre-git era.
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> 
> This is not true, the action RCU callbacks were introduced
> by:
> 
> commit c7de2cf053420d63bac85133469c965d4b1083e1
> Author: Eric Dumazet <eric.dumazet@...il.com>
> Date:   Wed Jun 9 02:09:23 2010 +0000
> 
>     pkt_sched: gen_kill_estimator() rcu fixes
> 
> 
> and the filter RCU callbacks were introduced by the patchset like this one:
> 
> 
> commit 1ce87720d456e471de0fbd814dc5d1fe10fc1c44
> Author: John Fastabend <john.fastabend@...il.com>
> Date:   Fri Sep 12 20:09:16 2014 -0700
> 
>     net: sched: make cls_u32 lockless
I don't think this bug were introduced by above two commits only.
Actually, this bug were introduced by several commits, at least the following:
1. refcnt and bindcnt are not atomic
2. passing actions using list instead of arrays (I think initially we are using arrays)
3. using RCU callbacks
So instead of blaming the latest commit, it is better to say it is a pre-git error.