[selinux-testsuite PATCH 0/4] inet_socket and mmap patches

[selinux-testsuite PATCH 0/4] inet_socket and mmap patches

[Jan Stancek]

Here are 4 patches that fix failures I have came across in recently
added tests: "inet_socket" and "mmap" on RHEL distros.

I have tested this series on:
                   i386   x86_64   ia64   ppc64   ppc64le   s390x
RHEL5.6/11           *      *        *      *       N/A       *
RHEL6.2/4/5/6/7      *      *       N/A     *       N/A       *
RHEL7.0/1/2         N/A     *       N/A     *        *        *

I have hit one known kernel issue on RHEL7.0/1:
  Bug 1102641 - BUG: It is not possible to communicate between local
                program and local ipv6 address when at least one
                'netlabelctl unlbl' rule is added
  https://bugzilla.redhat.com/show_bug.cgi?id=1102641

This didn't prevent tests from completing, but since harness we use
is using IPv6 by default, it was getting stuck on connect after test
completed.

Other than this one known kernel issue, all tests have passed.

Jan Stancek (4):
  tests/inet_socket: check 'ip xfrm policy ctx' support
  inet_socket: secon: use current pid
  mmap/mprotect_heap: make sure memory is allocated from heap
  mmap/mprotect_file_private_execmod: clear READ_IMPLIES_EXEC

 tests/inet_socket/ipsec-load               |  4 ++--
 tests/inet_socket/test                     | 31 ++++++++++++++++++------------
 tests/mmap/mprotect_file_private_execmod.c | 13 +++++++++++++
 tests/mmap/mprotect_heap.c                 |  9 ++++++++-
 4 files changed, 42 insertions(+), 15 deletions(-)

-- 
1.8.3.1

[selinux-testsuite PATCH 1/4] tests/inet_socket: check 'ip xfrm policy ctx' support

[Jan Stancek]

Early RHEL6 distros like RHEL6.0 do not support "ctx" parameter,
which is causing test to fail:
  Error: argument "ctx" is wrong: unknown

Signed-off-by: Jan Stancek <jstancek@redhat.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
---
 tests/inet_socket/test | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/tests/inet_socket/test b/tests/inet_socket/test
index 4deca746208c..4f6ee51cec5c 100755
--- a/tests/inet_socket/test
+++ b/tests/inet_socket/test
@@ -1,7 +1,14 @@
 #!/usr/bin/perl
-
-use Test;
-BEGIN { plan tests => 20}
+use Test::More;
+
+BEGIN {
+    # check if ip xfrm supports ctx parameter
+    if (system("ip xfrm policy help 2>&1 | grep ctx") != 0) {
+        plan skip_all => "ctx not supported in ip xfrm policy";
+    } else {
+        plan tests => 20;
+    }
+}
 
 $basedir = $0;  $basedir =~ s|(.*)/[^/]*|$1|;
 
@@ -17,7 +24,7 @@ sleep 1; # Give it a moment to initialize.
 
 # Verify that authorized client can communicate with the server.
 $result = system "runcon -t test_inet_client_t $basedir/client stream 65535";
-ok($result, 0);
+ok($result eq 0);
 
 # Verify that unauthorized client cannot communicate with the server.
 $result = system "runcon -t test_inet_bad_client_t -- $basedir/client stream 65535 2>&1";
@@ -35,7 +42,7 @@ sleep 1; # Give it a moment to initialize
 
 # Verify that authorized client can communicate with the server.
 $result = system "runcon -t test_inet_client_t $basedir/client dgram 65535";
-ok($result, 0);
+ok($result eq 0);
 
 # Verify that unauthorized client cannot communicate with the server.
 $result = system "runcon -t test_inet_bad_client_t -- $basedir/client dgram 65535 2>&1";
@@ -49,11 +56,11 @@ system "$basedir/netlabel-flush";
 
 # Verify that authorized domain can bind UDP sockets.
 $result = system "runcon -t test_inet_bind_t -- $basedir/bind dgram 65535 2>&1";
-ok($result, 0);
+ok($result eq 0);
 
 # Verify that authorized domain can bind TCP sockets.
 $result = system "runcon -t test_inet_bind_t -- $basedir/bind stream 65535 2>&1";
-ok($result, 0);
+ok($result eq 0);
 
 # Verify that domain without name_bind cannot bind UDP sockets.
 $result = system "runcon -t test_inet_no_name_bind_t -- $basedir/bind dgram 65535 2>&1";
@@ -73,7 +80,7 @@ ok($result);
 
 # Verify that authorized domain can connect to TCP socket.
 $result = system "runcon -t test_inet_connect_t -- $basedir/connect 65535 2>&1";
-ok($result, 0);
+ok($result eq 0);
 
 # Verify that domain without name_connect cannot connect to TCP socket.
 $result = system "runcon -t test_inet_no_name_connect_t -- $basedir/connect 65535 2>&1";
@@ -91,7 +98,7 @@ sleep 1; # Give it a moment to initialize.
 
 # Verify that authorized client can communicate with the server.
 $result = system "runcon -t test_inet_client_t $basedir/client stream 65535";
-ok($result, 0);
+ok($result eq 0);
 
 # Verify that unauthorized client cannot communicate with the server.
 $result = system "runcon -t test_inet_bad_client_t -- $basedir/client stream 65535 2>&1";
@@ -109,7 +116,7 @@ sleep 1; # Give it a moment to initialize
 
 # Verify that authorized client can communicate with the server.
 $result = system "runcon -t test_inet_client_t $basedir/client dgram 65535";
-ok($result, 0);
+ok($result eq 0);
 
 # Verify that unauthorized client cannot communicate with the server.
 $result = system "runcon -t test_inet_bad_client_t -- $basedir/client dgram 65535 2>&1";
@@ -133,7 +140,7 @@ sleep 1; # Give it a moment to initialize.
 
 # Verify that authorized client can communicate with the server.
 $result = system "runcon -t test_inet_client_t -- $basedir/client -n stream 65535";
-ok($result, 0);
+ok($result eq 0);
 
 # Verify that unauthorized client cannot communicate with the server.
 $result = system "runcon -t test_inet_bad_client_t -- $basedir/client -n stream 65535 2>&1";
@@ -151,7 +158,7 @@ sleep 1; # Give it a moment to initialize
 
 # Verify that authorized client can communicate with the server.
 $result = system "runcon -t test_inet_client_t $basedir/client -n dgram 65535";
-ok($result, 0);
+ok($result eq 0);
 
 # Verify that unauthorized client cannot communicate with the server.
 $result = system "runcon -t test_inet_bad_client_t -- $basedir/client -n dgram 65535 2>&1";
-- 
1.8.3.1

[selinux-testsuite PATCH 2/4] inet_socket: secon: use current pid

[Jan Stancek]

When running selinux-testsuite in automated environment,
such as Beaker, stdin is usually /dev/null. This causes
problem for inet_socket test:
  secon:  Couldn't read security context: Inappropriate ioctl for device

Signed-off-by: Jan Stancek <jstancek@redhat.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
---
 tests/inet_socket/ipsec-load | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/inet_socket/ipsec-load b/tests/inet_socket/ipsec-load
index ded6efb04722..b9d2c6e43544 100755
--- a/tests/inet_socket/ipsec-load
+++ b/tests/inet_socket/ipsec-load
@@ -3,8 +3,8 @@ echo 0 > /proc/sys/net/ipv4/conf/lo/disable_xfrm
 echo 0 > /proc/sys/net/ipv4/conf/lo/disable_policy
 ip xfrm policy flush
 ip xfrm state flush
-goodclientcon=`secon -u`:`secon -r`:test_inet_client_t:`secon -m`
-badclientcon=`secon -u`:`secon -r`:test_inet_bad_client_t:`secon -m`
+goodclientcon=`secon -u --pid $$`:`secon -r --pid $$`:test_inet_client_t:`secon -m --pid $$`
+badclientcon=`secon -u --pid $$`:`secon -r --pid $$`:test_inet_bad_client_t:`secon -m --pid $$`
 ip xfrm state add src 127.0.0.1 dst 127.0.0.1 proto ah spi 0x200 ctx $goodclientcon auth md5 0123456789012345
 ip xfrm state add src 127.0.0.1 dst 127.0.0.1 proto ah spi 0x250 ctx $badclientcon auth md5 0123456789012345
 ip xfrm policy add src 127.0.0.1 dst 127.0.0.1 proto tcp dir out ctx "system_u:object_r:test_spd_t:s0" tmpl proto ah mode transport level required
