[LTP] [PATCH] setregid: use common user and group names.

[LTP] [PATCH] setregid: use common user and group names.

[Sandeep Patil]

Android systems do not have groups names 'users' 'sys' etc. Replace them
with 'nobody' and 'daemon' for the purposes of setregid0[34] tests and
make the nomenclature across both of these tests more consistent.

After the change, both setregid03 and setregid04 can run
on Android systems.

Signed-off-by: Sandeep Patil <sspatil@google.com>
---
 .../kernel/syscalls/setregid/setregid03.c     | 51 ++++++++++---------
 .../kernel/syscalls/setregid/setregid04.c     | 18 +++----
 2 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/testcases/kernel/syscalls/setregid/setregid03.c b/testcases/kernel/syscalls/setregid/setregid03.c
index a51719e80..97102e0ee 100644
--- a/testcases/kernel/syscalls/setregid/setregid03.c
+++ b/testcases/kernel/syscalls/setregid/setregid03.c
@@ -41,7 +41,7 @@ static gid_t neg_one = -1;
 /* flag to tell parent if child passed or failed. */
 static int flag;
 
-struct group users, sys, root, bin;
+struct group nobody_gr, daemon_gr, root_gr, bin_gr;
 struct passwd nobody;
 /*
  * The following structure contains all test data.  Each structure in the array
@@ -57,27 +57,28 @@ struct test_data_t {
 	char *test_msg;
 } test_data[] = {
 	{
-	&sys.gr_gid, &bin.gr_gid, &pass, &sys, &bin,
-		    "After setregid(sys, bin),"}, {
-	&neg_one, &sys.gr_gid, &pass, &sys, &sys, "After setregid(-1, sys)"},
-	{
-	&neg_one, &bin.gr_gid, &pass, &sys, &bin, "After setregid(-1, bin),"},
-	{
-	&bin.gr_gid, &neg_one, &pass, &bin, &bin, "After setregid(bin, -1),"},
-	{
-	&neg_one, &neg_one, &pass, &bin, &bin, "After setregid(-1, -1),"}, {
-	&neg_one, &bin.gr_gid, &pass, &bin, &bin, "After setregid(-1, bin),"},
-	{
-	&bin.gr_gid, &neg_one, &pass, &bin, &bin, "After setregid(bin, -1),"},
-	{
-	&bin.gr_gid, &bin.gr_gid, &pass, &bin, &bin,
+	&daemon_gr.gr_gid, &bin_gr.gr_gid, &pass, &daemon_gr, &bin_gr,
+		    "After setregid(daemon, bin),"}, {
+	&neg_one, &daemon_gr.gr_gid, &pass, &daemon_gr, &daemon_gr,
+		    "After setregid(-1, daemon)"}, {
+	&neg_one, &bin_gr.gr_gid, &pass, &daemon_gr, &bin_gr,
+		    "After setregid(-1, bin),"}, {
+	&bin_gr.gr_gid, &neg_one, &pass, &bin_gr, &bin_gr,
+		    "After setregid(bin, -1),"}, {
+	&neg_one, &neg_one, &pass, &bin_gr, &bin_gr,
+		    "After setregid(-1, -1),"}, {
+	&neg_one, &bin_gr.gr_gid, &pass, &bin_gr, &bin_gr,
+		    "After setregid(-1, bin),"}, {
+	&bin_gr.gr_gid, &neg_one, &pass, &bin_gr, &bin_gr,
+		    "After setregid(bin, -1),"}, {
+	&bin_gr.gr_gid, &bin_gr.gr_gid, &pass, &bin_gr, &bin_gr,
 		    "After setregid(bin, bin),"}, {
-	&sys.gr_gid, &neg_one, &fail, &bin, &bin, "After setregid(sys, -1)"},
-	{
-	&neg_one, &sys.gr_gid, &fail, &bin, &bin, "After setregid(-1, sys)"},
-	{
-	&sys.gr_gid, &sys.gr_gid, &fail, &bin, &bin,
-		    "After setregid(sys, sys)"},};
+	&daemon_gr.gr_gid, &neg_one, &fail, &bin_gr, &bin_gr,
+		    "After setregid(daemon, -1)"}, {
+	&neg_one, &daemon_gr.gr_gid, &fail, &bin_gr, &bin_gr,
+		    "After setregid(-1, daemon)"}, {
+	&daemon_gr.gr_gid, &daemon_gr.gr_gid, &fail, &bin_gr, &bin_gr,
+		    "After setregid(daemon, daemon)"},};
 
 int TST_TOTAL = sizeof(test_data) / sizeof(test_data[0]);
 
@@ -102,7 +103,7 @@ int main(int ac, char **av)
 		tst_count = 0;
 
 		/* set the appropriate ownership values */
