diff mbox series

[v2,5/5] setsockopt0[38]: Use tst_is_compat_mode()

Message ID 20240521105348.126316-6-pvorel@suse.cz
State Accepted
Headers show
Series .needs_abi_bits + rewrite fork14 to new API | expand

Commit Message

Petr Vorel May 21, 2024, 10:53 a.m. UTC
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v2

I wonder if both shouldn't be TCONF or TINFO.

Kind regards,
Petr

 testcases/kernel/syscalls/setsockopt/setsockopt03.c | 5 ++---
 testcases/kernel/syscalls/setsockopt/setsockopt08.c | 6 ++----
 2 files changed, 4 insertions(+), 7 deletions(-)

Comments

Cyril Hrubis May 21, 2024, 3:12 p.m. UTC | #1
Hi!
>  testcases/kernel/syscalls/setsockopt/setsockopt03.c | 5 ++---
>  testcases/kernel/syscalls/setsockopt/setsockopt08.c | 6 ++----
>  2 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt03.c b/testcases/kernel/syscalls/setsockopt/setsockopt03.c
> index 7a1458277..d910280c8 100644
> --- a/testcases/kernel/syscalls/setsockopt/setsockopt03.c
> +++ b/testcases/kernel/syscalls/setsockopt/setsockopt03.c
> @@ -48,9 +48,8 @@ struct payload {
>  
>  static void setup(void)
>  {
> -	if (tst_kernel_bits() == 32 || sizeof(long) > 4)
> -		tst_res(TCONF,
> -			"The vulnerability was only present in 32-bit compat mode");
> +	if (!tst_is_compat_mode())
> +		tst_res(TCONF, "The vulnerability was only present in 32-bit compat mode");
>  }
>  
>  static void run(void)
> diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt08.c b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> index 7f8243de1..3b7bd8482 100644
> --- a/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> +++ b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> @@ -95,10 +95,8 @@ static void *buffer;
>  
>  void setup(void)
>  {
> -	if (tst_kernel_bits() == 32 || sizeof(long) > 4) {
> -		tst_res(TINFO,
> -			"The vulnerability was only present in 32-bit compat mode");
> -	}
> +	if (!tst_is_compat_mode())
> +		tst_res(TINFO, "The vulnerability was only present in 32-bit compat mode");
>  
>  	tst_setup_netns();
>  }

I guess this is something that should be looked into after the release,
either we will need .needs_compat flag or relax the condtions...
Petr Vorel May 21, 2024, 6:34 p.m. UTC | #2
Hi Cyril, all,

> I guess this is something that should be looked into after the release,
> either we will need .needs_compat flag or relax the condtions...

OK, the previous 4 got a review thus merged, this one is left out.

Thanks for your time!

Kind regards,
Petr
Petr Vorel Nov. 26, 2024, 2:35 p.m. UTC | #3
Hi Cyril,

> Hi!
> >  testcases/kernel/syscalls/setsockopt/setsockopt03.c | 5 ++---
> >  testcases/kernel/syscalls/setsockopt/setsockopt08.c | 6 ++----
> >  2 files changed, 4 insertions(+), 7 deletions(-)

> > diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt03.c b/testcases/kernel/syscalls/setsockopt/setsockopt03.c
> > index 7a1458277..d910280c8 100644
> > --- a/testcases/kernel/syscalls/setsockopt/setsockopt03.c
> > +++ b/testcases/kernel/syscalls/setsockopt/setsockopt03.c
> > @@ -48,9 +48,8 @@ struct payload {

> >  static void setup(void)
> >  {
> > -	if (tst_kernel_bits() == 32 || sizeof(long) > 4)
> > -		tst_res(TCONF,
> > -			"The vulnerability was only present in 32-bit compat mode");
> > +	if (!tst_is_compat_mode())
> > +		tst_res(TCONF, "The vulnerability was only present in 32-bit compat mode");
> >  }

> >  static void run(void)
> > diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt08.c b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> > index 7f8243de1..3b7bd8482 100644
> > --- a/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> > +++ b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> > @@ -95,10 +95,8 @@ static void *buffer;

> >  void setup(void)
> >  {
> > -	if (tst_kernel_bits() == 32 || sizeof(long) > 4) {
> > -		tst_res(TINFO,
> > -			"The vulnerability was only present in 32-bit compat mode");
> > -	}
> > +	if (!tst_is_compat_mode())
> > +		tst_res(TINFO, "The vulnerability was only present in 32-bit compat mode");

> >  	tst_setup_netns();
> >  }

> I guess this is something that should be looked into after the release,
> either we will need .needs_compat flag or relax the condtions...

Looking into this old patch, do we relax the conditions or use .needs_abi_bits = 32?
setsockopt08.c prints only TINFO, thus !tst_is_compat_mode() would have to be
used for the check.

Kind regards,
Petr
Cyril Hrubis Nov. 26, 2024, 3:02 p.m. UTC | #4
Hi!
> > I guess this is something that should be looked into after the release,
> > either we will need .needs_compat flag or relax the condtions...
> 
> Looking into this old patch, do we relax the conditions or use .needs_abi_bits = 32?
> setsockopt08.c prints only TINFO, thus !tst_is_compat_mode() would have to be
> used for the check.

I guess that doing:

	if (!tst_is_compat_mode())
		tst_res(TINFO, "The vunerability was only present in 32-bit compat mode");


Is a sensible approach. It does not hurt to run the test either way but
it makes it clear that the original CVE it's not going to be reproduced
without 32-bit compat mode.
Petr Vorel Nov. 26, 2024, 5:47 p.m. UTC | #5
Hi Cyril,

> Hi!
> > > I guess this is something that should be looked into after the release,
> > > either we will need .needs_compat flag or relax the condtions...

> > Looking into this old patch, do we relax the conditions or use .needs_abi_bits = 32?
> > setsockopt08.c prints only TINFO, thus !tst_is_compat_mode() would have to be
> > used for the check.

> I guess that doing:

> 	if (!tst_is_compat_mode())
> 		tst_res(TINFO, "The vunerability was only present in 32-bit compat mode");


> Is a sensible approach. It does not hurt to run the test either way but
> it makes it clear that the original CVE it's not going to be reproduced
> without 32-bit compat mode.

+1. I modified that and dared to merge with your RBT (9dacb1ac27).
Thanks for your review!

Kind regards,
Petr
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt03.c b/testcases/kernel/syscalls/setsockopt/setsockopt03.c
index 7a1458277..d910280c8 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt03.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt03.c
@@ -48,9 +48,8 @@  struct payload {
 
 static void setup(void)
 {
-	if (tst_kernel_bits() == 32 || sizeof(long) > 4)
-		tst_res(TCONF,
-			"The vulnerability was only present in 32-bit compat mode");
+	if (!tst_is_compat_mode())
+		tst_res(TCONF, "The vulnerability was only present in 32-bit compat mode");
 }
 
 static void run(void)
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt08.c b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
index 7f8243de1..3b7bd8482 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt08.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
@@ -95,10 +95,8 @@  static void *buffer;
 
 void setup(void)
 {
-	if (tst_kernel_bits() == 32 || sizeof(long) > 4) {
-		tst_res(TINFO,
-			"The vulnerability was only present in 32-bit compat mode");
-	}
+	if (!tst_is_compat_mode())
+		tst_res(TINFO, "The vulnerability was only present in 32-bit compat mode");
 
 	tst_setup_netns();
 }