On Mon 02-11-26 15:21:39, Christian Brauner wrote: > 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]>
Looks good. Feel free to add: Reviewed-by: Jan Kara <[email protected]> Honza > --- > 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 > -- Jan Kara <[email protected]> SUSE Labs, CR
