<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Jul 10, 2018 at 7:48 PM Dmitry V. Levin <<a href="mailto:ldv@altlinux.org">ldv@altlinux.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Jul 09, 2018 at 12:11:31AM +0800, Zhibin Li wrote:<br>
> Use check_quota function for invalid commands instead of manually syscall invocation.<br>
<br>
instead of manual quotactl syscall invocations.<br></blockquote><div>My mistake, it should be adjective instead of adverb :) </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> In this way injection can also be checked for these commands in the next commit.<br>
> <br>
> * tests/quotactl.c: (print_dqfmt): New function.<br>
<br>
* tests/quotactl.c (print_dqfmt): New function.<br>
<br>
> (invalid_cmd_str, invalid_id_str): New variables.<br>
<br>
(main): Add invalid_cmd_str and invalid_id_str local variables,<br>
remove unused variable rc, use check_quota and print_dqfmt instead of<br>
manual quotactl syscall invocations.<br>
<br>
> * tests/quotactl-xfs.c: Removed unused variable rc.<br>
> (invalid_cmd): New variable.<br>
<br>
* tests/quotactl-xfs.c (main): : Add invalid_cmd variable, remove unused<br>
variable rc, use check_quota instead of manual quotactl syscall<br>
invocations.<br>
<br></blockquote><div>I will revise the commit message to make it more clear (I should've taken more references).</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> ---<br>
>  tests/quotactl-xfs.c | 35 +++++++++++++++---------<br>
>  tests/quotactl.c     | 76 +++++++++++++++++++++++++++++++++++++++-------------<br>
>  2 files changed, 80 insertions(+), 31 deletions(-)<br>
> <br>
> diff --git a/tests/quotactl-xfs.c b/tests/quotactl-xfs.c<br>
> index 036b61d2..c08b1584 100644<br>
> --- a/tests/quotactl-xfs.c<br>
> +++ b/tests/quotactl-xfs.c<br>
> @@ -207,7 +207,7 @@ main(void)<br>
>       char bogus_addr_str[sizeof(void *) * 2 + sizeof("0x")];<br>
>       char unterminated_str[sizeof(void *) * 2 + sizeof("0x")];<br>
>  <br>
> -     long rc;<br>
> +     static char invalid_cmd[1024];<br>
>       TAIL_ALLOC_OBJECT_CONST_PTR(struct fs_disk_quota, xdq);<br>
>       TAIL_ALLOC_OBJECT_CONST_PTR(struct fs_quota_stat, xqstat);<br>
>       TAIL_ALLOC_OBJECT_CONST_PTR(struct fs_quota_statv, xqstatv);<br>
> @@ -234,12 +234,12 @@ main(void)<br>
>                   "|XFS_QUOTA_GDQ_ACCT|XFS_QUOTA_GDQ_ENFD"<br>
>                   "|XFS_QUOTA_PDQ_ENFD|0xdeadbec0]");<br>
>  <br>
> -     rc = syscall(__NR_quotactl, QCMD(Q_XQUOTAON, 0xfacefeed), bogus_dev,<br>
> -                  bogus_id, bogus_addr);<br>
> -     printf("quotactl(QCMD(Q_XQUOTAON, %#x /* ???QUOTA */)"<br>
> -            ", %s, %p) = %s\n",<br>
> -            QCMD_TYPE(QCMD(Q_XQUOTAON, 0xfacefeed)),<br>
> -            bogus_dev_str, bogus_addr, sprintrc(rc));<br>
> +     snprintf(invalid_cmd, sizeof(invalid_cmd),<br>
> +              "QCMD(Q_XQUOTAON, %#x /* ???QUOTA */)",<br>
> +              QCMD_TYPE(QCMD(Q_XQUOTAON, 0xfacefeed)));<br>
> +     check_quota(CQF_ID_SKIP,<br>
> +                 QCMD(Q_XQUOTAON, 0xfacefeed), invalid_cmd,<br>
> +                 bogus_dev, bogus_dev_str, bogus_addr);<br>
>  <br>
>  <br>
>       /* Q_XQUOTAOFF */<br>
> @@ -264,11 +264,11 @@ main(void)<br>
>       /* Q_XGETQUOTA */<br>
>  <br>
>       /* Trying our best to get successful result */<br>
> -     check_quota(CQF_ADDR_CB, ARG_STR(QCMD(Q_GETQUOTA, USRQUOTA)),<br>
> +     check_quota(CQF_ADDR_CB, ARG_STR(QCMD(Q_XGETQUOTA, USRQUOTA)),<br>
>                   ARG_STR("/dev/sda1"), getuid(), xdq, print_xdisk_quota,<br>
>                   (intptr_t) 1);<br>
<br>
This change is quite different from the description.<br>
If it's a fix (I bet it is), it should rather go to a separate commit.<br></blockquote><div>I got it. Sorry for messing fixes and improvements up in one patch. I should</div><div>send them separately. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> -     check_quota(CQF_ADDR_CB, ARG_STR(QCMD(Q_GETQUOTA, GRPQUOTA)),<br>
> +     check_quota(CQF_ADDR_CB, ARG_STR(QCMD(Q_XGETQUOTA, GRPQUOTA)),<br>
>                   ARG_STR(NULL), -1, xdq, print_xdisk_quota, (intptr_t) 2);<br>
<br>
Likewise.<br>
<br>
> @@ -298,21 +298,30 @@ main(void)<br>
>                   ARG_STR("/dev/sda1"), xqstat, print_xquota_stat, (intptr_t) 1);<br>
>  <br>
>       check_quota(CQF_ID_SKIP | CQF_ADDR_CB,<br>
> -                 ARG_STR(QCMD(Q_XGETQSTATV, PRJQUOTA)),<br>
> +                 ARG_STR(QCMD(Q_XGETQSTAT, USRQUOTA)),<br>
> +                 ARG_STR("NULL"), xqstat, print_xquota_stat, (intptr_t) 2);<br>
<br>
Likewise.<br>
<br>
> +     check_quota(CQF_ID_SKIP,<br>
> +                 ARG_STR(QCMD(Q_XGETQSTAT, PRJQUOTA)),<br>
>                   unterminated, unterminated_str,<br>
> -                 xqstat + 1, print_xquota_stat, (intptr_t) 2);<br>
> +                 xqstat + 1);<br>
>  <br>
>  <br>
>       /* Q_XGETQSTATV */<br>
>  <br>
>       check_quota(CQF_ID_SKIP | CQF_ADDR_CB,<br>
> -                 ARG_STR(QCMD(Q_XGETQSTAT, USRQUOTA)),<br>
> -                 ARG_STR("/dev/sda1"), xqstatv, print_xquota_statv, 1);<br>
> +                 ARG_STR(QCMD(Q_XGETQSTATV, USRQUOTA)),<br>
> +                 ARG_STR("/dev/sda1"), xqstatv, print_xquota_statv, (intptr_t) 1);<br>
<br>
Likewise.<br>
<br>
<br>
-- <br>
ldv<br>
</blockquote></div></div>