* Re: [PATCH 1/8] execmem: drop unused execmem_update_copy()
From: Mike Rapoport @ 2025-07-07 11:49 UTC (permalink / raw)
To: Christophe Leroy
Cc: Andrew Morton, Andy Lutomirski, Borislav Petkov, Daniel Gomez,
Dave Hansen, Ingo Molnar, Luis Chamberlain, Mark Rutland,
Masami Hiramatsu, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <7e52f721-1d8e-4c50-af33-bee3f0d2ac6e@csgroup.eu>
On Mon, Jul 07, 2025 at 12:10:43PM +0200, Christophe Leroy wrote:
>
> Le 04/07/2025 à 15:49, Mike Rapoport a écrit :
> > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> >
> > The execmem_update_copy() that used text poking was required when memory
> > allocated from ROX cache was always read-only. Since now its permissions
> > can be switched to read-write there is no need in a function that updates
> > memory with text poking.
>
> Erm. Looks like I missed the patch that introduced this change.
>
> On some variant of powerpc, namely book3s/32, this is not feasible.
The only user of EXECMEM_ROX_CACHE for now is x86-64, we can always revisit
when powerpc book3s/32 would want to opt in to cache usage.
And it seems that [MODULES_VADDR, MODULES_END] is already mapped with
"large pages", isn't it?
> The granularity for setting the NX (non exec) bit is 256 Mbytes sections.
> So the area dedicated to execmem [MODULES_VADDR; MODULES_END[ always have
> the NX bit unset.
>
> You can change any page within this area from ROX to RWX but you can't make
> it RW without X. If you want RW without X you must map it in the VMALLOC
> area, as VMALLOC area have NX bit always set.
So what will happen when one callse
set_memory_nx()
set_memory_rw()
in such areas?
> Christophe
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH 3/8] execmem: rework execmem_cache_free()
From: Mike Rapoport @ 2025-07-07 11:32 UTC (permalink / raw)
To: Peter Zijlstra, Liam R. Howlett
Cc: Andrew Morton, Andy Lutomirski, Borislav Petkov, Daniel Gomez,
Dave Hansen, Ingo Molnar, Luis Chamberlain, Mark Rutland,
Masami Hiramatsu, H. Peter Anvin, Petr Pavlu, Sami Tolvanen,
Steven Rostedt, Thomas Gleixner, linux-kernel, linux-mm,
linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250707111102.GF1613200@noisy.programming.kicks-ass.net>
On Mon, Jul 07, 2025 at 01:11:02PM +0200, Peter Zijlstra wrote:
> On Fri, Jul 04, 2025 at 04:49:38PM +0300, Mike Rapoport wrote:
> > static bool execmem_cache_free(void *ptr)
> > {
> > struct maple_tree *busy_areas = &execmem_cache.busy_areas;
> > unsigned long addr = (unsigned long)ptr;
> > MA_STATE(mas, busy_areas, addr, addr);
> > void *area;
> > + int err;
> > +
> > + guard(mutex)(&execmem_cache.mutex);
> >
> > area = mas_walk(&mas);
> > + if (!area)
> > return false;
> >
> > + err = __execmem_cache_free(&mas, ptr, GFP_KERNEL | __GFP_NORETRY);
> > + if (err)
> > + goto err_slowpath;
> >
> > schedule_work(&execmem_cache_clean_work);
> >
> > return true;
> > +
> > +err_slowpath:
> > + mas_store_gfp(&mas, pending_free_set(ptr), GFP_KERNEL);
> > + execmem_cache.pending_free_cnt++;
> > + schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
> > + return true;
> > }
>
> This is a bit if an anti-pattern, using guard() and error goto. Since
Good to know :)
> there is only the one site, its best to write it like so:
>
> static bool execmem_cache_free(void *ptr)
> {
> struct maple_tree *busy_areas = &execmem_cache.busy_areas;
> unsigned long addr = (unsigned long)ptr;
> MA_STATE(mas, busy_areas, addr, addr);
> void *area;
> int err;
>
> guard(mutex)(&execmem_cache.mutex);
>
> area = mas_walk(&mas);
> if (!area)
> return false;
>
> err = __execmem_cache_free(&mas, ptr, GFP_KERNEL | __GFP_NORETRY);
> if (err) {
> mas_store_gfp(&mas, pending_free_set(ptr), GFP_KERNEL);
> execmem_cache.pending_free_cnt++;
> schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
> return true;
> }
>
> schedule_work(&execmem_cache_clean_work);
> return true;
> }
>
> And now I have to ask what happens if mas_store_gfp() returns an error?
AFAIU it won't. mas points to exact slot we've got the area from, nothing else
can modify the tree because of the mutex, so that mas_store_gfp()
essentially updates the value at an existing entry.
I'll add a comment about it.
Added @Liam to make sure I'm not saying nonsense :)
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH 3/8] execmem: rework execmem_cache_free()
From: Peter Zijlstra @ 2025-07-07 11:11 UTC (permalink / raw)
To: Mike Rapoport
Cc: Andrew Morton, Andy Lutomirski, Borislav Petkov, Daniel Gomez,
Dave Hansen, Ingo Molnar, Luis Chamberlain, Mark Rutland,
Masami Hiramatsu, H. Peter Anvin, Petr Pavlu, Sami Tolvanen,
Steven Rostedt, Thomas Gleixner, linux-kernel, linux-mm,
linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250704134943.3524829-4-rppt@kernel.org>
On Fri, Jul 04, 2025 at 04:49:38PM +0300, Mike Rapoport wrote:
> static bool execmem_cache_free(void *ptr)
> {
> struct maple_tree *busy_areas = &execmem_cache.busy_areas;
> unsigned long addr = (unsigned long)ptr;
> MA_STATE(mas, busy_areas, addr, addr);
> void *area;
> + int err;
> +
> + guard(mutex)(&execmem_cache.mutex);
>
> area = mas_walk(&mas);
> + if (!area)
> return false;
>
> + err = __execmem_cache_free(&mas, ptr, GFP_KERNEL | __GFP_NORETRY);
> + if (err)
> + goto err_slowpath;
>
> schedule_work(&execmem_cache_clean_work);
>
> return true;
> +
> +err_slowpath:
> + mas_store_gfp(&mas, pending_free_set(ptr), GFP_KERNEL);
> + execmem_cache.pending_free_cnt++;
> + schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
> + return true;
> }
This is a bit if an anti-pattern, using guard() and error goto. Since
there is only the one site, its best to write it like so:
static bool execmem_cache_free(void *ptr)
{
struct maple_tree *busy_areas = &execmem_cache.busy_areas;
unsigned long addr = (unsigned long)ptr;
MA_STATE(mas, busy_areas, addr, addr);
void *area;
int err;
guard(mutex)(&execmem_cache.mutex);
area = mas_walk(&mas);
if (!area)
return false;
err = __execmem_cache_free(&mas, ptr, GFP_KERNEL | __GFP_NORETRY);
if (err) {
mas_store_gfp(&mas, pending_free_set(ptr), GFP_KERNEL);
execmem_cache.pending_free_cnt++;
schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
return true;
}
schedule_work(&execmem_cache_clean_work);
return true;
}
And now I have to ask what happens if mas_store_gfp() returns an error?
^ permalink raw reply
* Re: [PATCH 1/8] execmem: drop unused execmem_update_copy()
From: Christophe Leroy @ 2025-07-07 10:10 UTC (permalink / raw)
To: Mike Rapoport, Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Daniel Gomez, Dave Hansen,
Ingo Molnar, Luis Chamberlain, Mark Rutland, Masami Hiramatsu,
H. Peter Anvin, Peter Zijlstra, Petr Pavlu, Sami Tolvanen,
Steven Rostedt, Thomas Gleixner, linux-kernel, linux-mm,
linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250704134943.3524829-2-rppt@kernel.org>
Le 04/07/2025 à 15:49, Mike Rapoport a écrit :
> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>
> The execmem_update_copy() that used text poking was required when memory
> allocated from ROX cache was always read-only. Since now its permissions
> can be switched to read-write there is no need in a function that updates
> memory with text poking.
Erm. Looks like I missed the patch that introduced this change.
On some variant of powerpc, namely book3s/32, this is not feasible. The
granularity for setting the NX (non exec) bit is 256 Mbytes sections.
So the area dedicated to execmem [MODULES_VADDR; MODULES_END[ always
have the NX bit unset.
You can change any page within this area from ROX to RWX but you can't
make it RW without X. If you want RW without X you must map it in the
VMALLOC area, as VMALLOC area have NX bit always set.
Christophe
>
> Remove it.
>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
> include/linux/execmem.h | 13 -------------
> mm/execmem.c | 5 -----
> 2 files changed, 18 deletions(-)
>
> diff --git a/include/linux/execmem.h b/include/linux/execmem.h
> index 3be35680a54f..734fbe83d98e 100644
> --- a/include/linux/execmem.h
> +++ b/include/linux/execmem.h
> @@ -185,19 +185,6 @@ DEFINE_FREE(execmem, void *, if (_T) execmem_free(_T));
> struct vm_struct *execmem_vmap(size_t size);
> #endif
>
> -/**
> - * execmem_update_copy - copy an update to executable memory
> - * @dst: destination address to update
> - * @src: source address containing the data
> - * @size: how many bytes of memory shold be copied
> - *
> - * Copy @size bytes from @src to @dst using text poking if the memory at
> - * @dst is read-only.
> - *
> - * Return: a pointer to @dst or NULL on error
> - */
> -void *execmem_update_copy(void *dst, const void *src, size_t size);
> -
> /**
> * execmem_is_rox - check if execmem is read-only
> * @type - the execmem type to check
> diff --git a/mm/execmem.c b/mm/execmem.c
> index 2b683e7d864d..0712ebb4eb77 100644
> --- a/mm/execmem.c
> +++ b/mm/execmem.c
> @@ -399,11 +399,6 @@ void execmem_free(void *ptr)
> vfree(ptr);
> }
>
> -void *execmem_update_copy(void *dst, const void *src, size_t size)
> -{
> - return text_poke_copy(dst, src, size);
> -}
> -
> bool execmem_is_rox(enum execmem_type type)
> {
> return !!(execmem_info->ranges[type].flags & EXECMEM_ROX_CACHE);
^ permalink raw reply
* Re: [PATCH v14 3/7] rust: introduce module_param module
From: Miguel Ojeda @ 2025-07-06 20:00 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye, rust-for-linux,
linux-kernel, linux-kbuild, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Simona Vetter, Greg KH, Fiona Behrens,
Daniel Almeida, linux-modules
In-Reply-To: <87jz4orxh9.fsf@kernel.org>
On Fri, Jul 4, 2025 at 1:45 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> It is an inherited name from way back.
Yeah, that name comes from one of the very first PRs. I think we may
have discussed its name in one of the early calls or maybe I just came
up with the name.
Either way, it is almost 5 years old so I would suggest taking a look
at naming etc. with fresh eyes.
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: update Daniel Gomez's role and email address
From: Daniel Gomez @ 2025-07-06 8:15 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Daniel Gomez
Cc: linux-modules, linux-kernel
In-Reply-To: <20250704-add-dagomez-maintainer-v1-1-5fc32033c51c@samsung.com>
On Fri, 04 Jul 2025 21:39:59 +0200, Daniel Gomez wrote:
> Update Daniel Gomez's modules reviewer role to maintainer. This is
> according to the plan [1][2] of scaling with more reviewers for modules
> (for the incoming Rust support [3]) and rotate [4] every 6 months.
>
> [1] Link:
> https://lore.kernel.org/linux-modules/
> ZsPANzx4-5DrOl5m@bombadil.infradead.org
>
> [...]
Applied, thanks!
[1/1] MAINTAINERS: update Daniel Gomez's role and email address
commit: 6d6e69bd7c1c8c4f7b536a88d27d523cdee6a2f8
Best regards,
--
Daniel Gomez <da.gomez@samsung.com>
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: update Daniel Gomez's role and email address
From: Luis Chamberlain @ 2025-07-05 1:29 UTC (permalink / raw)
To: Linus Torvalds, Daniel Gomez
Cc: Petr Pavlu, Daniel Gomez, Sami Tolvanen, linux-modules,
linux-kernel, Daniel Gomez
In-Reply-To: <20250704-add-dagomez-maintainer-v1-1-5fc32033c51c@samsung.com>
On Fri, Jul 04, 2025 at 09:39:59PM +0200, Daniel Gomez wrote:
> From: Daniel Gomez <da.gomez@samsung.com>
>
> Update Daniel Gomez's modules reviewer role to maintainer. This is
> according to the plan [1][2] of scaling with more reviewers for modules
> (for the incoming Rust support [3]) and rotate [4] every 6 months.
>
> [1] Link:
> https://lore.kernel.org/linux-modules/
> ZsPANzx4-5DrOl5m@bombadil.infradead.org
>
> https://lore.kernel.org/linux-modules/
> a3701a9a-5b42-4581-a150-67d84601061c@suse.com
>
> [2] Link:
> https://lore.kernel.org/linux-modules/
> 458901be-1da8-4987-9c72-5aa3da6db15e@suse.com
>
> [3] Link:
> https://lore.kernel.org/linux-modules/
> 20250702-module-params-v3-v14-0-5b1cc32311af@kernel.org
>
> [4] Link:
> https://lore.kernel.org/linux-modules/
> Z3gDAnPlA3SZEbgl@bombadil.infradead.org
>
> Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
Linus -- just a heads up, Daniel is next up from the Rust world
to take on modules. After 6 months from now it will be Sami. Petr
has done great but the goal was to get more blood from the Rust
world. This has been working well so far so I think after Sami
I'll just go as a reviewer and eventually focus on other things.
Acked-by: Luis Chamberlain <mcgrof@kernel.org>
Luis
> ---
> There are fixes [1][2] to be sent for this release cycle that fix a bug
> introduced in v6.16-rc1 and in v6.4-rc1.
>
> [1] Link:
> https://lore.kernel.org/oe-lkp/202506041623.e45e4f7d-lkp@intel.com/
> https://lore.kernel.org/linux-modules/20250610163328.URcsSUC1@linutronix.de/
>
> [2] Link:
> https://lore.kernel.org/linux-modules/20250618122730.51324-1-petr.pavlu@suse.com/
> https://lore.kernel.org/linux-modules/20230319213542.1790479-3-mcgrof@kernel.org/
> ---
> MAINTAINERS | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4bac4ea21b64..bf07c0acd1e1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16820,8 +16820,8 @@ F: include/dt-bindings/clock/mobileye,eyeq5-clk.h
> MODULE SUPPORT
> M: Luis Chamberlain <mcgrof@kernel.org>
> M: Petr Pavlu <petr.pavlu@suse.com>
> +M: Daniel Gomez <da.gomez@kernel.org>
> R: Sami Tolvanen <samitolvanen@google.com>
> -R: Daniel Gomez <da.gomez@samsung.com>
> L: linux-modules@vger.kernel.org
> L: linux-kernel@vger.kernel.org
> S: Maintained
>
> ---
> base-commit: 60481cbdfae129753633cf03f061293b6e0c8bf4
> change-id: 20250704-add-dagomez-maintainer-d48e17d43f9e
>
> Best regards,
> --
> Daniel Gomez <da.gomez@samsung.com>
>
>
^ permalink raw reply
* [PATCH] MAINTAINERS: update Daniel Gomez's role and email address
From: Daniel Gomez @ 2025-07-04 19:39 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: linux-modules, linux-kernel, Daniel Gomez
From: Daniel Gomez <da.gomez@samsung.com>
Update Daniel Gomez's modules reviewer role to maintainer. This is
according to the plan [1][2] of scaling with more reviewers for modules
(for the incoming Rust support [3]) and rotate [4] every 6 months.
[1] Link:
https://lore.kernel.org/linux-modules/
ZsPANzx4-5DrOl5m@bombadil.infradead.org
https://lore.kernel.org/linux-modules/
a3701a9a-5b42-4581-a150-67d84601061c@suse.com
[2] Link:
https://lore.kernel.org/linux-modules/
458901be-1da8-4987-9c72-5aa3da6db15e@suse.com
[3] Link:
https://lore.kernel.org/linux-modules/
20250702-module-params-v3-v14-0-5b1cc32311af@kernel.org
[4] Link:
https://lore.kernel.org/linux-modules/
Z3gDAnPlA3SZEbgl@bombadil.infradead.org
Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
---
There are fixes [1][2] to be sent for this release cycle that fix a bug
introduced in v6.16-rc1 and in v6.4-rc1.
[1] Link:
https://lore.kernel.org/oe-lkp/202506041623.e45e4f7d-lkp@intel.com/
https://lore.kernel.org/linux-modules/20250610163328.URcsSUC1@linutronix.de/
[2] Link:
https://lore.kernel.org/linux-modules/20250618122730.51324-1-petr.pavlu@suse.com/
https://lore.kernel.org/linux-modules/20230319213542.1790479-3-mcgrof@kernel.org/
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 4bac4ea21b64..bf07c0acd1e1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16820,8 +16820,8 @@ F: include/dt-bindings/clock/mobileye,eyeq5-clk.h
MODULE SUPPORT
M: Luis Chamberlain <mcgrof@kernel.org>
M: Petr Pavlu <petr.pavlu@suse.com>
+M: Daniel Gomez <da.gomez@kernel.org>
R: Sami Tolvanen <samitolvanen@google.com>
-R: Daniel Gomez <da.gomez@samsung.com>
L: linux-modules@vger.kernel.org
L: linux-kernel@vger.kernel.org
S: Maintained
---
base-commit: 60481cbdfae129753633cf03f061293b6e0c8bf4
change-id: 20250704-add-dagomez-maintainer-d48e17d43f9e
Best regards,
--
Daniel Gomez <da.gomez@samsung.com>
^ permalink raw reply related
* Re: [PATCH v14 5/7] rust: module: update the module macro with module parameter support
From: Benno Lossin @ 2025-07-04 14:00 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye, rust-for-linux,
linux-kernel, linux-kbuild, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Simona Vetter, Greg KH, Fiona Behrens,
Daniel Almeida, linux-modules
In-Reply-To: <87zfdkqd28.fsf@kernel.org>
On Fri Jul 4, 2025 at 3:51 PM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@kernel.org> writes:
>> On Fri Jul 4, 2025 at 2:29 PM CEST, Andreas Hindborg wrote:
>>> "Benno Lossin" <lossin@kernel.org> writes:
>>>> On Wed Jul 2, 2025 at 3:18 PM CEST, Andreas Hindborg wrote:
>>>>> + perm: 0, // Will not appear in sysfs
>>>>> + level: -1,
>>>>> + flags: 0,
>>>>> + __bindgen_anon_1:
>>>>> + ::kernel::bindings::kernel_param__bindgen_ty_1 {{
>>>>> + arg: {param_name}.as_void_ptr()
>>>>> + }},
>>>>
>>>> Formatting?
>>>>
>>>> + __bindgen_anon_1: ::kernel::bindings::kernel_param__bindgen_ty_1 {{
>>>> + arg: {param_name}.as_void_ptr()
>>>> + }},
>>>
>>>
>>> That makes the line more than 100 characters after changing other
>>> formatting things. Perhaps I should just left shift all this?
>>
>> Not sure what you mean by left shift? When I tried it, it was fine, but
>> it could have changed with the other things... Do you have a branch with
>> your changes?
>
> Move all the code template so the least indented start at column 0.
>
> My WIP branch is here [1].
If you dedent the contents of the string once, then everything fits in
100 columns.
---
Cheers,
Benno
> Best regards,
> Andreas Hindborg
>
>
> [1] https://github.com/metaspace/linux/tree/module-params
^ permalink raw reply
* Re: [PATCH v14 5/7] rust: module: update the module macro with module parameter support
From: Andreas Hindborg @ 2025-07-04 13:51 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye, rust-for-linux,
linux-kernel, linux-kbuild, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Simona Vetter, Greg KH, Fiona Behrens,
Daniel Almeida, linux-modules
In-Reply-To: <DB3A6GR3TQON.C9N9U4V48R1D@kernel.org>
"Benno Lossin" <lossin@kernel.org> writes:
> On Fri Jul 4, 2025 at 2:29 PM CEST, Andreas Hindborg wrote:
>> "Benno Lossin" <lossin@kernel.org> writes:
>>> On Wed Jul 2, 2025 at 3:18 PM CEST, Andreas Hindborg wrote:
>>>> + perm: 0, // Will not appear in sysfs
>>>> + level: -1,
>>>> + flags: 0,
>>>> + __bindgen_anon_1:
>>>> + ::kernel::bindings::kernel_param__bindgen_ty_1 {{
>>>> + arg: {param_name}.as_void_ptr()
>>>> + }},
>>>
>>> Formatting?
>>>
>>> + __bindgen_anon_1: ::kernel::bindings::kernel_param__bindgen_ty_1 {{
>>> + arg: {param_name}.as_void_ptr()
>>> + }},
>>
>>
>> That makes the line more than 100 characters after changing other
>> formatting things. Perhaps I should just left shift all this?
>
> Not sure what you mean by left shift? When I tried it, it was fine, but
> it could have changed with the other things... Do you have a branch with
> your changes?
Move all the code template so the least indented start at column 0.
My WIP branch is here [1].
Best regards,
Andreas Hindborg
[1] https://github.com/metaspace/linux/tree/module-params
^ permalink raw reply
* [PATCH 8/8] x86/ftrace: enable EXECMEM_ROX_CACHE for ftrace allocations
From: Mike Rapoport @ 2025-07-04 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Daniel Gomez, Dave Hansen,
Ingo Molnar, Luis Chamberlain, Mark Rutland, Masami Hiramatsu,
Mike Rapoport, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250704134943.3524829-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
For the most part ftrace uses text poking and can handle ROX memory.
The only place that requires writable memory is create_trampoline() that
updates the allocated memory and in the end makes it ROX.
Use execmem_alloc_rw() in x86::ftrace::alloc_tramp() and enable ROX cache
for EXECMEM_FTRACE when configuration and CPU features allow that.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
arch/x86/kernel/ftrace.c | 2 +-
arch/x86/mm/init.c | 9 ++++++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 252e82bcfd2f..4450acec9390 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -263,7 +263,7 @@ void arch_ftrace_update_code(int command)
static inline void *alloc_tramp(unsigned long size)
{
- return execmem_alloc(EXECMEM_FTRACE, size);
+ return execmem_alloc_rw(EXECMEM_FTRACE, size);
}
static inline void tramp_free(void *tramp)
{
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 442fafd8ff52..bb57e93b4caf 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -1105,7 +1105,14 @@ struct execmem_info __init *execmem_arch_setup(void)
.pgprot = PAGE_KERNEL_ROX,
.alignment = MODULE_ALIGN,
},
- [EXECMEM_FTRACE ... EXECMEM_BPF] = {
+ [EXECMEM_FTRACE] = {
+ .flags = flags,
+ .start = start,
+ .end = MODULES_END,
+ .pgprot = pgprot,
+ .alignment = MODULE_ALIGN,
+ },
+ [EXECMEM_BPF] = {
.flags = EXECMEM_KASAN_SHADOW,
.start = start,
.end = MODULES_END,
--
2.47.2
^ permalink raw reply related
* [PATCH 7/8] x86/kprobes: enable EXECMEM_ROX_CACHE for kprobes allocations
From: Mike Rapoport @ 2025-07-04 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Daniel Gomez, Dave Hansen,
Ingo Molnar, Luis Chamberlain, Mark Rutland, Masami Hiramatsu,
Mike Rapoport, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250704134943.3524829-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
x86::alloc_insn_page() always allocates ROX memory.
Instead of overriding this method, add EXECMEM_KPROBES entry in
execmem_info with pgprot set to PAGE_KERNEL_ROX and use ROX cache when
configuration and CPU features allow it.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
arch/x86/kernel/kprobes/core.c | 18 ------------------
arch/x86/mm/init.c | 9 ++++++++-
2 files changed, 8 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 47cb8eb138ba..6079d15dab8c 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -481,24 +481,6 @@ static int prepare_singlestep(kprobe_opcode_t *buf, struct kprobe *p,
return len;
}
-/* Make page to RO mode when allocate it */
-void *alloc_insn_page(void)
-{
- void *page;
-
- page = execmem_alloc(EXECMEM_KPROBES, PAGE_SIZE);
- if (!page)
- return NULL;
-
- /*
- * TODO: Once additional kernel code protection mechanisms are set, ensure
- * that the page was not maliciously altered and it is still zeroed.
- */
- set_memory_rox((unsigned long)page, 1);
-
- return page;
-}
-
/* Kprobe x86 instruction emulation - only regs->ip or IF flag modifiers */
static void kprobe_emulate_ifmodifiers(struct kprobe *p, struct pt_regs *regs)
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index dbc63f0d538f..442fafd8ff52 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -1098,7 +1098,14 @@ struct execmem_info __init *execmem_arch_setup(void)
.pgprot = pgprot,
.alignment = MODULE_ALIGN,
},
- [EXECMEM_KPROBES ... EXECMEM_BPF] = {
+ [EXECMEM_KPROBES] = {
+ .flags = flags,
+ .start = start,
+ .end = MODULES_END,
+ .pgprot = PAGE_KERNEL_ROX,
+ .alignment = MODULE_ALIGN,
+ },
+ [EXECMEM_FTRACE ... EXECMEM_BPF] = {
.flags = EXECMEM_KASAN_SHADOW,
.start = start,
.end = MODULES_END,
--
2.47.2
^ permalink raw reply related
* [PATCH 6/8] execmem: drop writable parameter from execmem_fill_trapping_insns()
From: Mike Rapoport @ 2025-07-04 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Daniel Gomez, Dave Hansen,
Ingo Molnar, Luis Chamberlain, Mark Rutland, Masami Hiramatsu,
Mike Rapoport, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250704134943.3524829-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
After update of execmem_cache_free() that made memory writable before
updating it, there is no need to update read only memory, so the writable
parameter to execmem_fill_trapping_insns() is not needed. Drop it.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
arch/x86/mm/init.c | 8 ++------
include/linux/execmem.h | 3 +--
mm/execmem.c | 4 ++--
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 7456df985d96..dbc63f0d538f 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -1063,13 +1063,9 @@ unsigned long arch_max_swapfile_size(void)
static struct execmem_info execmem_info __ro_after_init;
#ifdef CONFIG_ARCH_HAS_EXECMEM_ROX
-void execmem_fill_trapping_insns(void *ptr, size_t size, bool writeable)
+void execmem_fill_trapping_insns(void *ptr, size_t size)
{
- /* fill memory with INT3 instructions */
- if (writeable)
- memset(ptr, INT3_INSN_OPCODE, size);
- else
- text_poke_set(ptr, INT3_INSN_OPCODE, size);
+ memset(ptr, INT3_INSN_OPCODE, size);
}
#endif
diff --git a/include/linux/execmem.h b/include/linux/execmem.h
index 4e510d1c609c..fe367bdadc3e 100644
--- a/include/linux/execmem.h
+++ b/include/linux/execmem.h
@@ -60,12 +60,11 @@ enum execmem_range_flags {
* will trap
* @ptr: pointer to memory to fill
* @size: size of the range to fill
- * @writable: is the memory poited by @ptr is writable or ROX
*
* A hook for architecures to fill execmem ranges with invalid instructions.
* Architectures that use EXECMEM_ROX_CACHE must implement this.
*/
-void execmem_fill_trapping_insns(void *ptr, size_t size, bool writable);
+void execmem_fill_trapping_insns(void *ptr, size_t size);
/**
* execmem_restore_rox - restore read-only-execute permissions
diff --git a/mm/execmem.c b/mm/execmem.c
index ec2a6aab143b..398e60c1002f 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -304,7 +304,7 @@ static int execmem_cache_populate(struct execmem_range *range, size_t size)
goto err_free_mem;
/* fill memory with instructions that will trap */
- execmem_fill_trapping_insns(p, alloc_size, /* writable = */ true);
+ execmem_fill_trapping_insns(p, alloc_size);
err = set_memory_rox((unsigned long)p, vm->nr_pages);
if (err)
@@ -363,7 +363,7 @@ static int __execmem_cache_free(struct ma_state *mas, void *ptr, gfp_t gfp_mask)
if (err)
return err;
- execmem_fill_trapping_insns(ptr, size, /* writable = */ true);
+ execmem_fill_trapping_insns(ptr, size);
execmem_restore_rox(ptr, size);
err = execmem_cache_add_locked(ptr, size, gfp_mask);
--
2.47.2
^ permalink raw reply related
* [PATCH 5/8] execmem: add fallback for failures in vmalloc(VM_ALLOW_HUGE_VMAP)
From: Mike Rapoport @ 2025-07-04 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Daniel Gomez, Dave Hansen,
Ingo Molnar, Luis Chamberlain, Mark Rutland, Masami Hiramatsu,
Mike Rapoport, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250704134943.3524829-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
When execmem populates ROX cache it uses vmalloc(VM_ALLOW_HUGE_VMAP).
Although vmalloc falls back to allocating base pages if high order
allocation fails, it may happen that it still cannot allocate enough
memory.
Right now ROX cache is only used by modules and in majority of cases the
allocations happen at boot time when there's plenty of free memory, but
upcoming enabling ROX cache for ftrace and kprobes would mean that execmem
allocations can happen when the system is under memory pressure and a
failure to allocate large page worth of memory becomes more likely.
Fallback to regular vmalloc() if vmalloc(VM_ALLOW_HUGE_VMAP) fails.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
mm/execmem.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/mm/execmem.c b/mm/execmem.c
index 3cb3a9d1c93f..ec2a6aab143b 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -291,6 +291,11 @@ static int execmem_cache_populate(struct execmem_range *range, size_t size)
alloc_size = round_up(size, PMD_SIZE);
p = execmem_vmalloc(range, alloc_size, PAGE_KERNEL, vm_flags);
+ if (!p) {
+ alloc_size = size;
+ p = execmem_vmalloc(range, alloc_size, PAGE_KERNEL, vm_flags);
+ }
+
if (!p)
return err;
@@ -457,7 +462,7 @@ void *execmem_alloc(enum execmem_type type, size_t size)
bool use_cache = range->flags & EXECMEM_ROX_CACHE;
unsigned long vm_flags = VM_FLUSH_RESET_PERMS;
pgprot_t pgprot = range->pgprot;
- void *p;
+ void *p = NULL;
size = PAGE_ALIGN(size);
--
2.47.2
^ permalink raw reply related
* [PATCH 4/8] execmem: move execmem_force_rw() and execmem_restore_rox() before use
From: Mike Rapoport @ 2025-07-04 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Daniel Gomez, Dave Hansen,
Ingo Molnar, Luis Chamberlain, Mark Rutland, Masami Hiramatsu,
Mike Rapoport, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250704134943.3524829-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
to avoid static declarations.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
mm/execmem.c | 44 +++++++++++++++++++++-----------------------
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/mm/execmem.c b/mm/execmem.c
index 1cc781244593..3cb3a9d1c93f 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -137,6 +137,27 @@ static int execmem_set_direct_map_valid(struct vm_struct *vm, bool valid)
return err;
}
+static int execmem_force_rw(void *ptr, size_t size)
+{
+ unsigned int nr = PAGE_ALIGN(size) >> PAGE_SHIFT;
+ unsigned long addr = (unsigned long)ptr;
+ int ret;
+
+ ret = set_memory_nx(addr, nr);
+ if (ret)
+ return ret;
+
+ return set_memory_rw(addr, nr);
+}
+
+int execmem_restore_rox(void *ptr, size_t size)
+{
+ unsigned int nr = PAGE_ALIGN(size) >> PAGE_SHIFT;
+ unsigned long addr = (unsigned long)ptr;
+
+ return set_memory_rox(addr, nr);
+}
+
static void execmem_cache_clean(struct work_struct *work)
{
struct maple_tree *free_areas = &execmem_cache.free_areas;
@@ -328,8 +349,6 @@ static inline void *pending_free_clear(void *ptr)
return (void *)((unsigned long)ptr & ~PENDING_FREE_MASK);
}
-static int execmem_force_rw(void *ptr, size_t size);
-
static int __execmem_cache_free(struct ma_state *mas, void *ptr, gfp_t gfp_mask)
{
size_t size = mas_range_len(mas);
@@ -410,27 +429,6 @@ static bool execmem_cache_free(void *ptr)
return true;
}
-static int execmem_force_rw(void *ptr, size_t size)
-{
- unsigned int nr = PAGE_ALIGN(size) >> PAGE_SHIFT;
- unsigned long addr = (unsigned long)ptr;
- int ret;
-
- ret = set_memory_nx(addr, nr);
- if (ret)
- return ret;
-
- return set_memory_rw(addr, nr);
-}
-
-int execmem_restore_rox(void *ptr, size_t size)
-{
- unsigned int nr = PAGE_ALIGN(size) >> PAGE_SHIFT;
- unsigned long addr = (unsigned long)ptr;
-
- return set_memory_rox(addr, nr);
-}
-
#else /* CONFIG_ARCH_HAS_EXECMEM_ROX */
/*
* when ROX cache is not used the permissions defined by architectures for
--
2.47.2
^ permalink raw reply related
* [PATCH 3/8] execmem: rework execmem_cache_free()
From: Mike Rapoport @ 2025-07-04 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Daniel Gomez, Dave Hansen,
Ingo Molnar, Luis Chamberlain, Mark Rutland, Masami Hiramatsu,
Mike Rapoport, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250704134943.3524829-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
Currently execmem_cache_free() ignores potential allocation failures that
may happen in execmem_cache_add(). Besides, it uses text poking to fill the
memory with trapping instructions before returning it to cache although it
would be more efficient to make that memory writable, update it using
memcpy and then restore ROX protection.
Rework execmem_cache_free() so that in case of an error it will defer
freeing of the memory to a delayed work.
With this the happy fast path will now change permissions to RW, fill the
memory with trapping instructions using memcpy, restore ROX permissions,
add the memory back to the free cache and clear the relevant entry in
busy_areas.
If any step in the fast path fails, the entry in busy_areas will be marked
as pending_free. These entries will be handled by a delayed work and freed
asynchronously.
To make the fast path faster, use __GFP_NORETRY for memory allocations and
let asynchronous handler try harder with GFP_KERNEL.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
mm/execmem.c | 120 +++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 97 insertions(+), 23 deletions(-)
diff --git a/mm/execmem.c b/mm/execmem.c
index 6b040fbc5f4f..1cc781244593 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -93,8 +93,15 @@ struct execmem_cache {
struct mutex mutex;
struct maple_tree busy_areas;
struct maple_tree free_areas;
+ unsigned int pending_free_cnt; /* protected by mutex */
};
+/* delay to schedule asynchronous free if fast path free fails */
+#define FREE_DELAY (msecs_to_jiffies(10))
+
+/* mark entries in busy_areas that should be freed asynchronously */
+#define PENDING_FREE_MASK (1 << (PAGE_SHIFT - 1))
+
static struct execmem_cache execmem_cache = {
.mutex = __MUTEX_INITIALIZER(execmem_cache.mutex),
.busy_areas = MTREE_INIT_EXT(busy_areas, MT_FLAGS_LOCK_EXTERN,
@@ -155,20 +162,17 @@ static void execmem_cache_clean(struct work_struct *work)
static DECLARE_WORK(execmem_cache_clean_work, execmem_cache_clean);
-static int execmem_cache_add(void *ptr, size_t size)
+static int execmem_cache_add_locked(void *ptr, size_t size, gfp_t gfp_mask)
{
struct maple_tree *free_areas = &execmem_cache.free_areas;
- struct mutex *mutex = &execmem_cache.mutex;
unsigned long addr = (unsigned long)ptr;
MA_STATE(mas, free_areas, addr - 1, addr + 1);
unsigned long lower, upper;
void *area = NULL;
- int err;
lower = addr;
upper = addr + size - 1;
- mutex_lock(mutex);
area = mas_walk(&mas);
if (area && mas.last == addr - 1)
lower = mas.index;
@@ -178,12 +182,14 @@ static int execmem_cache_add(void *ptr, size_t size)
upper = mas.last;
mas_set_range(&mas, lower, upper);
- err = mas_store_gfp(&mas, (void *)lower, GFP_KERNEL);
- mutex_unlock(mutex);
- if (err)
- return err;
+ return mas_store_gfp(&mas, (void *)lower, gfp_mask);
+}
- return 0;
+static int execmem_cache_add(void *ptr, size_t size, gfp_t gfp_mask)
+{
+ guard(mutex)(&execmem_cache.mutex);
+
+ return execmem_cache_add_locked(ptr, size, gfp_mask);
}
static bool within_range(struct execmem_range *range, struct ma_state *mas,
@@ -278,7 +284,7 @@ static int execmem_cache_populate(struct execmem_range *range, size_t size)
if (err)
goto err_free_mem;
- err = execmem_cache_add(p, alloc_size);
+ err = execmem_cache_add(p, alloc_size, GFP_KERNEL);
if (err)
goto err_reset_direct_map;
@@ -307,33 +313,101 @@ static void *execmem_cache_alloc(struct execmem_range *range, size_t size)
return __execmem_cache_alloc(range, size);
}
+static inline bool is_pending_free(void *ptr)
+{
+ return ((unsigned long)ptr & PENDING_FREE_MASK);
+}
+
+static inline void *pending_free_set(void *ptr)
+{
+ return (void *)((unsigned long)ptr | PENDING_FREE_MASK);
+}
+
+static inline void *pending_free_clear(void *ptr)
+{
+ return (void *)((unsigned long)ptr & ~PENDING_FREE_MASK);
+}
+
+static int execmem_force_rw(void *ptr, size_t size);
+
+static int __execmem_cache_free(struct ma_state *mas, void *ptr, gfp_t gfp_mask)
+{
+ size_t size = mas_range_len(mas);
+ int err;
+
+ err = execmem_force_rw(ptr, size);
+ if (err)
+ return err;
+
+ execmem_fill_trapping_insns(ptr, size, /* writable = */ true);
+ execmem_restore_rox(ptr, size);
+
+ err = execmem_cache_add_locked(ptr, size, gfp_mask);
+ if (err)
+ return err;
+
+ mas_store_gfp(mas, NULL, gfp_mask);
+ return 0;
+}
+
+static void execmem_cache_free_slow(struct work_struct *work);
+static DECLARE_DELAYED_WORK(execmem_cache_free_work, execmem_cache_free_slow);
+
+static void execmem_cache_free_slow(struct work_struct *work)
+{
+ struct maple_tree *busy_areas = &execmem_cache.busy_areas;
+ MA_STATE(mas, busy_areas, 0, ULONG_MAX);
+ void *area;
+
+ guard(mutex)(&execmem_cache.mutex);
+
+ if (!execmem_cache.pending_free_cnt)
+ return;
+
+ mas_for_each(&mas, area, ULONG_MAX) {
+ if (!is_pending_free(area))
+ continue;
+
+ pending_free_clear(area);
+ if (__execmem_cache_free(&mas, area, GFP_KERNEL))
+ continue;
+
+ execmem_cache.pending_free_cnt--;
+ }
+
+ if (execmem_cache.pending_free_cnt)
+ schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
+ else
+ schedule_work(&execmem_cache_clean_work);
+}
+
static bool execmem_cache_free(void *ptr)
{
struct maple_tree *busy_areas = &execmem_cache.busy_areas;
- struct mutex *mutex = &execmem_cache.mutex;
unsigned long addr = (unsigned long)ptr;
MA_STATE(mas, busy_areas, addr, addr);
- size_t size;
void *area;
+ int err;
+
+ guard(mutex)(&execmem_cache.mutex);
- mutex_lock(mutex);
area = mas_walk(&mas);
- if (!area) {
- mutex_unlock(mutex);
+ if (!area)
return false;
- }
- size = mas_range_len(&mas);
-
- mas_store_gfp(&mas, NULL, GFP_KERNEL);
- mutex_unlock(mutex);
-
- execmem_fill_trapping_insns(ptr, size, /* writable = */ false);
- execmem_cache_add(ptr, size);
+ err = __execmem_cache_free(&mas, ptr, GFP_KERNEL | __GFP_NORETRY);
+ if (err)
+ goto err_slowpath;
schedule_work(&execmem_cache_clean_work);
return true;
+
+err_slowpath:
+ mas_store_gfp(&mas, pending_free_set(ptr), GFP_KERNEL);
+ execmem_cache.pending_free_cnt++;
+ schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
+ return true;
}
static int execmem_force_rw(void *ptr, size_t size)
--
2.47.2
^ permalink raw reply related
* [PATCH 2/8] execmem: introduce execmem_alloc_rw()
From: Mike Rapoport @ 2025-07-04 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Daniel Gomez, Dave Hansen,
Ingo Molnar, Luis Chamberlain, Mark Rutland, Masami Hiramatsu,
Mike Rapoport, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250704134943.3524829-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
Some callers of execmem_alloc() require the memory to be temporarily
writable even when it is allocated from ROX cache. These callers use
execemem_make_temp_rw() right after the call to execmem_alloc().
Wrap this sequence in execmem_alloc_rw() API.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
arch/x86/kernel/alternative.c | 3 +--
include/linux/execmem.h | 38 ++++++++++++++++++++---------------
kernel/module/main.c | 13 ++----------
mm/execmem.c | 27 ++++++++++++++++++++++++-
4 files changed, 51 insertions(+), 30 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index ea1d984166cd..526a5fef93ab 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -120,7 +120,7 @@ struct its_array its_pages;
static void *__its_alloc(struct its_array *pages)
{
- void *page __free(execmem) = execmem_alloc(EXECMEM_MODULE_TEXT, PAGE_SIZE);
+ void *page __free(execmem) = execmem_alloc_rw(EXECMEM_MODULE_TEXT, PAGE_SIZE);
if (!page)
return NULL;
@@ -237,7 +237,6 @@ static void *its_alloc(void)
if (!page)
return NULL;
- execmem_make_temp_rw(page, PAGE_SIZE);
if (pages == &its_pages)
set_memory_x((unsigned long)page, 1);
diff --git a/include/linux/execmem.h b/include/linux/execmem.h
index 734fbe83d98e..4e510d1c609c 100644
--- a/include/linux/execmem.h
+++ b/include/linux/execmem.h
@@ -67,21 +67,6 @@ enum execmem_range_flags {
*/
void execmem_fill_trapping_insns(void *ptr, size_t size, bool writable);
-/**
- * execmem_make_temp_rw - temporarily remap region with read-write
- * permissions
- * @ptr: address of the region to remap
- * @size: size of the region to remap
- *
- * Remaps a part of the cached large page in the ROX cache in the range
- * [@ptr, @ptr + @size) as writable and not executable. The caller must
- * have exclusive ownership of this range and ensure nothing will try to
- * execute code in this range.
- *
- * Return: 0 on success or negative error code on failure.
- */
-int execmem_make_temp_rw(void *ptr, size_t size);
-
/**
* execmem_restore_rox - restore read-only-execute permissions
* @ptr: address of the region to remap
@@ -95,7 +80,6 @@ int execmem_make_temp_rw(void *ptr, size_t size);
*/
int execmem_restore_rox(void *ptr, size_t size);
#else
-static inline int execmem_make_temp_rw(void *ptr, size_t size) { return 0; }
static inline int execmem_restore_rox(void *ptr, size_t size) { return 0; }
#endif
@@ -165,6 +149,28 @@ struct execmem_info *execmem_arch_setup(void);
*/
void *execmem_alloc(enum execmem_type type, size_t size);
+/**
+ * execmem_alloc_rw - allocate writatble executable memory
+ * @type: type of the allocation
+ * @size: how many bytes of memory are required
+ *
+ * Allocates memory that will contain executable code, either generated or
+ * loaded from kernel modules.
+ *
+ * Allocates memory that will contain data coupled with executable code,
+ * like data sections in kernel modules.
+ *
+ * Forces writable permissions on the allocated memory and the caller is
+ * responsible to manage the permissions afterwards.
+ *
+ * For architectures that use ROX cache the permissions will be set to R+W.
+ * For architectures that don't use ROX cache the default permissions for @type
+ * will be used as they must be writable.
+ *
+ * Return: a pointer to the allocated memory or %NULL
+ */
+void *execmem_alloc_rw(enum execmem_type type, size_t size);
+
/**
* execmem_free - free executable memory
* @ptr: pointer to the memory that should be freed
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 413ac6ea3702..d009326ef7bb 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1292,20 +1292,11 @@ static int module_memory_alloc(struct module *mod, enum mod_mem_type type)
else
execmem_type = EXECMEM_MODULE_TEXT;
- ptr = execmem_alloc(execmem_type, size);
+ ptr = execmem_alloc_rw(execmem_type, size);
if (!ptr)
return -ENOMEM;
- if (execmem_is_rox(execmem_type)) {
- int err = execmem_make_temp_rw(ptr, size);
-
- if (err) {
- execmem_free(ptr);
- return -ENOMEM;
- }
-
- mod->mem[type].is_rox = true;
- }
+ mod->mem[type].is_rox = execmem_is_rox(execmem_type);
/*
* The pointer to these blocks of memory are stored on the module
diff --git a/mm/execmem.c b/mm/execmem.c
index 0712ebb4eb77..6b040fbc5f4f 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -336,7 +336,7 @@ static bool execmem_cache_free(void *ptr)
return true;
}
-int execmem_make_temp_rw(void *ptr, size_t size)
+static int execmem_force_rw(void *ptr, size_t size)
{
unsigned int nr = PAGE_ALIGN(size) >> PAGE_SHIFT;
unsigned long addr = (unsigned long)ptr;
@@ -358,6 +358,16 @@ int execmem_restore_rox(void *ptr, size_t size)
}
#else /* CONFIG_ARCH_HAS_EXECMEM_ROX */
+/*
+ * when ROX cache is not used the permissions defined by architectures for
+ * execmem ranges that are updated before use (e.g. EXECMEM_MODULE_TEXT) must
+ * be writable anyway
+ */
+static inline int execmem_force_rw(void *ptr, size_t size)
+{
+ return 0;
+}
+
static void *execmem_cache_alloc(struct execmem_range *range, size_t size)
{
return NULL;
@@ -387,6 +397,21 @@ void *execmem_alloc(enum execmem_type type, size_t size)
return kasan_reset_tag(p);
}
+void *execmem_alloc_rw(enum execmem_type type, size_t size)
+{
+ void *p __free(execmem) = execmem_alloc(type, size);
+ int err;
+
+ if (!p)
+ return NULL;
+
+ err = execmem_force_rw(p, size);
+ if (err)
+ return NULL;
+
+ return no_free_ptr(p);
+}
+
void execmem_free(void *ptr)
{
/*
--
2.47.2
^ permalink raw reply related
* [PATCH 1/8] execmem: drop unused execmem_update_copy()
From: Mike Rapoport @ 2025-07-04 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Daniel Gomez, Dave Hansen,
Ingo Molnar, Luis Chamberlain, Mark Rutland, Masami Hiramatsu,
Mike Rapoport, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250704134943.3524829-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
The execmem_update_copy() that used text poking was required when memory
allocated from ROX cache was always read-only. Since now its permissions
can be switched to read-write there is no need in a function that updates
memory with text poking.
Remove it.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
include/linux/execmem.h | 13 -------------
mm/execmem.c | 5 -----
2 files changed, 18 deletions(-)
diff --git a/include/linux/execmem.h b/include/linux/execmem.h
index 3be35680a54f..734fbe83d98e 100644
--- a/include/linux/execmem.h
+++ b/include/linux/execmem.h
@@ -185,19 +185,6 @@ DEFINE_FREE(execmem, void *, if (_T) execmem_free(_T));
struct vm_struct *execmem_vmap(size_t size);
#endif
-/**
- * execmem_update_copy - copy an update to executable memory
- * @dst: destination address to update
- * @src: source address containing the data
- * @size: how many bytes of memory shold be copied
- *
- * Copy @size bytes from @src to @dst using text poking if the memory at
- * @dst is read-only.
- *
- * Return: a pointer to @dst or NULL on error
- */
-void *execmem_update_copy(void *dst, const void *src, size_t size);
-
/**
* execmem_is_rox - check if execmem is read-only
* @type - the execmem type to check
diff --git a/mm/execmem.c b/mm/execmem.c
index 2b683e7d864d..0712ebb4eb77 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -399,11 +399,6 @@ void execmem_free(void *ptr)
vfree(ptr);
}
-void *execmem_update_copy(void *dst, const void *src, size_t size)
-{
- return text_poke_copy(dst, src, size);
-}
-
bool execmem_is_rox(enum execmem_type type)
{
return !!(execmem_info->ranges[type].flags & EXECMEM_ROX_CACHE);
--
2.47.2
^ permalink raw reply related
* [PATCH 0/8] x86: enable EXECMEM_ROX_CACHE for ftrace and kprobes
From: Mike Rapoport @ 2025-07-04 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Daniel Gomez, Dave Hansen,
Ingo Molnar, Luis Chamberlain, Mark Rutland, Masami Hiramatsu,
Mike Rapoport, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
linux-mm, linux-modules, linux-trace-kernel, x86
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
Hi,
These patches enable use of EXECMEM_ROX_CACHE for ftrace and kprobes
allocations on x86.
They also include some ground work in execmem.
Since the execmem model for caching large ROX pages changed from the
initial assumption that the memory that is allocated from ROX cache is
always ROX to the current state where memory can be temporarily made RW and
then restored to ROX, we can stop using text poking to update it. This also
saves the hassle of trying lock text_mutex in execmem_cache_free() when
kprobes already hold that mutex.
The patches 1-6 update and cleanup execmem ROX cache management,
patch 7 enables EXECMEM_ROX_CACHE for kprobes and
patch 8 enables EXECMEM_ROX_CACHE for frace.
The patches are also available at git:
https://git.kernel.org/rppt/h/execmem/x86-rox/ftrace%2bkprobes
Mike Rapoport (Microsoft) (8):
execmem: drop unused execmem_update_copy()
execmem: introduce execmem_alloc_rw()
execmem: rework execmem_cache_free()
execmem: move execmem_force_rw() and execmem_restore_rox() before use
execmem: add fallback for failures in vmalloc(VM_ALLOW_HUGE_VMAP)
execmem: drop writable parameter from execmem_fill_trapping_insns()
x86/kprobes: enable EXECMEM_ROX_CACHE for kprobes allocations
x86/ftrace: enable EXECMEM_ROX_CACHE for ftrace allocations
arch/x86/kernel/alternative.c | 3 +-
arch/x86/kernel/ftrace.c | 2 +-
arch/x86/kernel/kprobes/core.c | 18 ---
arch/x86/mm/init.c | 24 ++--
include/linux/execmem.h | 54 ++++-----
kernel/module/main.c | 13 +--
mm/execmem.c | 193 +++++++++++++++++++++++++--------
7 files changed, 189 insertions(+), 118 deletions(-)
base-commit: 86731a2a651e58953fc949573895f2fa6d456841
--
2.47.2
^ permalink raw reply
* Re: [PATCH v14 5/7] rust: module: update the module macro with module parameter support
From: Benno Lossin @ 2025-07-04 12:48 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye, rust-for-linux,
linux-kernel, linux-kbuild, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Simona Vetter, Greg KH, Fiona Behrens,
Daniel Almeida, linux-modules
In-Reply-To: <875xg8rvei.fsf@kernel.org>
On Fri Jul 4, 2025 at 2:29 PM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@kernel.org> writes:
>> On Wed Jul 2, 2025 at 3:18 PM CEST, Andreas Hindborg wrote:
>>> + perm: 0, // Will not appear in sysfs
>>> + level: -1,
>>> + flags: 0,
>>> + __bindgen_anon_1:
>>> + ::kernel::bindings::kernel_param__bindgen_ty_1 {{
>>> + arg: {param_name}.as_void_ptr()
>>> + }},
>>
>> Formatting?
>>
>> + __bindgen_anon_1: ::kernel::bindings::kernel_param__bindgen_ty_1 {{
>> + arg: {param_name}.as_void_ptr()
>> + }},
>
>
> That makes the line more than 100 characters after changing other
> formatting things. Perhaps I should just left shift all this?
Not sure what you mean by left shift? When I tried it, it was fine, but
it could have changed with the other things... Do you have a branch with
your changes?
---
Cheers,
Benno
^ permalink raw reply
* Re: [PATCH v14 5/7] rust: module: update the module macro with module parameter support
From: Andreas Hindborg @ 2025-07-04 12:29 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye, rust-for-linux,
linux-kernel, linux-kbuild, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Simona Vetter, Greg KH, Fiona Behrens,
Daniel Almeida, linux-modules
In-Reply-To: <DB1OK2PQZ790.S317HUWYJR3J@kernel.org>
"Benno Lossin" <lossin@kernel.org> writes:
> On Wed Jul 2, 2025 at 3:18 PM CEST, Andreas Hindborg wrote:
>> Allow module parameters to be declared in the rust `module!` macro.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> A few nits below, with those fixed
>
> Reviewed-by: Benno Lossin <lossin@kernel.org>
>
>> ---
>> rust/macros/helpers.rs | 25 +++++++
>> rust/macros/lib.rs | 31 +++++++++
>> rust/macros/module.rs | 177 ++++++++++++++++++++++++++++++++++++++++++++++---
>> 3 files changed, 223 insertions(+), 10 deletions(-)
>
>> + fn emit_params(&mut self, info: &ModuleInfo) {
>> + let Some(params) = &info.params else {
>> + return;
>> + };
>> +
>> + for param in params {
>> + let ops = param_ops_path(¶m.ptype);
>> +
>> + // Note: The spelling of these fields is dictated by the user space
>> + // tool `modinfo`.
>> + self.emit_param("parmtype", ¶m.name, ¶m.ptype);
>> + self.emit_param("parm", ¶m.name, ¶m.description);
>> +
>> + write!(
>> + self.param_buffer,
>> + "
>> + pub(crate) static {param_name}:
>> + ::kernel::module_param::ModuleParamAccess<{param_type}> =
>> + ::kernel::module_param::ModuleParamAccess::new({param_default});
>> +
>> + #[link_section = \"__param\"]
>> + #[used]
>> + static __{module_name}_{param_name}_struct:
>
> Does it make sense to move this static to a `const _: () = {};` block?
Yes, that makes sense.
>
>> + ::kernel::module_param::RacyKernelParam =
>> + ::kernel::module_param::RacyKernelParam::new(
>> + ::kernel::bindings::kernel_param {{
>> + name: if cfg!(MODULE) {{
>
> s/cfg/::core::cfg/
OK.
>
> :)
>
> Also there seems to only be a 2-space indentation here.
Fixed.
>
>> + ::kernel::c_str!(\"{param_name}\").as_bytes_with_nul()
>> + }} else {{
>> + ::kernel::c_str!(\"{module_name}.{param_name}\").as_bytes_with_nul()
>> + }}.as_ptr(),
>> + // SAFETY: `__this_module` is constructed by the kernel at load time
>> + // and will not be freed until the module is unloaded.
>> + #[cfg(MODULE)]
>> + mod_: unsafe {{
>> + (&::kernel::bindings::__this_module
>> + as *const ::kernel::bindings::module)
>> + .cast_mut()
>> + }},
>
> It doesn't stop with the improvements...
>
> https://github.com/Rust-for-Linux/linux/issues/1176
>
> Maybe we should also have one to use it here, but eh we can do that
> later (and it's not as bad to forget about :)
Applying `from_ref`.
>
>> + #[cfg(not(MODULE))]
>> + mod_: ::core::ptr::null_mut(),
>> + ops: &{ops} as *const ::kernel::bindings::kernel_param_ops,
>
> ::core::ptr::from_ref(&{ops})
👍
>
>> + perm: 0, // Will not appear in sysfs
>> + level: -1,
>> + flags: 0,
>> + __bindgen_anon_1:
>> + ::kernel::bindings::kernel_param__bindgen_ty_1 {{
>> + arg: {param_name}.as_void_ptr()
>> + }},
>
> Formatting?
>
> + __bindgen_anon_1: ::kernel::bindings::kernel_param__bindgen_ty_1 {{
> + arg: {param_name}.as_void_ptr()
> + }},
That makes the line more than 100 characters after changing other
formatting things. Perhaps I should just left shift all this?
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH v14 3/7] rust: introduce module_param module
From: Andreas Hindborg @ 2025-07-04 11:46 UTC (permalink / raw)
To: Benno Lossin
Cc: Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Nicolas Schier, Trevor Gross,
Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
Fiona Behrens, Daniel Almeida, linux-modules
In-Reply-To: <DB36LGJOWHZQ.GHXR3VGX7806@kernel.org>
"Benno Lossin" <lossin@kernel.org> writes:
> On Fri Jul 4, 2025 at 9:37 AM CEST, Andreas Hindborg wrote:
>> Andreas Hindborg <a.hindborg@kernel.org> writes:
>>
>>> "Danilo Krummrich" <dakr@kernel.org> writes:
>>>
>>>> On 7/2/25 3:18 PM, Andreas Hindborg wrote:
>>>>> + /// Get a shared reference to the parameter value.
>>>>> + // Note: When sysfs access to parameters are enabled, we have to pass in a
>>>>> + // held lock guard here.
>>>>> + pub fn get(&self) -> &T {
>>>>> + self.value.as_ref().unwrap_or(&self.default)
>>>>> + }
>>>>
>>>> I think you forgot to rename this.
>>>
>>> Yes, thanks for being persistent on this :)
>>
>> Actually, there is a discussion on whether to keep the API similar to
>> `std::sync::OnceLock` [1] but also whether to rename this to something
>> other than `OnceLock` [2]. Depending on how that resolves, it might make
>> sense to stay with `get` or rename to something else.
>
> But this is for the `ModuleParamAccess`, right? There I think it makes
> sense to choose `access` or `value`.
Right, sorry. My context window was too small here.
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH v14 3/7] rust: introduce module_param module
From: Andreas Hindborg @ 2025-07-04 11:45 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye, rust-for-linux,
linux-kernel, linux-kbuild, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Simona Vetter, Greg KH, Fiona Behrens,
Daniel Almeida, linux-modules
In-Reply-To: <DB1O6I32IYI4.OFHKKMD9JV40@kernel.org>
"Benno Lossin" <lossin@kernel.org> writes:
> On Wed Jul 2, 2025 at 3:18 PM CEST, Andreas Hindborg wrote:
>> Add types and traits for interfacing the C moduleparam API.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> I have some nits below, but overall
>
> Reviewed-by: Benno Lossin <lossin@kernel.org>
>
>> ---
>> rust/kernel/lib.rs | 1 +
>> rust/kernel/module_param.rs | 191 ++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 192 insertions(+)
>
> I really like how the `OnceLock` usage turned out here! Thanks for the
> quick impl!
>
>>
>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> index 6b4774b2b1c3..2b439ea06185 100644
>> --- a/rust/kernel/lib.rs
>> +++ b/rust/kernel/lib.rs
>> @@ -87,6 +87,7 @@
>> pub mod list;
>> pub mod miscdevice;
>> pub mod mm;
>> +pub mod module_param;
>> #[cfg(CONFIG_NET)]
>> pub mod net;
>> pub mod of;
>> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
>> new file mode 100644
>> index 000000000000..ca4be7e45ff7
>> --- /dev/null
>> +++ b/rust/kernel/module_param.rs
>> @@ -0,0 +1,191 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Support for module parameters.
>> +//!
>> +//! C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h)
>> +
>> +use crate::prelude::*;
>> +use crate::str::BStr;
>> +use bindings;
>> +use kernel::sync::once_lock::OnceLock;
>> +
>> +/// Newtype to make `bindings::kernel_param` [`Sync`].
>> +#[repr(transparent)]
>> +#[doc(hidden)]
>> +pub struct RacyKernelParam(bindings::kernel_param);
>
> Can you remind me why this is called `Racy`? Maybe add the explainer in
> a comment? (and if it's named racy, why is it okay?)
>
> If it doesn't have a real reason, maybe it should be called
> `KernelParam`?
It is an inherited name from way back. The type exists to allow a static
`bindings::kernel_param`, as this C type is not `Sync`.
I agree, it should just be `KernelParam`.
>
>> +
>> +impl RacyKernelParam {
>> + #[doc(hidden)]
>> + pub const fn new(val: bindings::kernel_param) -> Self {
>> + Self(val)
>> + }
>> +}
>> +
>> +// SAFETY: C kernel handles serializing access to this type. We never access it
>> +// from Rust module.
>> +unsafe impl Sync for RacyKernelParam {}
>> +
>> +/// Types that can be used for module parameters.
>> +// NOTE: This trait is `Copy` because drop could produce unsoundness during teardown.
>> +pub trait ModuleParam: Sized + Copy {
>> + /// The [`ModuleParam`] will be used by the kernel module through this type.
>> + ///
>> + /// This may differ from `Self` if, for example, `Self` needs to track
>> + /// ownership without exposing it or allocate extra space for other possible
>> + /// parameter values.
>> + // This is required to support string parameters in the future.
>> + type Value: ?Sized;
>
> This isn't used anywhere in the patchset and AFAIK the kernel is moving
> away from module params, so I'm not so sure if we're going to have
> strings as params.
The kernel dropping module parameters depends on who you ask. But
regardless, we should probably remove it for now.
>
> Or do you already have those patches ready/plan to use strings? If not,
> then I think we should just remove this type and when we actually need
> them add it.
They are in the old rust branch and they need some work. I do not have a
user for them, which is why I am not including them in this series.
>
>> +
>> + /// Parse a parameter argument into the parameter value.
>> + fn try_from_param_arg(arg: &BStr) -> Result<Self>;
>> +}
>> +
>
>> +impl<T> ModuleParamAccess<T> {
>> + #[doc(hidden)]
>> + pub const fn new(default: T) -> Self {
>> + Self {
>> + value: OnceLock::new(),
>> + default,
>> + }
>> + }
>> +
>> + /// Get a shared reference to the parameter value.
>> + // Note: When sysfs access to parameters are enabled, we have to pass in a
>> + // held lock guard here.
>> + pub fn get(&self) -> &T {
>> + self.value.as_ref().unwrap_or(&self.default)
>> + }
>> +
>> + /// Get a mutable pointer to `self`.
>> + ///
>> + /// NOTE: In most cases it is not safe deref the returned pointer.
>> + pub const fn as_void_ptr(&self) -> *mut c_void {
>> + (self as *const Self).cast_mut().cast()
>
> There is `core::ptr::from_ref` that we should use instead of the `as`
> cast.
Cool 👍
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH v14 3/7] rust: introduce module_param module
From: Benno Lossin @ 2025-07-04 9:59 UTC (permalink / raw)
To: Andreas Hindborg, Danilo Krummrich
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Nicolas Schier, Trevor Gross,
Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
Fiona Behrens, Daniel Almeida, linux-modules
In-Reply-To: <87zfdks8y2.fsf@kernel.org>
On Fri Jul 4, 2025 at 9:37 AM CEST, Andreas Hindborg wrote:
> Andreas Hindborg <a.hindborg@kernel.org> writes:
>
>> "Danilo Krummrich" <dakr@kernel.org> writes:
>>
>>> On 7/2/25 3:18 PM, Andreas Hindborg wrote:
>>>> + /// Get a shared reference to the parameter value.
>>>> + // Note: When sysfs access to parameters are enabled, we have to pass in a
>>>> + // held lock guard here.
>>>> + pub fn get(&self) -> &T {
>>>> + self.value.as_ref().unwrap_or(&self.default)
>>>> + }
>>>
>>> I think you forgot to rename this.
>>
>> Yes, thanks for being persistent on this :)
>
> Actually, there is a discussion on whether to keep the API similar to
> `std::sync::OnceLock` [1] but also whether to rename this to something
> other than `OnceLock` [2]. Depending on how that resolves, it might make
> sense to stay with `get` or rename to something else.
But this is for the `ModuleParamAccess`, right? There I think it makes
sense to choose `access` or `value`.
---
Cheers,
Benno
> Best regards,
> Andreas Hindborg
>
>
> [1] https://lore.kernel.org/all/35e1fef4-b715-4827-a498-bdde9b58b51c@penguintechs.org
> [2] https://lore.kernel.org/all/CAH5fLggY2Ei14nVJzLBEoR1Rut1GKU4SZX=+14tuRH1aSuQVTA@mail.gmail.com
^ permalink raw reply
* Re: [PATCH v14 3/7] rust: introduce module_param module
From: Andreas Hindborg @ 2025-07-04 7:37 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Benno Lossin, Nicolas Schier,
Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules
In-Reply-To: <875xg8tnv7.fsf@kernel.org>
Andreas Hindborg <a.hindborg@kernel.org> writes:
> "Danilo Krummrich" <dakr@kernel.org> writes:
>
>> On 7/2/25 3:18 PM, Andreas Hindborg wrote:
>>> + /// Get a shared reference to the parameter value.
>>> + // Note: When sysfs access to parameters are enabled, we have to pass in a
>>> + // held lock guard here.
>>> + pub fn get(&self) -> &T {
>>> + self.value.as_ref().unwrap_or(&self.default)
>>> + }
>>
>> I think you forgot to rename this.
>
> Yes, thanks for being persistent on this :)
Actually, there is a discussion on whether to keep the API similar to
`std::sync::OnceLock` [1] but also whether to rename this to something
other than `OnceLock` [2]. Depending on how that resolves, it might make
sense to stay with `get` or rename to something else.
Best regards,
Andreas Hindborg
[1] https://lore.kernel.org/all/35e1fef4-b715-4827-a498-bdde9b58b51c@penguintechs.org
[2] https://lore.kernel.org/all/CAH5fLggY2Ei14nVJzLBEoR1Rut1GKU4SZX=+14tuRH1aSuQVTA@mail.gmail.com
^ 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