[LTP] [PATCH v3 0/2] mount03: Convert to new API

[xuyang2018.jy@fujitsu.com]

Hi Petr

> Hi Xu,
> 
> ...
>>>>> -	SAFE_READ(0, otfd, readbuf, sizeof(readbuf));
>>>>> +	nobody_uid = ltpuser->pw_uid;
>>>>> +	nobody_gid = ltpuser->pw_gid;
> 
>>>>> -	SAFE_FSTAT(otfd, &file_stat);
>>>>> +	snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
>>>>> +	TST_RESOURCE_COPY(NULL, TESTBIN, file);
> 
>>>> In fact, old test case copy resource file when mount fileystem, but now,
>>>> you change this.  So in test_nosuid function, you test nosuid behaviour
>>>> in tmpdir instead of different filesystems.
> 
>>> old code in setup:
>>>       fs_type = tst_dev_fs_type();
>>>       device = tst_acquire_device(cleanup);
> 
>>>       if (!device)
>>>           tst_brkm(TCONF, cleanup, "Failed to obtain block device");
> 
>>>       tst_mkfs(cleanup, device, fs_type, NULL, NULL);
> 
>>>       SAFE_MKDIR(cleanup, mntpoint, DIR_MODE);
> 
>>>       SAFE_MOUNT(cleanup, device, mntpoint, fs_type, 0, NULL);
>>>       TST_RESOURCE_COPY(cleanup, "mount03_setuid_test", path_name);
> 
>>> new code:
>>>       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
>>>       SAFE_CP(TESTBIN, file);
> 
>>> Well, Li in his v2 removed the code because there is .mntpoint = MNTPOINT, in
>>> struct tst_test, therefore MNTPOINT is mounted in the filesystem, right?
> 
>>> But he also did SAFE_STAT and SAFE_CHMOD on MNTPOINT, which is IMHO wrong
>>> (or at least different from the old code).
> 
>> Yes, it is wrong. I guess Chen misundertand mntpoint usage(it just
>> create mntpoint instead mount dev to a moutpoint).
> 
>> So do you will fix this？
> 
> Yes, see the diff below. I'm waiting little longer if anybody else has some
> comments before merging it.
> 
> Thanks for your review.
> 
>> Best Regards
>> Yang Xu
> 
> diff --git testcases/kernel/syscalls/mount/mount03.c testcases/kernel/syscalls/mount/mount03.c
> index 74b018d78..9c58783d7 100644
> --- testcases/kernel/syscalls/mount/mount03.c
> +++ testcases/kernel/syscalls/mount/mount03.c
> @@ -15,7 +15,6 @@
>   #include <sys/types.h>
>   #include <sys/wait.h>
>   #include <pwd.h>
> -#include "old_resource.h"
>   #include "tst_test.h"
>   #include "lapi/mount.h"
>   
> @@ -145,7 +144,7 @@ static void setup(void)
>   	nobody_gid = ltpuser->pw_gid;
>   
>   	snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> -	TST_RESOURCE_COPY(NULL, TESTBIN, file);
> +	SAFE_CP(TESTBIN, file);

I still think we should test nosuid behaviour on different filesystem 
like other test function because we have expand it to all filesystems.

Also include tmpfs, so SAFE_CP should be in test_nosuid function 
otherwise may hit ENOENT problem.

different code as below:

[root@localhost mount]# git diff .
diff --git a/testcases/kernel/syscalls/mount/mount03.c 
b/testcases/kernel/syscalls/mount/mount03.c
index 74b018d78..b0582c76b 100644
--- a/testcases/kernel/syscalls/mount/mount03.c
+++ b/testcases/kernel/syscalls/mount/mount03.c
@@ -21,6 +21,7 @@

  #define MNTPOINT        "mntpoint"
  #define TESTBIN        "mount03_setuid_test"
+#define BIN_PATH           MNTPOINT"/"TESTBIN
  #define TEST_STR "abcdefghijklmnopqrstuvwxyz"
  #define FILE_MODE      0644
  #define SUID_MODE      0511
@@ -75,12 +76,19 @@ static void test_nosuid(void)
  {
         pid_t pid;
         int status;
+       struct stat st;
+
+       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
+       SAFE_CP(TESTBIN, file);
+       SAFE_STAT(file, &st);
+       if (st.st_mode != SUID_MODE)
+               SAFE_CHMOD(file, SUID_MODE);

         pid = SAFE_FORK();
         if (!pid) {
                 SAFE_SETGID(nobody_gid);
                 SAFE_SETREUID(-1, nobody_uid);
-               SAFE_EXECLP(TESTBIN, TESTBIN, NULL);
+               SAFE_EXECLP(BIN_PATH, TESTBIN, NULL);
         }

         SAFE_WAITPID(pid, &status, 0);
@@ -138,18 +146,10 @@ static struct tcase {

  static void setup(void)
  {
-       struct stat st;
         struct passwd *ltpuser = SAFE_GETPWNAM("nobody");

         nobody_uid = ltpuser->pw_uid;
         nobody_gid = ltpuser->pw_gid;
-
-       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
-       TST_RESOURCE_COPY(NULL, TESTBIN, file);
-
-       SAFE_STAT(file, &st);
-       if (st.st_mode != SUID_MODE)
-           SAFE_CHMOD(file, SUID_MODE);
  }

  static void cleanup(void)
[root@localhost mount]#


Best Regards
Yang Xu

>   
>   	SAFE_STAT(file, &st);
>   	if (st.st_mode != SUID_MODE)