[LTP] [PATCH 1/1] recvmmsg01: Workaround segfault on libc variant on 32 bit

[LTP] [PATCH 1/1] recvmmsg01: Workaround segfault on libc variant on 32 bit

[Petr Vorel]

EFAULT test segfaults on newer kernels (e.g. 6.4) on libc variant on
32bit.  Use typical LTP workaround to test by forked child + checking
the terminating signal.

NOTE: testing kernel variants could be done without forking child, but
use it as well for simplicity.

Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
NOTE: working on other 3 tests affected by the same problem
(mq_timedreceive01.c, mq_timedsend01.c,  sigtimedwait01.c).

 .../kernel/syscalls/recvmmsg/recvmmsg01.c     | 48 +++++++++++++++----
 1 file changed, 40 insertions(+), 8 deletions(-)

diff --git a/testcases/kernel/syscalls/recvmmsg/recvmmsg01.c b/testcases/kernel/syscalls/recvmmsg/recvmmsg01.c
index fb21ea1e70..46658b133d 100644
--- a/testcases/kernel/syscalls/recvmmsg/recvmmsg01.c
+++ b/testcases/kernel/syscalls/recvmmsg/recvmmsg01.c
@@ -47,7 +47,7 @@ static struct test_case tcase[] = {
 		.desc = "bad message vector address",
 		.fd = &receive_sockfd,
 		.exp_errno = EFAULT,
-		.msg_vec = (void*)&bad_addr,
+		.msg_vec = (void *)&bad_addr,
 	},
 	{
 		.desc = "negative seconds in timeout",
@@ -74,23 +74,54 @@ static struct test_case tcase[] = {
 	}
 };
 
-static void do_test(unsigned int i)
+static void verify_recvmmsg(unsigned int i, void *timeout)
 {
 	struct time64_variants *tv = &variants[tst_variant];
 	struct test_case *tc = &tcase[i];
-	void *timeout;
 
 	ts.type = tv->ts_type;
 	tst_ts_set_sec(&ts, tc->tv_sec);
 	tst_ts_set_nsec(&ts, tc->tv_nsec);
 
+	TST_EXP_FAIL2(tv->recvmmsg(*tc->fd, *tc->msg_vec, VLEN, 0, timeout),
+		      tc->exp_errno, "recvmmsg() %s", tc->desc);
+}
+
+static void test_bad_addr(unsigned int i)
+{
+	struct time64_variants *tv = &variants[tst_variant];
+	void *timeout = bad_addr;
+	pid_t pid;
+	int status;
+
+	pid = SAFE_FORK();
+	if (!pid) {
+		verify_recvmmsg(i, timeout);
+		_exit(!TST_PASS);
+	}
+
+	SAFE_WAITPID(pid, &status, 0);
+
+	if (WIFEXITED(status))
+		return;
+
+	if (tv->ts_type == TST_LIBC_TIMESPEC &&
+		WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV) {
+		tst_res(TPASS, "Child killed by signal");
+		return;
+	}
+
+	tst_res(TFAIL, "Child %s", tst_strstatus(status));
+}
+
+static void do_test(unsigned int i)
+{
+	struct test_case *tc = &tcase[i];
+
 	if (tc->bad_ts_addr)
-		timeout = bad_addr;
+		test_bad_addr(i);
 	else
-		timeout = tst_ts_get(&ts);
-
-	TST_EXP_FAIL2(tv->recvmmsg(*tc->fd, *tc->msg_vec, VLEN, 0, timeout),
-	             tc->exp_errno, "recvmmsg() %s", tc->desc);
+		verify_recvmmsg(i, tst_ts_get(&ts));
 }
 
 static void setup(void)
@@ -139,6 +170,7 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_variants = ARRAY_SIZE(variants),
+	.forks_child = 1,
 	.bufs = (struct tst_buffers []) {
 		{&iov, .iov_sizes = (int[]){1, -1}},
 		{&msg, .size = VLEN * sizeof(*msg)},
-- 
2.45.2


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] recvmmsg01: Workaround segfault on libc variant on 32 bit

[Wei Gao via ltp]

