[LTP] [PATCH v3] setsockopt08: Handle ENOPROTOOPT even with compatible config

[LTP] [PATCH v3] setsockopt08: Handle ENOPROTOOPT even with compatible config

[Richard Palethorpe]

One or more necessary modules can be missing even if they are present
in the config.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
Suggested-by: Martin Doucha <mdoucha@suse.cz>
---

V3:
* Use code suggested by Martin. Functionally it is the same as V2.

I'm not sure which is easier to read, but usually more indentation
makes things worse. So this is better in that regard.

 .../kernel/syscalls/setsockopt/setsockopt08.c      | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt08.c b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
index 33892f9b1..20abe85b4 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt08.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
@@ -101,6 +101,8 @@ void setup(void)
 
 void run(void)
 {
+	const char *const res_fmt_str =
+		"setsockopt(%d, IPPROTO_IP, IPT_SO_SET_REPLACE, %p, 1)";
 	struct ipt_replace *ipt_replace = buffer;
 	struct ipt_entry *ipt_entry = &ipt_replace->entries[0];
 	struct xt_entry_match *xt_entry_match =
@@ -110,6 +112,7 @@ void run(void)
 	struct xt_entry_target *xt_entry_tgt =
 		((struct xt_entry_target *) (&ipt_entry->elems[0] + match_size));
 	int fd = SAFE_SOCKET(AF_INET, SOCK_DGRAM, 0);
+	int result;
 
 	xt_entry_match->u.user.match_size = (u_int16_t)match_size;
 	strcpy(xt_entry_match->u.user.name, "state");
@@ -126,10 +129,13 @@ void run(void)
 	ipt_replace->num_counters = 1;
 	ipt_replace->size = ipt_entry->next_offset;
 
-	TST_EXP_FAIL(setsockopt(fd, IPPROTO_IP, IPT_SO_SET_REPLACE, buffer, 1),
-		     EINVAL,
-		     "setsockopt(%d, IPPROTO_IP, IPT_SO_SET_REPLACE, %p, 1)",
-		     fd, buffer);
+	TEST(setsockopt(fd, IPPROTO_IP, IPT_SO_SET_REPLACE, buffer, 1));
+
+	if (TST_RET == -1 && TST_ERR == ENOPROTOOPT)
+		tst_brk(TCONF | TTERRNO, res_fmt_str, fd, buffer);
+
+	result = (TST_RET == -1 && TST_ERR == EINVAL) ? TPASS : TFAIL;
+	tst_res(result | TTERRNO, res_fmt_str, fd, buffer);
 
 	SAFE_CLOSE(fd);
 }
-- 
2.31.1

[LTP] [PATCH v3] setsockopt08: Handle ENOPROTOOPT even with compatible config

[Martin Doucha]

Hi,
small nit below but otherwise it looks good.

Reviewed-by: Martin Doucha <mdoucha@suse.cz>

On 12. 08. 21 8:47, Richard Palethorpe wrote:
> One or more necessary modules can be missing even if they are present
> in the config.
> 
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> Suggested-by: Martin Doucha <mdoucha@suse.cz>
> ---
> 
> V3:
> * Use code suggested by Martin. Functionally it is the same as V2.
> 
> I'm not sure which is easier to read, but usually more indentation
> makes things worse. So this is better in that regard.
> 
>  .../kernel/syscalls/setsockopt/setsockopt08.c      | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt08.c b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> index 33892f9b1..20abe85b4 100644
> --- a/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> +++ b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> @@ -101,6 +101,8 @@ void setup(void)
>  
>  void run(void)
>  {
> +	const char *const res_fmt_str =
> +		"setsockopt(%d, IPPROTO_IP, IPT_SO_SET_REPLACE, %p, 1)";
>  	struct ipt_replace *ipt_replace = buffer;
>  	struct ipt_entry *ipt_entry = &ipt_replace->entries[0];
>  	struct xt_entry_match *xt_entry_match =
> @@ -110,6 +112,7 @@ void run(void)
>  	struct xt_entry_target *xt_entry_tgt =
>  		((struct xt_entry_target *) (&ipt_entry->elems[0] + match_size));
>  	int fd = SAFE_SOCKET(AF_INET, SOCK_DGRAM, 0);
> +	int result;

Unused variable ^

>  
>  	xt_entry_match->u.user.match_size = (u_int16_t)match_size;
>  	strcpy(xt_entry_match->u.user.name, "state");
> @@ -126,10 +129,13 @@ void run(void)
>  	ipt_replace->num_counters = 1;
>  	ipt_replace->size = ipt_entry->next_offset;
>  
> -	TST_EXP_FAIL(setsockopt(fd, IPPROTO_IP, IPT_SO_SET_REPLACE, buffer, 1),
> -		     EINVAL,
> -		     "setsockopt(%d, IPPROTO_IP, IPT_SO_SET_REPLACE, %p, 1)",
> -		     fd, buffer);
> +	TEST(setsockopt(fd, IPPROTO_IP, IPT_SO_SET_REPLACE, buffer, 1));
> +
> +	if (TST_RET == -1 && TST_ERR == ENOPROTOOPT)
> +		tst_brk(TCONF | TTERRNO, res_fmt_str, fd, buffer);
> +
> +	result = (TST_RET == -1 && TST_ERR == EINVAL) ? TPASS : TFAIL;
> +	tst_res(result | TTERRNO, res_fmt_str, fd, buffer);
>  
>  	SAFE_CLOSE(fd);
>  }
> 


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

