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: <Y4B17nBArWS1Iywo@hirez.programming.kicks-ass.net> Date: Fri, 25 Nov 2022 08:59:42 +0100 From: Peter Zijlstra <peterz@...radead.org> To: Dmitry Safonov <dima@...sta.com> Cc: linux-kernel@...r.kernel.org, David Ahern <dsahern@...nel.org>, Eric Dumazet <edumazet@...gle.com>, Ard Biesheuvel <ardb@...nel.org>, Bob Gilligan <gilligan@...sta.com>, "David S. Miller" <davem@...emloft.net>, Dmitry Safonov <0x7f454c46@...il.com>, Francesco Ruggeri <fruggeri@...sta.com>, Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>, Jakub Kicinski <kuba@...nel.org>, Jason Baron <jbaron@...mai.com>, Josh Poimboeuf <jpoimboe@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Salam Noureddine <noureddine@...sta.com>, Steven Rostedt <rostedt@...dmis.org>, netdev@...r.kernel.org Subject: Re: [PATCH v6 1/5] jump_label: Prevent key->enabled int overflow On Wed, Nov 23, 2022 at 05:38:55PM +0000, Dmitry Safonov wrote: > 1. With CONFIG_JUMP_LABEL=n static_key_slow_inc() doesn't have any > protection against key->enabled refcounter overflow. > 2. With CONFIG_JUMP_LABEL=y static_key_slow_inc_cpuslocked() > still may turn the refcounter negative as (v + 1) may overflow. > > key->enabled is indeed a ref-counter as it's documented in multiple > places: top comment in jump_label.h, Documentation/staging/static-keys.rst, > etc. > > As -1 is reserved for static key that's in process of being enabled, > functions would break with negative key->enabled refcount: > - for CONFIG_JUMP_LABEL=n negative return of static_key_count() > breaks static_key_false(), static_key_true() > - the ref counter may become 0 from negative side by too many > static_key_slow_inc() calls and lead to use-after-free issues. > > These flaws result in that some users have to introduce an additional > mutex and prevent the reference counter from overflowing themselves, > see bpf_enable_runtime_stats() checking the counter against INT_MAX / 2. > > Prevent the reference counter overflow by checking if (v + 1) > 0. > Change functions API to return whether the increment was successful. > > Signed-off-by: Dmitry Safonov <dima@...sta.com> > Acked-by: Jakub Kicinski <kuba@...nel.org> This looks good to me: Acked-by: Peter Zijlstra (Intel) <peterz@...radead.org> What is the plan for merging this? I'm assuming it would want to go through the network tree, but as already noted earlier it depends on a patch I have in tip/locking/core. Now I checked, tip/locking/core is *just* that one patch, so it might be possible to merge that branch and this series into the network tree and note that during the pull request to Linus.
Powered by blists - more mailing lists