[LTP] [PATCH 1/1] clone302: Fix short size test

[LTP] [PATCH 1/1] clone302: Fix short size test

[Petr Vorel]

Test uses sizeof(struct clone_args)-1 as invalid size. Original struct
size 64 was suitable (because > 64 results in EINVAL), but unrelated
change in 45f6916ba increased the size to 88, thus test failed to due
unexpected clone3() pass.

Depending on struct clone_args size is not good idea, let's use
arbitrary value 1.

Fixes: 1b0e8dd71 ("syscalls/clone3: New tests")
Suggested-by: Martin Doucha <mdoucha@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Value is really arbitrary, any hint for better value.
Or should we delete the test?

NOTE: tested only on x86_64.

Kind regards,
Petr

 testcases/kernel/syscalls/clone3/clone302.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/clone3/clone302.c b/testcases/kernel/syscalls/clone3/clone302.c
index b1b4ccebb..98848e4bc 100644
--- a/testcases/kernel/syscalls/clone3/clone302.c
+++ b/testcases/kernel/syscalls/clone3/clone302.c
@@ -34,7 +34,7 @@ static struct tcase {
 } tcases[] = {
 	{"invalid args", &invalid_args, sizeof(*valid_args), 0, NULL, SIGCHLD, 0, 0, 0, EFAULT},
 	{"zero size", &valid_args, 0, 0, NULL, SIGCHLD, 0, 0, 0, EINVAL},
-	{"short size", &valid_args, sizeof(*valid_args) - 1, 0, NULL, SIGCHLD, 0, 0, 0, EINVAL},
+	{"short size", &valid_args, 1, 0, NULL, SIGCHLD, 0, 0, 0, EINVAL},
 	{"extra size", &valid_args, sizeof(*valid_args) + 1, 0, NULL, SIGCHLD, 0, 0, 0, EFAULT},
 	{"sighand-no-VM", &valid_args, sizeof(*valid_args), CLONE_SIGHAND, NULL, SIGCHLD, 0, 0, 0, EINVAL},
 	{"thread-no-sighand", &valid_args, sizeof(*valid_args), CLONE_THREAD, NULL, SIGCHLD, 0, 0, 0, EINVAL},
-- 
2.40.1


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

Re: [LTP] [PATCH 1/1] clone302: Fix short size test

[Martin Doucha]

Hi,

On 01. 09. 23 16:44, Petr Vorel wrote:
> Test uses sizeof(struct clone_args)-1 as invalid size. Original struct
> size 64 was suitable (because > 64 results in EINVAL), but unrelated

Nit: <64

> change in 45f6916ba increased the size to 88, thus test failed to due
> unexpected clone3() pass.
> 
> Depending on struct clone_args size is not good idea, let's use
> arbitrary value 1.

This will work, but we could also define a "minimal" struct clone_args 
(without any fields other than the required ones) and use the size of 
that (-1 of course). That would give better coverage and leave no 
untested gap between 1 and minimal struct size.

> 
> Fixes: 1b0e8dd71 ("syscalls/clone3: New tests")
> Suggested-by: Martin Doucha <mdoucha@suse.cz>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Value is really arbitrary, any hint for better value.
> Or should we delete the test?
> 
> NOTE: tested only on x86_64.
> 
> Kind regards,
> Petr
> 
>   testcases/kernel/syscalls/clone3/clone302.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/syscalls/clone3/clone302.c b/testcases/kernel/syscalls/clone3/clone302.c
> index b1b4ccebb..98848e4bc 100644
> --- a/testcases/kernel/syscalls/clone3/clone302.c
> +++ b/testcases/kernel/syscalls/clone3/clone302.c
> @@ -34,7 +34,7 @@ static struct tcase {
>   } tcases[] = {
>   	{"invalid args", &invalid_args, sizeof(*valid_args), 0, NULL, SIGCHLD, 0, 0, 0, EFAULT},
>   	{"zero size", &valid_args, 0, 0, NULL, SIGCHLD, 0, 0, 0, EINVAL},
> -	{"short size", &valid_args, sizeof(*valid_args) - 1, 0, NULL, SIGCHLD, 0, 0, 0, EINVAL},
> +	{"short size", &valid_args, 1, 0, NULL, SIGCHLD, 0, 0, 0, EINVAL},
>   	{"extra size", &valid_args, sizeof(*valid_args) + 1, 0, NULL, SIGCHLD, 0, 0, 0, EFAULT},
>   	{"sighand-no-VM", &valid_args, sizeof(*valid_args), CLONE_SIGHAND, NULL, SIGCHLD, 0, 0, 0, EINVAL},
>   	{"thread-no-sighand", &valid_args, sizeof(*valid_args), CLONE_THREAD, NULL, SIGCHLD, 0, 0, 0, EINVAL},

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


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

Re: [LTP] [PATCH 1/1] clone302: Fix short size test

[Petr Vorel]

Hi Martin,

> Hi,

> On 01. 09. 23 16:44, Petr Vorel wrote:
> > Test uses sizeof(struct clone_args)-1 as invalid size. Original struct
> > size 64 was suitable (because > 64 results in EINVAL), but unrelated

> Nit: <64

Thanks! (I meant that, but fingers wrote the opposite comparator.)

> > change in 45f6916ba increased the size to 88, thus test failed to due
> > unexpected clone3() pass.

> > Depending on struct clone_args size is not good idea, let's use
> > arbitrary value 1.

> This will work, but we could also define a "minimal" struct clone_args
> (without any fields other than the required ones) and use the size of that
> (-1 of course). That would give better coverage and leave no untested gap
> between 1 and minimal struct size.

Very good idea. I'm setting this as changes requested.

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/1] clone302: Fix short size test

[Wei Gao via ltp]

On Fri, Sep 01, 2023 at 07:00:38PM +0200, Petr Vorel wrote:
> Hi Martin,
> 
> > Hi,
> 
> > On 01. 09. 23 16:44, Petr Vorel wrote:
> > > Test uses sizeof(struct clone_args)-1 as invalid size. Original struct
> > > size 64 was suitable (because > 64 results in EINVAL), but unrelated
> 
> > Nit: <64
> 
> Thanks! (I meant that, but fingers wrote the opposite comparator.)
> 
> > > change in 45f6916ba increased the size to 88, thus test failed to due
> > > unexpected clone3() pass.
> 
> > > Depending on struct clone_args size is not good idea, let's use
> > > arbitrary value 1.
> 
> > This will work, but we could also define a "minimal" struct clone_args
> > (without any fields other than the required ones) and use the size of that
> > (-1 of course). That would give better coverage and leave no untested gap
> > between 1 and minimal struct size.
> 
> Very good idea. I'm setting this as changes requested.

I have made another patch for fix this, hope this can work.
https://patchwork.ozlabs.org/project/ltp/patch/20230902055638.14256-1-wegao@suse.com/

> 
> Kind regards,
> Petr
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

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