On Fri, Nov 08, 2024 at 01:21:39PM +0100, Petr Vorel wrote:
> EFAULT test segfaults on newer kernels (e.g. 6.4) on libc variant on
> 32bit.  Use typical LTP workaround to test by forked child + checking
> the terminating signal.
> 
> NOTE: testing kernel variants could be done without forking child, but
> use it as well for simplicity.
> 
> Suggested-by: Cyril Hrubis <chrubis@suse.cz>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> NOTE: working on other 3 tests affected by the same problem
> (mq_timedreceive01.c, mq_timedsend01.c,  sigtimedwait01.c).
> 
>  .../kernel/syscalls/recvmmsg/recvmmsg01.c     | 48 +++++++++++++++----
>  1 file changed, 40 insertions(+), 8 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/recvmmsg/recvmmsg01.c b/testcases/kernel/syscalls/recvmmsg/recvmmsg01.c
> index fb21ea1e70..46658b133d 100644
> --- a/testcases/kernel/syscalls/recvmmsg/recvmmsg01.c
> +++ b/testcases/kernel/syscalls/recvmmsg/recvmmsg01.c
> @@ -47,7 +47,7 @@ static struct test_case tcase[] = {
>  		.desc = "bad message vector address",
>  		.fd = &receive_sockfd,
>  		.exp_errno = EFAULT,
> -		.msg_vec = (void*)&bad_addr,
> +		.msg_vec = (void *)&bad_addr,
>  	},
>  	{
>  		.desc = "negative seconds in timeout",
> @@ -74,23 +74,54 @@ static struct test_case tcase[] = {
>  	}
>  };
>  
> -static void do_test(unsigned int i)
> +static void verify_recvmmsg(unsigned int i, void *timeout)
>  {
>  	struct time64_variants *tv = &variants[tst_variant];
>  	struct test_case *tc = &tcase[i];
> -	void *timeout;
>  
>  	ts.type = tv->ts_type;
>  	tst_ts_set_sec(&ts, tc->tv_sec);
>  	tst_ts_set_nsec(&ts, tc->tv_nsec);
>  
> +	TST_EXP_FAIL2(tv->recvmmsg(*tc->fd, *tc->msg_vec, VLEN, 0, timeout),
> +		      tc->exp_errno, "recvmmsg() %s", tc->desc);
> +}
> +
> +static void test_bad_addr(unsigned int i)
> +{
> +	struct time64_variants *tv = &variants[tst_variant];
> +	void *timeout = bad_addr;
> +	pid_t pid;
> +	int status;
> +
> +	pid = SAFE_FORK();
> +	if (!pid) {
> +		verify_recvmmsg(i, timeout);
> +		_exit(!TST_PASS);
> +	}
> +
> +	SAFE_WAITPID(pid, &status, 0);
> +
> +	if (WIFEXITED(status))
> +		return;
> +
> +	if (tv->ts_type == TST_LIBC_TIMESPEC &&
> +		WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV) {
> +		tst_res(TPASS, "Child killed by signal");
very nit: 
1) s/Child killed by signal/Child killed by SIGSEGV signal/
2) suggest also print out tc->desc

Thanks for create patch fix our current issue.
Reviewed-by: Wei Gao <wegao@suse.com>
> +		return;
> +	}
> +
> +	tst_res(TFAIL, "Child %s", tst_strstatus(status));
> +}
> +
> +static void do_test(unsigned int i)
> +{
> +	struct test_case *tc = &tcase[i];
> +
>  	if (tc->bad_ts_addr)
> -		timeout = bad_addr;
> +		test_bad_addr(i);
>  	else
> -		timeout = tst_ts_get(&ts);
> -
> -	TST_EXP_FAIL2(tv->recvmmsg(*tc->fd, *tc->msg_vec, VLEN, 0, timeout),
> -	             tc->exp_errno, "recvmmsg() %s", tc->desc);
> +		verify_recvmmsg(i, tst_ts_get(&ts));
>  }
>  
>  static void setup(void)
> @@ -139,6 +170,7 @@ static struct tst_test test = {
>  	.setup = setup,
>  	.cleanup = cleanup,
>  	.test_variants = ARRAY_SIZE(variants),
> +	.forks_child = 1,
>  	.bufs = (struct tst_buffers []) {
>  		{&iov, .iov_sizes = (int[]){1, -1}},
>  		{&msg, .size = VLEN * sizeof(*msg)},
> -- 
> 2.45.2
> 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] recvmmsg01: Workaround segfault on libc variant on 32 bit

[Petr Vorel]

Hi Wei, all,

...
> > -static void do_test(unsigned int i)
> > +static void verify_recvmmsg(unsigned int i, void *timeout)
> >  {
> >  	struct time64_variants *tv = &variants[tst_variant];
> >  	struct test_case *tc = &tcase[i];
> > -	void *timeout;

> >  	ts.type = tv->ts_type;
> >  	tst_ts_set_sec(&ts, tc->tv_sec);
> >  	tst_ts_set_nsec(&ts, tc->tv_nsec);

> > +	TST_EXP_FAIL2(tv->recvmmsg(*tc->fd, *tc->msg_vec, VLEN, 0, timeout),
> > +		      tc->exp_errno, "recvmmsg() %s", tc->desc);
> > +}
> > +
> > +static void test_bad_addr(unsigned int i)
> > +{
> > +	struct time64_variants *tv = &variants[tst_variant];
> > +	void *timeout = bad_addr;
> > +	pid_t pid;
> > +	int status;
> > +
> > +	pid = SAFE_FORK();
> > +	if (!pid) {
> > +		verify_recvmmsg(i, timeout);
> > +		_exit(!TST_PASS);
> > +	}
> > +
> > +	SAFE_WAITPID(pid, &status, 0);
> > +
> > +	if (WIFEXITED(status))
> > +		return;
> > +
> > +	if (tv->ts_type == TST_LIBC_TIMESPEC &&
> > +		WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV) {
> > +		tst_res(TPASS, "Child killed by signal");
> very nit: 
> 1) s/Child killed by signal/Child killed by SIGSEGV signal/
I copy pasted it. Maybe better would be to print "Child killed by expected
signal" because it's IMHO irrelevant which signal it is, right?
Signal name is important only when there is other signal on TFAIL.

> 2) suggest also print out tc->desc
Here in test_bad_addr()? This is only used for a single testcase,
tc->desc is printed for the main testing with TST_EXP_FAIL2(),
therefore I don't think it will help debugging.

> Thanks for create patch fix our current issue.

Thanks for review. I'll try to get to the other testcases this week.

> Reviewed-by: Wei Gao <wegao@suse.com>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] recvmmsg01: Workaround segfault on libc variant on 32 bit

