On Wed, Apr 05, 2017 at 11:57:01AM -0700, Steve Beattie wrote: > On Tue, Apr 04, 2017 at 03:41:41PM -0500, Tyler Hicks wrote: > > I didn't mean to make this simple test improvement turn into something > > complex. I'm willing to ack your original patch if you don't see a quick > > and easy solution to this issue. > > Nah, let's do it right. V2 of the patch follows. Changes since v1: > > - compile error if neither SYS_getdents or SYS_getdents is defined
The only thing I spotted is this :) ^^^ duplciated "SYS_getdents". Acked-by: Seth Arnold <[email protected]> Thanks > - verify that getdents() and getdents64() return the same value if both > are available. > - propagate errno from getdents/getdents64 if failure occurs, instead > of synthetic error value. > > (For reviewing, because so much changes, it might just be easier to > review readdir.c after applying, rather than the diff itself.) > > Subject: tests: readdir - test both getdents() and getdents64() if available > > In commit 3649, Colin King fixed the readdir test build issue where > aarch64 only supports getdetns64(), not getdents(). Realistically, > however, we want to ensure mediation occurs on both syscalls where > they exist. This patch changes the test to attempt performing both > versions of getdents(). Also, if both versions exist, return a > failure if the result of each differs from the other. > > Bug: https://bugs.launchpad.net/bugs/1674245 > > Signed-off-by: Steve Beattie <[email protected]> > --- > tests/regression/apparmor/readdir.c | 101 > +++++++++++++++++++++++++++--------- > 1 file changed, 76 insertions(+), 25 deletions(-) > > Index: b/tests/regression/apparmor/readdir.c > =================================================================== > --- a/tests/regression/apparmor/readdir.c > +++ b/tests/regression/apparmor/readdir.c > @@ -1,10 +1,14 @@ > /* > * Copyright (C) 2002-2006 Novell/SUSE > + * Copyright (C) 2017 Canonical, Ltd. > * > * This program is free software; you can redistribute it and/or > * modify it under the terms of the GNU General Public License as > * published by the Free Software Foundation, version 2 of the > * License. > + * > + * We attempt to test both getdents() and getdents64() here, but > + * some architectures like aarch64 only support getdents64(). > */ > #define _GNU_SOURCE > > @@ -18,45 +22,92 @@ > #include <string.h> > #include <sys/syscall.h> > > -int main(int argc, char *argv[]) > +/* error if neither SYS_getdents or SYS_getdents64 is defined */ > +#if !defined(SYS_getdents) && !defined(SYS_getdents64) > + #error "Neither SYS_getdents or SYS_getdents64 is defined, something has > gone wrong!" > +#endif > + > +#ifdef SYS_getdents > +int my_readdir(char *dirname) > { > int fd; > -#if defined(SYS_getdents64) > - struct dirent64 dir; > -#else > struct dirent dir; > -#endif > > - if (argc != 2){ > - fprintf(stderr, "usage: %s dir\n", > - argv[0]); > - return 1; > + fd = open(dirname, O_RDONLY, 0); > + if (fd == -1) { > + printf("FAIL - open %s\n", strerror(errno)); > + return errno; > } > > - fd = open(argv[1], O_RDONLY, 0); > - if (fd == -1){ > - printf("FAIL - open %s\n", strerror(errno)); > - return 1; > + /* getdents isn't exported by glibc, so must use syscall() */ > + if (syscall(SYS_getdents, fd, &dir, sizeof(dir)) == -1){ > + printf("FAIL - getdents %s\n", strerror(errno)); > + close(fd); > + return errno; > } > > - /* > - if (fchdir(fd) == -1){ > - printf("FAIL - fchdir %s\n", strerror(errno)); > - return 1; > + close(fd); > + return 0; > +} > +#endif > + > +#ifdef SYS_getdents64 > +int my_readdir64(char *dirname) > +{ > + int fd; > + struct dirent64 dir; > + > + fd = open(dirname, O_RDONLY, 0); > + if (fd == -1) { > + printf("FAIL - open %s\n", strerror(errno)); > + return errno; > } > - */ > > /* getdents isn't exported by glibc, so must use syscall() */ > -#if defined(SYS_getdents64) > - if (syscall(SYS_getdents64, fd, &dir, sizeof(struct dirent64)) == -1){ > -#else > - if (syscall(SYS_getdents, fd, &dir, sizeof(struct dirent)) == -1){ > + if (syscall(SYS_getdents64, fd, &dir, sizeof(dir)) == -1){ > + printf("FAIL - getdents64 %s\n", strerror(errno)); > + close(fd); > + return errno; > + } > + > + close(fd); > + return 0; > +} > #endif > - printf("FAIL - getdents %s\n", strerror(errno)); > - return 1; > + > +int main(int argc, char *argv[]) > +{ > + int rc = 0, rc64 = 0, err = 0; > + > + if (argc != 2) { > + fprintf(stderr, "usage: %s dir\n", > + argv[0]); > + err = 1; > + goto err; > + } > + > +#ifdef SYS_getdents > + rc = my_readdir(argv[1]); > + err |= rc; > +#endif > + > +#ifdef SYS_getdents64 > + rc64 = my_readdir64(argv[1]); > + err |= rc64; > +#endif > + > +#if defined(SYS_getdents) && defined(SYS_getdents64) > + if (rc != rc64) { > + printf("FAIL - getdents and getdents64 returned different > values: %d vs %d\n", rc, rc64); > + err = 1; > } > +#endif > + > + if (err != 0) > + goto err; > > printf("PASS\n"); > > - return 0; > +err: > + return err; > }
signature.asc
Description: PGP signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
