1. Patchew
  2. linux
  3. View series

[PATCH] selftests/mm: handle fork() failure in migration tests

Hu Song posted 1 patch 1 month ago 
tools/testing/selftests/mm/migration.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
[PATCH] selftests/mm: handle fork() failure in migration tests
Posted by Hu Song 1 month ago 
From: Song Hu <husong@kylinos.cn>

When fork() fails and returns -1, the code falls into the else branch
and stores -1 into self->pids[i]. Later, kill(-1, SIGTERM) is called
which sends SIGTERM to every process the user has permission to signal,
potentially killing the entire user session.

Add an explicit check for fork() returning -1 (pid < 0). On failure,
clean up any already-forked children before failing the test via
ASSERT_GE(pid, 0). This fix is applied to all three fork() call sites
in shared_anon, shared_anon_thp, and shared_anon_htlb tests.

Signed-off-by: Hu Song <husong@kylinos.cn>
---
 tools/testing/selftests/mm/migration.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tools/testing/selftests/mm/migration.c b/tools/testing/selftests/mm/migration.c
index 60e78bbfc0e3..f433e4f195ad 100644
--- a/tools/testing/selftests/mm/migration.c
+++ b/tools/testing/selftests/mm/migration.c
@@ -163,6 +163,11 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
 	memset(ptr, 0xde, TWOMEG);
 	for (i = 0; i < self->nthreads - 1; i++) {
 		pid = fork();
+		if (pid < 0) {
+			while (--i >= 0)
+				kill(self->pids[i], SIGTERM);
+			ASSERT_GE(pid, 0);
+		}
 		if (!pid) {
 			prctl(PR_SET_PDEATHSIG, SIGHUP);
 			/* Parent may have died before prctl so check now. */
@@ -235,6 +240,11 @@ TEST_F_TIMEOUT(migration, shared_anon_thp, 2*RUNTIME)
 	memset(ptr, 0xde, TWOMEG);
 	for (i = 0; i < self->nthreads - 1; i++) {
 		pid = fork();
+		if (pid < 0) {
+			while (--i >= 0)
+				kill(self->pids[i], SIGTERM);
+			ASSERT_GE(pid, 0);
+		}
 		if (!pid) {
 			prctl(PR_SET_PDEATHSIG, SIGHUP);
 			/* Parent may have died before prctl so check now. */
@@ -295,6 +305,11 @@ TEST_F_TIMEOUT(migration, shared_anon_htlb, 2*RUNTIME)
 	memset(ptr, 0xde, TWOMEG);
 	for (i = 0; i < self->nthreads - 1; i++) {
 		pid = fork();
+		if (pid < 0) {
+			while (--i >= 0)
+				kill(self->pids[i], SIGTERM);
+			ASSERT_GE(pid, 0);
+		}
 		if (!pid) {
 			prctl(PR_SET_PDEATHSIG, SIGHUP);
 			/* Parent may have died before prctl so check now. */
-- 
2.25.1
Re: [PATCH] selftests/mm: handle fork() failure in migration tests
Posted by David Hildenbrand (Arm) 1 month ago 
On 5/11/26 11:34, Hu Song wrote:
> From: Song Hu <husong@kylinos.cn>
> 
> When fork() fails and returns -1, the code falls into the else branch
> and stores -1 into self->pids[i]. Later, kill(-1, SIGTERM) is called
> which sends SIGTERM to every process the user has permission to signal,
> potentially killing the entire user session.
> 
> Add an explicit check for fork() returning -1 (pid < 0). On failure,
> clean up any already-forked children before failing the test via
> ASSERT_GE(pid, 0). This fix is applied to all three fork() call sites
> in shared_anon, shared_anon_thp, and shared_anon_htlb tests.
> 
> Signed-off-by: Hu Song <husong@kylinos.cn>
> ---
>  tools/testing/selftests/mm/migration.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/tools/testing/selftests/mm/migration.c b/tools/testing/selftests/mm/migration.c
> index 60e78bbfc0e3..f433e4f195ad 100644
> --- a/tools/testing/selftests/mm/migration.c
> +++ b/tools/testing/selftests/mm/migration.c
> @@ -163,6 +163,11 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
>  	memset(ptr, 0xde, TWOMEG);
>  	for (i = 0; i < self->nthreads - 1; i++) {
>  		pid = fork();
> +		if (pid < 0) {
> +			while (--i >= 0)
> +				kill(self->pids[i], SIGTERM);



> +			ASSERT_GE(pid, 0);
> +		}
>  		if (!pid) {

I think "else if" reads nicer here.

But why do we care about cleaning up the other processes? IIUC, we don't do that
explicitly if e.g.,

	ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);

fails?


-- 
Cheers,

David
Re: [PATCH] selftests/mm: handle fork() failure in migration tests
Posted by Mike Rapoport 1 month ago 
On Mon, May 11, 2026 at 02:47:38PM +0200, David Hildenbrand (Arm) wrote:
> On 5/11/26 11:34, Hu Song wrote:
> > From: Song Hu <husong@kylinos.cn>
> > 
> > When fork() fails and returns -1, the code falls into the else branch
> > and stores -1 into self->pids[i]. Later, kill(-1, SIGTERM) is called
> > which sends SIGTERM to every process the user has permission to signal,
> > potentially killing the entire user session.
> > 
> > Add an explicit check for fork() returning -1 (pid < 0). On failure,
> > clean up any already-forked children before failing the test via
> > ASSERT_GE(pid, 0). This fix is applied to all three fork() call sites
> > in shared_anon, shared_anon_thp, and shared_anon_htlb tests.
> > 
> > Signed-off-by: Hu Song <husong@kylinos.cn>
> > ---
> >  tools/testing/selftests/mm/migration.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/mm/migration.c b/tools/testing/selftests/mm/migration.c
> > index 60e78bbfc0e3..f433e4f195ad 100644
> > --- a/tools/testing/selftests/mm/migration.c
> > +++ b/tools/testing/selftests/mm/migration.c
> > @@ -163,6 +163,11 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
> >  	memset(ptr, 0xde, TWOMEG);
> >  	for (i = 0; i < self->nthreads - 1; i++) {
> >  		pid = fork();
> > +		if (pid < 0) {
> > +			while (--i >= 0)
> > +				kill(self->pids[i], SIGTERM);
> 
> 
> 
> > +			ASSERT_GE(pid, 0);
> > +		}
> >  		if (!pid) {
> 
> I think "else if" reads nicer here.
> 
> But why do we care about cleaning up the other processes? IIUC, we don't do that
> explicitly if e.g.,
> 
> 	ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
> 
> fails?

We care about cleaning up all the forked processes because otherwise they
remain zombies :)

But killing some of them if a fork() failed does not help.

I have a more structured fix here:
https://lore.kernel.org/all/20260511162840.375890-5-rppt@kernel.org
 
> 
> -- 
> Cheers,
> 
> David

-- 
Sincerely yours,
Mike.
Re: [PATCH] selftests/mm: handle fork() failure in migration tests
Posted by Donet Tom 1 month ago 
Hi

On 5/11/26 3:04 PM, Hu Song wrote:
> From: Song Hu <husong@kylinos.cn>
>
> When fork() fails and returns -1, the code falls into the else branch
> and stores -1 into self->pids[i]. Later, kill(-1, SIGTERM) is called
> which sends SIGTERM to every process the user has permission to signal,
> potentially killing the entire user session.
>
> Add an explicit check for fork() returning -1 (pid < 0). On failure,
> clean up any already-forked children before failing the test via
> ASSERT_GE(pid, 0). This fix is applied to all three fork() call sites
> in shared_anon, shared_anon_thp, and shared_anon_htlb tests.
>
> Signed-off-by: Hu Song <husong@kylinos.cn>

I am seeing the below issue in checkpatch.pl.

WARNING: From:/Signed-off-by: email name mismatch: 'From: Song Hu 
<husong@kylinos.cn>' != 'Signed-off-by: Hu Song <husong@kylinos.cn>'

-Donet

> ---
>   tools/testing/selftests/mm/migration.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/tools/testing/selftests/mm/migration.c b/tools/testing/selftests/mm/migration.c
> index 60e78bbfc0e3..f433e4f195ad 100644
> --- a/tools/testing/selftests/mm/migration.c
> +++ b/tools/testing/selftests/mm/migration.c
> @@ -163,6 +163,11 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
>   	memset(ptr, 0xde, TWOMEG);
>   	for (i = 0; i < self->nthreads - 1; i++) {
>   		pid = fork();
> +		if (pid < 0) {
> +			while (--i >= 0)
> +				kill(self->pids[i], SIGTERM);
> +			ASSERT_GE(pid, 0);
> +		}
>   		if (!pid) {
>   			prctl(PR_SET_PDEATHSIG, SIGHUP);
>   			/* Parent may have died before prctl so check now. */
> @@ -235,6 +240,11 @@ TEST_F_TIMEOUT(migration, shared_anon_thp, 2*RUNTIME)
>   	memset(ptr, 0xde, TWOMEG);
>   	for (i = 0; i < self->nthreads - 1; i++) {
>   		pid = fork();
> +		if (pid < 0) {
> +			while (--i >= 0)
> +				kill(self->pids[i], SIGTERM);
> +			ASSERT_GE(pid, 0);
> +		}
>   		if (!pid) {
>   			prctl(PR_SET_PDEATHSIG, SIGHUP);
>   			/* Parent may have died before prctl so check now. */
> @@ -295,6 +305,11 @@ TEST_F_TIMEOUT(migration, shared_anon_htlb, 2*RUNTIME)
>   	memset(ptr, 0xde, TWOMEG);
>   	for (i = 0; i < self->nthreads - 1; i++) {
>   		pid = fork();
> +		if (pid < 0) {
> +			while (--i >= 0)
> +				kill(self->pids[i], SIGTERM);
> +			ASSERT_GE(pid, 0);
> +		}
>   		if (!pid) {
>   			prctl(PR_SET_PDEATHSIG, SIGHUP);
>   			/* Parent may have died before prctl so check now. */

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.