* [PATCH v2] tests/secretmem: add test
@ 2022-08-31 15:34 Christian Göttsche
2022-09-12 13:41 ` Ondrej Mosnacek
0 siblings, 1 reply; 4+ messages in thread
From: Christian Göttsche @ 2022-08-31 15:34 UTC (permalink / raw)
To: selinux
Add test for memfd_secret(2) anonymous inodes check added in 6.0 via
2bfe15c52612 ("mm: create security context for memfd_secret inodes").
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2:
- print mmap failures to stdout, since they are expected when
mapping with PROT_EXEC
---
.github/workflows/checks.yml | 4 ++
Vagrantfile | 10 +++--
policy/Makefile | 4 ++
policy/test_secretmem.te | 33 ++++++++++++++
tests/Makefile | 5 +++
tests/secretmem/.gitignore | 1 +
tests/secretmem/Makefile | 5 +++
tests/secretmem/secretmem.c | 83 ++++++++++++++++++++++++++++++++++++
tests/secretmem/test | 39 +++++++++++++++++
9 files changed, 180 insertions(+), 4 deletions(-)
create mode 100644 policy/test_secretmem.te
create mode 100644 tests/secretmem/.gitignore
create mode 100644 tests/secretmem/Makefile
create mode 100644 tests/secretmem/secretmem.c
create mode 100755 tests/secretmem/test
diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml
index 714bb49..ae81925 100644
--- a/.github/workflows/checks.yml
+++ b/.github/workflows/checks.yml
@@ -38,6 +38,10 @@ jobs:
run: vagrant up
- name: Wait for the machine to come up if rebooting
run: while ! vagrant ssh -- true; do sleep 1; done
+ - name: Show Vagrant VM details
+ run: |
+ vagrant ssh -- uname -a
+ vagrant ssh -- cat /proc/cmdline
- name: Run SELinux testsuite
run: vagrant ssh -- sudo make -C /root/testsuite test
- name: Check unwanted denials
diff --git a/Vagrantfile b/Vagrantfile
index f73586c..6f7675f 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -37,15 +37,12 @@ Vagrant.configure("2") do |config|
when 'default'
dnf_opts = ''
kernel_pkgs = 'kernel-devel-"$(uname -r)" kernel-modules-"$(uname -r)"'
- reboot_cmd = ''
when 'latest'
dnf_opts = ''
kernel_pkgs = 'kernel-devel kernel-modules'
- reboot_cmd = 'reboot'
when 'secnext'
dnf_opts = '--nogpgcheck --releasever rawhide --repofrompath kernel-secnext,https://repo.paul-moore.com/rawhide/x86_64'
kernel_pkgs = 'kernel-devel kernel-modules'
- reboot_cmd = 'reboot'
else
print("Invalid KERNEL_TYPE '#{ENV['KERNEL_TYPE']}'")
abort
@@ -93,7 +90,12 @@ EOF
jfsutils \
dosfstools \
#{kernel_pkgs}
+
#{extra_commands}
- #{reboot_cmd}
+
+ # for secretmem test
+ grubby --update-kernel=ALL --args=secretmem.enable=1
+
+ reboot
SCRIPT
end
diff --git a/policy/Makefile b/policy/Makefile
index b6f2f32..403802b 100644
--- a/policy/Makefile
+++ b/policy/Makefile
@@ -162,6 +162,10 @@ ifeq (x$(DISTRO),$(filter x$(DISTRO),xRHEL4 xRHEL5 xRHEL6))
TARGETS:=$(filter-out test_overlayfs.te test_mqueue.te test_ibpkey.te, $(TARGETS))
endif
+ifeq ($(shell grep -q anon_inode $(POLDEV)/include/support/all_perms.spt && echo true),true)
+TARGETS += test_secretmem.te
+endif
+
all: build
expand_check:
diff --git a/policy/test_secretmem.te b/policy/test_secretmem.te
new file mode 100644
index 0000000..357f41d
--- /dev/null
+++ b/policy/test_secretmem.te
@@ -0,0 +1,33 @@
+#################################
+#
+# Policy for testing secretmem operations
+#
+
+# Private type for secret memory
+type test_secretmem_inode_t;
+
+# Domain not allowed to create secret memory
+type test_nocreate_secretmem_t;
+testsuite_domain_type_minimal(test_nocreate_secretmem_t)
+
+# Domain allowed to create secret memory with the own domain type
+type test_create_secretmem_t;
+testsuite_domain_type_minimal(test_create_secretmem_t)
+allow test_create_secretmem_t self:anon_inode create;
+
+# Domain allowed to create secret memory with the own domain type and allowed to map WX
+type test_create_wx_secretmem_t;
+testsuite_domain_type_minimal(test_create_wx_secretmem_t)
+allow test_create_wx_secretmem_t self:anon_inode create;
+allow test_create_wx_secretmem_t self:process execmem;
+
+# Domain not allowed to create secret memory via a type transition to a private type
+type test_nocreate_transition_secretmem_t;
+testsuite_domain_type_minimal(test_nocreate_transition_secretmem_t)
+type_transition test_nocreate_transition_secretmem_t test_nocreate_transition_secretmem_t:anon_inode test_secretmem_inode_t "[secretmem]";
+
+# Domain allowed to create secret memory via a type transition to a private type
+type test_create_transition_secretmem_t;
+testsuite_domain_type_minimal(test_create_transition_secretmem_t)
+type_transition test_create_transition_secretmem_t test_create_transition_secretmem_t:anon_inode test_secretmem_inode_t "[secretmem]";
+allow test_create_transition_secretmem_t test_secretmem_inode_t:anon_inode create;
diff --git a/tests/Makefile b/tests/Makefile
index 8abd438..b50cbc6 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -145,6 +145,11 @@ SUBDIRS += vsock_socket
endif
endif
+# memfd_secret(2) is available since 5.14, but only confined since 6.0
+ifneq ($(shell ./kvercmp $$(uname -r) 6.0),-1)
+SUBDIRS += secretmem
+endif
+
ifeq ($(DISTRO),RHEL4)
SUBDIRS:=$(filter-out bounds dyntrace dyntrans inet_socket mmap nnp_nosuid overlay unix_socket, $(SUBDIRS))
endif
diff --git a/tests/secretmem/.gitignore b/tests/secretmem/.gitignore
new file mode 100644
index 0000000..17e1c19
--- /dev/null
+++ b/tests/secretmem/.gitignore
@@ -0,0 +1 @@
+secretmem
diff --git a/tests/secretmem/Makefile b/tests/secretmem/Makefile
new file mode 100644
index 0000000..c27d5f0
--- /dev/null
+++ b/tests/secretmem/Makefile
@@ -0,0 +1,5 @@
+TARGETS=secretmem
+
+all: $(TARGETS)
+clean:
+ rm -f $(TARGETS)
diff --git a/tests/secretmem/secretmem.c b/tests/secretmem/secretmem.c
new file mode 100644
index 0000000..0d541ee
--- /dev/null
+++ b/tests/secretmem/secretmem.c
@@ -0,0 +1,83 @@
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <sys/mman.h>
+#include <sys/syscall.h>
+
+#ifndef __NR_memfd_secret
+# define __NR_memfd_secret 447
+#endif
+
+#define TEXT "Hello World!\nHello World!\nHello World!\nHello World!\nHello World!\nHello World!\n"
+
+static int _memfd_secret(unsigned long flags)
+{
+ return syscall(__NR_memfd_secret, flags);
+}
+
+int main(int argc, const char *argv[])
+{
+ long page_size;
+ int fd, flags;
+ char *mem;
+ bool check = (argc == 2 && strcmp(argv[1], "check") == 0);
+ bool wx = (argc == 2 && strcmp(argv[1], "wx") == 0);
+
+ page_size = sysconf(_SC_PAGESIZE);
+ if (page_size <= 0) {
+ fprintf(stderr, "failed to get pagesize, got %ld: %s\n", page_size,
+ strerror(errno));
+ return EXIT_FAILURE;
+ }
+
+ fd = _memfd_secret(0);
+ if (fd < 0) {
+ printf("memfd_secret() failed: %s\n", strerror(errno));
+ if (check && errno != ENOSYS)
+ return EXIT_SUCCESS;
+
+ return EXIT_FAILURE;
+ }
+
+ if (check)
+ return EXIT_SUCCESS;
+
+ if (ftruncate(fd, page_size) < 0) {
+ fprintf(stderr, "ftruncate failed: %s\n", strerror(errno));
+ }
+
+ flags = PROT_READ | PROT_WRITE;
+ if (wx)
+ flags |= PROT_EXEC;
+
+ mem = mmap(NULL, page_size, flags, MAP_SHARED, fd, 0);
+ if (mem == MAP_FAILED || !mem) {
+ printf("unable to mmap secret memory: %s\n", strerror(errno));
+ close(fd);
+ return EXIT_FAILURE;
+ }
+
+ close(fd);
+
+ memcpy(mem, TEXT, sizeof TEXT);
+
+ if (memcmp(mem, TEXT, sizeof TEXT) != 0) {
+ fprintf(stderr, "data not synced (1)\n");
+ munmap(mem, page_size);
+ return EXIT_FAILURE;
+ }
+
+ if (strlen(mem) + 1 != sizeof TEXT) {
+ fprintf(stderr, "data not synced (2)\n");
+ munmap(mem, page_size);
+ return EXIT_FAILURE;
+ }
+
+ munmap(mem, page_size);
+
+ return EXIT_SUCCESS;
+}
diff --git a/tests/secretmem/test b/tests/secretmem/test
new file mode 100755
index 0000000..d17d423
--- /dev/null
+++ b/tests/secretmem/test
@@ -0,0 +1,39 @@
+#!/usr/bin/perl
+
+use Test::More;
+
+BEGIN {
+ $basedir = $0;
+ $basedir =~ s|(.*)/[^/]*|$1|;
+
+ $result =
+ system "runcon -t test_nocreate_secretmem_t $basedir/secretmem check";
+ if ( $result ne 0 ) {
+ plan skip_all => "memfd_secret(2) not available";
+ }
+ else {
+ plan tests => 6;
+ }
+}
+
+$result = system "runcon -t test_nocreate_secretmem_t $basedir/secretmem";
+ok( $result >> 8 eq 1 );
+
+$result = system "runcon -t test_create_secretmem_t $basedir/secretmem";
+ok( $result eq 0 );
+
+$result = system "runcon -t test_create_secretmem_t $basedir/secretmem wx";
+ok( $result >> 8 eq 1 );
+
+$result = system "runcon -t test_create_wx_secretmem_t $basedir/secretmem wx";
+ok( $result >> 8 eq 1 );
+
+$result =
+ system "runcon -t test_nocreate_transition_secretmem_t $basedir/secretmem";
+ok( $result >> 8 eq 1 );
+
+$result =
+ system "runcon -t test_create_transition_secretmem_t $basedir/secretmem";
+ok( $result eq 0 );
+
+exit;
--
2.37.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] tests/secretmem: add test
2022-08-31 15:34 [PATCH v2] tests/secretmem: add test Christian Göttsche
@ 2022-09-12 13:41 ` Ondrej Mosnacek
2022-10-11 14:02 ` Ondrej Mosnacek
0 siblings, 1 reply; 4+ messages in thread
From: Ondrej Mosnacek @ 2022-09-12 13:41 UTC (permalink / raw)
To: Christian Göttsche; +Cc: SElinux list
On Wed, Aug 31, 2022 at 5:34 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
> Add test for memfd_secret(2) anonymous inodes check added in 6.0 via
> 2bfe15c52612 ("mm: create security context for memfd_secret inodes").
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
> v2:
> - print mmap failures to stdout, since they are expected when
> mapping with PROT_EXEC
> ---
> .github/workflows/checks.yml | 4 ++
> Vagrantfile | 10 +++--
> policy/Makefile | 4 ++
> policy/test_secretmem.te | 33 ++++++++++++++
> tests/Makefile | 5 +++
> tests/secretmem/.gitignore | 1 +
> tests/secretmem/Makefile | 5 +++
> tests/secretmem/secretmem.c | 83 ++++++++++++++++++++++++++++++++++++
> tests/secretmem/test | 39 +++++++++++++++++
> 9 files changed, 180 insertions(+), 4 deletions(-)
> create mode 100644 policy/test_secretmem.te
> create mode 100644 tests/secretmem/.gitignore
> create mode 100644 tests/secretmem/Makefile
> create mode 100644 tests/secretmem/secretmem.c
> create mode 100755 tests/secretmem/test
This looks good to me, with some minor comments below.
> diff --git a/tests/secretmem/secretmem.c b/tests/secretmem/secretmem.c
> new file mode 100644
> index 0000000..0d541ee
> --- /dev/null
> +++ b/tests/secretmem/secretmem.c
> @@ -0,0 +1,83 @@
> +#include <errno.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <sys/mman.h>
> +#include <sys/syscall.h>
> +
> +#ifndef __NR_memfd_secret
> +# define __NR_memfd_secret 447
> +#endif
> +
> +#define TEXT "Hello World!\nHello World!\nHello World!\nHello World!\nHello World!\nHello World!\n"
> +
> +static int _memfd_secret(unsigned long flags)
> +{
> + return syscall(__NR_memfd_secret, flags);
> +}
> +
> +int main(int argc, const char *argv[])
> +{
> + long page_size;
> + int fd, flags;
> + char *mem;
> + bool check = (argc == 2 && strcmp(argv[1], "check") == 0);
> + bool wx = (argc == 2 && strcmp(argv[1], "wx") == 0);
> +
> + page_size = sysconf(_SC_PAGESIZE);
> + if (page_size <= 0) {
> + fprintf(stderr, "failed to get pagesize, got %ld: %s\n", page_size,
> + strerror(errno));
> + return EXIT_FAILURE;
> + }
> +
> + fd = _memfd_secret(0);
> + if (fd < 0) {
> + printf("memfd_secret() failed: %s\n", strerror(errno));
> + if (check && errno != ENOSYS)
> + return EXIT_SUCCESS;
> +
> + return EXIT_FAILURE;
> + }
> +
> + if (check)
> + return EXIT_SUCCESS;
> +
> + if (ftruncate(fd, page_size) < 0) {
> + fprintf(stderr, "ftruncate failed: %s\n", strerror(errno));
> + }
> +
> + flags = PROT_READ | PROT_WRITE;
> + if (wx)
> + flags |= PROT_EXEC;
> +
> + mem = mmap(NULL, page_size, flags, MAP_SHARED, fd, 0);
> + if (mem == MAP_FAILED || !mem) {
> + printf("unable to mmap secret memory: %s\n", strerror(errno));
> + close(fd);
> + return EXIT_FAILURE;
> + }
> +
> + close(fd);
> +
> + memcpy(mem, TEXT, sizeof TEXT);
Please use parentheses with sizeof. When the argument is a type name
they are mandatory so it's better to use them always for consistency.
See also:
https://lore.kernel.org/lkml/CA+55aFwey-q4716pYYSi=3R_ucw84zFspDXMXmzvzc72XSc9Lg@mail.gmail.com/
https://lore.kernel.org/lkml/CA+55aFwcJgAFiow1sSo7mkF9n0MpTw80gjAszazyBrRcmbph-g@mail.gmail.com/
> +
> + if (memcmp(mem, TEXT, sizeof TEXT) != 0) {
> + fprintf(stderr, "data not synced (1)\n");
> + munmap(mem, page_size);
> + return EXIT_FAILURE;
> + }
> +
> + if (strlen(mem) + 1 != sizeof TEXT) {
> + fprintf(stderr, "data not synced (2)\n");
> + munmap(mem, page_size);
> + return EXIT_FAILURE;
> + }
What is the point of this second check? The previous check already
asserts that the contents are char-to-char equal to TEXT (including
the terminating null character), which implies string length
equivalence as well.
> +
> + munmap(mem, page_size);
> +
> + return EXIT_SUCCESS;
> +}
--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] tests/secretmem: add test
2022-09-12 13:41 ` Ondrej Mosnacek
@ 2022-10-11 14:02 ` Ondrej Mosnacek
2022-10-11 16:10 ` Christian Göttsche
0 siblings, 1 reply; 4+ messages in thread
From: Ondrej Mosnacek @ 2022-10-11 14:02 UTC (permalink / raw)
To: Christian Göttsche; +Cc: SElinux list
On Mon, Sep 12, 2022 at 3:41 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> On Wed, Aug 31, 2022 at 5:34 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> > Add test for memfd_secret(2) anonymous inodes check added in 6.0 via
> > 2bfe15c52612 ("mm: create security context for memfd_secret inodes").
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> > v2:
> > - print mmap failures to stdout, since they are expected when
> > mapping with PROT_EXEC
> > ---
> > .github/workflows/checks.yml | 4 ++
> > Vagrantfile | 10 +++--
> > policy/Makefile | 4 ++
> > policy/test_secretmem.te | 33 ++++++++++++++
> > tests/Makefile | 5 +++
> > tests/secretmem/.gitignore | 1 +
> > tests/secretmem/Makefile | 5 +++
> > tests/secretmem/secretmem.c | 83 ++++++++++++++++++++++++++++++++++++
> > tests/secretmem/test | 39 +++++++++++++++++
> > 9 files changed, 180 insertions(+), 4 deletions(-)
> > create mode 100644 policy/test_secretmem.te
> > create mode 100644 tests/secretmem/.gitignore
> > create mode 100644 tests/secretmem/Makefile
> > create mode 100644 tests/secretmem/secretmem.c
> > create mode 100755 tests/secretmem/test
>
> This looks good to me, with some minor comments below.
>
> > diff --git a/tests/secretmem/secretmem.c b/tests/secretmem/secretmem.c
> > new file mode 100644
> > index 0000000..0d541ee
> > --- /dev/null
> > +++ b/tests/secretmem/secretmem.c
> > @@ -0,0 +1,83 @@
> > +#include <errno.h>
> > +#include <stdbool.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +
> > +#include <sys/mman.h>
> > +#include <sys/syscall.h>
> > +
> > +#ifndef __NR_memfd_secret
> > +# define __NR_memfd_secret 447
> > +#endif
> > +
> > +#define TEXT "Hello World!\nHello World!\nHello World!\nHello World!\nHello World!\nHello World!\n"
> > +
> > +static int _memfd_secret(unsigned long flags)
> > +{
> > + return syscall(__NR_memfd_secret, flags);
> > +}
> > +
> > +int main(int argc, const char *argv[])
> > +{
> > + long page_size;
> > + int fd, flags;
> > + char *mem;
> > + bool check = (argc == 2 && strcmp(argv[1], "check") == 0);
> > + bool wx = (argc == 2 && strcmp(argv[1], "wx") == 0);
> > +
> > + page_size = sysconf(_SC_PAGESIZE);
> > + if (page_size <= 0) {
> > + fprintf(stderr, "failed to get pagesize, got %ld: %s\n", page_size,
> > + strerror(errno));
> > + return EXIT_FAILURE;
> > + }
> > +
> > + fd = _memfd_secret(0);
> > + if (fd < 0) {
> > + printf("memfd_secret() failed: %s\n", strerror(errno));
> > + if (check && errno != ENOSYS)
> > + return EXIT_SUCCESS;
> > +
> > + return EXIT_FAILURE;
> > + }
> > +
> > + if (check)
> > + return EXIT_SUCCESS;
> > +
> > + if (ftruncate(fd, page_size) < 0) {
> > + fprintf(stderr, "ftruncate failed: %s\n", strerror(errno));
> > + }
> > +
> > + flags = PROT_READ | PROT_WRITE;
> > + if (wx)
> > + flags |= PROT_EXEC;
> > +
> > + mem = mmap(NULL, page_size, flags, MAP_SHARED, fd, 0);
> > + if (mem == MAP_FAILED || !mem) {
> > + printf("unable to mmap secret memory: %s\n", strerror(errno));
> > + close(fd);
> > + return EXIT_FAILURE;
> > + }
> > +
> > + close(fd);
> > +
> > + memcpy(mem, TEXT, sizeof TEXT);
>
> Please use parentheses with sizeof. When the argument is a type name
> they are mandatory so it's better to use them always for consistency.
> See also:
> https://lore.kernel.org/lkml/CA+55aFwey-q4716pYYSi=3R_ucw84zFspDXMXmzvzc72XSc9Lg@mail.gmail.com/
> https://lore.kernel.org/lkml/CA+55aFwcJgAFiow1sSo7mkF9n0MpTw80gjAszazyBrRcmbph-g@mail.gmail.com/
>
> > +
> > + if (memcmp(mem, TEXT, sizeof TEXT) != 0) {
> > + fprintf(stderr, "data not synced (1)\n");
> > + munmap(mem, page_size);
> > + return EXIT_FAILURE;
> > + }
> > +
> > + if (strlen(mem) + 1 != sizeof TEXT) {
> > + fprintf(stderr, "data not synced (2)\n");
> > + munmap(mem, page_size);
> > + return EXIT_FAILURE;
> > + }
>
> What is the point of this second check? The previous check already
> asserts that the contents are char-to-char equal to TEXT (including
> the terminating null character), which implies string length
> equivalence as well.
>
> > +
> > + munmap(mem, page_size);
> > +
> > + return EXIT_SUCCESS;
> > +}
You only pushed the updated version to the GitHub PR
(https://github.com/SELinuxProject/selinux-testsuite/pull/80), but
anyway I took that version and applied it (just removed the changelog
from the commit message and added my SOB):
https://github.com/SELinuxProject/selinux-testsuite/commit/77352e748f006c343d602e4be03ae0d2cfcca831
--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] tests/secretmem: add test
2022-10-11 14:02 ` Ondrej Mosnacek
@ 2022-10-11 16:10 ` Christian Göttsche
0 siblings, 0 replies; 4+ messages in thread
From: Christian Göttsche @ 2022-10-11 16:10 UTC (permalink / raw)
To: Ondrej Mosnacek; +Cc: SElinux list
On Tue, 11 Oct 2022 at 16:02, Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> On Mon, Sep 12, 2022 at 3:41 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > On Wed, Aug 31, 2022 at 5:34 PM Christian Göttsche
> > <cgzones@googlemail.com> wrote:
> > > Add test for memfd_secret(2) anonymous inodes check added in 6.0 via
> > > 2bfe15c52612 ("mm: create security context for memfd_secret inodes").
> > >
> > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > > ---
> > > v2:
> > > - print mmap failures to stdout, since they are expected when
> > > mapping with PROT_EXEC
> > > ---
> > > .github/workflows/checks.yml | 4 ++
> > > Vagrantfile | 10 +++--
> > > policy/Makefile | 4 ++
> > > policy/test_secretmem.te | 33 ++++++++++++++
> > > tests/Makefile | 5 +++
> > > tests/secretmem/.gitignore | 1 +
> > > tests/secretmem/Makefile | 5 +++
> > > tests/secretmem/secretmem.c | 83 ++++++++++++++++++++++++++++++++++++
> > > tests/secretmem/test | 39 +++++++++++++++++
> > > 9 files changed, 180 insertions(+), 4 deletions(-)
> > > create mode 100644 policy/test_secretmem.te
> > > create mode 100644 tests/secretmem/.gitignore
> > > create mode 100644 tests/secretmem/Makefile
> > > create mode 100644 tests/secretmem/secretmem.c
> > > create mode 100755 tests/secretmem/test
> >
> > This looks good to me, with some minor comments below.
> >
> > > diff --git a/tests/secretmem/secretmem.c b/tests/secretmem/secretmem.c
> > > new file mode 100644
> > > index 0000000..0d541ee
> > > --- /dev/null
> > > +++ b/tests/secretmem/secretmem.c
> > > @@ -0,0 +1,83 @@
> > > +#include <errno.h>
> > > +#include <stdbool.h>
> > > +#include <stdio.h>
> > > +#include <stdlib.h>
> > > +#include <string.h>
> > > +#include <unistd.h>
> > > +
> > > +#include <sys/mman.h>
> > > +#include <sys/syscall.h>
> > > +
> > > +#ifndef __NR_memfd_secret
> > > +# define __NR_memfd_secret 447
> > > +#endif
> > > +
> > > +#define TEXT "Hello World!\nHello World!\nHello World!\nHello World!\nHello World!\nHello World!\n"
> > > +
> > > +static int _memfd_secret(unsigned long flags)
> > > +{
> > > + return syscall(__NR_memfd_secret, flags);
> > > +}
> > > +
> > > +int main(int argc, const char *argv[])
> > > +{
> > > + long page_size;
> > > + int fd, flags;
> > > + char *mem;
> > > + bool check = (argc == 2 && strcmp(argv[1], "check") == 0);
> > > + bool wx = (argc == 2 && strcmp(argv[1], "wx") == 0);
> > > +
> > > + page_size = sysconf(_SC_PAGESIZE);
> > > + if (page_size <= 0) {
> > > + fprintf(stderr, "failed to get pagesize, got %ld: %s\n", page_size,
> > > + strerror(errno));
> > > + return EXIT_FAILURE;
> > > + }
> > > +
> > > + fd = _memfd_secret(0);
> > > + if (fd < 0) {
> > > + printf("memfd_secret() failed: %s\n", strerror(errno));
> > > + if (check && errno != ENOSYS)
> > > + return EXIT_SUCCESS;
> > > +
> > > + return EXIT_FAILURE;
> > > + }
> > > +
> > > + if (check)
> > > + return EXIT_SUCCESS;
> > > +
> > > + if (ftruncate(fd, page_size) < 0) {
> > > + fprintf(stderr, "ftruncate failed: %s\n", strerror(errno));
> > > + }
> > > +
> > > + flags = PROT_READ | PROT_WRITE;
> > > + if (wx)
> > > + flags |= PROT_EXEC;
> > > +
> > > + mem = mmap(NULL, page_size, flags, MAP_SHARED, fd, 0);
> > > + if (mem == MAP_FAILED || !mem) {
> > > + printf("unable to mmap secret memory: %s\n", strerror(errno));
> > > + close(fd);
> > > + return EXIT_FAILURE;
> > > + }
> > > +
> > > + close(fd);
> > > +
> > > + memcpy(mem, TEXT, sizeof TEXT);
> >
> > Please use parentheses with sizeof. When the argument is a type name
> > they are mandatory so it's better to use them always for consistency.
> > See also:
> > https://lore.kernel.org/lkml/CA+55aFwey-q4716pYYSi=3R_ucw84zFspDXMXmzvzc72XSc9Lg@mail.gmail.com/
> > https://lore.kernel.org/lkml/CA+55aFwcJgAFiow1sSo7mkF9n0MpTw80gjAszazyBrRcmbph-g@mail.gmail.com/
> >
> > > +
> > > + if (memcmp(mem, TEXT, sizeof TEXT) != 0) {
> > > + fprintf(stderr, "data not synced (1)\n");
> > > + munmap(mem, page_size);
> > > + return EXIT_FAILURE;
> > > + }
> > > +
> > > + if (strlen(mem) + 1 != sizeof TEXT) {
> > > + fprintf(stderr, "data not synced (2)\n");
> > > + munmap(mem, page_size);
> > > + return EXIT_FAILURE;
> > > + }
> >
> > What is the point of this second check? The previous check already
> > asserts that the contents are char-to-char equal to TEXT (including
> > the terminating null character), which implies string length
> > equivalence as well.
> >
> > > +
> > > + munmap(mem, page_size);
> > > +
> > > + return EXIT_SUCCESS;
> > > +}
>
> You only pushed the updated version to the GitHub PR
> (https://github.com/SELinuxProject/selinux-testsuite/pull/80), but
> anyway I took that version and applied it (just removed the changelog
> from the commit message and added my SOB):
> https://github.com/SELinuxProject/selinux-testsuite/commit/77352e748f006c343d602e4be03ae0d2cfcca831
>
Thanks,
I thought I had sent the new revision, but it seems I have missed that.
>
> --
> Ondrej Mosnacek
> Senior Software Engineer, Linux Security - SELinux kernel
> Red Hat, Inc.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-10-11 16:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-31 15:34 [PATCH v2] tests/secretmem: add test Christian Göttsche
2022-09-12 13:41 ` Ondrej Mosnacek
2022-10-11 14:02 ` Ondrej Mosnacek
2022-10-11 16:10 ` Christian Göttsche
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.