• Jann Horn's avatar
    pstore: Don't use semaphores in always-atomic-context code · 8126b1c7
    Jann Horn authored
    pstore_dump() is *always* invoked in atomic context (nowadays in an RCU
    read-side critical section, before that under a spinlock).
    It doesn't make sense to try to use semaphores here.
    
    This is mostly a revert of commit ea84b580 ("pstore: Convert buf_lock
    to semaphore"), except that two parts aren't restored back exactly as they
    were:
    
     - keep the lock initialization in pstore_register
     - in efi_pstore_write(), always set the "block" flag to false
     - omit "is_locked", that was unnecessary since
       commit 959217c8 ("pstore: Actually give up during locking failure")
     - fix the bailout message
    
    The actual problem that the buggy commit was trying to address may have
    been that the use of preemptible() in efi_pstore_write() was wrong - it
    only looks at preempt_count() and the state of IRQs, but __rcu_read_lock()
    doesn't touch either of those under CONFIG_PREEMPT_RCU.
    (Sidenote: CONFIG_PREEMPT_RCU means that the scheduler can preempt tasks in
    RCU read-side critical sections, but you're not allowed to actively
    block/reschedule.)
    
    Lockdep probably never caught the problem because it's very rare that you
    actually hit the contended case, so lockdep always just sees the
    down_trylock(), not the down_interruptible(), and so it can't tell that
    there's a problem.
    
    Fixes: ea84b580 ("pstore: Convert buf_lock to semaphore")
    Cc: stable@vger.kernel.org
    Acked-by: default avatarSebastian Andrzej Siewior <bigeasy@linutronix.de>
    Signed-off-by: default avatarJann Horn <jannh@google.com>
    Signed-off-by: default avatarKees Cook <keescook@chromium.org>
    Link: https://lore.kernel.org/r/20220314185953.2068993-1-jannh@google.com
    8126b1c7
platform.c 18.7 KB