linux-kernel - Re: [PATCH 2/3] static_call: Align static_call_is

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 for Android: free password hash cracker in your pocket

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

Message-ID: <c4e9dfdf-c83a-3314-8c55-5b2371a56ec8@rasmusvillemoes.dk>
Date:   Fri, 19 Mar 2021 15:40:46 +0100
From:   Rasmus Villemoes <linux@...musvillemoes.dk>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     x86@...nel.org, jpoimboe@...hat.com, jbaron@...mai.com,
        rostedt@...dmis.org, ardb@...nel.org, linux-kernel@...r.kernel.org,
        sumit.garg@...aro.org, oliver.sang@...el.com, jarkko@...nel.org
Subject: Re: [PATCH 2/3] static_call: Align static_call_is_init() patching
 condition

On 19/03/2021 15.13, Peter Zijlstra wrote:

>> Dunno, probably overkill, but perhaps we could have an atomic_t (or
>> refcount, whatever) init_ref inited to 1, with init_ref_get() doing an
>> inc_unless_zero, and iff you get a ref, you're free to call (/patch)
>> __init functions and access __initdata, but must do init_ref_put(), with
>> PID1 dropping its initial ref and waiting for it to drop to 0 before
>> doing the *free_initmem() calls.
> 
> I'd as soon simply add another SYSTEM state. That way we don't have to
> worry about who else looks at RUNNING for what etc..

I don't understand. How would that solve the

PID1                           PIDX
                               ok = system_state < INIT_MEM_GONE;
system_state = INIT_MEM_GONE;
free_initmem();
system_state = RUNNING;
                               if (ok)
                                   poke init mem

race? I really don't see any way arbitrary threads can reliably check
how far PID1 has progressed at one point in time and use that
information (a few lines) later to access init memory without
synchronizing with PID1.

AFAICT, having an atomic_t object representing the init memory and a
"no, you can't have a new reference" would guarantee correctness of the
jump_label/static_call patching: If we get a reference, we do the
patching just as for the rest of the kernel .text. If we don't, i.e.
observe atomic_load(init_ref)==0, that may not necessarily mean that
PID1 has actually discarded the memory yet, but no thread can currently
or in the future actually run __init code, so it need not be patched.

Rasmus