| Message ID | 20200508193721.29283-1-stephen.smalley.work@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [testsuite] tests/filesystem: fix quotas_test | expand |
On Fri, May 8, 2020 at 9:37 PM Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > As per the man page, quotactl(2) expects to be passed a pointer to > a 4-byte buffer for Q_GETFMT. The kernel copies a single u32 value. > On Ubuntu, this was detected as a stack smash when running the test. > Fix the test program. > > Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com> > --- > tests/filesystem/quotas_test.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/tests/filesystem/quotas_test.c b/tests/filesystem/quotas_test.c > index 8359811..1424362 100644 > --- a/tests/filesystem/quotas_test.c > +++ b/tests/filesystem/quotas_test.c > @@ -33,7 +33,7 @@ int main(int argc, char *argv[]) > int opt, result, qcmd, save_err, test_id = geteuid(); > char *context, *src = NULL, *tgt = NULL; > bool verbose = false; > - char fmt_buf[2]; > + unsigned int fmtval; I wish we could use something like uint32_t here to make the size explicit, but that's a C99 thing... OTOH the binder test code already happily uses C99 stuff, so I'm not sure how to best handle this... (Add -std=c99 to CFLAGS? Just use <stdint.h> and assume the compiler has it? Avoid using C99 library features?) > > while ((opt = getopt(argc, argv, "s:t:v")) != -1) { > switch (opt) { > @@ -77,7 +77,7 @@ int main(int argc, char *argv[]) > printf("User Quota - ON\n"); > > qcmd = QCMD(Q_GETFMT, USRQUOTA); > - result = quotactl(qcmd, src, test_id, fmt_buf); > + result = quotactl(qcmd, src, test_id, (caddr_t)&fmtval); > save_err = errno; > if (result < 0) { > fprintf(stderr, "quotactl(Q_GETFMT, USRQUOTA) Failed: %s\n", > @@ -85,7 +85,7 @@ int main(int argc, char *argv[]) > return save_err; > } > if (verbose) > - printf("User Format: 0x%x\n", fmt_buf[0]); > + printf("User Format: 0x%x\n", fmtval); > > qcmd = QCMD(Q_QUOTAOFF, USRQUOTA); > result = quotactl(qcmd, src, QFMT_VFS_V0, tgt); > @@ -113,7 +113,7 @@ int main(int argc, char *argv[]) > printf("Group Quota - ON\n"); > > qcmd = QCMD(Q_GETFMT, GRPQUOTA); > - result = quotactl(qcmd, src, test_id, fmt_buf); > + result = quotactl(qcmd, src, test_id, (caddr_t)&fmtval); > save_err = errno; > if (result < 0) { > fprintf(stderr, "quotactl(Q_GETFMT, GRPQUOTA) Failed: %s\n", > @@ -121,7 +121,7 @@ int main(int argc, char *argv[]) > return save_err; > } > if (verbose) > - printf("Group Format: 0x%x\n", fmt_buf[0]); > + printf("Group Format: 0x%x\n", fmtval); > > qcmd = QCMD(Q_QUOTAOFF, GRPQUOTA); > result = quotactl(qcmd, src, QFMT_VFS_V0, tgt); > -- > 2.23.1 > -- Ondrej Mosnacek <omosnace at redhat dot com> Software Engineer, Security Technologies Red Hat, Inc.
On Mon, May 11, 2020 at 8:05 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > On Fri, May 8, 2020 at 9:37 PM Stephen Smalley > <stephen.smalley.work@gmail.com> wrote: > > As per the man page, quotactl(2) expects to be passed a pointer to > > a 4-byte buffer for Q_GETFMT. The kernel copies a single u32 value. > > On Ubuntu, this was detected as a stack smash when running the test. > > Fix the test program. > > > > Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com> > > --- > > tests/filesystem/quotas_test.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/tests/filesystem/quotas_test.c b/tests/filesystem/quotas_test.c > > index 8359811..1424362 100644 > > --- a/tests/filesystem/quotas_test.c > > +++ b/tests/filesystem/quotas_test.c > > @@ -33,7 +33,7 @@ int main(int argc, char *argv[]) > > int opt, result, qcmd, save_err, test_id = geteuid(); > > char *context, *src = NULL, *tgt = NULL; > > bool verbose = false; > > - char fmt_buf[2]; > > + unsigned int fmtval; > > I wish we could use something like uint32_t here to make the size > explicit, but that's a C99 thing... OTOH the binder test code already > happily uses C99 stuff, so I'm not sure how to best handle this... > (Add -std=c99 to CFLAGS? Just use <stdint.h> and assume the compiler > has it? Avoid using C99 library features?) I'll add stdint.h and use uint32_t. We already use the fixed-size types in a couple other tests and in the userspace.
diff --git a/tests/filesystem/quotas_test.c b/tests/filesystem/quotas_test.c index 8359811..1424362 100644 --- a/tests/filesystem/quotas_test.c +++ b/tests/filesystem/quotas_test.c @@ -33,7 +33,7 @@ int main(int argc, char *argv[]) int opt, result, qcmd, save_err, test_id = geteuid(); char *context, *src = NULL, *tgt = NULL; bool verbose = false; - char fmt_buf[2]; + unsigned int fmtval; while ((opt = getopt(argc, argv, "s:t:v")) != -1) { switch (opt) { @@ -77,7 +77,7 @@ int main(int argc, char *argv[]) printf("User Quota - ON\n"); qcmd = QCMD(Q_GETFMT, USRQUOTA); - result = quotactl(qcmd, src, test_id, fmt_buf); + result = quotactl(qcmd, src, test_id, (caddr_t)&fmtval); save_err = errno; if (result < 0) { fprintf(stderr, "quotactl(Q_GETFMT, USRQUOTA) Failed: %s\n", @@ -85,7 +85,7 @@ int main(int argc, char *argv[]) return save_err; } if (verbose) - printf("User Format: 0x%x\n", fmt_buf[0]); + printf("User Format: 0x%x\n", fmtval); qcmd = QCMD(Q_QUOTAOFF, USRQUOTA); result = quotactl(qcmd, src, QFMT_VFS_V0, tgt); @@ -113,7 +113,7 @@ int main(int argc, char *argv[]) printf("Group Quota - ON\n"); qcmd = QCMD(Q_GETFMT, GRPQUOTA); - result = quotactl(qcmd, src, test_id, fmt_buf); + result = quotactl(qcmd, src, test_id, (caddr_t)&fmtval); save_err = errno; if (result < 0) { fprintf(stderr, "quotactl(Q_GETFMT, GRPQUOTA) Failed: %s\n", @@ -121,7 +121,7 @@ int main(int argc, char *argv[]) return save_err; } if (verbose) - printf("Group Format: 0x%x\n", fmt_buf[0]); + printf("Group Format: 0x%x\n", fmtval); qcmd = QCMD(Q_QUOTAOFF, GRPQUOTA); result = quotactl(qcmd, src, QFMT_VFS_V0, tgt);
As per the man page, quotactl(2) expects to be passed a pointer to a 4-byte buffer for Q_GETFMT. The kernel copies a single u32 value. On Ubuntu, this was detected as a stack smash when running the test. Fix the test program. Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com> --- tests/filesystem/quotas_test.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)