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

[Seth Arnold]

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 <seth.arnold at canonical.com>

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 <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;
>  }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170405/d4d0678e/attachment.pgp>