inet_socket and mmap patches

PATCH Applied Jan Stancek 2 participants 14 messages Nov 6, 2015 – Nov 19, 2015 SELinux security module

Share & tools

Jan Stancek

author

2015-11-06 13:07 UTC #0 lore raw

cover inet_socket and mmap patches

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(-)

Signature · 1 line

1.8.3.1

Jan Stancek

author

2015-11-06 13:07 UTC #1 lore raw

PATCH 1/4 tests/inet_socket: check 'ip xfrm policy ctx' support

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 <[email protected]>

Cc Paul Moore <[email protected]>

Cc Stephen Smalley <[email protected]>

---
 tests/inet_socket/test | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

tests/inet_socket/test +19 ~~-12~~

index 4deca746208c..4f6ee51cec5c 100755
@@ -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";

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2015-11-06 17:58 UTC #2 lore raw

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

Quoted 34 lines · 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 <[email protected]>
Cc Paul Moore <[email protected]>
Cc Stephen Smalley <[email protected]>
---
 tests/inet_socket/test | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)
tests/inet_socket/test +10 -3
index 4deca746208c..4f6ee51cec5c 100755
@@ -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|;
 

tests/inet_socket/test sleep 1; # Give it a moment to initialize. · line 27

Quoted 5 lines

tests/inet_socket/test +1 -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)?

Signature · 2 lines

paul moore
www.paul-moore.com

Jan Stancek

author

2015-11-06 21:44 UTC #3 lore raw

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

----- Original Message -----

Quoted 42 lines

From: "Paul Moore" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected], "Stephen Smalley" <[email protected]>
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
Quoted 35 lines · 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 <[email protected]>
Cc Paul Moore <[email protected]>
Cc Stephen Smalley <[email protected]>
---
 tests/inet_socket/test | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)
tests/inet_socket/test +10 -3
index 4deca746208c..4f6ee51cec5c 100755
@@ -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.
 

Quoted 8 lines

tests/inet_socket/test sleep 1; # Give it a moment to initialize. · line 27
Quoted 5 lines
tests/inet_socket/test +1 -1
 # 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

Quoted 5 lines

--
paul moore
www.paul-moore.com

Paul Moore

maintainer

2015-11-19 21:04 UTC #4 lore raw

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

Quoted 52 lines · Jan Stancek

----- Original Message -----
Quoted 42 lines
From: "Paul Moore" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected], "Stephen Smalley" <[email protected]>
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
Quoted 35 lines · 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 <[email protected]>
Cc Paul Moore <[email protected]>
Cc Stephen Smalley <[email protected]>
---
 tests/inet_socket/test | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)
tests/inet_socket/test +10 -3
index 4deca746208c..4f6ee51cec5c 100755
@@ -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.
 
Quoted 8 lines
tests/inet_socket/test sleep 1; # Give it a moment to initialize. · line 27
Quoted 5 lines
tests/inet_socket/test +1 -1
 # 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)?

Quoted 7 lines

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.

Quoted 2 lines

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

Signature · 2 lines

paul moore
www.paul-moore.com

Jan Stancek

author

2015-11-06 13:07 UTC #5 lore raw

PATCH 2/4 inet_socket: secon: use current pid

In reply to 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 <[email protected]>

Cc Paul Moore <[email protected]>

Cc Stephen Smalley <[email protected]>

---
 tests/inet_socket/ipsec-load | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

tests/inet_socket/ipsec-load +2 -2

index ded6efb04722..b9d2c6e43544 100755
@@ -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

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2015-11-06 18:07 UTC #6 lore raw

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

Quoted 11 lines · 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 <[email protected]>
Cc Paul Moore <[email protected]>
Cc Stephen Smalley <[email protected]>
---
 tests/inet_socket/ipsec-load | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Looks good to me, merged.

Quoted 19 lines

tests/inet_socket/ipsec-load +1 -2
index ded6efb04722..b9d2c6e43544 100755
@@ -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

Signature · 2 lines

paul moore
www.paul-moore.com

Jan Stancek

author

2015-11-06 13:07 UTC #7 lore raw

PATCH 3/4 mmap/mprotect_heap: make sure memory is allocated from heap

In reply to 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 <[email protected]>

Cc Paul Moore <[email protected]>

