[PATCH v3 01/15] lib: provide kmemdup_const()

Bartosz Golaszewski posted 15 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH v3 01/15] lib: provide kmemdup_const()
Posted by Bartosz Golaszewski 2 months, 1 week ago
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Provide a function similar to strdup_const() but for copying blocks of
memory that are likely to be placed in .rodata.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 include/linux/string.h |  1 +
 mm/util.c              | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index fdd3442c6bcbd786e177b6e87358e1065a0ffafc..1a86d61de91204563e4179938c4dfc77108e03aa 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -304,6 +304,7 @@ extern char *kstrndup(const char *s, size_t len, gfp_t gfp);
 extern void *kmemdup_noprof(const void *src, size_t len, gfp_t gfp) __realloc_size(2);
 #define kmemdup(...)	alloc_hooks(kmemdup_noprof(__VA_ARGS__))
 
+extern const void *kmemdup_const(const void *src, size_t len, gfp_t gfp);
 extern void *kvmemdup(const void *src, size_t len, gfp_t gfp) __realloc_size(2);
 extern char *kmemdup_nul(const char *s, size_t len, gfp_t gfp);
 extern void *kmemdup_array(const void *src, size_t count, size_t element_size, gfp_t gfp)
diff --git a/mm/util.c b/mm/util.c
index f814e6a59ab1d354b8cd04ebf3903626f6b23a6c..f4df9194b0c69c27ff06e6ba1d1137c559035470 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -142,6 +142,27 @@ void *kmemdup_noprof(const void *src, size_t len, gfp_t gfp)
 }
 EXPORT_SYMBOL(kmemdup_noprof);
 
