[LTP] [PATCH] cve-2015-3290: Fix tst_syscall() return value

[LTP] [PATCH] cve-2015-3290: Fix tst_syscall() return value

[Martin Doucha]

The modify_ldt() syscall returns 32-bit signed integer value. Recent changes
in tst_syscall() caused the value to be interpreted as unsigned on older
kernels/glibc, which breaks the cve-2015-3290 test. Add explicit type cast
to fix it.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/cve/cve-2015-3290.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/testcases/cve/cve-2015-3290.c b/testcases/cve/cve-2015-3290.c
index f61d2809b..a2a8fcedd 100644
--- a/testcases/cve/cve-2015-3290.c
+++ b/testcases/cve/cve-2015-3290.c
@@ -195,7 +195,8 @@ static void set_ldt(void)
 		.useable	 = 0
 	};
 
-	TEST(tst_syscall(__NR_modify_ldt, 1, &data_desc, sizeof(data_desc)));
+	TEST((int)tst_syscall(__NR_modify_ldt, 1, &data_desc,
+		sizeof(data_desc)));
 	if (TST_RET == -EINVAL) {
 		tst_brk(TCONF | TRERRNO,
 			"modify_ldt: 16-bit data segments are probably disabled");
-- 
2.39.0


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

Re: [LTP] [PATCH] cve-2015-3290: Fix tst_syscall() return value

[Petr Vorel]

Hi all,

> The modify_ldt() syscall returns 32-bit signed integer value. Recent changes
> in tst_syscall() caused the value to be interpreted as unsigned on older
> kernels/glibc, which breaks the cve-2015-3290 test. Add explicit type cast
> to fix it.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

This is caused by e5d2a05a9 ("regen.sh: Use intptr_t for tst_syscall return")
which changed returning tst_ret from int to intptr_t (which is also int for 32
bit archs, but long for 64 bit archs). This commit is also needed, thus I don't
suggest to revert it, but I wonder how many other tests it broke.

Kind regards,
Petr

> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
>  testcases/cve/cve-2015-3290.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

> diff --git a/testcases/cve/cve-2015-3290.c b/testcases/cve/cve-2015-3290.c
> index f61d2809b..a2a8fcedd 100644
> --- a/testcases/cve/cve-2015-3290.c
> +++ b/testcases/cve/cve-2015-3290.c
> @@ -195,7 +195,8 @@ static void set_ldt(void)
>  		.useable	 = 0
>  	};

> -	TEST(tst_syscall(__NR_modify_ldt, 1, &data_desc, sizeof(data_desc)));
> +	TEST((int)tst_syscall(__NR_modify_ldt, 1, &data_desc,
> +		sizeof(data_desc)));
>  	if (TST_RET == -EINVAL) {
>  		tst_brk(TCONF | TRERRNO,
>  			"modify_ldt: 16-bit data segments are probably disabled");

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

Re: [LTP] [PATCH] cve-2015-3290: Fix tst_syscall() return value

[Teo Couprie Diaz]

Hi all,

On 15/02/2023 15:20, Petr Vorel wrote:
> Hi all,
>
>> The modify_ldt() syscall returns 32-bit signed integer value. Recent changes
>> in tst_syscall() caused the value to be interpreted as unsigned on older
>> kernels/glibc, which breaks the cve-2015-3290 test. Add explicit type cast
>> to fix it.
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> This is caused by e5d2a05a9 ("regen.sh: Use intptr_t for tst_syscall return")
> which changed returning tst_ret from int to intptr_t (which is also int for 32
> bit archs, but long for 64 bit archs). This commit is also needed, thus I don't
> suggest to revert it, but I wonder how many other tests it broke.

I sent the tst_syscall return patch. I did some testing with what systems I
had available when working on it and, at the time, didn't see any 
regressions,
including for cve-2015-3290.

However this was with fairly recent kernels and libcs, and according to 
Martin
the test failed for them on an older combination.

I shared the test suite[0] I tested the patch with, containing all the 
uses I found.
It might be worthwhile to run it on a system where we now know one of 
the tests
was affected ? Hopefully, none more, but having a result would be better.

Best regards,
Téo

[0]: https://lists.linux.it/pipermail/ltp/2022-November/031640.html

>
> Kind regards,
> Petr
>
>> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
>> ---
>>   testcases/cve/cve-2015-3290.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>> diff --git a/testcases/cve/cve-2015-3290.c b/testcases/cve/cve-2015-3290.c
>> index f61d2809b..a2a8fcedd 100644
>> --- a/testcases/cve/cve-2015-3290.c
>> +++ b/testcases/cve/cve-2015-3290.c
>> @@ -195,7 +195,8 @@ static void set_ldt(void)
>>   		.useable	 = 0
>>   	};
>> -	TEST(tst_syscall(__NR_modify_ldt, 1, &data_desc, sizeof(data_desc)));
>> +	TEST((int)tst_syscall(__NR_modify_ldt, 1, &data_desc,
>> +		sizeof(data_desc)));
>>   	if (TST_RET == -EINVAL) {
>>   		tst_brk(TCONF | TRERRNO,
>>   			"modify_ldt: 16-bit data segments are probably disabled");

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

Re: [LTP] [PATCH] cve-2015-3290: Fix tst_syscall() return value

[Martin Doucha]

On 15. 02. 23 16:34, Teo Couprie Diaz wrote:
> I sent the tst_syscall return patch. I did some testing with what systems I
> had available when working on it and, at the time, didn't see any 
> regressions,
> including for cve-2015-3290.
> 
> However this was with fairly recent kernels and libcs, and according to 
> Martin
> the test failed for them on an older combination.
> 
> I shared the test suite[0] I tested the patch with, containing all the 
> uses I found.
> It might be worthwhile to run it on a system where we now know one of 
> the tests
> was affected ? Hopefully, none more, but having a result would be better.

Yes, we got failures on some older distros with kernel 4.x and older 
glibc. I've already tested the patch there myself. The CVE test works 
fine on kernels 5.x with newer glibc.

-- 
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


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

Re: [LTP] [PATCH] cve-2015-3290: Fix tst_syscall() return value

[Petr Vorel]

> Hi all,

> On 15/02/2023 15:20, Petr Vorel wrote:
> > Hi all,

> > > The modify_ldt() syscall returns 32-bit signed integer value. Recent changes
> > > in tst_syscall() caused the value to be interpreted as unsigned on older
> > > kernels/glibc, which breaks the cve-2015-3290 test. Add explicit type cast
> > > to fix it.
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>

> > This is caused by e5d2a05a9 ("regen.sh: Use intptr_t for tst_syscall return")
> > which changed returning tst_ret from int to intptr_t (which is also int for 32
> > bit archs, but long for 64 bit archs). This commit is also needed, thus I don't
> > suggest to revert it, but I wonder how many other tests it broke.

> I sent the tst_syscall return patch. I did some testing with what systems I
> had available when working on it and, at the time, didn't see any
> regressions,
> including for cve-2015-3290.

> However this was with fairly recent kernels and libcs, and according to
> Martin
> the test failed for them on an older combination.

FYI I was able to reproduce it on VM with SLE15-SP2 kernel (based on 5.3.18,
with many patches) and glibc 2.26-13.62.1 (obviously older supported systems are
also affected). Hopefully that's all (need to check).

Kind regards,
Petr

> I shared the test suite[0] I tested the patch with, containing all the uses
> I found.
> It might be worthwhile to run it on a system where we now know one of the
> tests
> was affected ? Hopefully, none more, but having a result would be better.

> Best regards,
> Téo

> [0]: https://lists.linux.it/pipermail/ltp/2022-November/031640.html


> > Kind regards,
> > Petr

> > > Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> > > ---
> > >   testcases/cve/cve-2015-3290.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > diff --git a/testcases/cve/cve-2015-3290.c b/testcases/cve/cve-2015-3290.c
> > > index f61d2809b..a2a8fcedd 100644
> > > --- a/testcases/cve/cve-2015-3290.c
> > > +++ b/testcases/cve/cve-2015-3290.c
> > > @@ -195,7 +195,8 @@ static void set_ldt(void)
> > >   		.useable	 = 0
> > >   	};
> > > -	TEST(tst_syscall(__NR_modify_ldt, 1, &data_desc, sizeof(data_desc)));
> > > +	TEST((int)tst_syscall(__NR_modify_ldt, 1, &data_desc,
> > > +		sizeof(data_desc)));
> > >   	if (TST_RET == -EINVAL) {
> > >   		tst_brk(TCONF | TRERRNO,
> > >   			"modify_ldt: 16-bit data segments are probably disabled");

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

Re: [LTP] [PATCH] cve-2015-3290: Fix tst_syscall() return value

[Richard Palethorpe]

Hello,

Martin Doucha <mdoucha@suse.cz> writes:

> The modify_ldt() syscall returns 32-bit signed integer value. Recent changes
> in tst_syscall() caused the value to be interpreted as unsigned on older
> kernels/glibc, which breaks the cve-2015-3290 test. Add explicit type cast
> to fix it.

This is strange, currently for the test to work the return 32bit return
value must be sign extended to 64bit so that it equals -EINVAL (right?).

Meanwhile kernel has never changed the return value, it's always 32bit
("which is a bug"). I can't find any change in glibc either that would
explain it.

I am missing something or is it a mystery?

>
> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
>  testcases/cve/cve-2015-3290.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/cve/cve-2015-3290.c b/testcases/cve/cve-2015-3290.c
> index f61d2809b..a2a8fcedd 100644
> --- a/testcases/cve/cve-2015-3290.c
> +++ b/testcases/cve/cve-2015-3290.c
> @@ -195,7 +195,8 @@ static void set_ldt(void)
>  		.useable	 = 0
>  	};
>  
> -	TEST(tst_syscall(__NR_modify_ldt, 1, &data_desc, sizeof(data_desc)));
> +	TEST((int)tst_syscall(__NR_modify_ldt, 1, &data_desc,
> +		sizeof(data_desc)));
>  	if (TST_RET == -EINVAL) {
>  		tst_brk(TCONF | TRERRNO,
>  			"modify_ldt: 16-bit data segments are probably disabled");
> -- 
> 2.39.0


-- 
Thank you,
Richard.

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

Re: [LTP] [PATCH] cve-2015-3290: Fix tst_syscall() return value

[Richard Palethorpe]

Teo Couprie Diaz <teo.coupriediaz@arm.com> writes:

> Hi all,
>
> On 15/02/2023 15:20, Petr Vorel wrote:
>> Hi all,
>>
>>> The modify_ldt() syscall returns 32-bit signed integer value. Recent changes
>>> in tst_syscall() caused the value to be interpreted as unsigned on older
>>> kernels/glibc, which breaks the cve-2015-3290 test. Add explicit type cast
>>> to fix it.
>> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>>
>> This is caused by e5d2a05a9 ("regen.sh: Use intptr_t for tst_syscall return")
>> which changed returning tst_ret from int to intptr_t (which is also int for 32
>> bit archs, but long for 64 bit archs). This commit is also needed, thus I don't
>> suggest to revert it, but I wonder how many other tests it broke.

I think modify_ldt is special. You could search for other syscalls which
only return 32bit to make sure.

>
> I sent the tst_syscall return patch. I did some testing with what systems I
> had available when working on it and, at the time, didn't see any
> regressions,
> including for cve-2015-3290.
>
> However this was with fairly recent kernels and libcs, and according
> to Martin
> the test failed for them on an older combination.
>
> I shared the test suite[0] I tested the patch with, containing all the
> uses I found.
> It might be worthwhile to run it on a system where we now know one of
> the tests
> was affected ? Hopefully, none more, but having a result would be
> better.

I think the testing you did was more than adequate.

>
> Best regards,
> Téo
>
> [0]: https://lists.linux.it/pipermail/ltp/2022-November/031640.html
>
>>
>> Kind regards,
>> Petr
>>
>>> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
>>> ---
>>>   testcases/cve/cve-2015-3290.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>> diff --git a/testcases/cve/cve-2015-3290.c b/testcases/cve/cve-2015-3290.c
>>> index f61d2809b..a2a8fcedd 100644
>>> --- a/testcases/cve/cve-2015-3290.c
>>> +++ b/testcases/cve/cve-2015-3290.c
>>> @@ -195,7 +195,8 @@ static void set_ldt(void)
>>>   		.useable	 = 0
>>>   	};
>>> -	TEST(tst_syscall(__NR_modify_ldt, 1, &data_desc, sizeof(data_desc)));
>>> +	TEST((int)tst_syscall(__NR_modify_ldt, 1, &data_desc,
>>> +		sizeof(data_desc)));
>>>   	if (TST_RET == -EINVAL) {
>>>   		tst_brk(TCONF | TRERRNO,
>>>   			"modify_ldt: 16-bit data segments are probably disabled");


-- 
Thank you,
Richard.

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