-		if (SETREGID(NULL, sys.gr_gid, bin.gr_gid) == -1)
+		if (SETREGID(NULL, daemon_gr.gr_gid, bin_gr.gr_gid) == -1)
 			tst_brkm(TBROK, NULL, "Initial setregid failed");
 
 		SAFE_SETEUID(NULL, nobody.pw_uid);
@@ -191,11 +192,11 @@ static void setup(void)
 		tst_brkm(TBROK, NULL, "%s must be a valid group", #group); \
 	} \
 	GID16_CHECK(junk->gr_gid, setregid, NULL); \
-	group = *(junk); \
+	group ## _gr = *(junk); \
 } while (0)
 
-	GET_GID(users);
-	GET_GID(sys);
+	GET_GID(nobody);
+	GET_GID(daemon);
 	GET_GID(bin);
 
 	TEST_PAUSE;
diff --git a/testcases/kernel/syscalls/setregid/setregid04.c b/testcases/kernel/syscalls/setregid/setregid04.c
index 0e0aae782..73f8bcb03 100644
--- a/testcases/kernel/syscalls/setregid/setregid04.c
+++ b/testcases/kernel/syscalls/setregid/setregid04.c
@@ -35,7 +35,7 @@ TCID_DEFINE(setregid04);
 
 static gid_t neg_one = -1;
 
-static struct group users_gr, daemon_gr, root_gr, bin_gr;
+static struct group nobody_gr, daemon_gr, root_gr, bin_gr;
 
 /*
  * The following structure contains all test data.  Each structure in the array
@@ -52,8 +52,8 @@ struct test_data_t {
 	{
 	&root_gr.gr_gid, &root_gr.gr_gid, &root_gr, &root_gr,
 		    "After setregid(root, root),"}, {
-	&users_gr.gr_gid, &neg_one, &users_gr, &root_gr,
-		    "After setregid(users, -1)"}, {
+	&nobody_gr.gr_gid, &neg_one, &nobody_gr, &root_gr,
+		    "After setregid(nobody, -1)"}, {
 	&root_gr.gr_gid, &neg_one, &root_gr, &root_gr,
 		    "After setregid(root,-1),"}, {
 	&neg_one, &neg_one, &root_gr, &root_gr,
@@ -62,12 +62,12 @@ struct test_data_t {
 		    "After setregid(-1, root)"}, {
 	&root_gr.gr_gid, &neg_one, &root_gr, &root_gr,
 		    "After setregid(root, -1),"}, {
-	&daemon_gr.gr_gid, &users_gr.gr_gid, &daemon_gr, &users_gr,
-		    "After setregid(daemon, users)"}, {
-	&neg_one, &neg_one, &daemon_gr, &users_gr,
+	&daemon_gr.gr_gid, &nobody_gr.gr_gid, &daemon_gr, &nobody_gr,
+		    "After setregid(daemon, nobody)"}, {
+	&neg_one, &neg_one, &daemon_gr, &nobody_gr,
 		    "After setregid(-1, -1)"}, {
-	&neg_one, &users_gr.gr_gid, &daemon_gr, &users_gr,
-		    "After setregid(-1, users)"}
+	&neg_one, &nobody_gr.gr_gid, &daemon_gr, &nobody_gr,
+		    "After setregid(-1, nobody)"}
 };
 
 int TST_TOTAL = sizeof(test_data) / sizeof(test_data[0]);
@@ -123,7 +123,7 @@ static void setup(void)
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
 	SAFE_GETGROUP(root);
-	SAFE_GETGROUP(users);
+	SAFE_GETGROUP(nobody);
 	SAFE_GETGROUP(daemon);
 	SAFE_GETGROUP(bin);
 
-- 
2.16.2.804.g6dcf76e118-goog

[LTP] [PATCH] setregid: use common user and group names.

[Petr Vorel]

Hi Sandeep,

> Android systems do not have groups names 'users' 'sys' etc. Replace them
> with 'nobody' and 'daemon' for the purposes of setregid0[34] tests and
> make the nomenclature across both of these tests more consistent.

> After the change, both setregid03 and setregid04 can run
> on Android systems.
I guess we don't care about older-but-still-quite-new releases and look for the future (as
'daemon' was added to aosp quite recently, 'shell' would also work for older).

[1] https://android.googlesource.com/platform/system%2Fcore/+/8e8648463d74005ac3111235b6bdea9c79d7a34a

Kind regards,
Petr

[LTP] [PATCH] setregid: use common user and group names.

[Sandeep Patil]

On Mon, Mar 26, 2018 at 02:42:00PM +0200, Petr Vorel wrote:
> Hi Sandeep,
> 
> > Android systems do not have groups names 'users' 'sys' etc. Replace them
> > with 'nobody' and 'daemon' for the purposes of setregid0[34] tests and
> > make the nomenclature across both of these tests more consistent.
> 
> > After the change, both setregid03 and setregid04 can run
> > on Android systems.
> I guess we don't care about older-but-still-quite-new releases and look for the future (as
> 'daemon' was added to aosp quite recently, 'shell' would also work for older).

'shell' is not common though, e.g. it didn't exist by default on the
debian system that I test the patches before I send them out after testing on
Android.

The only other group that I could find that existed on both Android and my
x86 workstation is 'audio'. I can change it to that if you like, but again, I
don't know how portable that would be vs 'daemon'.

Also, I wouldn't worry about the test breaking on slightly older Android. The
vast majority of LTS tests are run through VTS, where these tests were disabled[1]
due to the failure anyway. This change will allow us to start running them on
all android devices now onwards ..

<snip>

- ssp

[1] https://android.googlesource.com/platform/test/vts-testcase/kernel/+/master/ltp/configs/disabled_tests.py#59

[LTP] [PATCH] setregid: use common user and group names.

[Petr Vorel]

Hi Sandeep,

> On Mon, Mar 26, 2018 at 02:42:00PM +0200, Petr Vorel wrote:
> > Hi Sandeep,

> > > Android systems do not have groups names 'users' 'sys' etc. Replace them
> > > with 'nobody' and 'daemon' for the purposes of setregid0[34] tests and
> > > make the nomenclature across both of these tests more consistent.

> > > After the change, both setregid03 and setregid04 can run
> > > on Android systems.
> > I guess we don't care about older-but-still-quite-new releases and look for the future (as
> > 'daemon' was added to aosp quite recently, 'shell' would also work for older).

> 'shell' is not common though, e.g. it didn't exist by default on the
> debian system that I test the patches before I send them out after testing on
> Android.
Sorry I wasn't clear, I meant for older Android releases. Sure 'shell' isn't probably on
any linux distribution.

> The only other group that I could find that existed on both Android and my
> x86 workstation is 'audio'. I can change it to that if you like, but again, I
> don't know how portable that would be vs 'daemon'.

> Also, I wouldn't worry about the test breaking on slightly older Android. The
> vast majority of LTS tests are run through VTS, where these tests were disabled[1]
> due to the failure anyway. This change will allow us to start running them on
> all android devices now onwards ..

I thought that there could be preprocesor condition for Android defining different users
for it, but if you don't need it I'll merge it and just note your related commit in commit
message:
8e8648463 ("libcutils: Add "daemon" and "bin" users for testing only")

Kind regards,
Petr

> - ssp

> [1] https://android.googlesource.com/platform/test/vts-testcase/kernel/+/master/ltp/configs/disabled_tests.py#59

[LTP] [PATCH] setregid: use common user and group names.

[Petr Vorel]

Hi Sandeep,

> Android systems do not have groups names 'users' 'sys' etc. Replace them
> with 'nobody' and 'daemon' for the purposes of setregid0[34] tests and
> make the nomenclature across both of these tests more consistent.

> After the change, both setregid03 and setregid04 can run
> on Android systems.

> Signed-off-by: Sandeep Patil <sspatil@google.com>
> ---
>  .../kernel/syscalls/setregid/setregid03.c     | 51 ++++++++++---------
>  .../kernel/syscalls/setregid/setregid04.c     | 18 +++----
>  2 files changed, 35 insertions(+), 34 deletions(-)

> diff --git a/testcases/kernel/syscalls/setregid/setregid03.c b/testcases/kernel/syscalls/setregid/setregid03.c
> index a51719e80..97102e0ee 100644
> --- a/testcases/kernel/syscalls/setregid/setregid03.c
> +++ b/testcases/kernel/syscalls/setregid/setregid03.c
> @@ -41,7 +41,7 @@ static gid_t neg_one = -1;
>  /* flag to tell parent if child passed or failed. */
>  static int flag;

> -struct group users, sys, root, bin;
> +struct group nobody_gr, daemon_gr, root_gr, bin_gr;
>  struct passwd nobody;
>  /*
>   * The following structure contains all test data.  Each structure in the array
> @@ -57,27 +57,28 @@ struct test_data_t {
>  	char *test_msg;
>  } test_data[] = {
>  	{
> -	&sys.gr_gid, &bin.gr_gid, &pass, &sys, &bin,
> -		    "After setregid(sys, bin),"}, {
> -	&neg_one, &sys.gr_gid, &pass, &sys, &sys, "After setregid(-1, sys)"},
> -	{
> -	&neg_one, &bin.gr_gid, &pass, &sys, &bin, "After setregid(-1, bin),"},
> -	{
> -	&bin.gr_gid, &neg_one, &pass, &bin, &bin, "After setregid(bin, -1),"},
> -	{
> -	&neg_one, &neg_one, &pass, &bin, &bin, "After setregid(-1, -1),"}, {
> -	&neg_one, &bin.gr_gid, &pass, &bin, &bin, "After setregid(-1, bin),"},
> -	{
> -	&bin.gr_gid, &neg_one, &pass, &bin, &bin, "After setregid(bin, -1),"},
> -	{
> -	&bin.gr_gid, &bin.gr_gid, &pass, &bin, &bin,
> +	&daemon_gr.gr_gid, &bin_gr.gr_gid, &pass, &daemon_gr, &bin_gr,
> +		    "After setregid(daemon, bin),"}, {
> +	&neg_one, &daemon_gr.gr_gid, &pass, &daemon_gr, &daemon_gr,
> +		    "After setregid(-1, daemon)"}, {
> +	&neg_one, &bin_gr.gr_gid, &pass, &daemon_gr, &bin_gr,
> +		    "After setregid(-1, bin),"}, {
> +	&bin_gr.gr_gid, &neg_one, &pass, &bin_gr, &bin_gr,
> +		    "After setregid(bin, -1),"}, {
> +	&neg_one, &neg_one, &pass, &bin_gr, &bin_gr,
> +		    "After setregid(-1, -1),"}, {
> +	&neg_one, &bin_gr.gr_gid, &pass, &bin_gr, &bin_gr,
> +		    "After setregid(-1, bin),"}, {
> +	&bin_gr.gr_gid, &neg_one, &pass, &bin_gr, &bin_gr,
> +		    "After setregid(bin, -1),"}, {
> +	&bin_gr.gr_gid, &bin_gr.gr_gid, &pass, &bin_gr, &bin_gr,
>  		    "After setregid(bin, bin),"}, {
> -	&sys.gr_gid, &neg_one, &fail, &bin, &bin, "After setregid(sys, -1)"},
> -	{
> -	&neg_one, &sys.gr_gid, &fail, &bin, &bin, "After setregid(-1, sys)"},
> -	{
> -	&sys.gr_gid, &sys.gr_gid, &fail, &bin, &bin,
> -		    "After setregid(sys, sys)"},};
> +	&daemon_gr.gr_gid, &neg_one, &fail, &bin_gr, &bin_gr,
> +		    "After setregid(daemon, -1)"}, {
> +	&neg_one, &daemon_gr.gr_gid, &fail, &bin_gr, &bin_gr,
> +		    "After setregid(-1, daemon)"}, {
> +	&daemon_gr.gr_gid, &daemon_gr.gr_gid, &fail, &bin_gr, &bin_gr,
> +		    "After setregid(daemon, daemon)"},};

>  int TST_TOTAL = sizeof(test_data) / sizeof(test_data[0]);

> @@ -102,7 +103,7 @@ int main(int ac, char **av)
>  		tst_count = 0;

>  		/* set the appropriate ownership values */
> -		if (SETREGID(NULL, sys.gr_gid, bin.gr_gid) == -1)
> +		if (SETREGID(NULL, daemon_gr.gr_gid, bin_gr.gr_gid) == -1)
>  			tst_brkm(TBROK, NULL, "Initial setregid failed");

>  		SAFE_SETEUID(NULL, nobody.pw_uid);
> @@ -191,11 +192,11 @@ static void setup(void)
>  		tst_brkm(TBROK, NULL, "%s must be a valid group", #group); \
>  	} \
>  	GID16_CHECK(junk->gr_gid, setregid, NULL); \
> -	group = *(junk); \
> +	group ## _gr = *(junk); \
>  } while (0)

> -	GET_GID(users);
> -	GET_GID(sys);
> +	GET_GID(nobody);
> +	GET_GID(daemon);
>  	GET_GID(bin);

>  	TEST_PAUSE;
> diff --git a/testcases/kernel/syscalls/setregid/setregid04.c b/testcases/kernel/syscalls/setregid/setregid04.c
> index 0e0aae782..73f8bcb03 100644
> --- a/testcases/kernel/syscalls/setregid/setregid04.c
> +++ b/testcases/kernel/syscalls/setregid/setregid04.c
> @@ -35,7 +35,7 @@ TCID_DEFINE(setregid04);

>  static gid_t neg_one = -1;

> -static struct group users_gr, daemon_gr, root_gr, bin_gr;
> +static struct group nobody_gr, daemon_gr, root_gr, bin_gr;

>  /*
>   * The following structure contains all test data.  Each structure in the array
> @@ -52,8 +52,8 @@ struct test_data_t {
>  	{
>  	&root_gr.gr_gid, &root_gr.gr_gid, &root_gr, &root_gr,
>  		    "After setregid(root, root),"}, {
> -	&users_gr.gr_gid, &neg_one, &users_gr, &root_gr,
> -		    "After setregid(users, -1)"}, {
> +	&nobody_gr.gr_gid, &neg_one, &nobody_gr, &root_gr,
> +		    "After setregid(nobody, -1)"}, {
>  	&root_gr.gr_gid, &neg_one, &root_gr, &root_gr,
>  		    "After setregid(root,-1),"}, {
>  	&neg_one, &neg_one, &root_gr, &root_gr,
> @@ -62,12 +62,12 @@ struct test_data_t {
>  		    "After setregid(-1, root)"}, {
>  	&root_gr.gr_gid, &neg_one, &root_gr, &root_gr,
>  		    "After setregid(root, -1),"}, {
> -	&daemon_gr.gr_gid, &users_gr.gr_gid, &daemon_gr, &users_gr,
> -		    "After setregid(daemon, users)"}, {
> -	&neg_one, &neg_one, &daemon_gr, &users_gr,
> +	&daemon_gr.gr_gid, &nobody_gr.gr_gid, &daemon_gr, &nobody_gr,
> +		    "After setregid(daemon, nobody)"}, {
> +	&neg_one, &neg_one, &daemon_gr, &nobody_gr,
>  		    "After setregid(-1, -1)"}, {
> -	&neg_one, &users_gr.gr_gid, &daemon_gr, &users_gr,
> -		    "After setregid(-1, users)"}
> +	&neg_one, &nobody_gr.gr_gid, &daemon_gr, &nobody_gr,
> +		    "After setregid(-1, nobody)"}
>  };

>  int TST_TOTAL = sizeof(test_data) / sizeof(test_data[0]);
> @@ -123,7 +123,7 @@ static void setup(void)
>  	tst_sig(FORK, DEF_HANDLER, cleanup);

>  	SAFE_GETGROUP(root);
> -	SAFE_GETGROUP(users);
> +	SAFE_GETGROUP(nobody);
>  	SAFE_GETGROUP(daemon);
>  	SAFE_GETGROUP(bin);

Thanks, pushed, with added note in commit message.


Kind regards,
Petr

[LTP] [PATCH] setregid: use common user and group names.

[Sandeep Patil]

On Tue, Mar 27, 2018 at 11:43:13AM +0200, Petr Vorel wrote:
> Hi Sandeep,
> 
> > On Mon, Mar 26, 2018 at 02:42:00PM +0200, Petr Vorel wrote:
> > > Hi Sandeep,
> 
> > > > Android systems do not have groups names 'users' 'sys' etc. Replace them
> > > > with 'nobody' and 'daemon' for the purposes of setregid0[34] tests and
> > > > make the nomenclature across both of these tests more consistent.
> 
> > > > After the change, both setregid03 and setregid04 can run
> > > > on Android systems.
> > > I guess we don't care about older-but-still-quite-new releases and look for the future (as
> > > 'daemon' was added to aosp quite recently, 'shell' would also work for older).
> 
> > 'shell' is not common though, e.g. it didn't exist by default on the
> > debian system that I test the patches before I send them out after testing on
> > Android.
> Sorry I wasn't clear, I meant for older Android releases. Sure 'shell' isn't probably on
> any linux distribution.
> 
> > The only other group that I could find that existed on both Android and my
> > x86 workstation is 'audio'. I can change it to that if you like, but again, I
> > don't know how portable that would be vs 'daemon'.
> 
> > Also, I wouldn't worry about the test breaking on slightly older Android. The
> > vast majority of LTS tests are run through VTS, where these tests were disabled[1]
> > due to the failure anyway. This change will allow us to start running them on
> > all android devices now onwards ..
> 
> I thought that there could be preprocesor condition for Android defining different users
> for it, but if you don't need it I'll merge it.

Yes, we can probable ifdef it in one place using "#ifdef __ANDROID__" or
other equivalent, but I don't think its needed here. We want to avoid doing
that as much as possible. It is fair to say these test will be run with VTS
"now onwards" anyway ..

> and just note your related commit in commit
> message:
> 8e8648463 ("libcutils: Add "daemon" and "bin" users for testing only")

I'll resend the patch with this added, thanks for the review.

- ssp

[LTP] [PATCH] setregid: use common user and group names.

[Petr Vorel]

Hi Sandeep,

...
> > > Also, I wouldn't worry about the test breaking on slightly older Android. The
> > > vast majority of LTS tests are run through VTS, where these tests were disabled[1]
> > > due to the failure anyway. This change will allow us to start running them on
> > > all android devices now onwards ..

> > I thought that there could be preprocesor condition for Android defining different users
> > for it, but if you don't need it I'll merge it.

> Yes, we can probable ifdef it in one place using "#ifdef __ANDROID__" or
> other equivalent, but I don't think its needed here. We want to avoid doing
> that as much as possible. It is fair to say these test will be run with VTS
> "now onwards" anyway ..
Thanks for explanation. I agree, makes sense to avoid it where possible.

> > and just note your related commit in commit
> > message:
> > 8e8648463 ("libcutils: Add "daemon" and "bin" users for testing only")

> I'll resend the patch with this added, thanks for the review.
I'm sorry I've added it myself yesterday and pushed:
1b7cf9474 setregid: use common user and group names.

I'll be more patient next time.


> - ssp

Kind regards,
Petr

[LTP] [PATCH] setregid: use common user and group names.

[Sandeep Patil]

On Wed, Mar 28, 2018 at 08:07:00AM +0200, Petr Vorel wrote:
> Hi Sandeep,
> 
> ...
> > > > Also, I wouldn't worry about the test breaking on slightly older Android. The
> > > > vast majority of LTS tests are run through VTS, where these tests were disabled[1]
> > > > due to the failure anyway. This change will allow us to start running them on
> > > > all android devices now onwards ..
> 
> > > I thought that there could be preprocesor condition for Android defining different users
> > > for it, but if you don't need it I'll merge it.
> 
> > Yes, we can probable ifdef it in one place using "#ifdef __ANDROID__" or
> > other equivalent, but I don't think its needed here. We want to avoid doing
> > that as much as possible. It is fair to say these test will be run with VTS
> > "now onwards" anyway ..
> Thanks for explanation. I agree, makes sense to avoid it where possible.
> 
> > > and just note your related commit in commit
> > > message:
> > > 8e8648463 ("libcutils: Add "daemon" and "bin" users for testing only")
> 
> > I'll resend the patch with this added, thanks for the review.
> I'm sorry I've added it myself yesterday and pushed:
> 1b7cf9474 setregid: use common user and group names.
> 
> I'll be more patient next time.

Happy to see it merged, doesn't matter. Thanks for the feedback.

- ssp

[LTP] [PATCH] setregid: use common user and group names.

[Cyril Hrubis]

Hi!
FIY this seems cause failures on Debian 9 where I do get failures:

setregid03    1  TBROK  :  setregid03.c:198: nobody must be a valid group
setregid03    2  TBROK  :  setregid03.c:198: Remaining cases broken

setregid04    1  TBROK  :  setregid04.c:126: Couldn't find the `nobody' group
setregid04    2  TBROK  :  setregid04.c:126: Remaining cases broken

I guess that we need a fallback if nobody group does not exist.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] setregid: use common user and group names.

[Petr Vorel]

Hi,

> FIY this seems cause failures on Debian 9 where I do get failures:

> setregid03    1  TBROK  :  setregid03.c:198: nobody must be a valid group
> setregid03    2  TBROK  :  setregid03.c:198: Remaining cases broken

> setregid04    1  TBROK  :  setregid04.c:126: Couldn't find the `nobody' group
> setregid04    2  TBROK  :  setregid04.c:126: Remaining cases broken
Debian has nogroup as group for nobody. Sorry that I overlooked it in "1b7cf9474".

> I guess that we need a fallback if nobody group does not exist.
Check for "nogroup"?
Or allow to overwrite the group name with env. variable?

BTW I wonder why we need more users than just nobody/nobody or nobody/nogroup.
I'd be for having env. variable TST_USER and TST_GROUP, with helper, which would check for
them if they're not defined (similar we use in tst_net.sh), so user wouldn't have to care
for it.

BTW: code in IDcheck.sh expect difference in Debian, but that's useless (just to report
it):
if ! fe "nobody" "$passwd" || ! (fe "nogroup" "$group" || fe "nobody" "$group")
then
    MISSING_ENTRY=1
fi


Kind regards,
Petr

[LTP] [PATCH] setregid: use common user and group names.

[Cyril Hrubis]

Hi!
> > FIY this seems cause failures on Debian 9 where I do get failures:
> 
> > setregid03    1  TBROK  :  setregid03.c:198: nobody must be a valid group
> > setregid03    2  TBROK  :  setregid03.c:198: Remaining cases broken
> 
> > setregid04    1  TBROK  :  setregid04.c:126: Couldn't find the `nobody' group
> > setregid04    2  TBROK  :  setregid04.c:126: Remaining cases broken
> Debian has nogroup as group for nobody. Sorry that I overlooked it in "1b7cf9474".
> 
> > I guess that we need a fallback if nobody group does not exist.
> Check for "nogroup"?

Let's just change the code to fallback to nogroup if group nobody does
not exist for the release and then we can think of an more elaborate
solution.

> Or allow to overwrite the group name with env. variable?
>
> BTW I wonder why we need more users than just nobody/nobody or nobody/nogroup.
> I'd be for having env. variable TST_USER and TST_GROUP, with helper, which would check for
> them if they're not defined (similar we use in tst_net.sh), so user wouldn't have to care
> for it.

I would rather go for adding some API to the test library that would
supply the test with useable user and group ids. Something as
.needs_user_id and .needs_group_id to the tst_test structure and then
put all the logic to the test library.

> BTW: code in IDcheck.sh expect difference in Debian, but that's useless (just to report
> it):
> if ! fe "nobody" "$passwd" || ! (fe "nogroup" "$group" || fe "nobody" "$group")
> then
>     MISSING_ENTRY=1
> fi

We should probably get rid of IDcheck.sh I doubt that it's up-to-date
enough to be useful.

-- 
Cyril Hrubis
chrubis@suse.cz