* [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray
@ 2026-04-15 12:25 Thorsten Blum
2026-04-15 12:25 ` [PATCH 2/2] lib/string_helpers: annotate struct strarray with __counted_by_ptr Thorsten Blum
2026-04-15 14:42 ` [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray Andy Shevchenko
0 siblings, 2 replies; 6+ messages in thread
From: Thorsten Blum @ 2026-04-15 12:25 UTC (permalink / raw)
To: Andrew Morton, Kees Cook, Andy Shevchenko
Cc: Thorsten Blum, linux-hardening, linux-kernel
kasprintf_strarray() returns an array of N strings and kfree_strarray()
also frees N entries. However, kasprintf_strarray() currently allocates
N+1 char pointers. Allocate exactly N pointers instead of N+1.
Also update the kernel-doc for @n.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
lib/string_helpers.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 169eaf583494..6a8db441b6fd 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -752,7 +752,7 @@ EXPORT_SYMBOL_GPL(kstrdup_and_replace);
* kasprintf_strarray - allocate and fill array of sequential strings
* @gfp: flags for the slab allocator
* @prefix: prefix to be used
- * @n: amount of lines to be allocated and filled
+ * @n: number of strings to be allocated and filled
*
* Allocates and fills @n strings using pattern "%s-%zu", where prefix
* is provided by caller. The caller is responsible to free them with
@@ -765,7 +765,7 @@ char **kasprintf_strarray(gfp_t gfp, const char *prefix, size_t n)
char **names;
size_t i;
- names = kcalloc(n + 1, sizeof(char *), gfp);
+ names = kcalloc(n, sizeof(char *), gfp);
if (!names)
return NULL;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] lib/string_helpers: annotate struct strarray with __counted_by_ptr
2026-04-15 12:25 [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray Thorsten Blum
@ 2026-04-15 12:25 ` Thorsten Blum
2026-04-15 14:42 ` [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray Andy Shevchenko
1 sibling, 0 replies; 6+ messages in thread
From: Thorsten Blum @ 2026-04-15 12:25 UTC (permalink / raw)
To: Kees Cook, Andy Shevchenko, Andrew Morton
Cc: Thorsten Blum, linux-hardening, linux-kernel
Add the __counted_by_ptr() compiler attribute to 'array' to improve
bounds checking via CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
lib/string_helpers.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 6a8db441b6fd..98d6ed0eaab7 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -805,7 +805,7 @@ void kfree_strarray(char **array, size_t n)
EXPORT_SYMBOL_GPL(kfree_strarray);
struct strarray {
- char **array;
+ char **array __counted_by_ptr(n);
size_t n;
};
@@ -824,13 +824,13 @@ char **devm_kasprintf_strarray(struct device *dev, const char *prefix, size_t n)
if (!ptr)
return ERR_PTR(-ENOMEM);
+ ptr->n = n;
ptr->array = kasprintf_strarray(GFP_KERNEL, prefix, n);
if (!ptr->array) {
devres_free(ptr);
return ERR_PTR(-ENOMEM);
}
- ptr->n = n;
devres_add(dev, ptr);
return ptr->array;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray
2026-04-15 12:25 [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray Thorsten Blum
2026-04-15 12:25 ` [PATCH 2/2] lib/string_helpers: annotate struct strarray with __counted_by_ptr Thorsten Blum
@ 2026-04-15 14:42 ` Andy Shevchenko
2026-04-15 15:30 ` Thorsten Blum
2026-04-15 18:38 ` David Laight
1 sibling, 2 replies; 6+ messages in thread
From: Andy Shevchenko @ 2026-04-15 14:42 UTC (permalink / raw)
To: Thorsten Blum
Cc: Andrew Morton, Kees Cook, Andy Shevchenko, linux-hardening,
linux-kernel
On Wed, Apr 15, 2026 at 02:25:43PM +0200, Thorsten Blum wrote:
> kasprintf_strarray() returns an array of N strings and kfree_strarray()
> also frees N entries. However, kasprintf_strarray() currently allocates
> N+1 char pointers. Allocate exactly N pointers instead of N+1.
>
> Also update the kernel-doc for @n.
Have you checked all current users that they do not rely on the NULL terminated
array?
Note, that was done on purpose that once allocated it can allow user
to drop the track of the number of strings and rely on NULL terminator.
I.o.w. the number of strings may be just a local variable somewhere
where kasprintf_strarray() is called.
I tend to NAK this change, rather you can update kernel-doc to explain
why it's done this way (see above).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray
2026-04-15 14:42 ` [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray Andy Shevchenko
@ 2026-04-15 15:30 ` Thorsten Blum
2026-04-16 7:48 ` Andy Shevchenko
2026-04-15 18:38 ` David Laight
1 sibling, 1 reply; 6+ messages in thread
From: Thorsten Blum @ 2026-04-15 15:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andrew Morton, Kees Cook, Andy Shevchenko, linux-hardening,
linux-kernel
On Wed, Apr 15, 2026 at 05:42:41PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 15, 2026 at 02:25:43PM +0200, Thorsten Blum wrote:
> > kasprintf_strarray() returns an array of N strings and kfree_strarray()
> > also frees N entries. However, kasprintf_strarray() currently allocates
> > N+1 char pointers. Allocate exactly N pointers instead of N+1.
> >
> > Also update the kernel-doc for @n.
>
> Have you checked all current users that they do not rely on the NULL terminated
> array?
Yes, I've checked all call sites, and none of them rely on the NULL
terminator. Specifically, I checked:
drivers/gpio/gpio-mockup.c
which uses PROPERTY_ENTRY_STRING_ARRAY_LEN(), and
drivers/pinctrl/bcm/pinctrl-bcm4908.c
drivers/pinctrl/intel/pinctrl-intel-platform.c
drivers/pinctrl/meson/pinctrl-amlogic-a4.c
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
drivers/pinctrl/pinctrl-at91.c
drivers/pinctrl/pinctrl-rockchip.c
drivers/pinctrl/pinctrl-st.c
all of which use the size N to iterate over the returned array.
Also, kfree_strarray() explicitly takes the number of entries N,
indicating that callers are expected to keep track of it.
> Note, that was done on purpose that once allocated it can allow user
> to drop the track of the number of strings and rely on NULL terminator.
> I.o.w. the number of strings may be just a local variable somewhere
> where kasprintf_strarray() is called.
>
> I tend to NAK this change, rather you can update kernel-doc to explain
> why it's done this way (see above).
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray
2026-04-15 14:42 ` [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray Andy Shevchenko
2026-04-15 15:30 ` Thorsten Blum
@ 2026-04-15 18:38 ` David Laight
1 sibling, 0 replies; 6+ messages in thread
From: David Laight @ 2026-04-15 18:38 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Thorsten Blum, Andrew Morton, Kees Cook, Andy Shevchenko,
linux-hardening, linux-kernel
On Wed, 15 Apr 2026 17:42:41 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Wed, Apr 15, 2026 at 02:25:43PM +0200, Thorsten Blum wrote:
> > kasprintf_strarray() returns an array of N strings and kfree_strarray()
> > also frees N entries. However, kasprintf_strarray() currently allocates
> > N+1 char pointers. Allocate exactly N pointers instead of N+1.
> >
> > Also update the kernel-doc for @n.
>
> Have you checked all current users that they do not rely on the NULL terminated
> array?
>
> Note, that was done on purpose that once allocated it can allow user
> to drop the track of the number of strings and rely on NULL terminator.
> I.o.w. the number of strings may be just a local variable somewhere
> where kasprintf_strarray() is called.
>
> I tend to NAK this change, rather you can update kernel-doc to explain
> why it's done this way (see above).
>
It isn't as though it is going to make much difference to the memory
footprint.
Allocating an extra trailing NULL is just a bit safer.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray
2026-04-15 15:30 ` Thorsten Blum
@ 2026-04-16 7:48 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2026-04-16 7:48 UTC (permalink / raw)
To: Thorsten Blum
Cc: Andrew Morton, Kees Cook, Andy Shevchenko, linux-hardening,
linux-kernel
On Wed, Apr 15, 2026 at 05:30:50PM +0200, Thorsten Blum wrote:
> On Wed, Apr 15, 2026 at 05:42:41PM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 15, 2026 at 02:25:43PM +0200, Thorsten Blum wrote:
> > > kasprintf_strarray() returns an array of N strings and kfree_strarray()
> > > also frees N entries. However, kasprintf_strarray() currently allocates
> > > N+1 char pointers. Allocate exactly N pointers instead of N+1.
> > >
> > > Also update the kernel-doc for @n.
> >
> > Have you checked all current users that they do not rely on the NULL terminated
> > array?
>
> Yes, I've checked all call sites, and none of them rely on the NULL
> terminator. Specifically, I checked:
>
> drivers/gpio/gpio-mockup.c
>
> which uses PROPERTY_ENTRY_STRING_ARRAY_LEN(), and
>
> drivers/pinctrl/bcm/pinctrl-bcm4908.c
> drivers/pinctrl/intel/pinctrl-intel-platform.c
> drivers/pinctrl/meson/pinctrl-amlogic-a4.c
> drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> drivers/pinctrl/pinctrl-at91.c
> drivers/pinctrl/pinctrl-rockchip.c
> drivers/pinctrl/pinctrl-st.c
>
> all of which use the size N to iterate over the returned array.
Thanks for confirming.
> Also, kfree_strarray() explicitly takes the number of entries N,
> indicating that callers are expected to keep track of it.
Still we might have an API that requires a NULL terminated arrays (when it
doesn't take size), which a caller wants to use.
> > Note, that was done on purpose that once allocated it can allow user
> > to drop the track of the number of strings and rely on NULL terminator.
> > I.o.w. the number of strings may be just a local variable somewhere
> > where kasprintf_strarray() is called.
> >
> > I tend to NAK this change, rather you can update kernel-doc to explain
> > why it's done this way (see above).
Given pros and cons, and what David said I'm still not sure that this is
going to be a beneficial patch. I leave it Kees and Andrew to decide.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-04-16 7:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-15 12:25 [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray Thorsten Blum
2026-04-15 12:25 ` [PATCH 2/2] lib/string_helpers: annotate struct strarray with __counted_by_ptr Thorsten Blum
2026-04-15 14:42 ` [PATCH 1/2] lib/string_helpers: drop redundant allocation in kasprintf_strarray Andy Shevchenko
2026-04-15 15:30 ` Thorsten Blum
2026-04-16 7:48 ` Andy Shevchenko
2026-04-15 18:38 ` David Laight
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox