diff mbox series

[v6,1/3] syscalls/quotactl09: Test error when quota info hidden in filesystem

Message ID 1641973691-22981-1-git-send-email-xuyang2018.jy@fujitsu.com
State Accepted
Headers show
Series [v6,1/3] syscalls/quotactl09: Test error when quota info hidden in filesystem | expand

Commit Message

Yang Xu Jan. 12, 2022, 7:48 a.m. UTC 
This case is similar to quotactl06 but only some differences
1) use quotactl and quotactl_fd syscalls without visible quota file
2) remove some error for addr argument
3) test external_fd for quotactl_fd for EINVAL error when using Q_QUOTAON
4) add error case description when running

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 runtest/syscalls                              |   1 +
 testcases/kernel/syscalls/quotactl/.gitignore |   1 +
 .../kernel/syscalls/quotactl/quotactl09.c     | 195 ++++++++++++++++++
 3 files changed, 197 insertions(+)
 create mode 100644 testcases/kernel/syscalls/quotactl/quotactl09.c

Comments

Yang Xu Jan. 12, 2022, 9:18 a.m. UTC | #1 
Hi Cyril

As your suggestion, I modify this patch as below

diff --git a/testcases/kernel/syscalls/quotactl/quotactl09.c
b/testcases/kernel/syscalls/quotactl/quotactl09.c
index 79f506bdc..4dbfcc7ea 100644
--- a/testcases/kernel/syscalls/quotactl/quotactl09.c
+++ b/testcases/kernel/syscalls/quotactl/quotactl09.c
@@ -17,14 +17,14 @@
  *   allowed by the quota format
  * - EPERM when the caller lacked the required privilege
(CAP_SYS_ADMIN) for the
  *   specified operation
- * - EINVAL when cmd is Q_QUOTAON, but the fd refers to a regular
file(doesn't
- *   under this moutpoint)
+ * - ENOSYS when cmd is Q_QUOTAON, but the fd refers to a socket
  *
  * Minimum e2fsprogs version required is 1.43.
  */

 #include <errno.h>
 #include <sys/quota.h>
+#include <sys/socket.h>
 #include "tst_test.h"
 #include "tst_capability.h"
 #include "quotactl_syscall_var.h"
@@ -33,7 +33,7 @@

 static int32_t fmt_id = QFMT_VFS_V1;
 static int test_id, mount_flag;
-static int getnextquota_nsup, external_fd = -1;
+static int getnextquota_nsup, socket_fd = -1;

 static struct if_nextdqblk res_ndq;

@@ -60,27 +60,25 @@ static struct tcase {
        void *addr;
        int exp_err;
        int on_flag;
-       int use_external_fd;
        char *des;
 } tcases[] = {
-       {QCMD(Q_SETQUOTA, USRQUOTA), &fmt_id, NULL, EFAULT, 1, 0,
+       {QCMD(Q_SETQUOTA, USRQUOTA), &fmt_id, NULL, EFAULT, 1,
        "EFAULT when addr or special is invalid"},

-       {QCMD(OPTION_INVALID, USRQUOTA), &fmt_id, NULL, EINVAL, 0, 0,
+       {QCMD(OPTION_INVALID, USRQUOTA), &fmt_id, NULL, EINVAL, 0,
        "EINVAL when cmd or type is invalid"},

-       {QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, ENOTBLK, 0, 0,
+       {QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, ENOTBLK, 0,
        "ENOTBLK when special is not a block device"},

-       {QCMD(Q_SETQUOTA, USRQUOTA), &test_id, &set_dqmax, ERANGE, 1, 0,
+       {QCMD(Q_SETQUOTA, USRQUOTA), &test_id, &set_dqmax, ERANGE, 1,
        "ERANGE when cmd is Q_SETQUOTA, but the specified limits are out
of the range"},

-       {QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, EPERM, 0, 0,
+       {QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, EPERM, 0,
        "EPERM when the caller lacked the required privilege for
specified operations"},

-       {QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, EINVAL, 0, 1,
-       "EINVAL when cmd is Q_QUOTAON, but the fd refers to a regular
file(doesn't under"
-       "this mountpoint)"}
+       {QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, ENOSYS, 0,
+       "ENOSYS when cmd is Q_QUOTAON, but the fd refers to a socket"}
 };

 static void verify_quotactl(unsigned int n)
@@ -113,8 +111,12 @@ static void verify_quotactl(unsigned int n)
                        tst_res(TCONF, "quotactl_fd() doesn't have this
error, skip");
                        return;
                }
+               if (tc->exp_err == ENOSYS) {
+                       TST_EXP_FAIL(syscall(__NR_quotactl_fd,
socket_fd, tc->cmd, *tc->id, tc->addr), tc->exp_err,
"syscall(quotactl_fd)");
+                       return;
+               }
        } else {
-               if (tc->use_external_fd) {
+               if (tc->exp_err == ENOSYS) {
                        tst_res(TCONF, "quotactl() doesn't use fd, skip");
                        return;
                }
@@ -123,8 +125,8 @@ static void verify_quotactl(unsigned int n)
                TST_EXP_FAIL(do_quotactl(fd, tc->cmd, "/dev/null",
*tc->id, tc->addr),
                        ENOTBLK, "do_quotactl()");
        else
-               TST_EXP_FAIL(do_quotactl(tc->use_external_fd ?
external_fd : fd, tc->cmd,
-                       tst_device->dev, *tc->id, tc->addr),
tc->exp_err, "do_quotactl()");
+               TST_EXP_FAIL(do_quotactl(fd, tc->cmd, tst_device->dev,
*tc->id, tc->addr),
+                       tc->exp_err, "do_quotactl()");

        if (quota_on) {
                TST_EXP_PASS_SILENT(do_quotactl(fd, QCMD(Q_QUOTAOFF,
USRQUOTA), tst_device->dev,
@@ -146,12 +148,11 @@ static void setup(void)
        const char *const fs_opts[] = { "-O quota", NULL};

        quotactl_info();
-       external_fd = SAFE_CREAT("testfile", O_RDONLY);

        SAFE_MKFS(tst_device->dev, tst_device->fs_type, fs_opts, NULL);
        SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
        mount_flag = 1;
-
+       socket_fd = SAFE_SOCKET(PF_INET, SOCK_STREAM, 0);
        fd = SAFE_OPEN(MNTPOINT, O_RDONLY);
        TEST(do_quotactl(fd, QCMD(Q_GETNEXTQUOTA, USRQUOTA),
tst_device->dev,
                test_id, (void *) &res_ndq));
@@ -168,8 +169,8 @@ static void cleanup(void)
 {
        if (fd > -1)
                SAFE_CLOSE(fd);
-       if (external_fd > -1)
-               SAFE_CLOSE(external_fd);
+       if (socket_fd > -1)
+               SAFE_CLOSE(socket_fd);
        if (mount_flag && tst_umount(MNTPOINT))
                tst_res(TWARN | TERRNO, "umount(%s)", MNTPOINT);


How about this patch after this change?

Best Regards
Yang Xu

> This case is similar to quotactl06 but only some differences
> 1) use quotactl and quotactl_fd syscalls without visible quota file
> 2) remove some error for addr argument
> 3) test external_fd for quotactl_fd for EINVAL error when using Q_QUOTAON
> 4) add error case description when running
> 
> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
> ---
>   runtest/syscalls                              |   1 +
>   testcases/kernel/syscalls/quotactl/.gitignore |   1 +
>   .../kernel/syscalls/quotactl/quotactl09.c     | 195 ++++++++++++++++++
>   3 files changed, 197 insertions(+)
>   create mode 100644 testcases/kernel/syscalls/quotactl/quotactl09.c
> 
> diff --git a/runtest/syscalls b/runtest/syscalls
> index a0ca84b36..3b2deb64e 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1073,6 +1073,7 @@ quotactl05 quotactl05
>   quotactl06 quotactl06
>   quotactl07 quotactl07
>   quotactl08 quotactl08
> +quotactl09 quotactl09
> 
>   read01 read01
>   read02 read02
> diff --git a/testcases/kernel/syscalls/quotactl/.gitignore b/testcases/kernel/syscalls/quotactl/.gitignore
> index dab9b3420..94de2c8f2 100644
> --- a/testcases/kernel/syscalls/quotactl/.gitignore
> +++ b/testcases/kernel/syscalls/quotactl/.gitignore
> @@ -6,3 +6,4 @@
>   /quotactl06
>   /quotactl07
>   /quotactl08
> +/quotactl09
> diff --git a/testcases/kernel/syscalls/quotactl/quotactl09.c b/testcases/kernel/syscalls/quotactl/quotactl09.c
> new file mode 100644
> index 000000000..79f506bdc
> --- /dev/null
> +++ b/testcases/kernel/syscalls/quotactl/quotactl09.c
> @@ -0,0 +1,195 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu<xuyang2018.jy@fujitsu.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Tests basic error handling of the quotactl syscall without visible quota files
> + * (use quotactl and quotactl_fd syscall):
> + *
> + * - EFAULT when addr or special is invalid
> + * - EINVAL when cmd or type is invalid
> + * - ENOTBLK when special is not a block device
> + * - ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the range
> + *   allowed by the quota format
> + * - EPERM when the caller lacked the required privilege (CAP_SYS_ADMIN) for the
> + *   specified operation
> + * - EINVAL when cmd is Q_QUOTAON, but the fd refers to a regular file(doesn't
> + *   under this moutpoint)
> + *
> + * Minimum e2fsprogs version required is 1.43.
> + */
> +
> +#include<errno.h>
> +#include<sys/quota.h>
> +#include "tst_test.h"
> +#include "tst_capability.h"
> +#include "quotactl_syscall_var.h"
> +
> +#define OPTION_INVALID 999
> +
> +static int32_t fmt_id = QFMT_VFS_V1;
> +static int test_id, mount_flag;
> +static int getnextquota_nsup, external_fd = -1;
> +
> +static struct if_nextdqblk res_ndq;
> +
> +static struct dqblk set_dqmax = {
> +	.dqb_bsoftlimit = 0x7fffffffffffffffLL,  /* 2^63-1 */
> +	.dqb_valid = QIF_BLIMITS
> +};
> +
> +static struct tst_cap dropadmin = {
> +	.action = TST_CAP_DROP,
> +	.id = CAP_SYS_ADMIN,
> +	.name = "CAP_SYS_ADMIN",
> +};
> +
> +static struct tst_cap needadmin = {
> +	.action = TST_CAP_REQ,
> +	.id = CAP_SYS_ADMIN,
> +	.name = "CAP_SYS_ADMIN",
> +};
> +
> +static struct tcase {
> +	int cmd;
> +	int *id;
> +	void *addr;
> +	int exp_err;
> +	int on_flag;
> +	int use_external_fd;
> +	char *des;
> +} tcases[] = {
> +	{QCMD(Q_SETQUOTA, USRQUOTA),&fmt_id, NULL, EFAULT, 1, 0,
> +	"EFAULT when addr or special is invalid"},
> +
> +	{QCMD(OPTION_INVALID, USRQUOTA),&fmt_id, NULL, EINVAL, 0, 0,
> +	"EINVAL when cmd or type is invalid"},
> +
> +	{QCMD(Q_QUOTAON, USRQUOTA),&fmt_id, NULL, ENOTBLK, 0, 0,
> +	"ENOTBLK when special is not a block device"},
> +
> +	{QCMD(Q_SETQUOTA, USRQUOTA),&test_id,&set_dqmax, ERANGE, 1, 0,
> +	"ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the range"},
> +
> +	{QCMD(Q_QUOTAON, USRQUOTA),&fmt_id, NULL, EPERM, 0, 0,
> +	"EPERM when the caller lacked the required privilege for specified operations"},
> +
> +	{QCMD(Q_QUOTAON, USRQUOTA),&fmt_id, NULL, EINVAL, 0, 1,
> +	"EINVAL when cmd is Q_QUOTAON, but the fd refers to a regular file(doesn't under "
> +	"this mountpoint)"}
> +};
> +
> +static void verify_quotactl(unsigned int n)
> +{
> +	struct tcase *tc =&tcases[n];
> +	int quota_on = 0;
> +	int drop_flag = 0;
> +
> +	tst_res(TINFO, "Testing %s", tc->des);
> +	if (tc->cmd == QCMD(Q_GETNEXTQUOTA, USRQUOTA)&&  getnextquota_nsup) {
> +		tst_res(TCONF, "current system doesn't support Q_GETNEXTQUOTA");
> +		return;
> +	}
> +
> +	if (tc->on_flag) {
> +		TST_EXP_PASS_SILENT(do_quotactl(fd, QCMD(Q_QUOTAON, USRQUOTA), tst_device->dev,
> +			fmt_id, NULL), "do_quotactl(QCMD(Q_QUOTAON, USRQUOTA))");
> +		if (!TST_PASS)
> +			return;
> +		quota_on = 1;
> +	}
> +
> +	if (tc->exp_err == EPERM) {
> +		tst_cap_action(&dropadmin);
> +		drop_flag = 1;
> +	}
> +
> +	if (tst_variant) {
> +		if (tc->exp_err == ENOTBLK) {
> +			tst_res(TCONF, "quotactl_fd() doesn't have this error, skip");
> +			return;
> +		}
> +	} else {
> +		if (tc->use_external_fd) {
> +			tst_res(TCONF, "quotactl() doesn't use fd, skip");
> +			return;
> +		}
> +	}
> +	if (tc->exp_err == ENOTBLK)
> +		TST_EXP_FAIL(do_quotactl(fd, tc->cmd, "/dev/null", *tc->id, tc->addr),
> +			ENOTBLK, "do_quotactl()");
> +	else
> +		TST_EXP_FAIL(do_quotactl(tc->use_external_fd ? external_fd : fd, tc->cmd,
> +			tst_device->dev, *tc->id, tc->addr), tc->exp_err, "do_quotactl()");
> +
> +	if (quota_on) {
> +		TST_EXP_PASS_SILENT(do_quotactl(fd, QCMD(Q_QUOTAOFF, USRQUOTA), tst_device->dev,
> +			fmt_id, NULL), "do_quotactl(QCMD(Q_QUOTAOFF, USRQUOTA)");
> +		if (!TST_PASS)
> +			return;
> +		quota_on = 0;
> +	}
> +
> +	if (drop_flag) {
> +		tst_cap_action(&needadmin);
> +		drop_flag = 0;
> +	}
> +}
> +
> +static void setup(void)
> +{
> +	unsigned int i;
> +	const char *const fs_opts[] = { "-O quota", NULL};
> +
> +	quotactl_info();
> +	external_fd = SAFE_CREAT("testfile", O_RDONLY);
> +
> +	SAFE_MKFS(tst_device->dev, tst_device->fs_type, fs_opts, NULL);
> +	SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
> +	mount_flag = 1;
> +
> +	fd = SAFE_OPEN(MNTPOINT, O_RDONLY);
> +	TEST(do_quotactl(fd, QCMD(Q_GETNEXTQUOTA, USRQUOTA), tst_device->dev,
> +		test_id, (void *)&res_ndq));
> +	if (TST_ERR == EINVAL || TST_ERR == ENOSYS)
> +		getnextquota_nsup = 1;
> +
> +	for (i = 0; i<  ARRAY_SIZE(tcases); i++) {
> +		if (!tcases[i].addr)
> +			tcases[i].addr = tst_get_bad_addr(NULL);
> +	}
> +}
> +
> +static void cleanup(void)
> +{
> +	if (fd>  -1)
> +		SAFE_CLOSE(fd);
> +	if (external_fd>  -1)
> +		SAFE_CLOSE(external_fd);
> +	if (mount_flag&&  tst_umount(MNTPOINT))
> +		tst_res(TWARN | TERRNO, "umount(%s)", MNTPOINT);
> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_kconfigs = (const char *[]) {
> +		"CONFIG_QFMT_V2",
> +		NULL
> +	},
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.test = verify_quotactl,
> +	.dev_fs_type = "ext4",
> +	.mntpoint = MNTPOINT,
> +	.needs_device = 1,
> +	.needs_root = 1,
> +	.test_variants = QUOTACTL_SYSCALL_VARIANTS,
> +	.needs_cmds = (const char *[]) {
> +		"mkfs.ext4>= 1.43.0",
> +		NULL
> +	}
> +};
Cyril Hrubis Jan. 12, 2022, 9:46 a.m. UTC | #2 
Hi!
> How about this patch after this change?

Looks reasonable.
Yang Xu Jan. 13, 2022, 6:51 a.m. UTC | #3 
Hi Cyril
> Hi!
>> How about this patch after this change?
>
> Looks reasonable.
So do you have some comments for other two quotactl patches? or add RVB tag?

ps: I want to make them merged in ltp before this release.

Best Regargds
Yang Xu
>
Cyril Hrubis Jan. 13, 2022, 9:49 a.m. UTC | #4 
Hi!
> >> How about this patch after this change?
> >
> > Looks reasonable.
> So do you have some comments for other two quotactl patches? or add RVB tag?
> 
> ps: I want to make them merged in ltp before this release.

I will have a look today, I had half day off yesterday and haven't
managed to get to them.
Cyril Hrubis Jan. 13, 2022, 2:22 p.m. UTC | #5 
Hi!
> +/*\
> + * [Description]
> + *
> + * Tests basic error handling of the quotactl syscall without visible quota files
> + * (use quotactl and quotactl_fd syscall):
> + *
> + * - EFAULT when addr or special is invalid
> + * - EINVAL when cmd or type is invalid
> + * - ENOTBLK when special is not a block device
> + * - ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the range
> + *   allowed by the quota format
> + * - EPERM when the caller lacked the required privilege (CAP_SYS_ADMIN) for the
> + *   specified operation
> + * - EINVAL when cmd is Q_QUOTAON, but the fd refers to a regular file(doesn't
> + *   under this moutpoint)
> + *
> + * Minimum e2fsprogs version required is 1.43.
> + */
> +
> +#include <errno.h>
> +#include <sys/quota.h>
> +#include "tst_test.h"
> +#include "tst_capability.h"
> +#include "quotactl_syscall_var.h"
> +
> +#define OPTION_INVALID 999
> +
> +static int32_t fmt_id = QFMT_VFS_V1;
> +static int test_id, mount_flag;
> +static int getnextquota_nsup, external_fd = -1;
> +
> +static struct if_nextdqblk res_ndq;
> +
> +static struct dqblk set_dqmax = {
> +	.dqb_bsoftlimit = 0x7fffffffffffffffLL,  /* 2^63-1 */
> +	.dqb_valid = QIF_BLIMITS
> +};
> +
> +static struct tst_cap dropadmin = {
> +	.action = TST_CAP_DROP,
> +	.id = CAP_SYS_ADMIN,
> +	.name = "CAP_SYS_ADMIN",
> +};
> +
> +static struct tst_cap needadmin = {
> +	.action = TST_CAP_REQ,
> +	.id = CAP_SYS_ADMIN,
> +	.name = "CAP_SYS_ADMIN",
> +};
> +
> +static struct tcase {
> +	int cmd;
> +	int *id;
> +	void *addr;
> +	int exp_err;
> +	int on_flag;
> +	int use_external_fd;
> +	char *des;
> +} tcases[] = {
> +	{QCMD(Q_SETQUOTA, USRQUOTA), &fmt_id, NULL, EFAULT, 1, 0,
> +	"EFAULT when addr or special is invalid"},
> +
> +	{QCMD(OPTION_INVALID, USRQUOTA), &fmt_id, NULL, EINVAL, 0, 0,
> +	"EINVAL when cmd or type is invalid"},
> +
> +	{QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, ENOTBLK, 0, 0,
> +	"ENOTBLK when special is not a block device"},
> +
> +	{QCMD(Q_SETQUOTA, USRQUOTA), &test_id, &set_dqmax, ERANGE, 1, 0,
> +	"ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the range"},
> +
> +	{QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, EPERM, 0, 0,
> +	"EPERM when the caller lacked the required privilege for specified operations"},
> +
> +	{QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, EINVAL, 0, 1,
> +	"EINVAL when cmd is Q_QUOTAON, but the fd refers to a regular file(doesn't under "
> +	"this mountpoint)"}
> +};
> +
> +static void verify_quotactl(unsigned int n)
> +{
> +	struct tcase *tc = &tcases[n];
> +	int quota_on = 0;
> +	int drop_flag = 0;
> +
> +	tst_res(TINFO, "Testing %s", tc->des);
> +	if (tc->cmd == QCMD(Q_GETNEXTQUOTA, USRQUOTA) && getnextquota_nsup) {
> +		tst_res(TCONF, "current system doesn't support Q_GETNEXTQUOTA");
> +		return;
> +	}
> +
> +	if (tc->on_flag) {
> +		TST_EXP_PASS_SILENT(do_quotactl(fd, QCMD(Q_QUOTAON, USRQUOTA), tst_device->dev,
> +			fmt_id, NULL), "do_quotactl(QCMD(Q_QUOTAON, USRQUOTA))");
> +		if (!TST_PASS)
> +			return;
> +		quota_on = 1;
> +	}
> +
> +	if (tc->exp_err == EPERM) {
> +		tst_cap_action(&dropadmin);
> +		drop_flag = 1;
> +	}
> +
> +	if (tst_variant) {
> +		if (tc->exp_err == ENOTBLK) {
> +			tst_res(TCONF, "quotactl_fd() doesn't have this error, skip");
> +			return;
> +		}
> +	} else {
> +		if (tc->use_external_fd) {
> +			tst_res(TCONF, "quotactl() doesn't use fd, skip");
> +			return;
> +		}
> +	}
> +	if (tc->exp_err == ENOTBLK)
> +		TST_EXP_FAIL(do_quotactl(fd, tc->cmd, "/dev/null", *tc->id, tc->addr),
> +			ENOTBLK, "do_quotactl()");
> +	else
> +		TST_EXP_FAIL(do_quotactl(tc->use_external_fd ? external_fd : fd, tc->cmd,
> +			tst_device->dev, *tc->id, tc->addr), tc->exp_err, "do_quotactl()");
> +
> +	if (quota_on) {
> +		TST_EXP_PASS_SILENT(do_quotactl(fd, QCMD(Q_QUOTAOFF, USRQUOTA), tst_device->dev,
> +			fmt_id, NULL), "do_quotactl(QCMD(Q_QUOTAOFF, USRQUOTA)");
> +		if (!TST_PASS)
> +			return;
> +		quota_on = 0;

There is no need to clear this flag.

> +	}
> +
> +	if (drop_flag) {
> +		tst_cap_action(&needadmin);
> +		drop_flag = 0;

And this flag.

> +	}
> +}
> +
> +static void setup(void)
> +{
> +	unsigned int i;
> +	const char *const fs_opts[] = { "-O quota", NULL};
> +
> +	quotactl_info();
> +	external_fd = SAFE_CREAT("testfile", O_RDONLY);
> +
> +	SAFE_MKFS(tst_device->dev, tst_device->fs_type, fs_opts, NULL);
> +	SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);

Why don't we pass the fs_opts in tst_test instead and use .mount_device = 1?

Should be as easy as:

static struct tst_test test = {
	...
	.dev_fs_opts = (const char *const []){"-O quota", NULL},
	.mount_device = 1,
	...
};

Or is there a reason why this wouldn't work?


With the changes outlined in the other email and the minor issues I've
pointed out here fixed:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Yang Xu Jan. 14, 2022, 2:27 a.m. UTC | #6 
Hi Cyril
> Hi!
>> +/*\
>> + * [Description]
>> + *
>> + * Tests basic error handling of the quotactl syscall without visible quota files
>> + * (use quotactl and quotactl_fd syscall):
>> + *
>> + * - EFAULT when addr or special is invalid
>> + * - EINVAL when cmd or type is invalid
>> + * - ENOTBLK when special is not a block device
>> + * - ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the range
>> + *   allowed by the quota format
>> + * - EPERM when the caller lacked the required privilege (CAP_SYS_ADMIN) for the
>> + *   specified operation
>> + * - EINVAL when cmd is Q_QUOTAON, but the fd refers to a regular file(doesn't
>> + *   under this moutpoint)
>> + *
>> + * Minimum e2fsprogs version required is 1.43.
>> + */
>> +
>> +#include<errno.h>
>> +#include<sys/quota.h>
>> +#include "tst_test.h"
>> +#include "tst_capability.h"
>> +#include "quotactl_syscall_var.h"
>> +
>> +#define OPTION_INVALID 999
>> +
>> +static int32_t fmt_id = QFMT_VFS_V1;
>> +static int test_id, mount_flag;
>> +static int getnextquota_nsup, external_fd = -1;
>> +
>> +static struct if_nextdqblk res_ndq;
>> +
>> +static struct dqblk set_dqmax = {
>> +	.dqb_bsoftlimit = 0x7fffffffffffffffLL,  /* 2^63-1 */
>> +	.dqb_valid = QIF_BLIMITS
>> +};
>> +
>> +static struct tst_cap dropadmin = {
>> +	.action = TST_CAP_DROP,
>> +	.id = CAP_SYS_ADMIN,
>> +	.name = "CAP_SYS_ADMIN",
>> +};
>> +
>> +static struct tst_cap needadmin = {
>> +	.action = TST_CAP_REQ,
>> +	.id = CAP_SYS_ADMIN,
>> +	.name = "CAP_SYS_ADMIN",
>> +};
>> +
>> +static struct tcase {
>> +	int cmd;
>> +	int *id;
>> +	void *addr;
>> +	int exp_err;
>> +	int on_flag;
>> +	int use_external_fd;
>> +	char *des;
>> +} tcases[] = {
>> +	{QCMD(Q_SETQUOTA, USRQUOTA),&fmt_id, NULL, EFAULT, 1, 0,
>> +	"EFAULT when addr or special is invalid"},
>> +
>> +	{QCMD(OPTION_INVALID, USRQUOTA),&fmt_id, NULL, EINVAL, 0, 0,
>> +	"EINVAL when cmd or type is invalid"},
>> +
>> +	{QCMD(Q_QUOTAON, USRQUOTA),&fmt_id, NULL, ENOTBLK, 0, 0,
>> +	"ENOTBLK when special is not a block device"},
>> +
>> +	{QCMD(Q_SETQUOTA, USRQUOTA),&test_id,&set_dqmax, ERANGE, 1, 0,
>> +	"ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the range"},
>> +
>> +	{QCMD(Q_QUOTAON, USRQUOTA),&fmt_id, NULL, EPERM, 0, 0,
>> +	"EPERM when the caller lacked the required privilege for specified operations"},
>> +
>> +	{QCMD(Q_QUOTAON, USRQUOTA),&fmt_id, NULL, EINVAL, 0, 1,
>> +	"EINVAL when cmd is Q_QUOTAON, but the fd refers to a regular file(doesn't under "
>> +	"this mountpoint)"}
>> +};
>> +
>> +static void verify_quotactl(unsigned int n)
>> +{
>> +	struct tcase *tc =&tcases[n];
>> +	int quota_on = 0;
>> +	int drop_flag = 0;
>> +
>> +	tst_res(TINFO, "Testing %s", tc->des);
>> +	if (tc->cmd == QCMD(Q_GETNEXTQUOTA, USRQUOTA)&&  getnextquota_nsup) {
>> +		tst_res(TCONF, "current system doesn't support Q_GETNEXTQUOTA");
>> +		return;
>> +	}
>> +
>> +	if (tc->on_flag) {
>> +		TST_EXP_PASS_SILENT(do_quotactl(fd, QCMD(Q_QUOTAON, USRQUOTA), tst_device->dev,
>> +			fmt_id, NULL), "do_quotactl(QCMD(Q_QUOTAON, USRQUOTA))");
>> +		if (!TST_PASS)
>> +			return;
>> +		quota_on = 1;
>> +	}
>> +
>> +	if (tc->exp_err == EPERM) {
>> +		tst_cap_action(&dropadmin);
>> +		drop_flag = 1;
>> +	}
>> +
>> +	if (tst_variant) {
>> +		if (tc->exp_err == ENOTBLK) {
>> +			tst_res(TCONF, "quotactl_fd() doesn't have this error, skip");
>> +			return;
>> +		}
>> +	} else {
>> +		if (tc->use_external_fd) {
>> +			tst_res(TCONF, "quotactl() doesn't use fd, skip");
>> +			return;
>> +		}
>> +	}
>> +	if (tc->exp_err == ENOTBLK)
>> +		TST_EXP_FAIL(do_quotactl(fd, tc->cmd, "/dev/null", *tc->id, tc->addr),
>> +			ENOTBLK, "do_quotactl()");
>> +	else
>> +		TST_EXP_FAIL(do_quotactl(tc->use_external_fd ? external_fd : fd, tc->cmd,
>> +			tst_device->dev, *tc->id, tc->addr), tc->exp_err, "do_quotactl()");
>> +
>> +	if (quota_on) {
>> +		TST_EXP_PASS_SILENT(do_quotactl(fd, QCMD(Q_QUOTAOFF, USRQUOTA), tst_device->dev,
>> +			fmt_id, NULL), "do_quotactl(QCMD(Q_QUOTAOFF, USRQUOTA)");
>> +		if (!TST_PASS)
>> +			return;
>> +		quota_on = 0;
>
> There is no need to clear this flag.
Yes, good catch, will also remove this in quotactl06.c.
>
>> +	}
>> +
>> +	if (drop_flag) {
>> +		tst_cap_action(&needadmin);
>> +		drop_flag = 0;
>
> And this flag.
>
>> +	}
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	unsigned int i;
>> +	const char *const fs_opts[] = { "-O quota", NULL};
>> +
>> +	quotactl_info();
>> +	external_fd = SAFE_CREAT("testfile", O_RDONLY);
>> +
>> +	SAFE_MKFS(tst_device->dev, tst_device->fs_type, fs_opts, NULL);
>> +	SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
>
> Why don't we pass the fs_opts in tst_test instead and use .mount_device = 1?
The code is pasted from quotactl06.c there use 
tst_require_quota_support. Here we don't use it, so we can use 
.mount_device directly.

Best Regards
Yang Xu
>
> Should be as easy as:
>
> static struct tst_test test = {
> 	...
> 	.dev_fs_opts = (const char *const []){"-O quota", NULL},
> 	.mount_device = 1,
> 	...
> };
>
> Or is there a reason why this wouldn't work?
>
>
> With the changes outlined in the other email and the minor issues I've
> pointed out here fixed:
>
> Reviewed-by: Cyril Hrubis<chrubis@suse.cz>

>
>
diff mbox series

Patch

diff --git a/runtest/syscalls b/runtest/syscalls
index a0ca84b36..3b2deb64e 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1073,6 +1073,7 @@  quotactl05 quotactl05
 quotactl06 quotactl06
 quotactl07 quotactl07
 quotactl08 quotactl08
+quotactl09 quotactl09
 
 read01 read01
 read02 read02
diff --git a/testcases/kernel/syscalls/quotactl/.gitignore b/testcases/kernel/syscalls/quotactl/.gitignore
index dab9b3420..94de2c8f2 100644
--- a/testcases/kernel/syscalls/quotactl/.gitignore
+++ b/testcases/kernel/syscalls/quotactl/.gitignore
@@ -6,3 +6,4 @@ 
 /quotactl06
 /quotactl07
 /quotactl08
+/quotactl09
diff --git a/testcases/kernel/syscalls/quotactl/quotactl09.c b/testcases/kernel/syscalls/quotactl/quotactl09.c
new file mode 100644
index 000000000..79f506bdc
--- /dev/null
+++ b/testcases/kernel/syscalls/quotactl/quotactl09.c
@@ -0,0 +1,195 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Tests basic error handling of the quotactl syscall without visible quota files
+ * (use quotactl and quotactl_fd syscall):
+ *
+ * - EFAULT when addr or special is invalid
+ * - EINVAL when cmd or type is invalid
+ * - ENOTBLK when special is not a block device
+ * - ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the range
+ *   allowed by the quota format
+ * - EPERM when the caller lacked the required privilege (CAP_SYS_ADMIN) for the
+ *   specified operation
+ * - EINVAL when cmd is Q_QUOTAON, but the fd refers to a regular file(doesn't
+ *   under this moutpoint)
+ *
+ * Minimum e2fsprogs version required is 1.43.
+ */
+
+#include <errno.h>
+#include <sys/quota.h>
+#include "tst_test.h"
+#include "tst_capability.h"
+#include "quotactl_syscall_var.h"
+
+#define OPTION_INVALID 999
+
+static int32_t fmt_id = QFMT_VFS_V1;
+static int test_id, mount_flag;
+static int getnextquota_nsup, external_fd = -1;
+
+static struct if_nextdqblk res_ndq;
+
+static struct dqblk set_dqmax = {
+	.dqb_bsoftlimit = 0x7fffffffffffffffLL,  /* 2^63-1 */
+	.dqb_valid = QIF_BLIMITS
+};
+
+static struct tst_cap dropadmin = {
+	.action = TST_CAP_DROP,
+	.id = CAP_SYS_ADMIN,
+	.name = "CAP_SYS_ADMIN",
+};
+
+static struct tst_cap needadmin = {
+	.action = TST_CAP_REQ,
+	.id = CAP_SYS_ADMIN,
+	.name = "CAP_SYS_ADMIN",
+};
+
+static struct tcase {
+	int cmd;
+	int *id;
+	void *addr;
+	int exp_err;
+	int on_flag;
+	int use_external_fd;
+	char *des;
+} tcases[] = {
+	{QCMD(Q_SETQUOTA, USRQUOTA), &fmt_id, NULL, EFAULT, 1, 0,
+	"EFAULT when addr or special is invalid"},
+
+	{QCMD(OPTION_INVALID, USRQUOTA), &fmt_id, NULL, EINVAL, 0, 0,
+	"EINVAL when cmd or type is invalid"},
+
+	{QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, ENOTBLK, 0, 0,
+	"ENOTBLK when special is not a block device"},
+
+	{QCMD(Q_SETQUOTA, USRQUOTA), &test_id, &set_dqmax, ERANGE, 1, 0,
+	"ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the range"},
+
+	{QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, EPERM, 0, 0,
+	"EPERM when the caller lacked the required privilege for specified operations"},
+
+	{QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, NULL, EINVAL, 0, 1,
+	"EINVAL when cmd is Q_QUOTAON, but the fd refers to a regular file(doesn't under "
+	"this mountpoint)"}
+};
+
+static void verify_quotactl(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+	int quota_on = 0;
+	int drop_flag = 0;
+
+	tst_res(TINFO, "Testing %s", tc->des);
+	if (tc->cmd == QCMD(Q_GETNEXTQUOTA, USRQUOTA) && getnextquota_nsup) {
+		tst_res(TCONF, "current system doesn't support Q_GETNEXTQUOTA");
+		return;
+	}
+
+	if (tc->on_flag) {
+		TST_EXP_PASS_SILENT(do_quotactl(fd, QCMD(Q_QUOTAON, USRQUOTA), tst_device->dev,
+			fmt_id, NULL), "do_quotactl(QCMD(Q_QUOTAON, USRQUOTA))");
+		if (!TST_PASS)
+			return;
+		quota_on = 1;
+	}
+
+	if (tc->exp_err == EPERM) {
+		tst_cap_action(&dropadmin);
+		drop_flag = 1;
+	}
+
+	if (tst_variant) {
+		if (tc->exp_err == ENOTBLK) {
+			tst_res(TCONF, "quotactl_fd() doesn't have this error, skip");
+			return;
+		}
+	} else {
+		if (tc->use_external_fd) {
+			tst_res(TCONF, "quotactl() doesn't use fd, skip");
+			return;
+		}
+	}
+	if (tc->exp_err == ENOTBLK)
+		TST_EXP_FAIL(do_quotactl(fd, tc->cmd, "/dev/null", *tc->id, tc->addr),
+			ENOTBLK, "do_quotactl()");
+	else
+		TST_EXP_FAIL(do_quotactl(tc->use_external_fd ? external_fd : fd, tc->cmd,
+			tst_device->dev, *tc->id, tc->addr), tc->exp_err, "do_quotactl()");
+
+	if (quota_on) {
+		TST_EXP_PASS_SILENT(do_quotactl(fd, QCMD(Q_QUOTAOFF, USRQUOTA), tst_device->dev,
+			fmt_id, NULL), "do_quotactl(QCMD(Q_QUOTAOFF, USRQUOTA)");
+		if (!TST_PASS)
+			return;
+		quota_on = 0;
+	}
+
+	if (drop_flag) {
+		tst_cap_action(&needadmin);
+		drop_flag = 0;
+	}
+}
+
+static void setup(void)
+{
+	unsigned int i;
+	const char *const fs_opts[] = { "-O quota", NULL};
+
+	quotactl_info();
+	external_fd = SAFE_CREAT("testfile", O_RDONLY);
+
+	SAFE_MKFS(tst_device->dev, tst_device->fs_type, fs_opts, NULL);
+	SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
+	mount_flag = 1;
+
+	fd = SAFE_OPEN(MNTPOINT, O_RDONLY);
+	TEST(do_quotactl(fd, QCMD(Q_GETNEXTQUOTA, USRQUOTA), tst_device->dev,
+		test_id, (void *) &res_ndq));
+	if (TST_ERR == EINVAL || TST_ERR == ENOSYS)
+		getnextquota_nsup = 1;
+
+	for (i = 0; i < ARRAY_SIZE(tcases); i++) {
+		if (!tcases[i].addr)
+			tcases[i].addr = tst_get_bad_addr(NULL);
+	}
+}
+
+static void cleanup(void)
+{
+	if (fd > -1)
+		SAFE_CLOSE(fd);
+	if (external_fd > -1)
+		SAFE_CLOSE(external_fd);
+	if (mount_flag && tst_umount(MNTPOINT))
+		tst_res(TWARN | TERRNO, "umount(%s)", MNTPOINT);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_QFMT_V2",
+		NULL
+	},
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = verify_quotactl,
+	.dev_fs_type = "ext4",
+	.mntpoint = MNTPOINT,
+	.needs_device = 1,
+	.needs_root = 1,
+	.test_variants = QUOTACTL_SYSCALL_VARIANTS,
+	.needs_cmds = (const char *[]) {
+		"mkfs.ext4 >= 1.43.0",
+		NULL
+	}
+};