[apparmor] [patch v2] tests: readdir - test both getdents() and getdents64() if available

[Steve Beattie]

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
 - 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 <steve at nxnw.org>
---
 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;
 }

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170405/449c3d1e/attachment.pgp>