vmcore_cleanup() has a single caller, fadump's release_mem_store() on
powerpc. That checks fw_dump.dump_active without a lock and calls
vmcore_cleanup() without one, and kernfs only serializes the writers of
one open file. So two tasks that write to
/sys/kernel/fadump/release_mem through different files run
vmcore_cleanup() at the same time.

The first one removes /proc/vmcore and waits for its readers in
proc_remove(). The second one finds the entry already unlinked, returns
from proc_remove() right away and frees vmcore_list, the ELF headers
and the device dumps while those readers still walk them. Or both of
them free the same lists at once.

Serialize vmcore_cleanup() with a mutex of its own. It can't be
vmcore_mutex, removing /proc/vmcore calls release_vmcore() which takes
it. The second caller then finds nothing left to free.

Reported-by: Chris Mason <[email protected]>
Fixes: b500afff11f6 ("fadump: Invalidate registration and release reserved 
memory for general use.")
Cc: [email protected]
Signed-off-by: Christian Brauner (Amutable) <[email protected]>
---
 fs/proc/vmcore.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 1fe70c4ecc54..ccb795b85509 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -1764,6 +1764,11 @@ fs_initcall(vmcore_init);
 /* Cleanup function for vmcore module. */
 void vmcore_cleanup(void)
 {
+       /* Not vmcore_mutex, removing /proc/vmcore calls release_vmcore(). */
+       static DEFINE_MUTEX(vmcore_cleanup_mutex);
+
+       /* Two writers of fadump's release_mem can get here at once. */
+       guard(mutex)(&vmcore_cleanup_mutex);
        if (proc_vmcore) {
                proc_remove(proc_vmcore);
                proc_vmcore = NULL;

-- 
2.53.0


Reply via email to