-- 
1.8.3.1

[selinux-testsuite PATCH 3/4] mmap/mprotect_heap: make sure memory is allocated from heap

[Jan Stancek]

This test failed on ppc64 with 64k pagesize because memory
allocation used mmap() instead of advancing heap.

Use mallopt(M_MMAP_THRESHOLD,..) with large enough value
to discourage use of mmap().

Also set length in mprotect to pagesize, kernel silently aligns
it to pagesize anyway.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
---
 tests/mmap/mprotect_heap.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/mmap/mprotect_heap.c b/tests/mmap/mprotect_heap.c
index 691299493c3f..8b4321d6fc0b 100644
--- a/tests/mmap/mprotect_heap.c
+++ b/tests/mmap/mprotect_heap.c
@@ -3,6 +3,7 @@
 #include <stdlib.h>
 #include <errno.h>
 #include <sys/mman.h>
+#include <malloc.h>
 
 int main(void)
 {
@@ -10,13 +11,19 @@ int main(void)
 	int rc;
 	int pagesize = getpagesize();
 
+	rc = mallopt(M_MMAP_THRESHOLD, pagesize * 16);
+	if (rc != 1) {
+		fprintf(stderr, "mallopt failed: %d\n", rc);
+		exit(1);
+	}
+
 	rc = posix_memalign(&ptr, pagesize, pagesize);
 	if (rc) {
 		fprintf(stderr, "posix_memalign failed: %d\n", rc);
 		exit(1);
 	}
 
-	rc = mprotect(ptr, 4096, PROT_READ | PROT_EXEC);
+	rc = mprotect(ptr, pagesize, PROT_READ | PROT_EXEC);
 	if (rc < 0) {
 		perror("mprotect");
 		exit(1);
-- 
1.8.3.1

[selinux-testsuite PATCH 4/4] mmap/mprotect_file_private_execmod: clear READ_IMPLIES_EXEC

[Jan Stancek]

If READ_IMPLIES_EXEC personality is set, mmap-ed area will have
VM_EXEC vm_flag set, which causes selinux_file_mprotect() to
skip check for FILE__EXECMOD.

This caused that older (2.6.32) kernels on s390x always allowed
mprotect to succeed.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
---
 tests/mmap/mprotect_file_private_execmod.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/mmap/mprotect_file_private_execmod.c b/tests/mmap/mprotect_file_private_execmod.c
index ade19811290a..6d6b995fc03c 100644
--- a/tests/mmap/mprotect_file_private_execmod.c
+++ b/tests/mmap/mprotect_file_private_execmod.c
@@ -4,6 +4,11 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <sys/mman.h>
+#include <sys/personality.h>
+
+#ifndef READ_IMPLIES_EXEC
+#define READ_IMPLIES_EXEC 0x0400000
+#endif
 
 int main(int argc, char **argv)
 {
@@ -16,6 +21,14 @@ int main(int argc, char **argv)
 		exit(1);
 	}
 
+	/* clear READ_IMPLIES_EXEC if present, because it skips
+	 * check for FILE__EXECMOD in selinux_file_mprotect() */
+	rc = personality(0xffffffff);
+	if ((rc != -1) && (rc & READ_IMPLIES_EXEC)) {
+		rc &= ~READ_IMPLIES_EXEC;
+		personality(rc);
+	}
+
 	fd = open(argv[1], O_RDONLY);
 	if (fd < 0) {
 		perror(argv[1]);
-- 
1.8.3.1

Re: [selinux-testsuite PATCH 1/4] tests/inet_socket: check 'ip xfrm policy ctx' support

[Paul Moore]

On Fri, Nov 6, 2015 at 8:07 AM, Jan Stancek <jstancek@redhat.com> wrote:
> Early RHEL6 distros like RHEL6.0 do not support "ctx" parameter,
> which is causing test to fail:
>   Error: argument "ctx" is wrong: unknown
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> ---
>  tests/inet_socket/test | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/tests/inet_socket/test b/tests/inet_socket/test
> index 4deca746208c..4f6ee51cec5c 100755
> --- a/tests/inet_socket/test
> +++ b/tests/inet_socket/test
> @@ -1,7 +1,14 @@
>  #!/usr/bin/perl
> -
> -use Test;
> -BEGIN { plan tests => 20}
> +use Test::More;
> +
> +BEGIN {
> +    # check if ip xfrm supports ctx parameter
> +    if (system("ip xfrm policy help 2>&1 | grep ctx") != 0) {
> +        plan skip_all => "ctx not supported in ip xfrm policy";
> +    } else {
> +        plan tests => 20;
> +    }
> +}
>
>  $basedir = $0;  $basedir =~ s|(.*)/[^/]*|$1|;
>
> @@ -17,7 +24,7 @@ sleep 1; # Give it a moment to initialize.
>
>  # Verify that authorized client can communicate with the server.
>  $result = system "runcon -t test_inet_client_t $basedir/client stream 65535";
> -ok($result, 0);
> +ok($result eq 0);

My understanding of Perl is *very* basic - why this change (and the
similar ones in this patch)?

-- 
paul moore
www.paul-moore.com

Re: [selinux-testsuite PATCH 2/4] inet_socket: secon: use current pid

[Paul Moore]

On Friday, November 06, 2015 02:07:22 PM Jan Stancek wrote:
> When running selinux-testsuite in automated environment,
> such as Beaker, stdin is usually /dev/null. This causes
> problem for inet_socket test:
>   secon:  Couldn't read security context: Inappropriate ioctl for device
> 
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> ---
>  tests/inet_socket/ipsec-load | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Looks good to me, merged.

> diff --git a/tests/inet_socket/ipsec-load b/tests/inet_socket/ipsec-load
> index ded6efb04722..b9d2c6e43544 100755
> --- a/tests/inet_socket/ipsec-load
> +++ b/tests/inet_socket/ipsec-load
> @@ -3,8 +3,8 @@ echo 0 > /proc/sys/net/ipv4/conf/lo/disable_xfrm
>  echo 0 > /proc/sys/net/ipv4/conf/lo/disable_policy
>  ip xfrm policy flush
>  ip xfrm state flush
> -goodclientcon=`secon -u`:`secon -r`:test_inet_client_t:`secon -m`
> -badclientcon=`secon -u`:`secon -r`:test_inet_bad_client_t:`secon -m`
> +goodclientcon=`secon -u --pid $$`:`secon -r --pid
> $$`:test_inet_client_t:`secon -m --pid $$` +badclientcon=`secon -u --pid
> $$`:`secon -r --pid $$`:test_inet_bad_client_t:`secon -m --pid $$` ip xfrm
> state add src 127.0.0.1 dst 127.0.0.1 proto ah spi 0x200 ctx $goodclientcon
> auth md5 0123456789012345 ip xfrm state add src 127.0.0.1 dst 127.0.0.1
> proto ah spi 0x250 ctx $badclientcon auth md5 0123456789012345 ip xfrm
> policy add src 127.0.0.1 dst 127.0.0.1 proto tcp dir out ctx
> "system_u:object_r:test_spd_t:s0" tmpl proto ah mode transport level
> required

-- 
paul moore
www.paul-moore.com

Re: [selinux-testsuite PATCH 3/4] mmap/mprotect_heap: make sure memory is allocated from heap

[Paul Moore]

On Friday, November 06, 2015 02:07:23 PM Jan Stancek wrote:
> This test failed on ppc64 with 64k pagesize because memory
> allocation used mmap() instead of advancing heap.
> 
> Use mallopt(M_MMAP_THRESHOLD,..) with large enough value
> to discourage use of mmap().
> 
> Also set length in mprotect to pagesize, kernel silently aligns
> it to pagesize anyway.
> 
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> ---
>  tests/mmap/mprotect_heap.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/mmap/mprotect_heap.c b/tests/mmap/mprotect_heap.c
> index 691299493c3f..8b4321d6fc0b 100644
> --- a/tests/mmap/mprotect_heap.c
> +++ b/tests/mmap/mprotect_heap.c
> @@ -3,6 +3,7 @@
>  #include <stdlib.h>
>  #include <errno.h>
>  #include <sys/mman.h>
> +#include <malloc.h>
> 
>  int main(void)
>  {
> @@ -10,13 +11,19 @@ int main(void)
>  	int rc;
>  	int pagesize = getpagesize();
> 
> +	rc = mallopt(M_MMAP_THRESHOLD, pagesize * 16);
> +	if (rc != 1) {
> +		fprintf(stderr, "mallopt failed: %d\n", rc);
> +		exit(1);
> +	}

Can you explain how you arrived at 16?  It looks a bit like a magic number to 
me, which always gives me pause.  Based on a quick read of the man page, it 
would seem like any value greater than "pagesize" would work, yes?

>  	rc = posix_memalign(&ptr, pagesize, pagesize);
>  	if (rc) {
>  		fprintf(stderr, "posix_memalign failed: %d\n", rc);
>  		exit(1);
>  	}
> 
> -	rc = mprotect(ptr, 4096, PROT_READ | PROT_EXEC);
> +	rc = mprotect(ptr, pagesize, PROT_READ | PROT_EXEC);
>  	if (rc < 0) {
>  		perror("mprotect");
>  		exit(1);

-- 
paul moore
www.paul-moore.com

Re: [selinux-testsuite PATCH 4/4] mmap/mprotect_file_private_execmod: clear READ_IMPLIES_EXEC

[Paul Moore]

On Friday, November 06, 2015 02:07:24 PM Jan Stancek wrote:
> If READ_IMPLIES_EXEC personality is set, mmap-ed area will have
> VM_EXEC vm_flag set, which causes selinux_file_mprotect() to
> skip check for FILE__EXECMOD.
> 
> This caused that older (2.6.32) kernels on s390x always allowed
> mprotect to succeed.
> 
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> ---
>  tests/mmap/mprotect_file_private_execmod.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Merged, thanks.

> diff --git a/tests/mmap/mprotect_file_private_execmod.c
> b/tests/mmap/mprotect_file_private_execmod.c index
> ade19811290a..6d6b995fc03c 100644
> --- a/tests/mmap/mprotect_file_private_execmod.c
> +++ b/tests/mmap/mprotect_file_private_execmod.c
> @@ -4,6 +4,11 @@
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <sys/mman.h>
> +#include <sys/personality.h>
> +
> +#ifndef READ_IMPLIES_EXEC
> +#define READ_IMPLIES_EXEC 0x0400000
> +#endif
> 
>  int main(int argc, char **argv)
>  {
> @@ -16,6 +21,14 @@ int main(int argc, char **argv)
>  		exit(1);
>  	}
> 
> +	/* clear READ_IMPLIES_EXEC if present, because it skips
> +	 * check for FILE__EXECMOD in selinux_file_mprotect() */
> +	rc = personality(0xffffffff);
> +	if ((rc != -1) && (rc & READ_IMPLIES_EXEC)) {
> +		rc &= ~READ_IMPLIES_EXEC;
> +		personality(rc);
> +	}
> +
>  	fd = open(argv[1], O_RDONLY);
>  	if (fd < 0) {
>  		perror(argv[1]);

-- 
paul moore
www.paul-moore.com

Re: [selinux-testsuite PATCH 1/4] tests/inet_socket: check 'ip xfrm policy ctx' support

[Jan Stancek]

----- Original Message -----
> From: "Paul Moore" <paul@paul-moore.com>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: selinux@tycho.nsa.gov, "Stephen Smalley" <sds@tycho.nsa.gov>
> Sent: Friday, 6 November, 2015 6:58:38 PM
> Subject: Re: [selinux-testsuite PATCH 1/4] tests/inet_socket: check 'ip xfrm policy ctx' support
> 
> On Fri, Nov 6, 2015 at 8:07 AM, Jan Stancek <jstancek@redhat.com> wrote:
> > Early RHEL6 distros like RHEL6.0 do not support "ctx" parameter,
> > which is causing test to fail:
> >   Error: argument "ctx" is wrong: unknown
> >
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > Cc: Paul Moore <paul@paul-moore.com>
> > Cc: Stephen Smalley <sds@tycho.nsa.gov>
> > ---
> >  tests/inet_socket/test | 31 +++++++++++++++++++------------
> >  1 file changed, 19 insertions(+), 12 deletions(-)
> >
> > diff --git a/tests/inet_socket/test b/tests/inet_socket/test
> > index 4deca746208c..4f6ee51cec5c 100755
> > --- a/tests/inet_socket/test
> > +++ b/tests/inet_socket/test
> > @@ -1,7 +1,14 @@
> >  #!/usr/bin/perl
> > -
> > -use Test;
> > -BEGIN { plan tests => 20}
> > +use Test::More;
> > +
> > +BEGIN {
> > +    # check if ip xfrm supports ctx parameter
> > +    if (system("ip xfrm policy help 2>&1 | grep ctx") != 0) {
> > +        plan skip_all => "ctx not supported in ip xfrm policy";
> > +    } else {
> > +        plan tests => 20;
> > +    }
> > +}
> >
> >  $basedir = $0;  $basedir =~ s|(.*)/[^/]*|$1|;
> >
> > @@ -17,7 +24,7 @@ sleep 1; # Give it a moment to initialize.
> >
> >  # Verify that authorized client can communicate with the server.
> >  $result = system "runcon -t test_inet_client_t $basedir/client stream
> >  65535";
> > -ok($result, 0);
> > +ok($result eq 0);
> 
> My understanding of Perl is *very* basic - why this change (and the
> similar ones in this patch)?

I wanted to skip the test, but skip_all is supported only in Test::More
framework. And arguments in ok() have slightly different meaning.

In "Test" args are "ok($have, $expect);" [1]
In "Test::More" args are "ok($got eq $expected, $test_name);" [2]

This change adapts parameters to "Test::More" style.

[1] http://perldoc.perl.org/Test.html
[2] http://perldoc.perl.org/Test/More.html

> 
> --
> paul moore
> www.paul-moore.com
> 

Re: [selinux-testsuite PATCH 3/4] mmap/mprotect_heap: make sure memory is allocated from heap

[Jan Stancek]

----- Original Message -----
> From: "Paul Moore" <paul@paul-moore.com>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: selinux@tycho.nsa.gov, sds@tycho.nsa.gov
> Sent: Friday, 6 November, 2015 7:14:54 PM
> Subject: Re: [selinux-testsuite PATCH 3/4] mmap/mprotect_heap: make sure memory is allocated from heap
> 
> On Friday, November 06, 2015 02:07:23 PM Jan Stancek wrote:
> > This test failed on ppc64 with 64k pagesize because memory
> > allocation used mmap() instead of advancing heap.
> > 
> > Use mallopt(M_MMAP_THRESHOLD,..) with large enough value
> > to discourage use of mmap().
> > 
> > Also set length in mprotect to pagesize, kernel silently aligns
> > it to pagesize anyway.
> > 
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > Cc: Paul Moore <paul@paul-moore.com>
> > Cc: Stephen Smalley <sds@tycho.nsa.gov>
> > ---
> >  tests/mmap/mprotect_heap.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/mmap/mprotect_heap.c b/tests/mmap/mprotect_heap.c
> > index 691299493c3f..8b4321d6fc0b 100644
> > --- a/tests/mmap/mprotect_heap.c
> > +++ b/tests/mmap/mprotect_heap.c
> > @@ -3,6 +3,7 @@
> >  #include <stdlib.h>
> >  #include <errno.h>
> >  #include <sys/mman.h>
> > +#include <malloc.h>
> > 
> >  int main(void)
> >  {
> > @@ -10,13 +11,19 @@ int main(void)
> >  	int rc;
> >  	int pagesize = getpagesize();
> > 
> > +	rc = mallopt(M_MMAP_THRESHOLD, pagesize * 16);
> > +	if (rc != 1) {
> > +		fprintf(stderr, "mallopt failed: %d\n", rc);
> > +		exit(1);
> > +	}
> 
> Can you explain how you arrived at 16?  It looks a bit like a magic number to
> me, which always gives me pause.  Based on a quick read of the man page, it
> would seem like any value greater than "pagesize" would work, yes?

I thought 2 would be enough - as you can find aligned page sized block within.
But when I tried it, it failed. It started working with 3 * pagesize.
I picked 16 as rule of thumb. Now, that I'm looking at man page I guess
we can as well use DEFAULT_MMAP_THRESHOLD_MAX.

Regards,
Jan

> 
> >  	rc = posix_memalign(&ptr, pagesize, pagesize);
> >  	if (rc) {
> >  		fprintf(stderr, "posix_memalign failed: %d\n", rc);
> >  		exit(1);
> >  	}
> > 
> > -	rc = mprotect(ptr, 4096, PROT_READ | PROT_EXEC);
> > +	rc = mprotect(ptr, pagesize, PROT_READ | PROT_EXEC);
> >  	if (rc < 0) {
> >  		perror("mprotect");
> >  		exit(1);
> 
> --
> paul moore
> www.paul-moore.com
> 
> 

[selinux-testsuite PATCH v2 3/4] mmap/mprotect_heap: make sure memory is allocated from heap

[Jan Stancek]

This test failed on ppc64 with 64k pagesize because memory
allocation used mmap() instead of advancing heap.

Use mallopt(M_MMAP_THRESHOLD,..) with large enough value
to discourage use of mmap().

Also set length in mprotect to pagesize, kernel silently aligns
it to pagesize anyway.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
---
 tests/mmap/mprotect_heap.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tests/mmap/mprotect_heap.c b/tests/mmap/mprotect_heap.c
index 691299493c3f..23de4de5be62 100644
--- a/tests/mmap/mprotect_heap.c
+++ b/tests/mmap/mprotect_heap.c
@@ -3,6 +3,11 @@
 #include <stdlib.h>
 #include <errno.h>
 #include <sys/mman.h>
+#include <malloc.h>
+
+#ifndef DEFAULT_MMAP_THRESHOLD_MAX
+#define DEFAULT_MMAP_THRESHOLD_MAX 512*1024
+#endif
 
 int main(void)
 {
@@ -10,13 +15,19 @@ int main(void)
 	int rc;
 	int pagesize = getpagesize();
 
+	rc = mallopt(M_MMAP_THRESHOLD, DEFAULT_MMAP_THRESHOLD_MAX);
+	if (rc != 1) {
+		fprintf(stderr, "mallopt failed: %d\n", rc);
+		exit(1);
+	}
+
 	rc = posix_memalign(&ptr, pagesize, pagesize);
 	if (rc) {
 		fprintf(stderr, "posix_memalign failed: %d\n", rc);
 		exit(1);
 	}
 
-	rc = mprotect(ptr, 4096, PROT_READ | PROT_EXEC);
+	rc = mprotect(ptr, pagesize, PROT_READ | PROT_EXEC);
 	if (rc < 0) {
 		perror("mprotect");
 		exit(1);
-- 
1.8.3.1

Re: [selinux-testsuite PATCH v2 3/4] mmap/mprotect_heap: make sure memory is allocated from heap

[Paul Moore]

On Monday, November 16, 2015 10:46:26 AM Jan Stancek wrote:
> This test failed on ppc64 with 64k pagesize because memory
> allocation used mmap() instead of advancing heap.
> 
> Use mallopt(M_MMAP_THRESHOLD,..) with large enough value
> to discourage use of mmap().
> 
> Also set length in mprotect to pagesize, kernel silently aligns
> it to pagesize anyway.
> 
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> ---
>  tests/mmap/mprotect_heap.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

Merged.  This looks much better, thanks.

> diff --git a/tests/mmap/mprotect_heap.c b/tests/mmap/mprotect_heap.c
> index 691299493c3f..23de4de5be62 100644
> --- a/tests/mmap/mprotect_heap.c
> +++ b/tests/mmap/mprotect_heap.c
> @@ -3,6 +3,11 @@
>  #include <stdlib.h>
>  #include <errno.h>
>  #include <sys/mman.h>
> +#include <malloc.h>
> +
> +#ifndef DEFAULT_MMAP_THRESHOLD_MAX
> +#define DEFAULT_MMAP_THRESHOLD_MAX 512*1024
> +#endif
> 
>  int main(void)
>  {
> @@ -10,13 +15,19 @@ int main(void)
>  	int rc;
>  	int pagesize = getpagesize();
> 
> +	rc = mallopt(M_MMAP_THRESHOLD, DEFAULT_MMAP_THRESHOLD_MAX);
> +	if (rc != 1) {
> +		fprintf(stderr, "mallopt failed: %d\n", rc);
> +		exit(1);
> +	}
> +
>  	rc = posix_memalign(&ptr, pagesize, pagesize);
>  	if (rc) {
>  		fprintf(stderr, "posix_memalign failed: %d\n", rc);
>  		exit(1);
>  	}
> 
> -	rc = mprotect(ptr, 4096, PROT_READ | PROT_EXEC);
> +	rc = mprotect(ptr, pagesize, PROT_READ | PROT_EXEC);
>  	if (rc < 0) {
>  		perror("mprotect");
>  		exit(1);

-- 
paul moore
www.paul-moore.com

Re: [selinux-testsuite PATCH 1/4] tests/inet_socket: check 'ip xfrm policy ctx' support

[Paul Moore]

On Fri, Nov 6, 2015 at 4:44 PM, Jan Stancek <jstancek@redhat.com> wrote:
> ----- Original Message -----
>> From: "Paul Moore" <paul@paul-moore.com>
>> To: "Jan Stancek" <jstancek@redhat.com>
>> Cc: selinux@tycho.nsa.gov, "Stephen Smalley" <sds@tycho.nsa.gov>
>> Sent: Friday, 6 November, 2015 6:58:38 PM
>> Subject: Re: [selinux-testsuite PATCH 1/4] tests/inet_socket: check 'ip xfrm policy ctx' support
>>
>> On Fri, Nov 6, 2015 at 8:07 AM, Jan Stancek <jstancek@redhat.com> wrote:
>> > Early RHEL6 distros like RHEL6.0 do not support "ctx" parameter,
>> > which is causing test to fail:
>> >   Error: argument "ctx" is wrong: unknown
>> >
>> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
>> > Cc: Paul Moore <paul@paul-moore.com>
>> > Cc: Stephen Smalley <sds@tycho.nsa.gov>
>> > ---
>> >  tests/inet_socket/test | 31 +++++++++++++++++++------------
>> >  1 file changed, 19 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/tests/inet_socket/test b/tests/inet_socket/test
>> > index 4deca746208c..4f6ee51cec5c 100755
>> > --- a/tests/inet_socket/test
>> > +++ b/tests/inet_socket/test
>> > @@ -1,7 +1,14 @@
>> >  #!/usr/bin/perl
>> > -
>> > -use Test;
>> > -BEGIN { plan tests => 20}
>> > +use Test::More;
>> > +
>> > +BEGIN {
>> > +    # check if ip xfrm supports ctx parameter
>> > +    if (system("ip xfrm policy help 2>&1 | grep ctx") != 0) {
>> > +        plan skip_all => "ctx not supported in ip xfrm policy";
>> > +    } else {
>> > +        plan tests => 20;
>> > +    }
>> > +}
>> >
>> >  $basedir = $0;  $basedir =~ s|(.*)/[^/]*|$1|;
>> >
>> > @@ -17,7 +24,7 @@ sleep 1; # Give it a moment to initialize.
>> >
>> >  # Verify that authorized client can communicate with the server.
>> >  $result = system "runcon -t test_inet_client_t $basedir/client stream
>> >  65535";
>> > -ok($result, 0);
>> > +ok($result eq 0);
>>
>> My understanding of Perl is *very* basic - why this change (and the
>> similar ones in this patch)?
>
> I wanted to skip the test, but skip_all is supported only in Test::More
> framework. And arguments in ok() have slightly different meaning.
>
> In "Test" args are "ok($have, $expect);" [1]
> In "Test::More" args are "ok($got eq $expected, $test_name);" [2]
>
> This change adapts parameters to "Test::More" style.

That's really annoying that Test::More changes the ok() function;
bummer.  Okay, I'll merge the patch, thanks for the clarification.

> [1] http://perldoc.perl.org/Test.html
> [2] http://perldoc.perl.org/Test/More.html

-- 
paul moore
www.paul-moore.com