| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <el6nfiplc3pitfief24cbcsd4dhvrp5hxwoz3dzccb5kilcogq@qv4pqrzekkfw>
Date: Wed, 27 Aug 2025 21:46:01 -0400
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Steven Rostedt <rostedt@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
bpf@...r.kernel.org, x86@...nel.org,
Masami Hiramatsu <mhiramat@...nel.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Josh Poimboeuf <jpoimboe@...nel.org>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>,
Jiri Olsa <jolsa@...nel.org>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Andrii Nakryiko <andrii@...nel.org>,
Indu Bhagat <indu.bhagat@...cle.com>,
"Jose E. Marchesi" <jemarch@....org>,
Beau Belgrave <beaub@...ux.microsoft.com>,
Jens Remus <jremus@...ux.ibm.com>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Florian Weimer <fweimer@...hat.com>, Sam James <sam@...too.org>,
Kees Cook <kees@...nel.org>, Carlos O'Donell <codonell@...hat.com>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>, David Hildenbrand <david@...hat.com>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
Vlastimil Babka <vbabka@...e.cz>, Mike Rapoport <rppt@...nel.org>,
Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
linux-mm@...ck.org
Subject: Re: [PATCH v10 02/11] unwind_user/sframe: Store sframe section data
in per-mm maple tree
* Steven Rostedt <rostedt@...nel.org> [250827 16:24]:
> From: Josh Poimboeuf <jpoimboe@...nel.org>
>
> Associate an sframe section with its mm by adding it to a per-mm maple
> tree which is indexed by the corresponding text address range. A single
> sframe section can be associated with multiple text ranges.
>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Borislav Petkov <bp@...en8.de>
> Cc: Dave Hansen <dave.hansen@...ux.intel.com>
> Cc: "H. Peter Anvin" <hpa@...or.com>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: David Hildenbrand <david@...hat.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> Cc: "Liam R. Howlett" <Liam.Howlett@...cle.com>
> Cc: Vlastimil Babka <vbabka@...e.cz>
> Cc: Mike Rapoport <rppt@...nel.org>
> Cc: Suren Baghdasaryan <surenb@...gle.com>
> Cc: Michal Hocko <mhocko@...e.com>
> Cc: x86@...nel.org
> Cc: linux-mm@...ck.org
> Signed-off-by: Josh Poimboeuf <jpoimboe@...nel.org>
> Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> ---
> arch/x86/include/asm/mmu.h | 2 +-
> include/linux/mm_types.h | 3 +++
> include/linux/sframe.h | 13 +++++++++
> kernel/fork.c | 10 +++++++
> kernel/unwind/sframe.c | 55 +++++++++++++++++++++++++++++++++++---
> mm/init-mm.c | 2 ++
> 6 files changed, 81 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/mmu.h b/arch/x86/include/asm/mmu.h
> index 0fe9c569d171..227a32899a59 100644
> --- a/arch/x86/include/asm/mmu.h
> +++ b/arch/x86/include/asm/mmu.h
> @@ -87,7 +87,7 @@ typedef struct {
> .context = { \
> .ctx_id = 1, \
> .lock = __MUTEX_INITIALIZER(mm.context.lock), \
> - }
> + },
>
> void leave_mm(void);
> #define leave_mm leave_mm
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 08bc2442db93..31fbd6663047 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -1210,6 +1210,9 @@ struct mm_struct {
> #ifdef CONFIG_MM_ID
> mm_id_t mm_id;
> #endif /* CONFIG_MM_ID */
> +#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
> + struct maple_tree sframe_mt;
> +#endif
> } __randomize_layout;
>
> /*
> diff --git a/include/linux/sframe.h b/include/linux/sframe.h
> index 0584f661f698..73bf6f0b30c2 100644
> --- a/include/linux/sframe.h
> +++ b/include/linux/sframe.h
> @@ -22,18 +22,31 @@ struct sframe_section {
> signed char fp_off;
> };
>
> +#define INIT_MM_SFRAME .sframe_mt = MTREE_INIT(sframe_mt, 0),
> +extern void sframe_free_mm(struct mm_struct *mm);
> +
> extern int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
> unsigned long text_start, unsigned long text_end);
> extern int sframe_remove_section(unsigned long sframe_addr);
>
> +static inline bool current_has_sframe(void)
> +{
> + struct mm_struct *mm = current->mm;
> +
> + return mm && !mtree_empty(&mm->sframe_mt);
> +}
> +
> #else /* !CONFIG_HAVE_UNWIND_USER_SFRAME */
>
> +#define INIT_MM_SFRAME
> +static inline void sframe_free_mm(struct mm_struct *mm) {}
> static inline int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
> unsigned long text_start, unsigned long text_end)
> {
> return -ENOSYS;
> }
> static inline int sframe_remove_section(unsigned long sframe_addr) { return -ENOSYS; }
> +static inline bool current_has_sframe(void) { return false; }
>
> #endif /* CONFIG_HAVE_UNWIND_USER_SFRAME */
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index af673856499d..496781b389bc 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -106,6 +106,7 @@
> #include <linux/pidfs.h>
> #include <linux/tick.h>
> #include <linux/unwind_deferred.h>
> +#include <linux/sframe.h>
>
> #include <asm/pgalloc.h>
> #include <linux/uaccess.h>
> @@ -690,6 +691,7 @@ void __mmdrop(struct mm_struct *mm)
> mm_destroy_cid(mm);
> percpu_counter_destroy_many(mm->rss_stat, NR_MM_COUNTERS);
> futex_hash_free(mm);
> + sframe_free_mm(mm);
>
> free_mm(mm);
> }
> @@ -1027,6 +1029,13 @@ static void mmap_init_lock(struct mm_struct *mm)
> #endif
> }
>
> +static void mm_init_sframe(struct mm_struct *mm)
> +{
> +#ifdef CONFIG_HAVE_UNWIND_USER_SFRAME
> + mt_init(&mm->sframe_mt);
> +#endif
> +}
> +
> static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
> struct user_namespace *user_ns)
> {
> @@ -1055,6 +1064,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
> mm->pmd_huge_pte = NULL;
> #endif
> mm_init_uprobes_state(mm);
> + mm_init_sframe(mm);
> hugetlb_count_init(mm);
>
> if (current->mm) {
> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index 20287f795b36..fa7d87ffd00a 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c
> @@ -122,15 +122,64 @@ int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
> if (ret)
> goto err_free;
>
> - /* TODO nowhere to store it yet - just free it and return an error */
> - ret = -ENOSYS;
> + ret = mtree_insert_range(sframe_mt, sec->text_start, sec->text_end, sec, GFP_KERNEL);
> + if (ret) {
> + dbg("mtree_insert_range failed: text=%lx-%lx\n",
> + sec->text_start, sec->text_end);
> + goto err_free;
> + }
> +
> + return 0;
>
> err_free:
> free_section(sec);
> return ret;
> }
>
> +static int __sframe_remove_section(struct mm_struct *mm,
> + struct sframe_section *sec)
> +{
> + if (!mtree_erase(&mm->sframe_mt, sec->text_start)) {
> + dbg("mtree_erase failed: text=%lx\n", sec->text_start);
> + return -EINVAL;
> + }
> +
> + free_section(sec);
> +
> + return 0;
> +}
> +
> int sframe_remove_section(unsigned long sframe_start)
> {
> - return -ENOSYS;
> + struct mm_struct *mm = current->mm;
> + struct sframe_section *sec;
> + unsigned long index = 0;
> + bool found = false;
> + int ret = 0;
> +
> + mt_for_each(&mm->sframe_mt, sec, index, ULONG_MAX) {
> + if (sec->sframe_start == sframe_start) {
> + found = true;
> + ret |= __sframe_remove_section(mm, sec);
> + }
> + }
If you use the advanced interface you have to handle the locking, but it
will be faster. I'm not sure how frequent you loop across many entries,
but you can do something like:
MA_SATE(mas, &mm->sframe_mt, index, index);
mas_lock(&mas);
mas_for_each(&mas, sec, ULONG_MAX) {
...
}
mas_unlock(&mas);
The maple state contains memory addresses of internal nodes, so you
cannot just edit the tree without it being either unlocked (which
negates the gains you would have) or by using it in the modification.
This seems like a good choice considering the __sframe_remove_section()
is called from only one place. You can pass the struct ma_state through
to the remove function and use it with mas_erase().
Actually, reading it again, why are you starting a search at 0? And
why are you deleting everything after the sframe_start to ULONG_MAX?
This seems incorrect. Can you explain your plan a bit here?
> +
> + if (!found || ret)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +void sframe_free_mm(struct mm_struct *mm)
> +{
> + struct sframe_section *sec;
> + unsigned long index = 0;
> +
> + if (!mm)
> + return;
> +
> + mt_for_each(&mm->sframe_mt, sec, index, ULONG_MAX)
> + free_section(sec);
> +
> + mtree_destroy(&mm->sframe_mt);
The same goes for this function. mt_for_each will start at the top of
the tree, lock, find your result, unlock. Each search starts from the
top of tree because it was unlocked. In the mas_ functions, the tree is
iterated in place which can be significantly faster depending on the
tree size.
Since you are not going to edit the tree you can use a maple state:
struct sframe_section *sec;
MA_STATE(mas, &mm->sframe_mt, 0, 0);
mas_lock(&mas);
mas_for_each(&mas, sec, ULONG_MAX)
free_section(sec);
mas_unlock(&mas);
mtree_destroy(&mm->sframe_mt);
> }
> diff --git a/mm/init-mm.c b/mm/init-mm.c
> index 4600e7605cab..b32fcf167cc2 100644
> --- a/mm/init-mm.c
> +++ b/mm/init-mm.c
> @@ -11,6 +11,7 @@
> #include <linux/atomic.h>
> #include <linux/user_namespace.h>
> #include <linux/iommu.h>
> +#include <linux/sframe.h>
> #include <asm/mmu.h>
>
> #ifndef INIT_MM_CONTEXT
> @@ -46,6 +47,7 @@ struct mm_struct init_mm = {
> .user_ns = &init_user_ns,
> .cpu_bitmap = CPU_BITS_NONE,
> INIT_MM_CONTEXT(init_mm)
> + INIT_MM_SFRAME
> };
>
> void setup_initial_init_mm(void *start_code, void *end_code,
> --
> 2.50.1
>
>
Powered by blists - more mailing lists