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

Tyler Hicks tyhicks at canonical.com
Tue Apr 4 19:03:53 UTC 2017


On 04/04/2017 01:14 PM, Steve Beattie wrote:
> Hey Colin,
> 
> On Tue, Apr 04, 2017 at 03:16:29PM -0000, Colin Ian King wrote:
>> Colin Ian King has proposed merging lp:~colin-king/apparmor/fix-arm64-test-builds into lp:apparmor.
>>
>> Requested reviews:
>>   AppArmor Developers (apparmor-dev)
>>
>> For more details, see:
>> https://code.launchpad.net/~colin-king/apparmor/fix-arm64-test-builds/+merge/321876
>>
>> This fixes build issues for the readdir test for arm64 where getdents(2)
>> is not wired up as a system call but gendents64(2) is available.
>> This changes the preference to use the 64 bit system call by default
>> if it is available on 64 bit systems.
> 
> Thanks for providing this fix.
> 
> Realastically, though, where an architecture supports both getdents()
> and getdents64(), we ought to be ensuring that apparmor mediation occurs
> on both syscall paths. What follows is a patch that attempts to do that.

Smart thinking. I like the idea but have one nitpick and one objection
below...

> 
> 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().
> 
> Bug: https://bugs.launchpad.net/bugs/1674245
> 
> Signed-off-by: Steve Beattie <steve at nxnw.org>
> ---
>  tests/regression/apparmor/readdir.c |   80 ++++++++++++++++++++++++++----------
>  1 file changed, 59 insertions(+), 21 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,79 @@
>  #include <string.h>
>  #include <sys/syscall.h>
>  
> -int main(int argc, char *argv[])
> +#ifdef SYS_getdents
> +int my_readdir(char *dirname)

Nitpick...

Please use my_getdents() and my_getdents64() since these are getdents
wrappers and not readdir wrappers.

>  {
>  	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]);
> +	fd = open(dirname, O_RDONLY, 0);
> +	if (fd == -1) {
> +		printf("FAIL - open  %s\n", strerror(errno));
>  		return 1;
>  	}
>  
> -	fd = open(argv[1], O_RDONLY, 0);
> -	if (fd == -1){
> -		printf("FAIL - open  %s\n", strerror(errno));
> +	/* 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 1;
>  	}
>  
> -	/*
> -	if (fchdir(fd) == -1){
> -		printf("FAIL - fchdir  %s\n", strerror(errno));
> +	close(fd);
> +	return 0;
> +}
> +#else
> +int my_readdir(char *dirname) { 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 1;
>  	}
> -	*/
>  
>  	/* getdents isn't exported by glibc, so must use syscall() */
> -#if defined(SYS_getdents64)
> -	if (syscall(SYS_getdents64, fd, &dir, sizeof(struct dirent64)) == -1){
> +	if (syscall(SYS_getdents64, fd, &dir, sizeof(dir)) == -1){
> +		printf("FAIL - getdents64  %s\n", strerror(errno));
> +		close(fd);
> +		return 1;
> +	}
> +
> +	close(fd);
> +	return 0;
> +}
>  #else
> -	if (syscall(SYS_getdents, fd, &dir, sizeof(struct dirent)) == -1){
> +int my_readdir64(char *dirname) { return 0; }
>  #endif
> -		printf("FAIL - getdents  %s\n", strerror(errno));
> -		return 1;
> +
> +int main(int argc, char *argv[])
> +{
> +	int rc = 0;
> +
> +	if (argc != 2) {
> +		fprintf(stderr, "usage: %s dir\n",
> +			argv[0]);
> +		rc = 1;
> +		goto err;
>  	}
>  
> +	rc = my_readdir(argv[1]);
> +	if (rc != 0)
> +		goto err;
> +
> +	rc = my_readdir64(argv[1]);
> +	if (rc != 0)
> +		goto err;

Objection...

In an expected fail test case, getdents64() will not be tested because
getdents() will have already failed.

The test script should pass the expected behavior to the test program
and the test program will need to verify that both syscalls match the
expected behavior.

Alternatively, a shortcut would be to test both syscalls and if the
results are different, raise an ERROR instead of a FAIL.

Tyler

> +
>  	printf("PASS\n");
>  
> -	return 0;
> +err:
> +	return rc;
>  }
> 
> 
> 
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170404/f19ee32b/attachment.pgp>


More information about the AppArmor mailing list