Message ID | 20190621102628.4800-4-liwang@redhat.com |
---|---|
State | Superseded |
Headers | show |
Series | Test for Memory Protection Key | expand |
----- Original Message ----- > Signed-off-by: Li Wang <liwang@redhat.com> > + > +static void pkey_tests(int pkey, int prot, int flags, int fd) > +{ > + char *buffer; > + > + if (fd == 0) { > + fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0664); > + } > + > + buffer = SAFE_MMAP(NULL, psize, prot, flags, fd, 0); > + > + if (pkey_mprotect(buffer, psize, prot, pkey) == -1) > + tst_brk(TBROK, "pkey_mprotect failed"); > + > + tst_res(TPASS, "apply pkey to the buffer area success"); > + > + if (fd > 0) { > + SAFE_CLOSE(fd); > + } > + > + SAFE_MUNMAP(buffer, psize); > +} > + Hi, pkey02 doesn't try to read/write as pkey01, but otherwise two tests look very similar. Could we try to read/write here as well for all combinations of map flags? Then pkey01 could be dropped since pkey02 would cover more than just 1 combination. Or is there a different reason behind separate tests, that I'm missing? Regards, Jan
On Mon, Jun 24, 2019 at 2:56 PM Jan Stancek <jstancek@redhat.com> wrote: > > > ----- Original Message ----- > > Signed-off-by: Li Wang <liwang@redhat.com> > > + > > +static void pkey_tests(int pkey, int prot, int flags, int fd) > > +{ > > + char *buffer; > > + > > + if (fd == 0) { > > + fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0664); > > + } > > + > > + buffer = SAFE_MMAP(NULL, psize, prot, flags, fd, 0); > > + > > + if (pkey_mprotect(buffer, psize, prot, pkey) == -1) > > + tst_brk(TBROK, "pkey_mprotect failed"); > > + > > + tst_res(TPASS, "apply pkey to the buffer area success"); > > + > > + if (fd > 0) { > > + SAFE_CLOSE(fd); > > + } > > + > > + SAFE_MUNMAP(buffer, psize); > > +} > > + > > Hi, > > pkey02 doesn't try to read/write as pkey01, but otherwise two tests look > very similar. > > Could we try to read/write here as well for all combinations of map flags? > Then pkey01 could be dropped since pkey02 would cover more than just 1 > combination. > Or is there a different reason behind separate tests, that I'm missing? > > The pkey02 is a follow new test idea(test more types of memory) after I completed pkey01. Yes, the diffenrence bettwen them is pkey02 cover more map flags but not do R/W verification. That's because I'm hoping to add {0, 0x0} to the test which does not trigger SIGSEGV in pkey02, it seems a bit tricky to distinguish the SIGSEGV is come from 0x0(if bug there) or PKEY_DISABLE_ACCESS progress.
On Mon, Jun 24, 2019 at 3:27 PM Li Wang <liwang@redhat.com> wrote: > > > On Mon, Jun 24, 2019 at 2:56 PM Jan Stancek <jstancek@redhat.com> wrote: > >> >> >> ----- Original Message ----- >> > Signed-off-by: Li Wang <liwang@redhat.com> >> > + >> > +static void pkey_tests(int pkey, int prot, int flags, int fd) >> > +{ >> > + char *buffer; >> > + >> > + if (fd == 0) { >> > + fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0664); >> > + } >> > + >> > + buffer = SAFE_MMAP(NULL, psize, prot, flags, fd, 0); >> > + >> > + if (pkey_mprotect(buffer, psize, prot, pkey) == -1) >> > + tst_brk(TBROK, "pkey_mprotect failed"); >> > + >> > + tst_res(TPASS, "apply pkey to the buffer area success"); >> > + >> > + if (fd > 0) { >> > + SAFE_CLOSE(fd); >> > + } >> > + >> > + SAFE_MUNMAP(buffer, psize); >> > +} >> > + >> >> Hi, >> >> pkey02 doesn't try to read/write as pkey01, but otherwise two tests look >> very similar. >> >> Could we try to read/write here as well for all combinations of map flags? >> Then pkey01 could be dropped since pkey02 would cover more than just 1 >> combination. >> Or is there a different reason behind separate tests, that I'm missing? >> >> > The pkey02 is a follow new test idea(test more types of memory) after I > completed pkey01. > > Yes, the diffenrence bettwen them is pkey02 cover more map flags but not > do R/W verification. That's because I'm hoping to add {0, 0x0} to the test > which does not trigger SIGSEGV in pkey02, it seems a bit tricky to > distinguish the SIGSEGV is come from 0x0(if bug there) > or PKEY_DISABLE_ACCESS progress. > > Well, a smple way I can find is to remove the {0, 0x0} from struct tcase, and test the access right 0x0 in lastly. So, what do you think about the new key02(merge key01 and old key02) in this attatchment?
----- Original Message ----- > On Mon, Jun 24, 2019 at 3:27 PM Li Wang <liwang@redhat.com> wrote: > > > > > > > On Mon, Jun 24, 2019 at 2:56 PM Jan Stancek <jstancek@redhat.com> wrote: > > > >> > >> > >> ----- Original Message ----- > >> > Signed-off-by: Li Wang <liwang@redhat.com> > >> > + > >> > +static void pkey_tests(int pkey, int prot, int flags, int fd) > >> > +{ > >> > + char *buffer; > >> > + > >> > + if (fd == 0) { > >> > + fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0664); > >> > + } > >> > + > >> > + buffer = SAFE_MMAP(NULL, psize, prot, flags, fd, 0); > >> > + > >> > + if (pkey_mprotect(buffer, psize, prot, pkey) == -1) > >> > + tst_brk(TBROK, "pkey_mprotect failed"); > >> > + > >> > + tst_res(TPASS, "apply pkey to the buffer area success"); > >> > + > >> > + if (fd > 0) { > >> > + SAFE_CLOSE(fd); > >> > + } > >> > + > >> > + SAFE_MUNMAP(buffer, psize); > >> > +} > >> > + > >> > >> Hi, > >> > >> pkey02 doesn't try to read/write as pkey01, but otherwise two tests look > >> very similar. > >> > >> Could we try to read/write here as well for all combinations of map flags? > >> Then pkey01 could be dropped since pkey02 would cover more than just 1 > >> combination. > >> Or is there a different reason behind separate tests, that I'm missing? > >> > >> > > The pkey02 is a follow new test idea(test more types of memory) after I > > completed pkey01. > > > > Yes, the diffenrence bettwen them is pkey02 cover more map flags but not > > do R/W verification. That's because I'm hoping to add {0, 0x0} to the test > > which does not trigger SIGSEGV in pkey02, it seems a bit tricky to > > distinguish the SIGSEGV is come from 0x0(if bug there) > > or PKEY_DISABLE_ACCESS progress. > > > > > Well, a smple way I can find is to remove the {0, 0x0} from struct tcase, > and test the access right 0x0 in lastly. You could also keep {0, 0x0}, but extend struct with "int fault_expected", and set it to 0/1 depending on flags. > > So, what do you think about the new key02(merge key01 and old key02) in > this attatchment? That works too. Some comments below: if (fd > 0) { SAFE_CLOSE(fd); } 0 is valid fd, brackets are not needed. All tst_* functions that report failures should also have TERRNO. tst_res(TFAIL, "Child unexpectedly ended") could print also exit code. Regards, Jan
diff --git a/runtest/syscalls b/runtest/syscalls index 6ea991f12..fddf071f9 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -722,6 +722,7 @@ mprotect03 mprotect03 mprotect04 mprotect04 pkey01 pkey01 +pkey02 pkey02 mq_notify01 mq_notify01 mq_notify02 mq_notify02 diff --git a/testcases/kernel/syscalls/pkeys/.gitignore b/testcases/kernel/syscalls/pkeys/.gitignore index 6fd5addb8..2f9ea0a1f 100644 --- a/testcases/kernel/syscalls/pkeys/.gitignore +++ b/testcases/kernel/syscalls/pkeys/.gitignore @@ -1 +1,2 @@ /pkey01 +/pkey02 diff --git a/testcases/kernel/syscalls/pkeys/pkey02.c b/testcases/kernel/syscalls/pkeys/pkey02.c new file mode 100644 index 000000000..ed6ea3c9d --- /dev/null +++ b/testcases/kernel/syscalls/pkeys/pkey02.c @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2019 Red Hat, Inc. + * + * Test for Memory Protection Keys on different memory area. + */ + +#define _GNU_SOURCE +#include <stdio.h> +#include <unistd.h> +#include <errno.h> +#include <stdlib.h> +#include <sys/syscall.h> +#include <sys/mman.h> +#include <sys/wait.h> + +#include "pkey.h" + +#define TMP_DIR "tmp_pkey" +#define TEST_FILE TMP_DIR"/testfile" +#define STR "abcdefghijklmnopqrstuvwxyz12345\n" + +static int psize; + +static struct tcase { + unsigned long flags; + unsigned long access_rights; +} tcases[] = { + {0, 0x0}, + {0, PKEY_DISABLE_ACCESS}, + {0, PKEY_DISABLE_WRITE}, +}; + +static void setup(void) +{ + int i, fd; + + psize = getpagesize(); + SAFE_MKDIR(TMP_DIR, 0664); + SAFE_MOUNT(TMP_DIR, TMP_DIR, "tmpfs", 0, NULL); + + check_pkey_support(); + + fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0664); + for (i = 0; i < 128; i++) + SAFE_WRITE(1, fd, STR, strlen(STR)); + + SAFE_CLOSE(fd); +} + +static void cleanup(void) +{ + SAFE_UMOUNT(TMP_DIR); + SAFE_RMDIR(TMP_DIR); +} + +static struct mmap_param { + int prot; + int flags; + int fd; +} mmap_params[] = { + {PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1}, + {PROT_READ, MAP_ANONYMOUS | MAP_SHARED, -1}, + {PROT_READ, MAP_PRIVATE, 0}, + {PROT_READ, MAP_SHARED, 0}, + {PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1}, + {PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED, -1}, + {PROT_WRITE, MAP_PRIVATE, 0}, + {PROT_WRITE, MAP_SHARED, 0}, + {PROT_EXEC, MAP_ANONYMOUS | MAP_PRIVATE, -1}, + {PROT_EXEC, MAP_ANONYMOUS | MAP_SHARED, -1}, + {PROT_EXEC, MAP_PRIVATE, 0}, + {PROT_EXEC, MAP_SHARED, 0}, + {PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1}, + {PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED, -1}, + {PROT_READ | PROT_WRITE, MAP_PRIVATE, 0}, + {PROT_READ | PROT_WRITE, MAP_SHARED, 0}, + {PROT_READ | PROT_WRITE | PROT_EXEC, MAP_ANONYMOUS | MAP_PRIVATE, -1}, + {PROT_READ | PROT_WRITE | PROT_EXEC, MAP_ANONYMOUS | MAP_SHARED, -1}, + {PROT_READ | PROT_WRITE | PROT_EXEC, MAP_PRIVATE, 0}, + {PROT_READ | PROT_WRITE | PROT_EXEC, MAP_SHARED, 0}, +}; + +static void pkey_tests(int pkey, int prot, int flags, int fd) +{ + char *buffer; + + if (fd == 0) { + fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0664); + } + + buffer = SAFE_MMAP(NULL, psize, prot, flags, fd, 0); + + if (pkey_mprotect(buffer, psize, prot, pkey) == -1) + tst_brk(TBROK, "pkey_mprotect failed"); + + tst_res(TPASS, "apply pkey to the buffer area success"); + + if (fd > 0) { + SAFE_CLOSE(fd); + } + + SAFE_MUNMAP(buffer, psize); +} + +static void verify_pkey(unsigned int i) +{ + int pkey; + long unsigned int j; + + struct tcase *tc = &tcases[i]; + struct mmap_param *mpa; + + for (j = 0; j < ARRAY_SIZE(mmap_params); j++) { + mpa = &mmap_params[j]; + + pkey = pkey_alloc(tc->flags, tc->access_rights); + if (pkey == -1) + tst_brk(TBROK, "pkey_alloc failed"); + + pkey_tests(pkey, mpa->prot, mpa->flags, mpa->fd); + + if (pkey_free(pkey) == -1) + tst_brk(TBROK, "pkey_free failed"); + } +} + +static struct tst_test test = { + .tcnt = ARRAY_SIZE(tcases), + .forks_child = 1, + .test = verify_pkey, + .setup = setup, + .cleanup = cleanup, +};
Signed-off-by: Li Wang <liwang@redhat.com> --- runtest/syscalls | 1 + testcases/kernel/syscalls/pkeys/.gitignore | 1 + testcases/kernel/syscalls/pkeys/pkey02.c | 134 +++++++++++++++++++++ 3 files changed, 136 insertions(+) create mode 100644 testcases/kernel/syscalls/pkeys/pkey02.c