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

[Petr Vorel <pvorel@suse.cz>]

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?

> > * 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

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