* [PATCH v2] tracing/synthetic: Free type string on error path
From: Yu Peng @ 2026-07-07 13:24 UTC (permalink / raw)
To: rostedt
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, linux-kernel,
Yu Peng
parse_synth_field() builds a "__data_loc ..." type string before
assigning it to field->type. If the seq_buf check fails, the temporary
string is not owned by field and is leaked. Free it before leaving.
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Yu Peng <pengyu@kylinos.cn>
---
Changes in v2:
- Use __free(kfree) and no_free_ptr() as suggested by Steven.
kernel/trace/trace_events_synth.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
index e6871230bde96..ad2e70258291b 100644
--- a/kernel/trace/trace_events_synth.c
+++ b/kernel/trace/trace_events_synth.c
@@ -828,7 +828,7 @@ static struct synth_field *parse_synth_field(int argc, char **argv,
} else if (size == 0) {
if (synth_field_is_string(field->type) ||
synth_field_is_stack(field->type)) {
- char *type;
+ char *type __free(kfree) = NULL;
len = sizeof("__data_loc ") + strlen(field->type) + 1;
type = kzalloc(len, GFP_KERNEL);
@@ -844,7 +844,7 @@ static struct synth_field *parse_synth_field(int argc, char **argv,
s.buffer[s.len] = '\0';
kfree(field->type);
- field->type = type;
+ field->type = no_free_ptr(type);
field->is_dynamic = true;
size = sizeof(u64);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2 4/4] mm/mlock: migrate folios out of CMA when mlocking a range
From: Wandun @ 2026-07-07 13:36 UTC (permalink / raw)
To: vbabka, david, rostedt, mhiramat, Alexander.Krabler, hughd, fvdl,
bigeasy, linux-mm, linux-kernel, linux-trace-kernel,
linux-rt-devel
Cc: akpm, surenb, mhocko, jackmanb, hannes, ziy, ljs, riel, liam,
harry, jannh, lance.yang, mathieu.desnoyers, matthew.brost,
joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple,
pfalcato
In-Reply-To: <20260707125925.3725177-5-chenwandun1@gmail.com>
On 7/7/26 20:59, Wandun Chen wrote:
> From: Wandun Chen <chenwandun@lixiang.com>
>
> The region covered by mlock[all] may contain CMA pages. cma_alloc installs
> migration entries in the page table, if a memory access occurs at this
> point, it must wait for the migration to complete, which may cause
> latency spikes on the RT kernels.
>
> Try to move the migration cost into the mlock[all] caller, which is
> typically a setup path. So reduce the chance of latency spikes on RT
> kernels by migrating the currently mapped CMA pages out of CMA region.
>
> Suggested-by: Frank van der Linden <fvdl@google.com>
> Signed-off-by: Wandun Chen <chenwandun@lixiang.com>
> Link: https://lore.kernel.org/all/CAPTztWZpnX1j8-7yeppVUsxE=O9hbVeqricDjZt8_pnN7a-kBQ@mail.gmail.com/#t
> ---
> mm/mlock.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 118 insertions(+), 1 deletion(-)
>
> diff --git a/mm/mlock.c b/mm/mlock.c
> index ac65de40b22b..f56c685533f5 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -25,6 +25,7 @@
> #include <linux/memcontrol.h>
> #include <linux/mm_inline.h>
> #include <linux/secretmem.h>
> +#include <linux/migrate.h>
> #include <linux/compaction.h>
>
> #include "internal.h"
> @@ -428,6 +429,119 @@ static int mlock_pte_range(pmd_t *pmd, unsigned long addr,
> return 0;
> }
>
> +#ifdef CONFIG_CMA
> +static int mlock_collect_migratable_pte_range(pmd_t *pmd, unsigned long addr,
> + unsigned long end, struct mm_walk *walk)
> +{
> + struct vm_area_struct *vma = walk->vma;
> + struct list_head *folio_list = walk->private;
> + spinlock_t *ptl;
> + pte_t *start_pte, *pte;
> + pte_t ptent;
> + struct folio *folio;
> + unsigned int step = 1;
> +
> + if (!(vma->vm_flags & VM_LOCKED))
> + return 0;
> +
> + ptl = pmd_trans_huge_lock(pmd, vma);
> + if (ptl) {
> + if (!pmd_present(*pmd)) {
> + if (unlikely(softleaf_is_migration(softleaf_from_pmd(*pmd)))) {
> + spin_unlock(ptl);
> + pmd_migration_entry_wait(vma->vm_mm, pmd);
> + walk->action = ACTION_AGAIN;
> + return 0;
> + }
> + goto out;
> + }
> + if (is_huge_zero_pmd(*pmd))
> + goto out;
> + folio = pmd_folio(*pmd);
> + if (folio_is_zone_device(folio))
> + goto out;
> + if (is_migrate_cma_page(&folio->page))
> + isolate_folio_to_list(folio, folio_list);
> + goto out;
> + }
> +
> + start_pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> + if (!start_pte) {
> + walk->action = ACTION_AGAIN;
> + return 0;
> + }
> +
> + for (pte = start_pte; addr != end; pte += step, addr += step * PAGE_SIZE) {
> + step = 1;
> + ptent = ptep_get(pte);
> + if (!pte_present(ptent)) {
> + if (unlikely(softleaf_is_migration(softleaf_from_pte(ptent)))) {
> + pte_unmap_unlock(start_pte, ptl);
> + migration_entry_wait(vma->vm_mm, pmd, addr);
> + walk->action = ACTION_AGAIN;
> + return 0;
> + }
> + continue;
> + }
> + folio = vm_normal_folio(vma, addr, ptent);
> + if (!folio || folio_is_zone_device(folio))
> + continue;
> + step = folio_mlock_step(folio, pte, addr, end);
> + if (is_migrate_cma_page(&folio->page))
Here should be, sorry about this.
if (!is_migrate_cma_page(&folio->page))
continue;
Best regards
Wandun
> + continue;
> + isolate_folio_to_list(folio, folio_list);
> + }
> + pte_unmap(start_pte);
> +out:
> + spin_unlock(ptl);
> + cond_resched();
> + return 0;
> +}
> +
> +static const struct mm_walk_ops mlock_collect_migratable_ops = {
> + .pmd_entry = mlock_collect_migratable_pte_range,
> + .walk_lock = PGWALK_RDLOCK,
> +};
> +
> +static void mlock_migrate_cma_range(unsigned long start, unsigned long len)
> +{
> + struct mm_struct *mm = current->mm;
> + unsigned long end = start + len;
> + LIST_HEAD(folio_list);
> + struct migration_target_control mtc = {
> + .nid = NUMA_NO_NODE,
> + .gfp_mask = GFP_HIGHUSER | __GFP_NOWARN,
> + .reason = MR_SYSCALL,
> + };
> +
> + if (compaction_allow_unevictable())
> + return;
> +
> + lru_cache_disable();
> +
> + if (mmap_read_lock_killable(mm))
> + goto out;
> +
> + walk_page_range(mm, start, end, &mlock_collect_migratable_ops,
> + &folio_list);
> + mmap_read_unlock(mm);
> +
> + if (list_empty(&folio_list))
> + goto out;
> +
> + if (migrate_pages(&folio_list, alloc_migration_target, NULL,
> + (unsigned long)&mtc, MIGRATE_SYNC, MR_SYSCALL, NULL))
> + putback_movable_pages(&folio_list);
> +out:
> + lru_cache_enable();
> +}
> +#else
> +static inline void mlock_migrate_cma_range(unsigned long start,
> + unsigned long len)
> +{
> +}
> +#endif /* CONFIG_CMA */
> +
> /*
> * mlock_vma_pages_range() - mlock any pages already in the range,
> * or munlock all pages in the range.
> @@ -678,6 +792,7 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla
> error = __mm_populate(start, len, 0);
> if (error)
> return __mlock_posix_error_return(error);
> + mlock_migrate_cma_range(start, len);
> return 0;
> }
>
> @@ -790,8 +905,10 @@ SYSCALL_DEFINE1(mlockall, int, flags)
> capable(CAP_IPC_LOCK))
> ret = apply_mlockall_flags(flags);
> mmap_write_unlock(current->mm);
> - if (!ret && (flags & MCL_CURRENT))
> + if (!ret && (flags & MCL_CURRENT)) {
> mm_populate(0, TASK_SIZE);
> + mlock_migrate_cma_range(0, TASK_SIZE);
> + }
>
> return ret;
> }
^ permalink raw reply
* Re: [PATCH v3 04/11] arm64/mm: Add set_memory_device() and set_memory_normal()
From: Mike Rapoport @ 2026-07-07 13:36 UTC (permalink / raw)
To: Robin Murphy
Cc: Will Deacon, Thierry Reding, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Hunter, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Sowjanya Komatineni, Luca Ceresoli, Mikko Perttunen, Yury Norov,
Rasmus Villemoes, Russell King, Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Suren Baghdasaryan, Michal Hocko,
Marek Szyprowski, Sumit Semwal, Benjamin Gaignard, Brian Starkey,
John Stultz, T.J. Mercier, Christian König, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Catalin Marinas,
Thierry Reding, devicetree, linux-tegra, linux-kernel, dri-devel,
linux-media, linux-arm-kernel, linux-s390, linux-mm, iommu,
linaro-mm-sig, linux-trace-kernel, Thierry Reding, Chun Ng
In-Reply-To: <b0a1bdd7-46ab-4025-8775-c9273892444e@arm.com>
On Tue, Jul 07, 2026 at 02:17:29PM +0100, Robin Murphy wrote:
>
> Given the precedent of memblock_mark_nomap(), as long as the reusable
> reserved-memory regions also get split into distinct memblocks, then it
> seems like in principle we ought to be able to give them a new
> MEMBLOCK_PTEMAP (or whatever) flag which could then be picked up in
> map_mem() without needing to override force_pte_mapping() globally?
Please don't. _nomap() caused enough pain.
> Cheers,
> Robin.
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH v2 1/4] mm/migrate: do not migrate folios mapped into VM_LOCKED VMAs under compaction
From: Lorenzo Stoakes @ 2026-07-07 13:44 UTC (permalink / raw)
To: Wandun Chen
Cc: vbabka, david, rostedt, mhiramat, Alexander.Krabler, hughd, fvdl,
bigeasy, linux-mm, linux-kernel, linux-trace-kernel,
linux-rt-devel, akpm, surenb, mhocko, jackmanb, hannes, ziy, riel,
liam, harry, jannh, lance.yang, mathieu.desnoyers, matthew.brost,
joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple,
pfalcato
In-Reply-To: <20260707125925.3725177-2-chenwandun1@gmail.com>
(Being really nitty, your subject line is too long)
Please don't reference legacy VMA flags for newer patches 'do not migrate
folios mapped into mlocked VMAs...' works just as well.
On Tue, Jul 07, 2026 at 08:59:22PM +0800, Wandun Chen wrote:
> From: Wandun Chen <chenwandun@lixiang.com>
>
> When compact_unevictable_allowed=0, unevictable pages should not be
> migrated. However, mlock_folio_batch in the mlock[all] syscall introduces
> a race, mlock_folio() sets PG_mlocked immediately but defers PG_unevictable
> to mlock_folio_batch(), causing pages that are about to become unevictable
> to be migrated, which violates the intent of compact_unevictable_allowed,
> and causes spike latency in RT kernels [1].
>
> In order to fix this, migration is forbidden for pages mapped into VMAs
> marked with VM_LOCKED. In addition, two early-return paths are introduced,
Please don't reference legacy VMA flags. -> VMA_LOCKED_BIT.
> filter out mlocked pages, return early to avoid unnecessary operations.
>
> Fixes: 90d07210ab55 ("mm: mlock: use folios and a folio batch internally")
Hmmmm why do you think my patch caused this? That was just a folio conversion?
Also I didn't think we liked having fixes spotted about a series with non-fixes
tags?
> Reported-by: Alexander Krabler <Alexander.Krabler@kuka.com>
> Closes: https://lore.kernel.org/all/DU0PR01MB10385345F7153F334100981888259A@DU0PR01MB10385.eurprd01.prod.exchangelabs.com/ [1]
> Suggested-by: Vlastimil Babka <vbabka@suse.cz>
> Signed-off-by: Wandun Chen <chenwandun@lixiang.com>
> Link: https://lore.kernel.org/linux-rt-users/33275585-f2db-4779-89f0-3ae24b455a67@suse.cz/#t
> ---
> include/linux/compaction.h | 6 ++++++
> include/linux/rmap.h | 3 +++
> mm/compaction.c | 8 +++++++-
> mm/migrate.c | 23 +++++++++++++++++++----
> mm/rmap.c | 12 +++++++++---
> 5 files changed, 44 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/compaction.h b/include/linux/compaction.h
> index f29ef0653546..04e60f65b976 100644
> --- a/include/linux/compaction.h
> +++ b/include/linux/compaction.h
> @@ -106,6 +106,7 @@ bool compaction_zonelist_suitable(struct alloc_context *ac, int order,
> extern void __meminit kcompactd_run(int nid);
> extern void __meminit kcompactd_stop(int nid);
> extern void wakeup_kcompactd(pg_data_t *pgdat, int order, int highest_zoneidx);
> +extern bool compaction_allow_unevictable(void);
Don't use extern. We remove extern as we go it's not needed.
>
> #else
> static inline void reset_isolation_suitable(pg_data_t *pgdat)
> @@ -131,6 +132,11 @@ static inline void wakeup_kcompactd(pg_data_t *pgdat,
> {
> }
>
> +static inline bool compaction_allow_unevictable(void)
> +{
> + return true;
> +}
> +
> #endif /* CONFIG_COMPACTION */
>
> struct node;
> diff --git a/include/linux/rmap.h b/include/linux/rmap.h
> index 8dc0871e5f00..359c7426b6b9 100644
> --- a/include/linux/rmap.h
> +++ b/include/linux/rmap.h
> @@ -102,6 +102,9 @@ enum ttu_flags {
> * do a final flush if necessary */
> TTU_RMAP_LOCKED = 0x80, /* do not grab rmap lock:
> * caller holds it */
> + TTU_RESPECT_MLOCK = 0x100,/* leave VM_LOCKED vmas mapped instead
-> VMA_LOCKED_BIT please. Also maybe just say mlock'd?
> + * of installing a migration entry
> + */
> };
>
> #ifdef CONFIG_MMU
> diff --git a/mm/compaction.c b/mm/compaction.c
> index f08765ade014..5d256930e389 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1116,7 +1116,8 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
> is_unevictable = folio_test_unevictable(folio);
>
> /* Compaction might skip unevictable pages but CMA takes them */
> - if (!(mode & ISOLATE_UNEVICTABLE) && is_unevictable)
> + if (!(mode & ISOLATE_UNEVICTABLE) &&
> + (is_unevictable || folio_test_mlocked(folio)))
Maybe just change is_unevictable to include this check?
Like:
is_unevictable = folio_test_unevictable(folio) ||
folio_test_mlocked(folio);
?
Also later you have:
if (((mode & ISOLATE_ASYNC_MIGRATE) && is_dirty) ||
(mapping && is_unevictable)) {
...
Which doesn't account for mlock as-is? Is that correct?
> goto isolate_fail_put;
>
> /*
> @@ -1898,6 +1899,11 @@ typedef enum {
> * compactable pages.
> */
> static int sysctl_compact_unevictable_allowed __read_mostly = CONFIG_COMPACT_UNEVICTABLE_DEFAULT;
> +
> +bool compaction_allow_unevictable(void)
> +{
> + return sysctl_compact_unevictable_allowed;
> +}
You add this helper but isolate_migratepages() still references
sysctl_compact_unevictable_allowed directly?
> /*
> * Tunable for proactive compaction. It determines how
> * aggressively the kernel should compact memory in the
> diff --git a/mm/migrate.c b/mm/migrate.c
> index a786549551e3..3a15eb13e82b 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1202,7 +1202,7 @@ static void migrate_folio_done(struct folio *src,
> static int migrate_folio_unmap(new_folio_t get_new_folio,
> free_folio_t put_new_folio, unsigned long private,
> struct folio *src, struct folio **dstp, enum migrate_mode mode,
> - struct list_head *ret)
> + struct list_head *ret, enum migrate_reason reason)
> {
> struct folio *dst;
> int rc = -EAGAIN;
> @@ -1210,6 +1210,7 @@ static int migrate_folio_unmap(new_folio_t get_new_folio,
> struct anon_vma *anon_vma = NULL;
> bool locked = false;
> bool dst_locked = false;
> + enum ttu_flags ttu = 0;
You reference ttu only in an if-block below no? So why are you declaring
this here? Move it to the if-block.
>
> dst = get_new_folio(src, private);
> if (!dst)
> @@ -1249,9 +1250,15 @@ static int migrate_folio_unmap(new_folio_t get_new_folio,
> folio_lock(src);
> }
> locked = true;
> - if (folio_test_mlocked(src))
> + if (folio_test_mlocked(src)) {
> old_folio_state |= FOLIO_WAS_MLOCKED;
>
> + if (reason == MR_COMPACTION && !compaction_allow_unevictable()) {
This should really be a helper since you repeat yourself and it's not
obvious what this is checking.
Like:
static migrate_mlock_allowed(enum migrate_reason reason)
{
/* Only compaction is disallowed. */
if (reason != MR_COMPACTION)
return true;
/* If we can compact unevictable folios, we are ok. */
if (compaction_allow_unevictable())
return true;
/* Conservative: if any folio could be mlock()'d, disallow. */
return false;
}
Then you could self-document what you're checking and avoid code duplication below.
> + rc = -EBUSY;
> + goto out;
> + }
> + }
> +
> if (folio_test_writeback(src)) {
> /*
> * Only in the case of a full synchronous migration is it
> @@ -1324,7 +1331,14 @@ static int migrate_folio_unmap(new_folio_t get_new_folio,
> /* Establish migration ptes */
> VM_BUG_ON_FOLIO(folio_test_anon(src) &&
> !folio_test_ksm(src) && !anon_vma, src);
Useful to convert VM_BUG_*() -> VM_WARN_*() (possibly _ONCE() here also) as we go!
> - try_to_migrate(src, mode == MIGRATE_ASYNC ? TTU_BATCH_FLUSH : 0);
> +
> + if (mode == MIGRATE_ASYNC)
> + ttu |= TTU_BATCH_FLUSH;
> +
> + if (reason == MR_COMPACTION && !compaction_allow_unevictable())
See above about deduplicating.
> + ttu |= TTU_RESPECT_MLOCK;
Hmm. I don't love 'respect mlock'. I guess we only know about the reason
being compaction here.
But I'm confused anyway. We have the folio, why aren't we just checking for
PG_mlocked() here instead of getting the rmap to see if it's mapped
anywhere with VMA_LOCKED_BIT?
> +
> + try_to_migrate(src, ttu);
> old_folio_state |= FOLIO_WAS_MAPPED;
> }
>
> @@ -1905,7 +1919,8 @@ static int migrate_pages_batch(struct list_head *from,
> }
>
> rc = migrate_folio_unmap(get_new_folio, put_new_folio,
> - private, folio, &dst, mode, ret_folios);
> + private, folio, &dst, mode, ret_folios,
> + reason);
> /*
> * The rules are:
> * 0: folio will be put on unmap_folios list,
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 0fb7a1b82cf3..3cb7f6337d38 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -2420,6 +2420,9 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,
> unsigned long pfn;
> unsigned long hsz = 0;
>
> + if ((flags & TTU_RESPECT_MLOCK) && (vma->vm_flags & VM_LOCKED))
Please use the modern API for VMA flags. So:
if ((flags & TTU_RESPECT_MLOCK) && vma_test(VMA_LOCKED_BIT))
> + return false;
> +
> /*
> * When racing against e.g. zap_pte_range() on another cpu,
> * in between its ptep_get_and_clear_full() and folio_remove_rmap_*(),
> @@ -2741,11 +2744,14 @@ void try_to_migrate(struct folio *folio, enum ttu_flags flags)
> };
>
> /*
> - * Migration always ignores mlock and only supports TTU_RMAP_LOCKED and
> - * TTU_SPLIT_HUGE_PMD, TTU_SYNC, and TTU_BATCH_FLUSH flags.
> + * Migration normally ignores mlock, but TTU_RESPECT_MLOCK asks it to
> + * leave folios mapped into VM_LOCKED vmas alone. Only TTU_RMAP_LOCKED,
Again -> VMA_LOCKED_BIT.
> + * TTU_SPLIT_HUGE_PMD, TTU_SYNC, TTU_BATCH_FLUSH and TTU_RESPECT_MLOCK
> + * are supported.
> */
> if (WARN_ON_ONCE(flags & ~(TTU_RMAP_LOCKED | TTU_SPLIT_HUGE_PMD |
> - TTU_SYNC | TTU_BATCH_FLUSH)))
> + TTU_SYNC | TTU_BATCH_FLUSH |
> + TTU_RESPECT_MLOCK)))
> return;
>
> if (folio_is_zone_device(folio) &&
> --
> 2.43.0
>
Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH v2 1/4] mm/migrate: do not migrate folios mapped into VM_LOCKED VMAs under compaction
From: Lorenzo Stoakes @ 2026-07-07 13:55 UTC (permalink / raw)
To: Wandun Chen
Cc: vbabka, david, rostedt, mhiramat, Alexander.Krabler, hughd, fvdl,
bigeasy, linux-mm, linux-kernel, linux-trace-kernel,
linux-rt-devel, akpm, surenb, mhocko, jackmanb, hannes, ziy, riel,
liam, harry, jannh, lance.yang, mathieu.desnoyers, matthew.brost,
joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple,
pfalcato
In-Reply-To: <akz7ythMxfIZeT0d@lucifer>
On Tue, Jul 07, 2026 at 02:44:50PM +0100, Lorenzo Stoakes wrote:
> See above about deduplicating.
>
> > + ttu |= TTU_RESPECT_MLOCK;
>
> Hmm. I don't love 'respect mlock'. I guess we only know about the reason
> being compaction here.
>
> But I'm confused anyway. We have the folio, why aren't we just checking for
> PG_mlocked() here instead of getting the rmap to see if it's mapped
> anywhere with VMA_LOCKED_BIT?
Also, since compaction_allow_unevictable() is a function that is accessible
elsewhere, you could literally just have a TTU_MIGRATION here instead and have
the rmap logic call compaction_allow_unevictable() instead rather than this.
And then you could adapt the function I suggested before not to take a reason
parameter but rather a 'is_migration' one instead possibly and then pass (ttu &
TTU_MIGRATION) in.
BUT. I still question whether this is at all needed since you have the folio you
can check for PG_mlocked...
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH v3 04/11] arm64/mm: Add set_memory_device() and set_memory_normal()
From: Robin Murphy @ 2026-07-07 14:15 UTC (permalink / raw)
To: Mike Rapoport
Cc: Will Deacon, Thierry Reding, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Hunter, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Sowjanya Komatineni, Luca Ceresoli, Mikko Perttunen, Yury Norov,
Rasmus Villemoes, Russell King, Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Suren Baghdasaryan, Michal Hocko,
Marek Szyprowski, Sumit Semwal, Benjamin Gaignard, Brian Starkey,
John Stultz, T.J. Mercier, Christian König, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Catalin Marinas,
Thierry Reding, devicetree, linux-tegra, linux-kernel, dri-devel,
linux-media, linux-arm-kernel, linux-s390, linux-mm, iommu,
linaro-mm-sig, linux-trace-kernel, Thierry Reding, Chun Ng
In-Reply-To: <ak0A6eLp0Pw8iKK0@kernel.org>
On 07/07/2026 2:36 pm, Mike Rapoport wrote:
> On Tue, Jul 07, 2026 at 02:17:29PM +0100, Robin Murphy wrote:
>>
>> Given the precedent of memblock_mark_nomap(), as long as the reusable
>> reserved-memory regions also get split into distinct memblocks, then it
>> seems like in principle we ought to be able to give them a new
>> MEMBLOCK_PTEMAP (or whatever) flag which could then be picked up in
>> map_mem() without needing to override force_pte_mapping() globally?
>
> Please don't. _nomap() caused enough pain.
Indeed I was there for pretty much the whole pfn_valid() saga :)
Bad example maybe - in this case the only actual similarity to nomap
would be the fact that it would also be set by the of_reserved_mem code
based on what it finds in DT; in all other aspects it should be
functionally closer to something like MEMBLOCK_RSRV_NOINIT, i.e. just
carrying information through the mm init phase, then ceasing to matter
at all once the linear mapping is done.
Cheers,
Robin.
^ permalink raw reply
* Re: [PATCH v2 2/4] mm/mlock: wait for migration to finish when mlocking a folio
From: Lorenzo Stoakes @ 2026-07-07 14:33 UTC (permalink / raw)
To: Wandun Chen
Cc: vbabka, david, rostedt, mhiramat, Alexander.Krabler, hughd, fvdl,
bigeasy, linux-mm, linux-kernel, linux-trace-kernel,
linux-rt-devel, akpm, surenb, mhocko, jackmanb, hannes, ziy, riel,
liam, harry, jannh, lance.yang, mathieu.desnoyers, matthew.brost,
joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple,
pfalcato
In-Reply-To: <20260707125925.3725177-3-chenwandun1@gmail.com>
On Tue, Jul 07, 2026 at 08:59:23PM +0800, Wandun Chen wrote:
> From: Wandun Chen <chenwandun@lixiang.com>
>
> In RT kernels, sysctl_compact_unevictable_allowed is false by default,
> when the mlock/mlockall system call try to lock all the present page,
> the mlock_pte_range function skips non-present entries. If these
> non-present entries are migration entries, and the migration is not
> guaranteed to have completed before the mlock/mlockall, it may result
> in a page fault on subsequent access, which then waits for the
> migration to finish, causing spike latency in RT kernels.
Is this really noticable and measurable?
I suppose not too many ranges should be mock
>
> Fix it by waiting for the migration to complete during the mlock/mlockall
> syscall when sysctl_compact_unevictable_allowed is false.
Probably better to reference the sysctl itself rather than an arbitrary varible
name.
>
> Fixes: 90d07210ab55 ("mm: mlock: use folios and a folio batch internally")
Again I have no idea why you chose this as the fixes target, especially for
this?
But in any case, why is this separate from the previous commit? I sthis an
entirely separate fix?
> Suggested-by: Vlastimil Babka <vbabka@suse.cz>
> Signed-off-by: Wandun Chen <chenwandun@lixiang.com>
> Link: https://lore.kernel.org/lkml/c8793c0f-7156-4cb7-9e6e-7909397e2fff@kernel.org/#t
A nit but please strip that #t.
> ---
> mm/mlock.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/mm/mlock.c b/mm/mlock.c
> index 97e49038d8d3..ac65de40b22b 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -25,6 +25,7 @@
> #include <linux/memcontrol.h>
> #include <linux/mm_inline.h>
> #include <linux/secretmem.h>
> +#include <linux/compaction.h>
>
> #include "internal.h"
>
> @@ -361,8 +362,17 @@ static int mlock_pte_range(pmd_t *pmd, unsigned long addr,
>
> ptl = pmd_trans_huge_lock(pmd, vma);
> if (ptl) {
> - if (!pmd_present(*pmd))
> + if (!pmd_present(*pmd)) {
> + if (unlikely((vma->vm_flags & VM_LOCKED) &&
-> vma_test(vma, VMA_LOCKED_BIT)...
What is the basis for your unlikely()? Don't use likely()/unlikely() just
because it feels right. Leave them out unless you have actual profiling data to
prove it.
Remove it please.
But I'm also confused here - why are you checking for VMAs that already had
VMA_LOCKED_BIT set when you are about to set it?
Surely this shouldn't be predicated on VMA_LOCKED_BIT?
> + !compaction_allow_unevictable() &&
Why do we care about compaction here? Are we assuming the softleaf migration
entry is present only for compaction reasons?
So these migration entries could be there for any reason? Does it matter? Is it
such a big deal, on mlock, to wait for migration entries? I think probably not.
But then maybe some weird workload gets affected... hmm.
> + softleaf_is_migration(softleaf_from_pmd(*pmd)))) {
Use pmd_is_migration_entry()? :)
And technically you should use pmdp_get() I think? Maybe a situation where we
don't care/need the READ_ONCE() though.
> + spin_unlock(ptl);
> + pmd_migration_entry_wait(vma->vm_mm, pmd);
> + walk->action = ACTION_AGAIN;
> + return 0;
> + }
This code is really disgusting, let's please not just copy/paste open-coded
nested horror shows like this.
A good rule of thumb is if you see a bunch of code 'poking out' like this and
certainly if you copy/paste it or something very much like it, break it out into
a function.
So:
static bool wait_for_migration(const struct vm_area_struct *vma, softleaf_t entry)
{
>>>> As above I'm not sure should even be checking this?
if (!vma_test(vma, VMA_LOCKED_BIT))
return false;
>>>> As above I'm not sure this makes sense?
if (compaction_allowed_unevictable())
return false;
return softleaf_is_migration(entry);
}
Then call it like:
if (ptl) {
const pmd_t pmd = pmdp_get(pmd);
if (!pmd_present(pmd)) {
const softleaf_t entry = softleaf_from_pmd(pmd);
if (!wait_for_migration(vma, entry))
goto out;
spin_unlock(ptl);
pmd_migration_entry_wait(vma->vm_mm, pmd);
walk->action = ACTION_AGAIN;
return 0;
}
if (is_huge_zero_pmd(pmd))
goto out;
(etc. going *pmd -> pmd)
But we could probably do even better than that, and assuming your dubious checks
above aren't needed, we don't even need a helper then:
if (ptl) {
const pmd_t pmd = pmdp_get(pmd); // possibly just *pmd?
const softleaf_t entry = softleaf_from_pmd(pmd);
if (softleaf_is_migration(entry)) {
/* Wait for migration entries. */
spin_unlock(ptl);
pmd_migration_entry_wait(vma->vm_mm, pmd);
walk->action = ACTION_AGAIN;
return 0;
}
if (!pmd_present(pmd))
goto out;
etc.
And similar for the PTE stuff below.
This works because softleaf_from_pmd() (and pte) will give you a 'none' softleaf
if the entry is not a softlaf entry. So you can unconditionally call it on a
PMD.
> goto out;
> + }
> if (is_huge_zero_pmd(*pmd))
> goto out;
> folio = pmd_folio(*pmd);
> @@ -383,8 +393,17 @@ static int mlock_pte_range(pmd_t *pmd, unsigned long addr,
>
> for (pte = start_pte; addr != end; pte++, addr += PAGE_SIZE) {
> ptent = ptep_get(pte);
> - if (!pte_present(ptent))
> + if (!pte_present(ptent)) {
> + if (unlikely((vma->vm_flags & VM_LOCKED) &&
> + !compaction_allow_unevictable() &&
> + softleaf_is_migration(softleaf_from_pte(ptent)))) {
> + pte_unmap_unlock(start_pte, ptl);
> + migration_entry_wait(vma->vm_mm, pmd, addr);
> + walk->action = ACTION_AGAIN;
> + return 0;
> + }
> continue;
> + }
> folio = vm_normal_folio(vma, addr, ptent);
> if (!folio || folio_is_zone_device(folio))
> continue;
> --
> 2.43.0
>
Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH] ring-buffer: serialize read-page order with subbuffer resize
From: Steven Rostedt @ 2026-07-07 14:36 UTC (permalink / raw)
To: Yousef Alhouseen
Cc: mhiramat, mathieu.desnoyers, petr.pavlu, linux-trace-kernel,
linux-kernel
In-Reply-To: <CAMuQ4bUNbCM7oxnrXaT_oAoX+Guoo4_Bu0eO5dLR3OZ62Eu0Xg@mail.gmail.com>
On Tue, 30 Jun 2026 14:16:48 -0700
Yousef Alhouseen <alhouseenyousef@gmail.com> wrote:
> Agreed. I’ll add an explicit resize-in-progress flag, set it around
> the order transition, and make the external read-page
> allocation/free/read paths reject work while it is set. I’ll check the
> flag under the locks that serialize each path so it cannot race the
> transition, then compile and test the resulting v2.
Did you ever get around to sending a v2?
-- Steve
^ permalink raw reply
* Re: [PATCH 01/30] mm: move vma_start_pgoff() into mm.h and clean up
From: David Hildenbrand (Arm) @ 2026-07-07 14:41 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton
Cc: Russell King, Dinh Nguyen, Simon Schuster,
James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Marek Szyprowski, Peter Zijlstra,
Arnaldo Carvalho de Melo, Namhyung Kim, Masami Hiramatsu,
Oleg Nesterov, Steven Rostedt, SeongJae Park, Miaohe Lin,
Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
Harry Yoo, Jann Horn
In-Reply-To: <b28b698df4c009e85c4728446ca5863d8e633164.1782735110.git.ljs@kernel.org>
On 6/29/26 14:23, Lorenzo Stoakes wrote:
> vma_last_pgoff() already lives there, so it's a bit odd to keep
> vma_start_pgoff() in mm/interval_tree.c. Move them together.
>
> These each return unsigned long, which pgoff_t is typedef'd to. Make this
> consistent and have these functions return pgoff_t instead.
>
> Additionally, express vma_last_pgoff() in terms of vma_start_pgoff(), since
> we wrap the vma->vm_pgoff access, we may as well use it here.
>
> Also while we're here, const-ify the VMA and cleanup a bit.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> ---
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH] tracing/user_events: fix use-after-free of enabler in user_event_mm_dup()
From: Steven Rostedt @ 2026-07-07 14:42 UTC (permalink / raw)
To: Michael Bommarito
Cc: Beau Belgrave, XIAO WU, Masami Hiramatsu, Mathieu Desnoyers,
linux-trace-kernel, linux-kernel, stable
In-Reply-To: <CAJJ9bXzJpYRE-NjOjiArpuJWGnFXr+jq7ukbEEdEhK9YPCbYrQ@mail.gmail.com>
On Mon, 6 Jul 2026 16:11:03 -0400
Michael Bommarito <michael.bommarito@gmail.com> wrote:
> On Mon, Jul 6, 2026 at 4:06 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> > I'm taking in the OP patch, but this looks like a separate issue.
> >
> > Any update on this?
>
> Sorry, had gone fishing. I'll have v2 in the next day or so
>
Ah, you're going to send a new version. I'll drop the one I pulled then.
-- Steve
^ permalink raw reply
* Re: [PATCH] tracing/user_events: fix use-after-free of enabler in user_event_mm_dup()
From: Michael Bommarito @ 2026-07-07 14:43 UTC (permalink / raw)
To: Steven Rostedt
Cc: Beau Belgrave, XIAO WU, Masami Hiramatsu, Mathieu Desnoyers,
linux-trace-kernel, linux-kernel, stable
In-Reply-To: <20260707104205.582db193@gandalf.local.home>
> Ah, you're going to send a new version. I'll drop the one I pulled then.
Saw your pull for-linus and I was just about to send a separate patch
set for the second UAF with a ktest (as 2/2). I can do either way,
just let me know which is easier
^ permalink raw reply
* Re: [PATCH 02/30] mm: add kdoc comments for vma_start/last_pgoff()
From: David Hildenbrand (Arm) @ 2026-07-07 14:46 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton
Cc: Russell King, Dinh Nguyen, Simon Schuster,
James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Marek Szyprowski, Peter Zijlstra,
Arnaldo Carvalho de Melo, Namhyung Kim, Masami Hiramatsu,
Oleg Nesterov, Steven Rostedt, SeongJae Park, Miaohe Lin,
Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
Harry Yoo, Jann Horn
In-Reply-To: <8c618dfd7de419e3b797b8bd1cd921d4c5b8878b.1782735110.git.ljs@kernel.org>
On 6/29/26 14:23, Lorenzo Stoakes wrote:
> Describe what vma_start_pgoff() and vma_last_pgoff() actually provide in
> detail.
>
> This is in order that we can differentiate this between functions that will
> be added in a subsequent patch which provide a different page offset.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> ---
> include/linux/mm.h | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 059144435729..2f00c75e66bd 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4278,11 +4278,37 @@ static inline unsigned long vma_pages(const struct vm_area_struct *vma)
> return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> }
>
> +/**
> + * vma_start_pgoff() - Get the page offset of the start of @vma
> + * @vma: The VMA whose page offset is required.
> + *
> + * If the VMA is file-backed, this is the page offset into the file.
> + *
> + * If the VMA is anonymous, this is the virtual page offset of the start of the
> + * VMA - if unfaulted, then vma->vm_start >> PAGE_SHIFT, if faulted then the
> + * virtual page offset at the time of first fault.
> + *
> + * Note that if @vma is a MAP_PRIVATE file-backed mapping, then this returns the
> + * file offset.
There is the ugly case of @vma being a MAP_PRIVATE PFNMAP mapping, where it
returns something different.
(remap_pfn_range_prepare_vma() -> vma->vm_pgoff set to PFN)
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v2 4/4] mm/mlock: migrate folios out of CMA when mlocking a range
From: Lorenzo Stoakes @ 2026-07-07 14:54 UTC (permalink / raw)
To: Wandun Chen
Cc: vbabka, david, rostedt, mhiramat, Alexander.Krabler, hughd, fvdl,
bigeasy, linux-mm, linux-kernel, linux-trace-kernel,
linux-rt-devel, akpm, surenb, mhocko, jackmanb, hannes, ziy, riel,
liam, harry, jannh, lance.yang, mathieu.desnoyers, matthew.brost,
joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple,
pfalcato
In-Reply-To: <20260707125925.3725177-5-chenwandun1@gmail.com>
On Tue, Jul 07, 2026 at 08:59:25PM +0800, Wandun Chen wrote:
> From: Wandun Chen <chenwandun@lixiang.com>
>
> The region covered by mlock[all] may contain CMA pages. cma_alloc installs
> migration entries in the page table, if a memory access occurs at this
> point, it must wait for the migration to complete, which may cause
> latency spikes on the RT kernels.
>
> Try to move the migration cost into the mlock[all] caller, which is
> typically a setup path. So reduce the chance of latency spikes on RT
> kernels by migrating the currently mapped CMA pages out of CMA region.
'reduce the chances of latency' so do you have any data to back this invasive
change or not?
And for RT, but nothing in here at all checks for RT? You're using this
compaction sysctl as an RT check somehow? That's gross.
This doesn't feel like the right solution.
>
> Suggested-by: Frank van der Linden <fvdl@google.com>
> Signed-off-by: Wandun Chen <chenwandun@lixiang.com>
> Link: https://lore.kernel.org/all/CAPTztWZpnX1j8-7yeppVUsxE=O9hbVeqricDjZt8_pnN7a-kBQ@mail.gmail.com/#t
> ---
> mm/mlock.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 118 insertions(+), 1 deletion(-)
>
> diff --git a/mm/mlock.c b/mm/mlock.c
> index ac65de40b22b..f56c685533f5 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -25,6 +25,7 @@
> #include <linux/memcontrol.h>
> #include <linux/mm_inline.h>
> #include <linux/secretmem.h>
> +#include <linux/migrate.h>
> #include <linux/compaction.h>
>
> #include "internal.h"
> @@ -428,6 +429,119 @@ static int mlock_pte_range(pmd_t *pmd, unsigned long addr,
> return 0;
> }
>
> +#ifdef CONFIG_CMA
Ugh yuck. This is horrible. Why are we polluting mlock.c with CMA stuff?
Also no comment?
> +static int mlock_collect_migratable_pte_range(pmd_t *pmd, unsigned long addr,
> + unsigned long end, struct mm_walk *walk)
> +{
You've literally copy/pasted mlock_pte_range(), this is disgusting.
Please don't copy/paste code like this, this isn't PHP, it's the kernel, it's
completely unacceptable.
> + struct vm_area_struct *vma = walk->vma;
> + struct list_head *folio_list = walk->private;
> + spinlock_t *ptl;
> + pte_t *start_pte, *pte;
> + pte_t ptent;
> + struct folio *folio;
> + unsigned int step = 1;
> +
> + if (!(vma->vm_flags & VM_LOCKED))
Once again vma_test(vma, VMA_LOCKED_BIT) please.
But also, again (since you copy/pasted), why? You're literally mlocking here...
You going for that 'mlocking already mlocked ranges' sweet spot or am I missing
something?
> + return 0;
> +
> + ptl = pmd_trans_huge_lock(pmd, vma);
> + if (ptl) {
> + if (!pmd_present(*pmd)) {
> + if (unlikely(softleaf_is_migration(softleaf_from_pmd(*pmd)))) {
OK so you're not actually checking for CMA here at all, you're just doing the
migration wait stuff... again here? What?
> + spin_unlock(ptl);
> + pmd_migration_entry_wait(vma->vm_mm, pmd);
> + walk->action = ACTION_AGAIN;
> + return 0;
> + }
> + goto out;
> + }
> + if (is_huge_zero_pmd(*pmd))
> + goto out;
> + folio = pmd_folio(*pmd);
> + if (folio_is_zone_device(folio))
> + goto out;
> + if (is_migrate_cma_page(&folio->page))
Err isn't this per-page, and you just checked that this is a huge folio, and
you're only checking the first page? That seems wrong?
> + isolate_folio_to_list(folio, folio_list);
Then you just take the whole folio?
It's really horrible that you're burying this in copy/pasted code from the rest.
> + goto out;
> + }
> +
> + start_pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> + if (!start_pte) {
> + walk->action = ACTION_AGAIN;
> + return 0;
> + }
> +
> + for (pte = start_pte; addr != end; pte += step, addr += step * PAGE_SIZE) {
> + step = 1;
> + ptent = ptep_get(pte);
> + if (!pte_present(ptent)) {
> + if (unlikely(softleaf_is_migration(softleaf_from_pte(ptent)))) {
> + pte_unmap_unlock(start_pte, ptl);
> + migration_entry_wait(vma->vm_mm, pmd, addr);
BTW I also wonder if we have guaranteed forward progress here if say a migration
happened to migrate back and forth again across a range...?
> + walk->action = ACTION_AGAIN;
> + return 0;
> + }
Again, since you copy/pasted, same comment - this is disgusting.
And again I'm confused why you're waiting here again after you did it previously
due to the previous commit that already does it?
And why for a non-CMA range, if somebody happens to set CONFIG_CMA they now get
this done twice?
> + continue;
> + }
> + folio = vm_normal_folio(vma, addr, ptent);
> + if (!folio || folio_is_zone_device(folio))
> + continue;
> + step = folio_mlock_step(folio, pte, addr, end);
> + if (is_migrate_cma_page(&folio->page))
> + continue;
You mean ! surely?
Why are you inverting this vs. the above? you replied to the patch so maybe you
correct this there.
> + isolate_folio_to_list(folio, folio_list);
And again you bury the actual point of this in one easily missed line and zero
comments.
No.
> + }
> + pte_unmap(start_pte);
> +out:
> + spin_unlock(ptl);
> + cond_resched();
> + return 0;
> +}
Yeah this is just completely unacceptable. You have to find a way to deduplicate
code. Copy/paste code dumps are not ok.
But at the same time, you're dumping CMA crap in core mm mlock code which is
horrible, you've also dumped some migration code so you're really mixing things
up here horribly, and it doesn't seem justified?
> +
> +static const struct mm_walk_ops mlock_collect_migratable_ops = {
> + .pmd_entry = mlock_collect_migratable_pte_range,
> + .walk_lock = PGWALK_RDLOCK,
> +};
> +
> +static void mlock_migrate_cma_range(unsigned long start, unsigned long len)
> +{
> + struct mm_struct *mm = current->mm;
> + unsigned long end = start + len;
> + LIST_HEAD(folio_list);
> + struct migration_target_control mtc = {
> + .nid = NUMA_NO_NODE,
> + .gfp_mask = GFP_HIGHUSER | __GFP_NOWARN,
> + .reason = MR_SYSCALL,
> + };
> +
> + if (compaction_allow_unevictable())
> + return;
Again you're assuming compaction for some reason. Why?
It feels like you're just gating specific behaviour for your workload on this
flag and assuming that's ok.
> +
> + lru_cache_disable();
What, why?
> +
> + if (mmap_read_lock_killable(mm))
> + goto out;
OK so you got a fatal signal and you don't bother telling anybody about it and
just skip migration?...
> +
Weird whitespace...
> + walk_page_range(mm, start, end, &mlock_collect_migratable_ops,
> + &folio_list);
> + mmap_read_unlock(mm);
> +
> + if (list_empty(&folio_list))
> + goto out;
> +
> + if (migrate_pages(&folio_list, alloc_migration_target, NULL,
> + (unsigned long)&mtc, MIGRATE_SYNC, MR_SYSCALL, NULL))
> + putback_movable_pages(&folio_list);
> +out:
> + lru_cache_enable();
> +}
> +#else
> +static inline void mlock_migrate_cma_range(unsigned long start,
inline in a .c file? Why? Drop it.
> + unsigned long len)
> +{
> +}
> +#endif /* CONFIG_CMA */
> +
> /*
> * mlock_vma_pages_range() - mlock any pages already in the range,
> * or munlock all pages in the range.
> @@ -678,6 +792,7 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla
> error = __mm_populate(start, len, 0);
> if (error)
> return __mlock_posix_error_return(error);
> + mlock_migrate_cma_range(start, len);
Err, unconditionally?
> return 0;
> }
>
> @@ -790,8 +905,10 @@ SYSCALL_DEFINE1(mlockall, int, flags)
> capable(CAP_IPC_LOCK))
> ret = apply_mlockall_flags(flags);
> mmap_write_unlock(current->mm);
> - if (!ret && (flags & MCL_CURRENT))
> + if (!ret && (flags & MCL_CURRENT)) {
> mm_populate(0, TASK_SIZE);
> + mlock_migrate_cma_range(0, TASK_SIZE);
Err what? Why are you doing this? You're forcing a wait on migration of
literally everything across the whole of the process whether or not they're CMA
ranges, but as a one time thing?
> + }
>
> return ret;
> }
> --
> 2.43.0
>
In general this feels like the wrong solution for a specific workload that
sticks horrible stuff in core mm and I don't really love it :)
Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH v2 4/4] mm/mlock: migrate folios out of CMA when mlocking a range
From: Lorenzo Stoakes @ 2026-07-07 14:57 UTC (permalink / raw)
To: Wandun
Cc: vbabka, david, rostedt, mhiramat, Alexander.Krabler, hughd, fvdl,
bigeasy, linux-mm, linux-kernel, linux-trace-kernel,
linux-rt-devel, akpm, surenb, mhocko, jackmanb, hannes, ziy, riel,
liam, harry, jannh, lance.yang, mathieu.desnoyers, matthew.brost,
joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple,
pfalcato
In-Reply-To: <bd88f65e-000b-4dde-92f7-28af6a34e977@gmail.com>
On Tue, Jul 07, 2026 at 09:36:17PM +0800, Wandun wrote:
<snip>
> > diff --git a/mm/mlock.c b/mm/mlock.c
> > index ac65de40b22b..f56c685533f5 100644
> > --- a/mm/mlock.c
> > +++ b/mm/mlock.c
<snip>
> > +static int mlock_collect_migratable_pte_range(pmd_t *pmd, unsigned long addr,
> > + unsigned long end, struct mm_walk *walk)
> > +{
> > + struct vm_area_struct *vma = walk->vma;
> > + struct list_head *folio_list = walk->private;
> > + spinlock_t *ptl;
> > + pte_t *start_pte, *pte;
> > + pte_t ptent;
> > + struct folio *folio;
> > + unsigned int step = 1;
> > +
> > + if (!(vma->vm_flags & VM_LOCKED))
> > + return 0;
> > +
> > + ptl = pmd_trans_huge_lock(pmd, vma);
> > + if (ptl) {
> > + if (!pmd_present(*pmd)) {
> > + if (unlikely(softleaf_is_migration(softleaf_from_pmd(*pmd)))) {
> > + spin_unlock(ptl);
> > + pmd_migration_entry_wait(vma->vm_mm, pmd);
> > + walk->action = ACTION_AGAIN;
> > + return 0;
> > + }
> > + goto out;
> > + }
> > + if (is_huge_zero_pmd(*pmd))
> > + goto out;
> > + folio = pmd_folio(*pmd);
> > + if (folio_is_zone_device(folio))
> > + goto out;
> > + if (is_migrate_cma_page(&folio->page))
> > + isolate_folio_to_list(folio, folio_list);
> > + goto out;
> > + }
> > +
> > + start_pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> > + if (!start_pte) {
> > + walk->action = ACTION_AGAIN;
> > + return 0;
> > + }
> > +
> > + for (pte = start_pte; addr != end; pte += step, addr += step * PAGE_SIZE) {
> > + step = 1;
> > + ptent = ptep_get(pte);
> > + if (!pte_present(ptent)) {
> > + if (unlikely(softleaf_is_migration(softleaf_from_pte(ptent)))) {
> > + pte_unmap_unlock(start_pte, ptl);
> > + migration_entry_wait(vma->vm_mm, pmd, addr);
> > + walk->action = ACTION_AGAIN;
> > + return 0;
> > + }
> > + continue;
> > + }
> > + folio = vm_normal_folio(vma, addr, ptent);
> > + if (!folio || folio_is_zone_device(folio))
> > + continue;
> > + step = folio_mlock_step(folio, pte, addr, end);
> > + if (is_migrate_cma_page(&folio->page))
> Here should be, sorry about this.
> if (!is_migrate_cma_page(&folio->page))
> continue;
But above you do the:
if (is_migrate_cma_page(&folio->page))
isolate_folio_to_list(folio, folio_list);
Pattern :) so these should be consistent. But you're copy/pasting and need to
not do that so it's kind of by the by.
Anyway, yes I assumed you meant this. But see the main review, I am not really a
fan of the implementation, overall.
>
> Best regards
> Wandun
Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH] tracing: Fix NULL pointer dereference in func_set_flag()
From: Steven Rostedt @ 2026-07-07 15:14 UTC (permalink / raw)
To: Yuanhe Shu
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, sashiko-bot@kernel.org,
sashiko-reviews@lists.linux.dev
In-Reply-To: <20260624061715.1445655-1-xiangzao@linux.alibaba.com>
From: https://sashiko.dev/#/patchset/20260624061715.1445655-1-xiangzao%40linux.alibaba.com
> commit 2140381622b995438751cdb59eb4bae6923f6a60
> Author: Yuanhe Shu <xiangzao@linux.alibaba.com>
> tracing: Fix NULL pointer dereference in func_set_flag()
> This commit fixes a NULL pointer dereference in func_set_flag() by
> moving the tracer type check before dereferencing tr->current_trace_flags.
> > diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
> > index f283391a4dc80..cd37f20137581 100644
> > --- a/kernel/trace/trace_functions.c
> > +++ b/kernel/trace/trace_functions.c
> > @@ -458,12 +458,12 @@ func_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
> > ftrace_func_t func;
> > u32 new_flags;
> >
> > - /* Do nothing if already set. */
> > - if (!!set == !!(tr->current_trace_flags->val & bit))
> > + /* We can change this flag only when current tracer is function. */
> > + if (tr->current_trace != &function_trace)
> > return 0;
> >
> > - /* We can change this flag only when not running. */
> > - if (tr->current_trace != &function_trace)
> > + /* Do nothing if already set. */
> > + if (!!set == !!(tr->current_trace_flags->val & bit))
> > return 0;
>
> This isn't a bug introduced by this patch, but while reviewing this fix for
> tr->current_trace_flags, I noticed a similar NULL pointer dereference
> can occur in print_graph_function() in kernel/trace/trace_functions_graph.c.
> If a user enables the function_graph tracer, populating the ring buffer
> with graph events, and then switches to a tracer that lacks flags and
> does not automatically clear the ring buffer (such as hwlat, osnoise, or
> timerlat), tr->current_trace_flags becomes NULL.
> When the user subsequently reads the trace buffer, the fallback event handler
> calls print_graph_function(), which unconditionally dereferences
> tr->current_trace_flags:
>
> kernel/trace/trace_functions_graph.c:print_graph_function() {
> struct trace_array *tr = iter->tr;
> return print_graph_function_flags(iter, tr->current_trace_flags->val);
> }
>
> Does this unconditional dereference of tr->current_trace_flags->val also
> need a similar safety check to prevent a kernel crash?
No, because even though hwlat, osnoise and timerlat may not clear the main
ring buffer, it reads it differently. Thus the print_graph_function will
not be called.
Unless you can figure out a way to get to that function with one of those
other tracers?
-- Steve
^ permalink raw reply
* Re: [PATCH] tracing: Add "within" filter for call-stack-based event filtering
From: Steven Rostedt @ 2026-07-07 15:20 UTC (permalink / raw)
To: Chen Jun
Cc: mhiramat, mathieu.desnoyers, linux-kernel, linux-trace-kernel,
sashiko-bot@kernel.org, sashiko-reviews@lists.linux.dev
In-Reply-To: <20260608145556.94931-1-chenjun102@huawei.com>
From: https://sashiko.dev/#/patchset/20260608145556.94931-1-chenjun102%40huawei.com
> commit dfbe6e67884d4ff73b44c46b51995f1945758a89
> Author: Chen Jun <chenjun102@huawei.com>
> tracing: Add "within" filter for call-stack-based event filtering
> This commit introduces a "within" filter predicate for trace events, allowing
> users to filter events based on whether a specific function appears in the
> current call stack. It uses kallsyms_lookup_name() to resolve the function to
> an address range during setup, and stack_trace_save() to capture the stack at
> runtime to evaluate the filter condition.
>
> > diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> > index 609325f579427..34e1a7f0b3cd0 100644
> > --- a/kernel/trace/trace_events_filter.c
> > +++ b/kernel/trace/trace_events_filter.c
> [ ... ]
> > @@ -1009,6 +1010,22 @@ static int filter_pred_function(struct filter_pred *pred, void *event)
> > return pred->op == OP_EQ ? ret : !ret;
> > }
> >
> > +/* Filter predicate for within. */
> > +static int filter_pred_within(struct filter_pred *pred, void *event)
> > +{
> > +#ifdef CONFIG_STACKTRACE
> > + unsigned long entries[16];
> > + unsigned int nr_entries;
> > + int i;
> > +
> > + nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 0);
>
> Is an array size of 16 entries sufficient here? Since the skipnr parameter is
> 0, several of these frames will be consumed by internal tracing and filter
> evaluation functions (such as filter_pred_within() and
> trace_event_buffer_commit()).
>
> This leaves very few slots for the actual call chain, meaning higher-level
> functions like "vfs_read" will likely be truncated from the stack trace.
> Could this cause the filter condition to evaluate to false incorrectly,
> missing the events we are trying to capture?
>
This is a current limitation that may be dealt with in the future.
> > + for (i = 0; i < nr_entries; i++)
> > + if (pred->val <= entries[i] && entries[i] < pred->val2)
> > + return !pred->not;
> > +#endif
> > + return pred->not;
> > +}
>
> [ ... ]
> > @@ -2002,10 +2021,28 @@ static int parse_pred(const char *str, void *data,
> >
> > } else if (field->filter_type == FILTER_DYN_STRING) {
> > pred->fn_num = FILTER_PRED_FN_STRLOC;
> > - } else if (field->filter_type == FILTER_RDYN_STRING)
> > + } else if (field->filter_type == FILTER_RDYN_STRING) {
> > pred->fn_num = FILTER_PRED_FN_STRRELLOC;
> > - else {
> > + } else if (field->filter_type == FILTER_WITHIN) {
> > + unsigned long func;
> > +
> > + if (op == OP_GLOB)
> > + goto err_free;
>
> Does this goto need a preceding call to parse_error()? Other error paths in
> this function initialize the error state before jumping to err_free.
> Without it, users might see an unhelpful generic error message instead of a
> syntax error indicating that the glob operator isn't supported here.
This is a legitimate issue.
Chen, can you send a v2 and add a proper error message here?
Thanks,
-- Steve
> > +
> > + pred->fn_num = FILTER_PRED_FN_WITHIN;
> > + func = kallsyms_lookup_name(pred->regex->pattern);
^ permalink raw reply
* Re: [PATCH] tracing/user_events: fix use-after-free of enabler in user_event_mm_dup()
From: Steven Rostedt @ 2026-07-07 15:27 UTC (permalink / raw)
To: Michael Bommarito
Cc: Beau Belgrave, XIAO WU, Masami Hiramatsu, Mathieu Desnoyers,
linux-trace-kernel, linux-kernel, stable
In-Reply-To: <CAJJ9bXy78yKmOb+x-THk4EwJxY=0si04YAMtmOu-SzarVJwRBQ@mail.gmail.com>
On Tue, 7 Jul 2026 10:43:40 -0400
Michael Bommarito <michael.bommarito@gmail.com> wrote:
> > Ah, you're going to send a new version. I'll drop the one I pulled then.
>
> Saw your pull for-linus and I was just about to send a separate patch
> set for the second UAF with a ktest (as 2/2). I can do either way,
> just let me know which is easier
I'll drop it and take your new one.
-- Steve
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/3] tracing: Expose tracepoint BTF ids via tracefs
From: Steven Rostedt @ 2026-07-07 16:00 UTC (permalink / raw)
To: Mykyta Yatsenko
Cc: bpf, ast, andrii, daniel, kafai, kernel-team, eddyz87, memxor,
Mykyta Yatsenko, linux-trace-kernel
In-Reply-To: <20260518-generic_tracepoint-v2-0-b755a5cf67bb@meta.com>
On Mon, 18 May 2026 08:23:14 -0700
Mykyta Yatsenko <mykyta.yatsenko5@gmail.com> wrote:
> BPF and other consumers that want to attach to or decode a generic
> tracepoint need three pieces of BTF information for it:
>
> - the BTF of the object that owns the tracepoint's types
> - the FUNC_PROTO describing the tracepoint arguments (with names),
> consumed by raw_tp / tp_btf BPF programs
> - the STRUCT id of trace_event_raw_<call>, the ring-buffer record
> consumed by classic BPF_PROG_TYPE_TRACEPOINT programs
>
> Today none of this is easily discoverable from userspace. The kernel
> knows the ids - resolve_btfids fills them in at link time - but
> consumers have to search them by the naming convention
> ("__bpf_trace_<name>", "trace_event_raw_<name>"), walking BTF for
> every tracepoint.
I'll pull this in even though it adds 100K to the kernel:
text data bss dec hex filename
40506047 15709518 16661184 72876749 45802cd /tmp/vmlinux.new
40499064 15615294 16661184 72775542 4567776 /tmp/vmlinux.old
-- Steve
^ permalink raw reply
* Re: [PATCH net-next] net/tcp: Add explicit tracepoint for tcp_syn_ack_timeout()
From: Emil Tsalapatis @ 2026-07-07 16:08 UTC (permalink / raw)
To: Eric Dumazet, Emil Tsalapatis
Cc: netdev, linux-trace-kernel, ncardwell, kuniyu, rostedt, mhiramat,
davem, kuba, pabeni
In-Reply-To: <CANn89iL23Rsu3iGXxhw4pAnCB-JgU7fnrryDLXKj4jHqSs=HOw@mail.gmail.com>
On Tue Jul 7, 2026 at 3:52 AM EDT, Eric Dumazet wrote:
> On Mon, Jul 6, 2026 at 6:01 PM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>>
>> Clang can inline the tcp_syn_ack_timeout() function during compilation,
>> making it impossible to use kprobes for tracing without preventing
>> inlining. Add an explicit tracepoint to it instead.
>
> So much copy/pasting for a very small issue :/
>
>>
>> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>> ---
>> include/trace/events/tcp.h | 72 ++++++++++++++++++++++++++++++++++++++
>> net/ipv4/tcp_timer.c | 3 ++
>> 2 files changed, 75 insertions(+)
>>
>
> tcp_syn_ack_timeout() is hardly a fast path, so you can instead:
>
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index 322db13333c7..ab2c3de19e46 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -748,7 +748,7 @@ static void tcp_write_timer(struct timer_list *t)
> sock_put(sk);
> }
>
> -void tcp_syn_ack_timeout(const struct request_sock *req)
> +noinline_for_tracing void tcp_syn_ack_timeout(const struct request_sock *req)
> {
> struct net *net = read_pnet(&inet_rsk(req)->ireq_net);
Sounds good, I will respin and just mark it noinline.
^ permalink raw reply
* Re: [PATCH 11/30] mm/vma: introduce and use vmg_pages(), vmg_[start, end]_pgoff()
From: Gregory Price @ 2026-07-07 16:25 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Russell King, Dinh Nguyen, Simon Schuster,
James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
Harry Yoo, Jann Horn
In-Reply-To: <f7b4f8a611ab4d36eb3cf2e394610a3744a93895.1782735110.git.ljs@kernel.org>
On Mon, Jun 29, 2026 at 01:23:22PM +0100, Lorenzo Stoakes wrote:
> In the VMA logic we often need to determine the number of pages in the
> specified merge range, as well as the start and end page offsets of that
> range.
>
> Introduce and use helpers for these purposes.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Gregory Price <gourry@gourry.net>
^ permalink raw reply
* Re: [PATCH 12/30] mm/vma: clean up anon_vma_compatible()
From: Gregory Price @ 2026-07-07 16:26 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Russell King, Dinh Nguyen, Simon Schuster,
James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
Harry Yoo, Jann Horn
In-Reply-To: <5a7a07bd2a774989849b0fea84f758059ed914df.1782735110.git.ljs@kernel.org>
On Mon, Jun 29, 2026 at 01:23:23PM +0100, Lorenzo Stoakes wrote:
> Break up the existing very large conditional, add comments and use
> vma_[start/end]_pgoff() to make clearer what we're doing here.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Gregory Price <gourry@gourry.net>
^ permalink raw reply
* [PATCH 0/2] tracing/user_events: fix use-after-free in user_event_mm_dup()
From: Michael Bommarito @ 2026-07-07 16:59 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
Cc: Beau Belgrave, XIAO WU, linux-trace-kernel, linux-kernel, stable
This replaces the earlier single patch "tracing/user_events: fix
use-after-free of enabler in user_event_mm_dup()" that is in the tracing
for-linus branch; Steven agreed to drop that one and take this instead.
user_event_enabler_destroy() removes an enabler from the mm enabler list
that user_event_mm_dup() walks locklessly under rcu_read_lock() during
fork(), then drops the enabler's event reference and frees the enabler
without waiting for a grace period. A concurrent fork() walker can
therefore both dereference the freed enabler and take a reference on a
user_event that the put has already freed -- two use-after-frees, one on
the enabler and one on the user_event. The enabler use-after-free was
found first; XIAO WU then reported the user_event one, with a PoC and a
KASAN slab-use-after-free (write) in user_event_mm_dup(), and the
enabler-only fix did not address it.
Patch 1 holds both the enabler and its event reference until an RCU grace
period has elapsed, by deferring the put and the free to a work item
queued with queue_rcu_work(). The approach was suggested by Beau
Belgrave; it supersedes the enabler-only fix.
Patch 2 adjusts two user_events selftests that assumed the event is torn
down the instant an unregister returns; with the deferred put, DIAG_IOCSDEL
can briefly return -EBUSY, so they now wait for the delete to take effect.
Verified under KASAN on x86-64: the race faults on the unpatched kernel
(and panics with kasan.fault=panic), a benign serialized control is clean,
and the patched kernel is clean across repeated runs. The user_events
selftests pass on both kernels with patch 2 applied.
Michael Bommarito (2):
tracing/user_events: fix use-after-free in user_event_mm_dup()
selftests/user_events: wait for deferred event teardown after
unregister
kernel/trace/trace_events_user.c | 39 +++++++++++++++----
.../testing/selftests/user_events/abi_test.c | 24 +++++++++++-
.../testing/selftests/user_events/perf_test.c | 26 +++++++++++--
3 files changed, 78 insertions(+), 11 deletions(-)
base-commit: f24ca6729076623c9a0547ecc71e4fc1c4b65c3c
--
2.53.0
^ permalink raw reply
* [PATCH 1/2] tracing/user_events: fix use-after-free in user_event_mm_dup()
From: Michael Bommarito @ 2026-07-07 16:59 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
Cc: Beau Belgrave, XIAO WU, linux-trace-kernel, linux-kernel, stable
In-Reply-To: <20260707165912.2560537-1-michael.bommarito@gmail.com>
user_event_mm_dup() walks the parent mm's enabler list locklessly under
rcu_read_lock() during fork() (from copy_process()); it does not take
event_mutex:
rcu_read_lock();
list_for_each_entry_rcu(enabler, &old_mm->enablers, mm_enablers_link)
enabler->event = user_event_get(orig->event);
user_event_enabler_destroy() removes an enabler from that list with
list_del_rcu() and then, without waiting for a grace period, drops the
enabler's user_event reference with user_event_put() and frees the enabler
with kfree(). A reader that loaded the enabler before the list_del_rcu()
can still be walking it, which leads to two use-after-frees:
- kfree(enabler) frees the enabler while that reader dereferences
enabler->event.
- user_event_put() may drop the last reference to the user_event, which
is then freed (via delayed_destroy_user_event() on a work queue), while
the same reader does user_event_get(orig->event) on it.
Both are reachable by an unprivileged task that can open user_events_data:
one multithreaded process that registers an enabler and then concurrently
unregisters it and calls fork() triggers the race. KASAN reports a
slab-use-after-free in user_event_mm_dup() during clone(), with a
"refcount_t: addition on 0" warning when the user_event is freed.
The enabler use-after-free was found first; the user_event one was reported
by XIAO WU, and the earlier enabler-only fix did not address it.
Defer both the user_event_put() and the kfree(enabler) to a work item
queued with queue_rcu_work(), so they run only after an RCU grace period,
once all readers walking the enabler list have finished. The put must run
in process context because user_event_put() takes event_mutex on the last
reference, so a work queue is used rather than call_rcu(). The now-unlocked
put lets the locked argument of user_event_enabler_destroy() be removed;
all callers are updated.
Fixes: 7235759084a4 ("tracing/user_events: Use remote writes for event enablement")
Cc: stable@vger.kernel.org
Reported-by: XIAO WU <xiaowu.417@qq.com>
Closes: https://lore.kernel.org/all/tencent_89647CE40DC452B891C65C94D1B271DE8E07@qq.com/
Suggested-by: Beau Belgrave <beaub@linux.microsoft.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
Since v1 (the enabler-only fix "tracing/user_events: fix use-after-free
of enabler in user_event_mm_dup()", being dropped from for-linus):
- also fix the user_event-object UAF reported by XIAO WU; defer both the
put and the free with queue_rcu_work() instead of kfree_rcu().
- drop the locked argument now that the put runs from the work item.
- add patch 2 for the selftest teardown timing.
kernel/trace/trace_events_user.c | 39 ++++++++++++++++++++++++++------
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
index c4ba484f7b38b..8c82ecb735f41 100644
--- a/kernel/trace/trace_events_user.c
+++ b/kernel/trace/trace_events_user.c
@@ -109,6 +109,9 @@ struct user_event_enabler {
/* Track enable bit, flags, etc. Aligned for bitops. */
unsigned long values;
+
+ /* Defer the event put and enabler free past an RCU grace period. */
+ struct rcu_work put_rwork;
};
/* Bits 0-5 are for the bit to update upon enable/disable (0-63 allowed) */
@@ -396,17 +399,39 @@ static struct user_event_group *user_event_group_create(void)
return NULL;
};
-static void user_event_enabler_destroy(struct user_event_enabler *enabler,
- bool locked)
+static void delayed_user_event_enabler_put(struct work_struct *work)
{
- list_del_rcu(&enabler->mm_enablers_link);
+ struct user_event_enabler *enabler = container_of(to_rcu_work(work),
+ struct user_event_enabler, put_rwork);
/* No longer tracking the event via the enabler */
- user_event_put(enabler->event, locked);
+ user_event_put(enabler->event, false);
+ /* Run from queue_rcu_work(), the RCU grace period has elapsed */
kfree(enabler);
}
+static void user_event_enabler_destroy(struct user_event_enabler *enabler)
+{
+ list_del_rcu(&enabler->mm_enablers_link);
+
+ /*
+ * The enabler is removed from an RCU-traversed list
+ * (user_event_mm_dup() walks mm->enablers under rcu_read_lock() only),
+ * and readers there dereference enabler->event and take a new ref on
+ * it. Both the put of that event reference and the free of the enabler
+ * therefore have to wait for a grace period so no reader can be looking
+ * at the enabler or racing the last put of its event.
+ *
+ * The put itself must not run in RCU context: when it drops the last
+ * reference user_event_put() takes event_mutex, which cannot be taken
+ * from a softirq/RCU callback. Defer both to a work item scheduled
+ * after a grace period via queue_rcu_work().
+ */
+ INIT_RCU_WORK(&enabler->put_rwork, delayed_user_event_enabler_put);
+ queue_rcu_work(system_percpu_wq, &enabler->put_rwork);
+}
+
static int user_event_mm_fault_in(struct user_event_mm *mm, unsigned long uaddr,
int attempt)
{
@@ -464,7 +489,7 @@ static void user_event_enabler_fault_fixup(struct work_struct *work)
/* User asked for enabler to be removed during fault */
if (test_bit(ENABLE_VAL_FREEING_BIT, ENABLE_BITOPS(enabler))) {
- user_event_enabler_destroy(enabler, true);
+ user_event_enabler_destroy(enabler);
goto out;
}
@@ -764,7 +789,7 @@ static void user_event_mm_destroy(struct user_event_mm *mm)
struct user_event_enabler *enabler, *next;
list_for_each_entry_safe(enabler, next, &mm->enablers, mm_enablers_link)
- user_event_enabler_destroy(enabler, false);
+ user_event_enabler_destroy(enabler);
mmdrop(mm->mm);
kfree(mm);
@@ -2645,7 +2670,7 @@ static long user_events_ioctl_unreg(unsigned long uarg)
flags |= enabler->values & ENABLE_VAL_COMPAT_MASK;
if (!test_bit(ENABLE_VAL_FAULTING_BIT, ENABLE_BITOPS(enabler)))
- user_event_enabler_destroy(enabler, true);
+ user_event_enabler_destroy(enabler);
/* Removed at least one */
ret = 0;
--
2.53.0
^ permalink raw reply related
* [PATCH 2/2] selftests/user_events: wait for deferred event teardown after unregister
From: Michael Bommarito @ 2026-07-07 16:59 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
Cc: Beau Belgrave, XIAO WU, linux-trace-kernel, linux-kernel, stable
In-Reply-To: <20260707165912.2560537-1-michael.bommarito@gmail.com>
Unregistering a user event now defers the drop of the enabler's event
reference (and the freeing of the enabler) past an RCU grace period. As a
result DIAG_IOCSDEL can transiently fail with -EBUSY while that last
reference is still being dropped, where it previously succeeded
immediately.
Two tests assumed the delete takes effect the instant the unregister
returns:
- abi_test "flags" deletes the event right after disabling it.
- perf_test's fixture teardown clear() deletes __test_event before the
next test registers the same name; a stale event makes the following
registration fail with -EADDRINUSE.
Retry the delete until it succeeds (or the event is already gone) with a
bounded wait, matching the existing wait_for_delete() idiom in the same
suite, so the tests are robust to the deferred teardown.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
.../testing/selftests/user_events/abi_test.c | 24 ++++++++++++++++-
.../testing/selftests/user_events/perf_test.c | 26 ++++++++++++++++---
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/user_events/abi_test.c b/tools/testing/selftests/user_events/abi_test.c
index 85892b3b719cc..9e2f84d281afc 100644
--- a/tools/testing/selftests/user_events/abi_test.c
+++ b/tools/testing/selftests/user_events/abi_test.c
@@ -132,6 +132,28 @@ static int event_delete(void)
return ret;
}
+/*
+ * Deleting an event drops its last reference, but an unregister may defer
+ * that put (and the freeing of the associated enabler) past an RCU grace
+ * period. The delete can therefore transiently fail with -EBUSY while the
+ * previous reference is still being dropped. Retry for up to ~10 seconds.
+ */
+static int wait_for_event_delete(void)
+{
+ int i, ret;
+
+ for (i = 0; i < 10000; ++i) {
+ ret = event_delete();
+
+ if (ret == 0)
+ return 0;
+
+ usleep(1000);
+ }
+
+ return ret;
+}
+
static int reg_enable_multi(void *enable, int size, int bit, int flags,
char *args)
{
@@ -262,7 +284,7 @@ TEST_F(user, flags) {
ASSERT_TRUE(event_exists());
/* Ensure we can delete it */
- ASSERT_EQ(0, event_delete());
+ ASSERT_EQ(0, wait_for_event_delete());
/* USER_EVENT_REG_MAX or above is not allowed */
ASSERT_EQ(-1, reg_enable_flags(&self->check, sizeof(int), 0,
diff --git a/tools/testing/selftests/user_events/perf_test.c b/tools/testing/selftests/user_events/perf_test.c
index cafec0e52eb31..5727cb5b914cf 100644
--- a/tools/testing/selftests/user_events/perf_test.c
+++ b/tools/testing/selftests/user_events/perf_test.c
@@ -85,6 +85,7 @@ static int get_offset(void)
static int clear(int *check)
{
struct user_unreg unreg = {0};
+ int i, ret;
unreg.size = sizeof(unreg);
unreg.disable_bit = 31;
@@ -99,13 +100,32 @@ static int clear(int *check)
if (errno != ENOENT)
return -1;
- if (ioctl(fd, DIAG_IOCSDEL, "__test_event") == -1)
- if (errno != ENOENT)
+ /*
+ * Deleting the event drops its last reference, but the unregister
+ * above defers that put (and the freeing of the enabler) past an RCU
+ * grace period. The delete can therefore transiently fail with -EBUSY
+ * until that reference is dropped. Retry for up to ~10 seconds so the
+ * event is actually gone before the next test registers the same name.
+ */
+ for (i = 0; i < 10000; ++i) {
+ ret = ioctl(fd, DIAG_IOCSDEL, "__test_event");
+
+ if (ret == 0 || errno == ENOENT) {
+ ret = 0;
+ break;
+ }
+
+ if (errno != EBUSY) {
+ close(fd);
return -1;
+ }
+
+ usleep(1000);
+ }
close(fd);
- return 0;
+ return ret;
}
FIXTURE(user) {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH 2/2] selftests/user_events: wait for deferred event teardown after unregister
From: Steven Rostedt @ 2026-07-07 17:41 UTC (permalink / raw)
To: Michael Bommarito
Cc: Masami Hiramatsu, Mathieu Desnoyers, Beau Belgrave, XIAO WU,
linux-trace-kernel, linux-kernel, stable
In-Reply-To: <20260707165912.2560537-3-michael.bommarito@gmail.com>
On Tue, 7 Jul 2026 12:59:12 -0400
Michael Bommarito <michael.bommarito@gmail.com> wrote:
>
> +/*
> + * Deleting an event drops its last reference, but an unregister may defer
> + * that put (and the freeing of the associated enabler) past an RCU grace
> + * period. The delete can therefore transiently fail with -EBUSY while the
> + * previous reference is still being dropped. Retry for up to ~10 seconds.
> + */
> +static int wait_for_event_delete(void)
> +{
> + int i, ret;
> +
> + for (i = 0; i < 10000; ++i) {
> + ret = event_delete();
> +
> + if (ret == 0)
> + return 0;
> +
> + usleep(1000);
> + }
> +
> + return ret;
> +}
> +
Care to address Sashiko's comment: https://sashiko.dev/#/patchset/20260707165912.2560537-2-michael.bommarito%40gmail.com
I'll pull in patch 1 and start testing it as this one is just the tools
change, it doesn't need my testing (my tests only tests kernel changes)
-- Steve
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox