Re: [LTP] [PATCH][RESEND] Issue observed with chmod05, fchmod05 test cases

[Subrata M. <su...@li...>]

On Tue, 2009-09-15 at 15:57 +0530, Sharyathi Nagesh wrote:
> Subrata
>    Excuse me for the late response, I have update the patch as well as
> the description. 
> Please re-verify
> Thanks
> Sharyathi
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> differences
> between files
> attachment
> (fix_supplementary_group_id_issue.patch)
> 
> 
> This Patch fixes bug in the test cases chmod05.c and fchmod05.c. The
> test case uses 2 UserIds nobody and bin.
> The issue is observed when one of the UserId is the supplementary
> group Ids of the rooti (either nobody or bin).
> In that case the Posix standard mentions that S_IGSID bit should not
> be cleared by the systemcall, chmod() 
> or fchmod(), which causes the test to fail. 
> This patch fixes the issue by specifically clearing the supplementary
> group ID of the process.
> The main intention of the test case is to test if S_IGSID bit is
> cleared in case the non privilage user tries to 
> set file modes. Hence felt clearing the supplementary group Ids is the
> best way to fix the issue.
> At the same time I am reverting the earlier patch submitted by Wei
> Yongjun <yj...@cn...> 

Thanks.

Regards--
Subrata

> 
> Signed-off-by: Sharyathi Nagesh <sha...@in...>
> 
> Index: ltp-full-20090831/testcases/kernel/syscalls/fchmod/fchmod05.c
> ===================================================================
> ---
> ltp-full-20090831.orig/testcases/kernel/syscalls/fchmod/fchmod05.c  2009-08-31 10:45:51.000000000 +0530
> +++
> ltp-full-20090831/testcases/kernel/syscalls/fchmod/fchmod05.c       2009-09-15 15:37:20.000000000 +0530
> @@ -218,12 +218,16 @@
>                 tst_brkm(TBROK, cleanup, "mkdir(2) of %s failed",
> TESTDIR);
>         }
> 
> -       if (chown(TESTDIR, nobody_u->pw_uid, nobody_u->pw_gid) == -1)
> +       if(setgroups(1, &nobody_u->pw_gid) == -1)
> +               tst_brkm(TBROK, cleanup, "Couldn't change
> supplementary group Id: %s",
> +                               strerror(errno));
> +
> +       if (chown(TESTDIR, nobody_u->pw_uid, bin_group->gr_gid) == -1)
>                 tst_brkm(TBROK, cleanup, "Couldn't change owner of
> testdir: %s",
>                                 strerror(errno));
> 
> -       /* change to nobody:bin */
> -       if (setegid(bin_group->gr_gid) == -1 ||
> seteuid(nobody_u->pw_uid) == -1)
> +       /* change to nobody:nobody */
> +       if (setegid(nobody_u->pw_gid) == -1 ||
> seteuid(nobody_u->pw_uid) == -1)
>                 tst_brkm(TBROK, cleanup, "Couldn't switch to
> nobody:nobody: %s",
>                                 strerror(errno));
> 
> Index: ltp-full-20090831/testcases/kernel/syscalls/chmod/chmod05.c
> ===================================================================
> ---
> ltp-full-20090831.orig/testcases/kernel/syscalls/chmod/chmod05.c    2009-08-31 10:45:53.000000000 +0530
> +++
> ltp-full-20090831/testcases/kernel/syscalls/chmod/chmod05.c 2009-09-15
> 15:41:27.000000000 +0530
> @@ -236,11 +236,15 @@
>         if (mkdir(TESTDIR, MODE_RWX) < 0)
>                 tst_brkm(TBROK|TERRNO, cleanup, "mkdir(%s) failed",
> TESTDIR);
> 
> -       if (chown(TESTDIR, nobody_u->pw_uid, nobody_u->pw_gid) == -1)
> +       if(setgroups(1, &nobody_u->pw_gid) == -1)
> +               tst_brkm(TBROK, cleanup, "Couldn't change
> supplementary group Id: %s",
> +                               strerror(errno));
> +
> +       if (chown(TESTDIR, nobody_u->pw_uid, bin_group->gr_gid) == -1)
>                 tst_brkm(TBROK|TERRNO, cleanup, "chown() of testdir
> failed");
> 
> -       /* change to nobody:bin */
> -       if (setegid(bin_group->gr_gid) == -1 ||
> +       /* change to nobody:nobody */
> +       if (setegid(nobody_u->pw_gid) == -1 || 
>                  seteuid(nobody_u->pw_uid) == -1)
>                 tst_brkm(TBROK|TERRNO, cleanup, "Couldn't switch to
> nobody:nobody");
>  }
>