[LTP] [PATCH v3] setsockopt08: Handle ENOPROTOOPT even with compatible config

[Richard Palethorpe]

Hello,

Martin Doucha <mdoucha@suse.cz> writes:

> Hi,
> small nit below but otherwise it looks good.
>
> Reviewed-by: Martin Doucha <mdoucha@suse.cz>

Cheers.

>
> On 12. 08. 21 8:47, Richard Palethorpe wrote:
>> One or more necessary modules can be missing even if they are present
>> in the config.
>> 
>> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
>> Suggested-by: Martin Doucha <mdoucha@suse.cz>
>> ---
>> 
>> V3:
>> * Use code suggested by Martin. Functionally it is the same as V2.
>> 
>> I'm not sure which is easier to read, but usually more indentation
>> makes things worse. So this is better in that regard.
>> 
>>  .../kernel/syscalls/setsockopt/setsockopt08.c      | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>> 
>> diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt08.c b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
>> index 33892f9b1..20abe85b4 100644
>> --- a/testcases/kernel/syscalls/setsockopt/setsockopt08.c
>> +++ b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
>> @@ -101,6 +101,8 @@ void setup(void)
>>  
>>  void run(void)
>>  {
>> +	const char *const res_fmt_str =
>> +		"setsockopt(%d, IPPROTO_IP, IPT_SO_SET_REPLACE, %p, 1)";
>>  	struct ipt_replace *ipt_replace = buffer;
>>  	struct ipt_entry *ipt_entry = &ipt_replace->entries[0];
>>  	struct xt_entry_match *xt_entry_match =
>> @@ -110,6 +112,7 @@ void run(void)
>>  	struct xt_entry_target *xt_entry_tgt =
>>  		((struct xt_entry_target *) (&ipt_entry->elems[0] + match_size));
>>  	int fd = SAFE_SOCKET(AF_INET, SOCK_DGRAM, 0);
>> +	int result;
>
> Unused variable ^

It is used

>
>>  
>>  	xt_entry_match->u.user.match_size = (u_int16_t)match_size;
>>  	strcpy(xt_entry_match->u.user.name, "state");
>> @@ -126,10 +129,13 @@ void run(void)
>>  	ipt_replace->num_counters = 1;
>>  	ipt_replace->size = ipt_entry->next_offset;
>>  
>> -	TST_EXP_FAIL(setsockopt(fd, IPPROTO_IP, IPT_SO_SET_REPLACE, buffer, 1),
>> -		     EINVAL,
>> -		     "setsockopt(%d, IPPROTO_IP, IPT_SO_SET_REPLACE, %p, 1)",
>> -		     fd, buffer);
>> +	TEST(setsockopt(fd, IPPROTO_IP, IPT_SO_SET_REPLACE, buffer, 1));
>> +
>> +	if (TST_RET == -1 && TST_ERR == ENOPROTOOPT)
>> +		tst_brk(TCONF | TTERRNO, res_fmt_str, fd, buffer);
>> +
>> +	result = (TST_RET == -1 && TST_ERR == EINVAL) ? TPASS : TFAIL;
        ^^^^^^
>> +	tst_res(result | TTERRNO, res_fmt_str, fd, buffer);
>>  
>>  	SAFE_CLOSE(fd);
>>  }
>> 


-- 
Thank you,
Richard.

[LTP] [PATCH v4 1/2] setsockopt08: Handle ENOPROTOOPT even with compatible config

[Richard Palethorpe]

One or more necessary modules can be missing even if they are present
in the config.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
Suggested-by: Martin Doucha <mdoucha@suse.cz>
---

V4: Add TINFO patch

 .../kernel/syscalls/setsockopt/setsockopt08.c      | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt08.c b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
index 33892f9b1..20abe85b4 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt08.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
@@ -101,6 +101,8 @@ void setup(void)
 
 void run(void)
 {
+	const char *const res_fmt_str =
+		"setsockopt(%d, IPPROTO_IP, IPT_SO_SET_REPLACE, %p, 1)";
 	struct ipt_replace *ipt_replace = buffer;
 	struct ipt_entry *ipt_entry = &ipt_replace->entries[0];
 	struct xt_entry_match *xt_entry_match =
@@ -110,6 +112,7 @@ void run(void)
 	struct xt_entry_target *xt_entry_tgt =
 		((struct xt_entry_target *) (&ipt_entry->elems[0] + match_size));
 	int fd = SAFE_SOCKET(AF_INET, SOCK_DGRAM, 0);
+	int result;
 
 	xt_entry_match->u.user.match_size = (u_int16_t)match_size;
 	strcpy(xt_entry_match->u.user.name, "state");
@@ -126,10 +129,13 @@ void run(void)
 	ipt_replace->num_counters = 1;
 	ipt_replace->size = ipt_entry->next_offset;
 
-	TST_EXP_FAIL(setsockopt(fd, IPPROTO_IP, IPT_SO_SET_REPLACE, buffer, 1),
-		     EINVAL,
-		     "setsockopt(%d, IPPROTO_IP, IPT_SO_SET_REPLACE, %p, 1)",
-		     fd, buffer);
+	TEST(setsockopt(fd, IPPROTO_IP, IPT_SO_SET_REPLACE, buffer, 1));
+
+	if (TST_RET == -1 && TST_ERR == ENOPROTOOPT)
+		tst_brk(TCONF | TTERRNO, res_fmt_str, fd, buffer);
+
+	result = (TST_RET == -1 && TST_ERR == EINVAL) ? TPASS : TFAIL;
+	tst_res(result | TTERRNO, res_fmt_str, fd, buffer);
 
 	SAFE_CLOSE(fd);
 }
-- 
2.31.1

[LTP] [PATCH v4 2/2] setsockopt08: Avoid confusion by removing TCONF

[Richard Palethorpe]

When not in compat mode this only tests that bad offsets result in
EINVAL. When in compat mode, we also test for the vulnerability.

Therefor I view this as two tests. The TCONF was supposed to indicate
the configuration didn't allow one of these tests. However it just
caused confusion with multiple people asking me to change it to
tst_brk.

So change it to TINFO.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/kernel/syscalls/setsockopt/setsockopt08.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt08.c b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
index 20abe85b4..1ffc69178 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt08.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
@@ -91,7 +91,7 @@ static void *buffer;
 void setup(void)
 {
 	if (tst_kernel_bits() == 32 || sizeof(long) > 4) {
-		tst_res(TCONF,
+		tst_res(TINFO,
 			"The vulnerability was only present in 32-bit compat mode");
 	}
 
-- 
2.31.1

[LTP] [PATCH v4 2/2] setsockopt08: Avoid confusion by removing TCONF

[Martin Doucha]

On 16. 08. 21 13:16, Richard Palethorpe via ltp wrote:
> When not in compat mode this only tests that bad offsets result in
> EINVAL. When in compat mode, we also test for the vulnerability.
> 
> Therefor I view this as two tests. The TCONF was supposed to indicate
> the configuration didn't allow one of these tests. However it just
> caused confusion with multiple people asking me to change it to
> tst_brk.
> 
> So change it to TINFO.
> 
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> ---

Hi,
for both patches:

Reviewed-by: Martin Doucha <mdoucha@suse.cz>

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

[LTP] [PATCH v4 2/2] setsockopt08: Avoid confusion by removing TCONF

[Petr Vorel]

Hi Richie, Martin,

Thanks both, patchset merged.

Kind regards,
Petr