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

[xuyang2018.jy@fujitsu.com]

Hi Petr> Hi Xu,
> 
>> Hi Petr
> 
>>> Hi,
> 
>>> I wanted to speedup mount03 rewrite [1], thus I finished the work.
> 
>>> Changes include:
>>> * simplify code by using:
>>>     - SAFE macros
>>>     - TST_EXP_FAIL() instead of TST_EXP_FAIL2()
>>>     - remove if () check from SAFE macros (that's the point of safe macros
>>> 	to not having to use if () check
> 
>>> * fix mount03_setuid_test call, so it can expect 0 exit code
>>> I wonder myself why this fixes it:
>>> -		SAFE_SETREUID(nobody_uid, nobody_gid);
> 
>> Why here is nobody_gid?
> 
>>> +		SAFE_SETGID(nobody_gid);
>>> +		SAFE_SETREUID(-1, nobody_uid);
> 
>> What problem do you meet?
> 
> Using original code SAFE_SETREUID(nobody_uid, nobody_gid);
> causes mount03_setuid_test to fail (exit 1).
> The same code is in creat08.c, creat09.c, open10.c.
> Did I answer your question?

I look mount03_setuid_test code today, nosuid mount option should
expect setuid failed when using a non-privileged user even this program 
has set-user-id bit.

Old api also think PASS when mount03_setuid_test exit 1

So I think you should use  SAFE_SETREUID(nobody_uid, nobody_uid);
and then use code as below:

	if (WIFEXITED(status)) {
		switch (WEXITSTATUS(status)) {
		case EXIT_FAILURE:
			tst_res(TPASS, "%s passed", TESTBIN);
			return;
		case EXIT_SUCCESS:
			tst_res(TFAIL, "%s failed", TESTBIN);
			return;
		default:
		case TBROK:
			break;
		}
	}

Best Regards
Yang Xu
> 
>>> * add missing TST_RESOURCE_COPY()
>>> @Cyril: is it really needed?
> 
>> IMO, if we use resourcein test struct, we don't need it because ltp lib
>> has move it to tmpdir, we  can just use SAFE_COPY ie prctl06.c.
> 
> Ah, thanks!
> SAFE_CP(TESTBIN, file);
> 
> ...
>>> +#define FLAG_DESC(x) .flag = x, .desc = #x
>>> +static struct tcase {
>>> +	unsigned int flag;
>>> +	char *desc;
>>> +	void (*test)(void);
>>> +} tcases[] = {
>>> +	{FLAG_DESC(MS_RDONLY), test_rdonly},
>>> +	{FLAG_DESC(MS_NODEV), test_nodev},
>>> +	{FLAG_DESC(MS_NOEXEC), test_noexec},
>>> +	{FLAG_DESC(MS_SYNCHRONOUS), test_synchronous},
>>> +	{FLAG_DESC(MS_RDONLY), test_remount},
>>> +	{FLAG_DESC(MS_NOSUID), test_nosuid},
>>> +	{FLAG_DESC(MS_NOATIME), test_noatime},
>>> +};
> 
>>> -	sleep(1);
>>> +static void setup(void)
>>> +{
>>> +	struct stat st;
>>> +	struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
> 
>>> -	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).
> 
> Kind regards,
> Petr
> 
>> Best Regards
>> Yang Xu