[Cyril Hrubis]

Hi!
> -static void do_test(unsigned int i)
> +static void verify_recvmmsg(unsigned int i, void *timeout)
>  {
>  	struct time64_variants *tv = &variants[tst_variant];
>  	struct test_case *tc = &tcase[i];
> -	void *timeout;
>  
>  	ts.type = tv->ts_type;
>  	tst_ts_set_sec(&ts, tc->tv_sec);
>  	tst_ts_set_nsec(&ts, tc->tv_nsec);
>  
> +	TST_EXP_FAIL2(tv->recvmmsg(*tc->fd, *tc->msg_vec, VLEN, 0, timeout),
> +		      tc->exp_errno, "recvmmsg() %s", tc->desc);
> +}
> +
> +static void test_bad_addr(unsigned int i)
> +{
> +	struct time64_variants *tv = &variants[tst_variant];
> +	void *timeout = bad_addr;
> +	pid_t pid;
> +	int status;
> +
> +	pid = SAFE_FORK();
> +	if (!pid) {
> +		verify_recvmmsg(i, timeout);
> +		_exit(!TST_PASS);

This should be just exit(0). The child should either crash or exit with
0 any other result should be considered a bug.

> +	}
> +
> +	SAFE_WAITPID(pid, &status, 0);
> +
> +	if (WIFEXITED(status))
                             ^
			     && !WEXITSTATUS(status))
> +		return;
> +
> +	if (tv->ts_type == TST_LIBC_TIMESPEC &&
> +		WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV) {
> +		tst_res(TPASS, "Child killed by signal");
> +		return;
> +	}
> +
> +	tst_res(TFAIL, "Child %s", tst_strstatus(status));
> +}

The rest looks good to me, with the minor fixed applied:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] recvmmsg01: Workaround segfault on libc variant on 32 bit

[Petr Vorel]

Hi all,

...
> > +static void test_bad_addr(unsigned int i)
> > +{
> > +	struct time64_variants *tv = &variants[tst_variant];
> > +	void *timeout = bad_addr;
> > +	pid_t pid;
> > +	int status;
> > +
> > +	pid = SAFE_FORK();
> > +	if (!pid) {
> > +		verify_recvmmsg(i, timeout);
> > +		_exit(!TST_PASS);

> This should be just exit(0). The child should either crash or exit with
> 0 any other result should be considered a bug.

+1

> > +	}
> > +
> > +	SAFE_WAITPID(pid, &status, 0);
> > +
> > +	if (WIFEXITED(status))
>                              ^
> 			     && !WEXITSTATUS(status))

Ah, good point. I'll send a patch adding this to other tests, where it is
missing.

Merged with these fixes + wording from Wai.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] recvmmsg01: Workaround segfault on libc variant on 32 bit

[Petr Vorel]

...
> > +static void test_bad_addr(unsigned int i)
> > +{
> > +	struct time64_variants *tv = &variants[tst_variant];
> > +	void *timeout = bad_addr;
> > +	pid_t pid;
> > +	int status;
> > +
> > +	pid = SAFE_FORK();
> > +	if (!pid) {
> > +		verify_recvmmsg(i, timeout);
> > +		_exit(!TST_PASS);

> This should be just exit(0). The child should either crash or exit with
> 0 any other result should be considered a bug.

Hm, I changed before merge it to _exit(0), instead of exit(0). As Jan explained
[1] _exit() is just for signal handlers and exit() will also flush open streams.
I guess I should fix it.

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/CAASaF6yDG9VfWOzaSbzEMSGq5LXrqkNfLF7UmMcdWNvqmYywtA@mail.gmail.com/

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp