Re: [LTP] [PATCH] setfsuid02: using -1 as invalid fsuid for setfsuid()

[Petr Vorel <pvorel@suse.cz>]

Hi Avinesh,

> a uid which does not have an entry in the /etc/passwd
> file is not really an invalid fsuid for setfsuid(), so changing
> the test to use -1 as an invalid fsuid.
> And second setfsuid(-1) call is to verify that preceding call has
> actually failed and there is no change in the fsuid.

Here was supposed to be
Fixes: 85f0b8478 ("setfsuid02: Rewrite using new LTP API")

as the problem was introduced in your rewrite, right?

> diff --git a/testcases/kernel/syscalls/setfsuid/setfsuid02.c b/testcases/kernel/syscalls/setfsuid/setfsuid02.c
> index 850f17834..f5aa1c004 100644
> --- a/testcases/kernel/syscalls/setfsuid/setfsuid02.c
> +++ b/testcases/kernel/syscalls/setfsuid/setfsuid02.c
> @@ -21,9 +21,7 @@ static void run(void)
>  	uid_t invalid_uid, current_uid;

>  	current_uid = geteuid();
> -	invalid_uid = 1;
> -	while (getpwuid(invalid_uid))
> -		invalid_uid++;
> +	invalid_uid = -1;

IMHO it should be casted

invalid_uid = (uid_t)-1;

as the code in shadow-utils:
https://github.com/shadow-maint/shadow/blob/eaebea55a495a56317ed85e959b3599f73c6bdf2/src/newusers.c#L342-L345

This does not work on 16-bit version, because uid_t is unsigned int,
-1 overflows the allowed value when we do check:

UID16_CHECK(invalid_uid, setfsuid);

setfsuid02.c:26: TBROK: uid -1 of invalid_uid is too large for testing 16-bit version of setfsuid()

It also does not make sense to check invalid_uid, it should have been
current_uid in 85f0b8478 (my bad not catching this):

UID16_CHECK(current_uid, setfsuid);

Please, always try to test 16-bit variant (most of us does not bother to test
32-bit compatibility, but at least to see TCONF is worth).

If you agree, I merge it with changes below.

Kind regards,
Petr

diff --git testcases/kernel/syscalls/setfsuid/setfsuid02.c testcases/kernel/syscalls/setfsuid/setfsuid02.c
index f5aa1c004..92e1979fa 100644
--- testcases/kernel/syscalls/setfsuid/setfsuid02.c
+++ testcases/kernel/syscalls/setfsuid/setfsuid02.c
@@ -21,9 +21,9 @@ static void run(void)
 	uid_t invalid_uid, current_uid;
 
 	current_uid = geteuid();
-	invalid_uid = -1;
+	invalid_uid = (uid_t)-1;
 
-	UID16_CHECK(invalid_uid, setfsuid);
+	UID16_CHECK(current_uid, setfsuid);
 
 	TST_EXP_VAL_SILENT(SETFSUID(invalid_uid), current_uid);
 	TST_EXP_VAL(SETFSUID(invalid_uid), current_uid,

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