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: <alpine.LFD.2.20.1705160955310.1856@ja.home.ssi.bg> Date: Tue, 16 May 2017 10:46:46 +0300 (EEST) From: Julian Anastasov <ja@....bg> To: Cong Wang <xiyou.wangcong@...il.com> 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 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. May be we can add a fi->fib_flags & RTNH_F_DEAD check there but the problem is that it is set also on NETDEV_DOWN. While attempts to add route to device with !(dev->flags & IFF_UP) is rejected by fib_check_nh(), fib_create_info still can create routes when cfg->fc_scope == RT_SCOPE_HOST. So, RTNH_F_DEAD check in fib_find_info can avoid the reuse of fib info for host routes while device is down but not unregistered. As result, many fib infos can be created instead of one that is reused. Adding new RTNH_F_* flag to properly handle this does not look good... Regards -- Julian Anastasov <ja@....bg>
Powered by blists - more mailing lists