Skip to content

Commit d9b0532

Browse files
Sebastian Andrzej Siewiorbp3tk0v
authored andcommitted
futex: Move futex_hash_free() back to __mmput()
To avoid a memory leak via mm_alloc() + mmdrop() the futex cleanup code has been moved to __mmdrop(). This resulted in a warnings if the futex hash table has been allocated via vmalloc() the mmdrop() was invoked from atomic context. The free path must stay in __mmput() to ensure it is invoked from preemptible context. In order to avoid the memory leak, delay the allocation of mm_struct::mm->futex_ref to futex_hash_allocate(). This works because neither the per-CPU counter nor the private hash has been allocated and therefore - futex_private_hash() callers (such as exit_pi_state_list()) don't acquire reference if there is no private hash yet. There is also no reference put. - Regular callers (futex_hash()) fallback to global hash. No reference counting here. The futex_ref member can be allocated in futex_hash_allocate() before the private hash itself is allocated. This happens either while the first thread is created or on request. In both cases the process has just a single thread so there can be either futex operation in progress or the request to create a private hash. Move futex_hash_free() back to __mmput(); Move the allocation of mm_struct::futex_ref to futex_hash_allocate(). [ bp: Fold a follow-up fix to prevent a use-after-free: https://lore.kernel.org/r/[email protected] ] Fixes: e703b7e ("futex: Move futex cleanup to __mmdrop()") Closes: https://lore.kernel.org/all/[email protected]/ Reported-by: Jakub Kicinski <[email protected]> Signed-off-by: Sebastian Andrzej Siewior <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Signed-off-by: Borislav Petkov (AMD) <[email protected]> Link: https://lkml.kernel.org/r/[email protected]
1 parent 1b237f1 commit d9b0532

File tree

2 files changed

+13
-5
lines changed

2 files changed

+13
-5
lines changed

kernel/fork.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,6 @@ void __mmdrop(struct mm_struct *mm)
689689
mm_pasid_drop(mm);
690690
mm_destroy_cid(mm);
691691
percpu_counter_destroy_many(mm->rss_stat, NR_MM_COUNTERS);
692-
futex_hash_free(mm);
693692

694693
free_mm(mm);
695694
}
@@ -1138,6 +1137,7 @@ static inline void __mmput(struct mm_struct *mm)
11381137
if (mm->binfmt)
11391138
module_put(mm->binfmt->module);
11401139
lru_gen_del_mm(mm);
1140+
futex_hash_free(mm);
11411141
mmdrop(mm);
11421142
}
11431143

kernel/futex/core.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,12 +1722,9 @@ int futex_mm_init(struct mm_struct *mm)
17221722
RCU_INIT_POINTER(mm->futex_phash, NULL);
17231723
mm->futex_phash_new = NULL;
17241724
/* futex-ref */
1725+
mm->futex_ref = NULL;
17251726
atomic_long_set(&mm->futex_atomic, 0);
17261727
mm->futex_batches = get_state_synchronize_rcu();
1727-
mm->futex_ref = alloc_percpu(unsigned int);
1728-
if (!mm->futex_ref)
1729-
return -ENOMEM;
1730-
this_cpu_inc(*mm->futex_ref); /* 0 -> 1 */
17311728
return 0;
17321729
}
17331730

@@ -1801,6 +1798,17 @@ static int futex_hash_allocate(unsigned int hash_slots, unsigned int flags)
18011798
}
18021799
}
18031800

1801+
if (!mm->futex_ref) {
1802+
/*
1803+
* This will always be allocated by the first thread and
1804+
* therefore requires no locking.
1805+
*/
1806+
mm->futex_ref = alloc_percpu(unsigned int);
1807+
if (!mm->futex_ref)
1808+
return -ENOMEM;
1809+
this_cpu_inc(*mm->futex_ref); /* 0 -> 1 */
1810+
}
1811+
18041812
fph = kvzalloc(struct_size(fph, queues, hash_slots),
18051813
GFP_KERNEL_ACCOUNT | __GFP_NOWARN);
18061814
if (!fph)

0 commit comments

Comments
 (0)