+/**
+ * kmemdup_const - conditionally duplicate a region of memory
+ *
+ * @src: memory region to duplicate
+ * @len: memory region length,
+ * @gfp: GFP mask to use
+ *
+ * Return: source address if it is in .rodata or the return value of kmemdup()
+ * to which the function falls back otherwise.
+ *
+ * Note: the returned address must not be passed to kfree(), the caller must
+ * use kfree_const() instead.
+ */
+const void *kmemdup_const(const void *src, size_t len, gfp_t gfp)
+{
+	if (is_kernel_rodata((unsigned long)src))
+		return src;
+
+	return kmemdup(src, len, gfp);
+}
+
 /**
  * kmemdup_array - duplicate a given array.
  *

-- 
2.48.1
Re: [PATCH v3 01/15] lib: provide kmemdup_const()
Posted by Lorenzo Stoakes 2 months, 1 week ago
On Thu, Jul 24, 2025 at 11:24:29AM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Provide a function similar to strdup_const() but for copying blocks of
> memory that are likely to be placed in .rodata.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

LGTM afaict aside from nit below, so:

Acked-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  include/linux/string.h |  1 +
>  mm/util.c              | 21 +++++++++++++++++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index fdd3442c6bcbd786e177b6e87358e1065a0ffafc..1a86d61de91204563e4179938c4dfc77108e03aa 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -304,6 +304,7 @@ extern char *kstrndup(const char *s, size_t len, gfp_t gfp);
>  extern void *kmemdup_noprof(const void *src, size_t len, gfp_t gfp) __realloc_size(2);
>  #define kmemdup(...)	alloc_hooks(kmemdup_noprof(__VA_ARGS__))
>
> +extern const void *kmemdup_const(const void *src, size_t len, gfp_t gfp);

Please drop extern, it's unnecessary.

>  extern void *kvmemdup(const void *src, size_t len, gfp_t gfp) __realloc_size(2);
>  extern char *kmemdup_nul(const char *s, size_t len, gfp_t gfp);
>  extern void *kmemdup_array(const void *src, size_t count, size_t element_size, gfp_t gfp)
> diff --git a/mm/util.c b/mm/util.c
> index f814e6a59ab1d354b8cd04ebf3903626f6b23a6c..f4df9194b0c69c27ff06e6ba1d1137c559035470 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -142,6 +142,27 @@ void *kmemdup_noprof(const void *src, size_t len, gfp_t gfp)
>  }
>  EXPORT_SYMBOL(kmemdup_noprof);
>
> +/**
> + * kmemdup_const - conditionally duplicate a region of memory
> + *
> + * @src: memory region to duplicate
> + * @len: memory region length,
> + * @gfp: GFP mask to use
> + *
> + * Return: source address if it is in .rodata or the return value of kmemdup()
> + * to which the function falls back otherwise.
> + *
> + * Note: the returned address must not be passed to kfree(), the caller must
> + * use kfree_const() instead.
> + */
> +const void *kmemdup_const(const void *src, size_t len, gfp_t gfp)
> +{
> +	if (is_kernel_rodata((unsigned long)src))
> +		return src;
> +
> +	return kmemdup(src, len, gfp);
> +}
> +
>  /**
>   * kmemdup_array - duplicate a given array.
>   *
>
> --
> 2.48.1
>
Re: [PATCH v3 01/15] lib: provide kmemdup_const()
Posted by Andy Shevchenko 2 months, 1 week ago
On Thu, Jul 24, 2025 at 1:10 PM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
> On Thu, Jul 24, 2025 at 11:24:29AM +0200, Bartosz Golaszewski wrote:

...

> >  extern void *kmemdup_noprof(const void *src, size_t len, gfp_t gfp) __realloc_size(2);
> >  #define kmemdup(...) alloc_hooks(kmemdup_noprof(__VA_ARGS__))
> >
> > +extern const void *kmemdup_const(const void *src, size_t len, gfp_t gfp);
>
> Please drop extern, it's unnecessary.

It's all over the header. This should be done as a precursor patch and
I know that usually people push back on doing that. I gave up on this.
Kernel is going to rot sooner or later... :-(

> >  extern void *kvmemdup(const void *src, size_t len, gfp_t gfp) __realloc_size(2);
> >  extern char *kmemdup_nul(const char *s, size_t len, gfp_t gfp);
> >  extern void *kmemdup_array(const void *src, size_t count, size_t element_size, gfp_t gfp)


-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v3 01/15] lib: provide kmemdup_const()
Posted by Lorenzo Stoakes 2 months, 1 week ago
On Thu, Jul 24, 2025 at 01:12:49PM +0200, Andy Shevchenko wrote:
> On Thu, Jul 24, 2025 at 1:10 PM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> > On Thu, Jul 24, 2025 at 11:24:29AM +0200, Bartosz Golaszewski wrote:
> > > +extern const void *kmemdup_const(const void *src, size_t len, gfp_t gfp);
> >
> > Please drop extern, it's unnecessary.
>
> It's all over the header. This should be done as a precursor patch and
> I know that usually people push back on doing that. I gave up on this.
> Kernel is going to rot sooner or later... :-(

In mm we just update as we go, this is probably the best approach to avoid
unnecessary churn.
Re: [PATCH v3 01/15] lib: provide kmemdup_const()
Posted by Andy Shevchenko 2 months, 1 week ago
On Thu, Jul 24, 2025 at 12:15:11PM +0100, Lorenzo Stoakes wrote:
> On Thu, Jul 24, 2025 at 01:12:49PM +0200, Andy Shevchenko wrote:
> > On Thu, Jul 24, 2025 at 1:10 PM Lorenzo Stoakes
> > <lorenzo.stoakes@oracle.com> wrote:
> > > On Thu, Jul 24, 2025 at 11:24:29AM +0200, Bartosz Golaszewski wrote:
> > > > +extern const void *kmemdup_const(const void *src, size_t len, gfp_t gfp);
> > >
> > > Please drop extern, it's unnecessary.
> >
> > It's all over the header. This should be done as a precursor patch and
> > I know that usually people push back on doing that. I gave up on this.
> > Kernel is going to rot sooner or later... :-(
> 
> In mm we just update as we go, this is probably the best approach to avoid
> unnecessary churn.

I agree on the idea of eliminating it, but also I agree on the consistency over
redundancy. That's why I prefer to see this done at once for all (in the same
header) than doing one-by-one. And this approach got a lot of pushes back, while
the former even more pushed back on the (in)consistency matters.

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH v3 01/15] lib: provide kmemdup_const()
Posted by David Hildenbrand 2 months, 1 week ago
On 24.07.25 13:52, Andy Shevchenko wrote:
> On Thu, Jul 24, 2025 at 12:15:11PM +0100, Lorenzo Stoakes wrote:
>> On Thu, Jul 24, 2025 at 01:12:49PM +0200, Andy Shevchenko wrote:
>>> On Thu, Jul 24, 2025 at 1:10 PM Lorenzo Stoakes
>>> <lorenzo.stoakes@oracle.com> wrote:
>>>> On Thu, Jul 24, 2025 at 11:24:29AM +0200, Bartosz Golaszewski wrote:
>>>>> +extern const void *kmemdup_const(const void *src, size_t len, gfp_t gfp);
>>>>
>>>> Please drop extern, it's unnecessary.
>>>
>>> It's all over the header. This should be done as a precursor patch and
>>> I know that usually people push back on doing that. I gave up on this.
>>> Kernel is going to rot sooner or later... :-(
>>
>> In mm we just update as we go, this is probably the best approach to avoid
>> unnecessary churn.
> 
> I agree on the idea of eliminating it, but also I agree on the consistency over
> redundancy. That's why I prefer to see this done at once for all (in the same
> header) than doing one-by-one. And this approach got a lot of pushes back, while
> the former even more pushed back on the (in)consistency matters.

No new extern where unnecessary.

-- 
Cheers,

David / dhildenb

Re: [PATCH v3 01/15] lib: provide kmemdup_const()
Posted by Andy Shevchenko 2 months, 1 week ago
On Thu, Jul 24, 2025 at 11:24 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Provide a function similar to strdup_const() but for copying blocks of
> memory that are likely to be placed in .rodata.

Makes sense, one nit-pick below.

...

>  extern void *kmemdup_noprof(const void *src, size_t len, gfp_t gfp) __realloc_size(2);
>  #define kmemdup(...)   alloc_hooks(kmemdup_noprof(__VA_ARGS__))
>
> +extern const void *kmemdup_const(const void *src, size_t len, gfp_t gfp);

Can we locate this in the similar order to the C-file? I would put it
before kmemdup_array().

>  extern void *kvmemdup(const void *src, size_t len, gfp_t gfp) __realloc_size(2);
>  extern char *kmemdup_nul(const char *s, size_t len, gfp_t gfp);
>  extern void *kmemdup_array(const void *src, size_t count, size_t element_size, gfp_t gfp)

(below left for the context)
> +
>  /**
>   * kmemdup_array - duplicate a given array.

-- 
With Best Regards,
Andy Shevchenko