Cc Stephen Smalley <[email protected]>

---
 tests/mmap/mprotect_heap.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

tests/mmap/mprotect_heap.c +8 -1

index 691299493c3f..8b4321d6fc0b 100644
@@ -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);

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2015-11-06 18:15 UTC #8 lore raw

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

Quoted 29 lines · 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 <[email protected]>
Cc Paul Moore <[email protected]>
Cc Stephen Smalley <[email protected]>
---
 tests/mmap/mprotect_heap.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
tests/mmap/mprotect_heap.c +1 -0
index 691299493c3f..8b4321d6fc0b 100644
@@ -3,6 +3,7 @@
 #include <stdlib.h>
 #include <errno.h>
 #include <sys/mman.h>
+#include <malloc.h>
 
 int main(void)
 {

tests/mmap/mprotect_heap.c int main(void) · line 18

Quoted 8 lines

tests/mmap/mprotect_heap.c +5 -0

@@ -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?

Quoted 11 lines

tests/mmap/mprotect_heap.c +1 -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);

Signature · 2 lines

paul moore
www.paul-moore.com

Jan Stancek

author

2015-11-06 21:57 UTC #9 lore raw

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

----- Original Message -----

Quoted 39 lines

From: "Paul Moore" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected], [email protected]
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
Quoted 32 lines · 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 <[email protected]>
Cc Paul Moore <[email protected]>
Cc Stephen Smalley <[email protected]>
---
 tests/mmap/mprotect_heap.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
tests/mmap/mprotect_heap.c +1 -0
index 691299493c3f..8b4321d6fc0b 100644
@@ -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();
 

Quoted 9 lines

tests/mmap/mprotect_heap.c int main(void) · line 18
Quoted 5 lines
tests/mmap/mprotect_heap.c +5 -0
+	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

Quoted 18 lines

tests/mmap/mprotect_heap.c int main(void) · line 29

Quoted 12 lines

tests/mmap/mprotect_heap.c +1 -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

Jan Stancek

author

2015-11-16 09:46 UTC #10 lore raw

PATCH v2 3/4 mmap/mprotect_heap: make sure memory is allocated from heap

In reply to 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 <[email protected]>

Cc Paul Moore <[email protected]>

Cc Stephen Smalley <[email protected]>

---
 tests/mmap/mprotect_heap.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

tests/mmap/mprotect_heap.c +12 -1

index 691299493c3f..23de4de5be62 100644
@@ -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);

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2015-11-19 20:46 UTC #11 lore raw

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

Quoted 9 lines · 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.

Quoted 6 lines

Signed-off-by Jan Stancek <[email protected]>
Cc Paul Moore <[email protected]>
Cc Stephen Smalley <[email protected]>
---
 tests/mmap/mprotect_heap.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Merged. This looks much better, thanks.

Quoted 37 lines

tests/mmap/mprotect_heap.c +12 -1

index 691299493c3f..23de4de5be62 100644
@@ -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);

Signature · 2 lines

paul moore
www.paul-moore.com

Jan Stancek

author

2015-11-06 13:07 UTC #12 lore raw

PATCH 4/4 mmap/mprotect_file_private_execmod: clear READ_IMPLIES_EXEC

In reply to 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 <[email protected]>

Cc Paul Moore <[email protected]>

Cc Stephen Smalley <[email protected]>

---
 tests/mmap/mprotect_file_private_execmod.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

tests/mmap/mprotect_file_private_execmod.c +13 -0

index ade19811290a..6d6b995fc03c 100644
@@ -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]);

Signature · 1 line

1.8.3.1

Paul Moore

maintainer Applied

2015-11-06 18:25 UTC #13 lore raw

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

Quoted 7 lines · 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.

Quoted 6 lines

Signed-off-by Jan Stancek <[email protected]>
Cc Paul Moore <[email protected]>
Cc Stephen Smalley <[email protected]>
---
 tests/mmap/mprotect_file_private_execmod.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Merged, thanks.

Quoted 32 lines

(quoted diff) +0 -0

b/tests/mmap/mprotect_file_private_execmod.c index ade19811290a..6d6b995fc03c 100644

tests/mmap/mprotect_file_private_execmod.c +13 -0

@@ -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]);

Signature · 2 lines

paul moore
www.paul-moore.com