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
| ||
|
Message-ID: <CAM_iQpVuqBkQxq=g4n5t33Gv0mxR_S0iOtQxgBHKceam8pmwfQ@mail.gmail.com> Date: Tue, 16 May 2017 10:53:24 -0700 From: Cong Wang <xiyou.wangcong@...il.com> To: Julian Anastasov <ja@....bg> Cc: Eric Dumazet <eric.dumazet@...il.com>, David Miller <davem@...emloft.net>, Linux Kernel Network Developers <netdev@...r.kernel.org>, Andrey Konovalov <andreyknvl@...gle.com>, Eric Dumazet <edumazet@...gle.com> Subject: Re: [Patch net] ipv4: restore rt->fi for reference counting On Tue, May 16, 2017 at 12:46 AM, Julian Anastasov <ja@....bg> wrote: > > Hello, > > On Mon, 15 May 2017, Cong Wang wrote: > >> On Mon, May 15, 2017 at 1:37 PM, Julian Anastasov <ja@....bg> wrote: >> > Any user that does not set FIB_LOOKUP_NOREF >> > will need nh_dev refcounts. The assumption is that the >> > NHs are accessed, who knows, may be even after RCU grace >> > period. As result, we can not use dev_put on NETDEV_UNREGISTER. >> > So, we should check if there are users that do not >> > set FIB_LOOKUP_NOREF, at first look, I don't see such ones >> > for IPv4. >> >> I see, although we do have FIB_LOOKUP_NOREF set all the times, >> there are other places we hold fib_clntref too, for example >> mlxsw_sp_router_fib_event_work(), it actually uses nh_dev too... >> >> So I am afraid moving dev_put() to fib_release_info() is not a solution >> here. I have to rethink about it. > > At first look, they use fib_info_hold() to get fib_clntref > reference from places where fib_treeref is not fatally decreased > to 0 but later a work is used which finishes the job. I guess, we > can convert such places to use just a fib_treeref reference. > They can use such new method instead of fib_info_hold: > > void fib_treeref_get(struct fib_info *fi) > { > spin_lock_bh(&fib_info_lock); > fi->fib_treeref++; > spin_unlock_bh(&fib_info_lock); > } > > They will use fib_release_info() to put the reference. But > on FIB_EVENT_ENTRY_DEL there is a small window where the > scheduled work delays the unlink of fib info from the > hash tables, i.e. there is a risk fib_find_info to reuse > a dead fib info. > Right, that seems risky. How about adding a check for ->fib_dead in these work's? If the last treeref is gone, probably it is no longer needed to continue to do the offloading work.
Powered by blists - more mailing lists