tools/testing/selftests/clone3/clone3_set_tid.c | 2 +- tools/testing/selftests/mm/mlock-random-test.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Clone3_set_tid uses getline(&line, &len, f) in a loop to read the
child's process status. The code expects that getline allocates the
buffer for the line on the first loop iteration. For this, glibc[1]
requires char *line to be set to NULL:
> ssize_t getline(char **restrict lineptr, ...)
> If *lineptr is set to NULL before the call, then getline() will
> allocate a buffer for storing the line.
However, char *line is only declared, leading to an undefined
initialization value. Fix this by properly initializing it to NULL.
Same issue fixed in mlock-random-test.
[1] https://man7.org/linux/man-pages/man3/getline.3.html
Fixes: 41585bbeeef9 ("selftests: add tests for clone3() with *set_tid")
Fixes: 26b4224d9961 ("selftests: expanding more mlock selftest")
Signed-off-by: Chris Gellermann <christian.gellermann@codasip.com>
---
tools/testing/selftests/clone3/clone3_set_tid.c | 2 +-
tools/testing/selftests/mm/mlock-random-test.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/clone3/clone3_set_tid.c b/tools/testing/selftests/clone3/clone3_set_tid.c
index 5c944aee6b41..485efa7c9eed 100644
--- a/tools/testing/selftests/clone3/clone3_set_tid.c
+++ b/tools/testing/selftests/clone3/clone3_set_tid.c
@@ -141,7 +141,7 @@ int main(int argc, char *argv[])
{
FILE *f;
char buf;
- char *line;
+ char *line = NULL;
int status;
int ret = -1;
size_t len = 0;
diff --git a/tools/testing/selftests/mm/mlock-random-test.c b/tools/testing/selftests/mm/mlock-random-test.c
index 9d349c151360..16294bc7dae6 100644
--- a/tools/testing/selftests/mm/mlock-random-test.c
+++ b/tools/testing/selftests/mm/mlock-random-test.c
@@ -84,7 +84,7 @@ int get_proc_locked_vm_size(void)
int get_proc_page_size(unsigned long addr)
{
FILE *smaps;
- char *line;
+ char *line = NULL;
unsigned long mmupage_size = 0;
size_t size;
--
2.47.3
On Tue, 26 May 2026 13:38:48 +0200 Chris Gellermann <christian.gellermann@codasip.com> wrote:
> Subject: [PATCH] selftest: Fix UB of getline due to missing var init
hm, what's "UB". Please expand the acronym.
> Clone3_set_tid uses getline(&line, &len, f) in a loop to read the
> child's process status. The code expects that getline allocates the
> buffer for the line on the first loop iteration. For this, glibc[1]
> requires char *line to be set to NULL:
>
> > ssize_t getline(char **restrict lineptr, ...)
> > If *lineptr is set to NULL before the call, then getline() will
> > allocate a buffer for storing the line.
>
> However, char *line is only declared, leading to an undefined
> initialization value. Fix this by properly initializing it to NULL.
Does the test crash? If not, how come? Luck?
> Same issue fixed in mlock-random-test.
>
> [1] https://man7.org/linux/man-pages/man3/getline.3.html
The two affected files are testing significantly different parts of the
kernel.
> Fixes: 41585bbeeef9 ("selftests: add tests for clone3() with *set_tid")
> Fixes: 26b4224d9961 ("selftests: expanding more mlock selftest")
And these were separated by three years.
So can you please split this into a two-patch series? And I suggest
you add "Cc: <stable@vger.kernel.org>" to each one. Please retain David's
ack on both.
Thanks.
On Tue, May 26, 2026 at 11:34:09AM -0700, Andrew Morton wrote:
> On Tue, 26 May 2026 13:38:48 +0200 Chris Gellermann <christian.gellermann@codasip.com> wrote:
>
> > Subject: [PATCH] selftest: Fix UB of getline due to missing var init
>
> hm, what's "UB". Please expand the acronym.
>
> > Clone3_set_tid uses getline(&line, &len, f) in a loop to read the
> > child's process status. The code expects that getline allocates the
> > buffer for the line on the first loop iteration. For this, glibc[1]
> > requires char *line to be set to NULL:
> >
> > > ssize_t getline(char **restrict lineptr, ...)
> > > If *lineptr is set to NULL before the call, then getline() will
> > > allocate a buffer for storing the line.
> >
> > However, char *line is only declared, leading to an undefined
> > initialization value. Fix this by properly initializing it to NULL.
>
> Does the test crash? If not, how come? Luck?
>
> > Same issue fixed in mlock-random-test.
> >
> > [1] https://man7.org/linux/man-pages/man3/getline.3.html
>
> The two affected files are testing significantly different parts of the
> kernel.
>
> > Fixes: 41585bbeeef9 ("selftests: add tests for clone3() with *set_tid")
> > Fixes: 26b4224d9961 ("selftests: expanding more mlock selftest")
>
> And these were separated by three years.
>
> So can you please split this into a two-patch series? And I suggest
> you add "Cc: <stable@vger.kernel.org>" to each one. Please retain David's
> ack on both.
Since this looks fine (I also wondered about the fixes too of course), feel free
to add my tag to this too:
Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>
>
> Thanks.
Cheers, Lorenzo
Clone3_set_tid uses getline(&line, ...) in a loop to read the child's
process status. The code expects that getline allocates the buffer for
the line on the first loop iteration. According to the Open Group
Spec[1], char *line has to be null pointer for this:
> ssize_t getline(char **restrict lineptr, ...);
> If *lineptr is a null pointer or if the object pointed to by *lineptr
> is of insufficient size, an object shall be allocated as if by
malloc()
> or the object shall be reallocated as if by realloc()[...].
However, char *line is only declared, leading to an undefined value
that is potentially non-null. In an example run with Musl v1.2.6, the
realloc call[2] of getdelim, which implements getline, triggers a
segfault:
./run_kselftest.sh --test clone3:clone3_set_tid
[ 1366.165898] kselftest: Running tests in clone3
...
[ 1367.799244] clone3_set_tid[811]: unhandled signal 11 code 0x1 at
0x0000000000000000 in libc.so[68184,3fbf69f000+4c000]
[ 1367.802808] CPU: 0 UID: 0 PID: 811 Comm: clone3_set_tid Not tainted
..
[ 1367.804188] epc: 0x0000003fbf6b0184
[ 1367.804188] ra : 0x0000003fbf6d4664
[ 1367.804188] sp : 0x0000003fce5f2e40
[ 1367.805314] gp : 0x0000002aaab0dfb8
[ 1367.805314] tp : 0x0000003fbf6f14a8
[ 1367.805314] t0 : 0x0000003fbf63d000
...
Looking at the realloc implementation, Musl mallocs for a null pointer
memory. But for a non-null pointer, it assumes it's passed a valid
pointer to the heap and tries to access its meta-data. This leads to the
segfault we see:
void *realloc(void *p, size_t n)
{
if (!p) return malloc(n);
if (size_overflows(n)) return 0;
struct meta *g = get_meta(p);
...
}
Fix this by properly initializing the line pointer to NULL.
[1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/getline.html
[2] https://git.musl-libc.org/cgit/musl/tree/src/stdio/getdelim.c#n38
Fixes: 41585bbeeef9 ("selftests: add tests for clone3() with *set_tid")
Cc: stable@vger.kernel.org
Acked-by: David Hildenbrand (arm) <david@kernel.org>
Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>
Signed-off-by: Chris Gellermann <christian.gellermann@codasip.com>
---
tools/testing/selftests/clone3/clone3_set_tid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/clone3/clone3_set_tid.c b/tools/testing/selftests/clone3/clone3_set_tid.c
index 5c944aee6b41..485efa7c9eed 100644
--- a/tools/testing/selftests/clone3/clone3_set_tid.c
+++ b/tools/testing/selftests/clone3/clone3_set_tid.c
@@ -141,7 +141,7 @@ int main(int argc, char *argv[])
{
FILE *f;
char buf;
- char *line;
+ char *line = NULL;
int status;
int ret = -1;
size_t len = 0;
--
2.47.3
Hm you're combining 2 of my least favourite things in one :)
- Doing a >1 patch series with patch N+1 in-reply-to patch N
- Doing a vN+1 in reply to a vN series.
Just for future, please send series independent of each other not in reply to
other series, and if there's more than 1 patch, send a cover letter and have all
the patches reply to that!
Thanks, Lorenzo
On Wed, Jun 03, 2026 at 12:43:09PM +0200, Chris Gellermann wrote:
> Clone3_set_tid uses getline(&line, ...) in a loop to read the child's
> process status. The code expects that getline allocates the buffer for
> the line on the first loop iteration. According to the Open Group
> Spec[1], char *line has to be null pointer for this:
>
> > ssize_t getline(char **restrict lineptr, ...);
> > If *lineptr is a null pointer or if the object pointed to by *lineptr
> > is of insufficient size, an object shall be allocated as if by
> malloc()
> > or the object shall be reallocated as if by realloc()[...].
>
> However, char *line is only declared, leading to an undefined value
> that is potentially non-null. In an example run with Musl v1.2.6, the
> realloc call[2] of getdelim, which implements getline, triggers a
> segfault:
>
> ./run_kselftest.sh --test clone3:clone3_set_tid
> [ 1366.165898] kselftest: Running tests in clone3
> ...
> [ 1367.799244] clone3_set_tid[811]: unhandled signal 11 code 0x1 at
> 0x0000000000000000 in libc.so[68184,3fbf69f000+4c000]
> [ 1367.802808] CPU: 0 UID: 0 PID: 811 Comm: clone3_set_tid Not tainted
> ..
> [ 1367.804188] epc: 0x0000003fbf6b0184
> [ 1367.804188] ra : 0x0000003fbf6d4664
> [ 1367.804188] sp : 0x0000003fce5f2e40
> [ 1367.805314] gp : 0x0000002aaab0dfb8
> [ 1367.805314] tp : 0x0000003fbf6f14a8
> [ 1367.805314] t0 : 0x0000003fbf63d000
> ...
>
> Looking at the realloc implementation, Musl mallocs for a null pointer
> memory. But for a non-null pointer, it assumes it's passed a valid
> pointer to the heap and tries to access its meta-data. This leads to the
> segfault we see:
>
> void *realloc(void *p, size_t n)
> {
> if (!p) return malloc(n);
> if (size_overflows(n)) return 0;
>
> struct meta *g = get_meta(p);
> ...
> }
>
> Fix this by properly initializing the line pointer to NULL.
>
> [1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/getline.html
> [2] https://git.musl-libc.org/cgit/musl/tree/src/stdio/getdelim.c#n38
>
> Fixes: 41585bbeeef9 ("selftests: add tests for clone3() with *set_tid")
> Cc: stable@vger.kernel.org
> Acked-by: David Hildenbrand (arm) <david@kernel.org>
> Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>
> Signed-off-by: Chris Gellermann <christian.gellermann@codasip.com>
> ---
> tools/testing/selftests/clone3/clone3_set_tid.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/clone3/clone3_set_tid.c b/tools/testing/selftests/clone3/clone3_set_tid.c
> index 5c944aee6b41..485efa7c9eed 100644
> --- a/tools/testing/selftests/clone3/clone3_set_tid.c
> +++ b/tools/testing/selftests/clone3/clone3_set_tid.c
> @@ -141,7 +141,7 @@ int main(int argc, char *argv[])
> {
> FILE *f;
> char buf;
> - char *line;
> + char *line = NULL;
> int status;
> int ret = -1;
> size_t len = 0;
> --
> 2.47.3
>
Sorry about the mess. It's my first time upstreaming something. > Just for future, please send series independent of each other not in reply to > other series, and if there's more than 1 patch, send a cover letter and have all > the patches reply to that! Sure, will do that. Best, Chris
This is another occurrence of using getline where the code assumes that
getline allocates memory to store the line, but the pointer passed to
it is uninitialized and potentially a non-null pointer. This
violates the Open Group Spec[1] and caused a segfault in a similar
situation in selftest/clone3/clone3_set_tid. Fix it by initializing the
line pointer to NULL.
The issue has been found by simply grepping through the selftest code
after running into the issue in clone3_set_tid. Whether it segfaults in
its current state is unknown to me. But it's good to be addressed due to
defensive reasons.
[1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/getline.html
Fixes: 26b4224d9961 ("selftests: expanding more mlock selftest")
Cc: stable@vger.kernel.org
Acked-by: David Hildenbrand (arm) <david@kernel.org>
Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>
Signed-off-by: Chris Gellermann <christian.gellermann@codasip.com>
---
tools/testing/selftests/mm/mlock-random-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/mm/mlock-random-test.c b/tools/testing/selftests/mm/mlock-random-test.c
index 9d349c151360..16294bc7dae6 100644
--- a/tools/testing/selftests/mm/mlock-random-test.c
+++ b/tools/testing/selftests/mm/mlock-random-test.c
@@ -84,7 +84,7 @@ int get_proc_locked_vm_size(void)
int get_proc_page_size(unsigned long addr)
{
FILE *smaps;
- char *line;
+ char *line = NULL;
unsigned long mmupage_size = 0;
size_t size;
--
2.47.3
On Tue, May 26, 2026 at 01:38:48PM +0200, Chris Gellermann wrote:
> Clone3_set_tid uses getline(&line, &len, f) in a loop to read the
> child's process status. The code expects that getline allocates the
> buffer for the line on the first loop iteration. For this, glibc[1]
> requires char *line to be set to NULL:
>
> > ssize_t getline(char **restrict lineptr, ...)
> > If *lineptr is set to NULL before the call, then getline() will
> > allocate a buffer for storing the line.
>
> However, char *line is only declared, leading to an undefined
> initialization value. Fix this by properly initializing it to NULL.
>
> Same issue fixed in mlock-random-test.
>
> [1] https://man7.org/linux/man-pages/man3/getline.3.html
>
> Fixes: 41585bbeeef9 ("selftests: add tests for clone3() with *set_tid")
> Fixes: 26b4224d9961 ("selftests: expanding more mlock selftest")
You'll need separate commits for each I think? That'd at least make life
easier. You can send them as a series.
> Signed-off-by: Chris Gellermann <christian.gellermann@codasip.com>
> ---
> tools/testing/selftests/clone3/clone3_set_tid.c | 2 +-
> tools/testing/selftests/mm/mlock-random-test.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/clone3/clone3_set_tid.c b/tools/testing/selftests/clone3/clone3_set_tid.c
> index 5c944aee6b41..485efa7c9eed 100644
> --- a/tools/testing/selftests/clone3/clone3_set_tid.c
> +++ b/tools/testing/selftests/clone3/clone3_set_tid.c
> @@ -141,7 +141,7 @@ int main(int argc, char *argv[])
> {
> FILE *f;
> char buf;
> - char *line;
> + char *line = NULL;
> int status;
> int ret = -1;
> size_t len = 0;
> diff --git a/tools/testing/selftests/mm/mlock-random-test.c b/tools/testing/selftests/mm/mlock-random-test.c
> index 9d349c151360..16294bc7dae6 100644
> --- a/tools/testing/selftests/mm/mlock-random-test.c
> +++ b/tools/testing/selftests/mm/mlock-random-test.c
> @@ -84,7 +84,7 @@ int get_proc_locked_vm_size(void)
> int get_proc_page_size(unsigned long addr)
> {
> FILE *smaps;
> - char *line;
> + char *line = NULL;
Strange this didn't result in noticeable bugs but maybe perceived as flakes
or such?
> unsigned long mmupage_size = 0;
> size_t size;
>
> --
> 2.47.3
>
Cheers, Lorenzo
On 5/26/26 13:38, Chris Gellermann wrote:
> Clone3_set_tid uses getline(&line, &len, f) in a loop to read the
> child's process status. The code expects that getline allocates the
> buffer for the line on the first loop iteration. For this, glibc[1]
> requires char *line to be set to NULL:
>
>> ssize_t getline(char **restrict lineptr, ...)
>> If *lineptr is set to NULL before the call, then getline() will
>> allocate a buffer for storing the line.
>
> However, char *line is only declared, leading to an undefined
> initialization value. Fix this by properly initializing it to NULL.
>
> Same issue fixed in mlock-random-test.
>
> [1] https://man7.org/linux/man-pages/man3/getline.3.html
>
> Fixes: 41585bbeeef9 ("selftests: add tests for clone3() with *set_tid")
> Fixes: 26b4224d9961 ("selftests: expanding more mlock selftest")
> Signed-off-by: Chris Gellermann <christian.gellermann@codasip.com>
> ---
Acked-by: David Hildenbrand (arm) <david@kernel.org>
--
Cheers,
David
© 2016 - 2026 Red Hat, Inc.