netdev - Re: [Patch net 8/9] act_ife: move tcfa

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]

Date:   Mon, 20 Aug 2018 11:29:52 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     xiyou.wangcong@...il.com
Cc:     netdev@...r.kernel.org, jhs@...atatu.com, vladbu@...lanox.com
Subject: Re: [Patch net 8/9] act_ife: move tcfa_lock down to where necessary

From: Cong Wang <xiyou.wangcong@...il.com>
Date: Sun, 19 Aug 2018 12:22:12 -0700

> The only time we need to take tcfa_lock is when adding
> a new metainfo to an existing ife->metalist. We don't need
> to take tcfa_lock so early and so broadly in tcf_ife_init().
> 
> This means we can always take ife_mod_lock first, avoid the
> reverse locking ordering warning as reported by Vlad.
> 
> Reported-by: Vlad Buslov <vladbu@...lanox.com>
> Tested-by: Vlad Buslov <vladbu@...lanox.com>
> Cc: Vlad Buslov <vladbu@...lanox.com>
> Cc: Jamal Hadi Salim <jhs@...atatu.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@...il.com>

After this change we no longer call populate_metalist() in an atomic
context via tcf_ife_init(), and populate_metalist passes 'exists'
down to add_metainfo() as an 'atomic' indicator.  It doesn't have this
meaning if you aren't holding the tcfa_lock in the callers with BH
disabled.

Therefore, add_metainfo()'s 'atomic' indication is inaccurate in this
call chain and will use GFP_ATOMIC unnecessarily.

Probably the thing to just is just pass 'false' down to add_metainfo()
in populate_metalist().