* [LTP] [PATCH 1/2] lib: allow checkpoints to be used by any uid @ 2017-10-04 10:42 Jan Stancek 2017-10-04 10:42 ` [LTP] [PATCH 2/2] security/dirtyc0w: synchronize parent and child Jan Stancek 0 siblings, 1 reply; 6+ messages in thread From: Jan Stancek @ 2017-10-04 10:42 UTC (permalink / raw) To: ltp Allow unprivileged child to synchronize with privileged parent. Use chmod after open, because effective permissions set by open() are modified by the process's umask: (mode & ~umask). Signed-off-by: Jan Stancek <jstancek@redhat.com> --- lib/tst_test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/tst_test.c b/lib/tst_test.c index 26414e31ca77..233b370794a6 100644 --- a/lib/tst_test.c +++ b/lib/tst_test.c @@ -93,6 +93,7 @@ static void setup_ipc(void) ipc_fd = open(shm_path, O_CREAT | O_EXCL | O_RDWR, 0600); if (ipc_fd < 0) tst_brk(TBROK | TERRNO, "open(%s)", shm_path); + SAFE_CHMOD(shm_path, 0666); SAFE_FTRUNCATE(ipc_fd, size); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [LTP] [PATCH 2/2] security/dirtyc0w: synchronize parent and child 2017-10-04 10:42 [LTP] [PATCH 1/2] lib: allow checkpoints to be used by any uid Jan Stancek @ 2017-10-04 10:42 ` Jan Stancek 2017-10-04 12:02 ` Cyril Hrubis 0 siblings, 1 reply; 6+ messages in thread From: Jan Stancek @ 2017-10-04 10:42 UTC (permalink / raw) To: ltp Add checkpoint to guarantee that parent doesn't send signal to child before it sets up signal handler. Signed-off-by: Jan Stancek <jstancek@redhat.com> --- testcases/kernel/security/dirtyc0w/dirtyc0w.c | 14 +++++++++++++- testcases/kernel/security/dirtyc0w/dirtyc0w_child.c | 3 +++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/testcases/kernel/security/dirtyc0w/dirtyc0w.c b/testcases/kernel/security/dirtyc0w/dirtyc0w.c index b8094ba977ab..5658f3233f59 100644 --- a/testcases/kernel/security/dirtyc0w/dirtyc0w.c +++ b/testcases/kernel/security/dirtyc0w/dirtyc0w.c @@ -36,6 +36,7 @@ * mm: remove gup_flags FOLL_WRITE games from __get_user_pages() */ +#include <errno.h> #include <sys/mman.h> #include <fcntl.h> #include <pthread.h> @@ -49,6 +50,7 @@ #define FNAME "test" #define STR "this is not a test\n" +#define TEST_APP "dirtyc0w_child" static uid_t nobody_uid; static gid_t nobody_gid; @@ -67,6 +69,7 @@ void dirtyc0w_test(void) { int i, fd, pid, fail = 0; char c; + char *av[] = {TEST_APP, NULL}; /* Create file */ fd = SAFE_OPEN(FNAME, O_WRONLY|O_CREAT|O_EXCL, 0444); @@ -78,9 +81,11 @@ void dirtyc0w_test(void) if (!pid) { SAFE_SETGID(nobody_gid); SAFE_SETUID(nobody_uid); - SAFE_EXECLP("dirtyc0w_child", "dirtyc0w_child", NULL); + (void)execve(TEST_APP, av, tst_ipc_envp); + tst_brk(TBROK|TERRNO, "execve failed"); } + TST_CHECKPOINT_WAIT(0); for (i = 0; i < 100; i++) { usleep(10000); @@ -102,10 +107,17 @@ void dirtyc0w_test(void) tst_res(TPASS, "Bug not reproduced"); } +static const char *const resource_files[] = { + TEST_APP, + NULL, +}; + static struct tst_test test = { .needs_tmpdir = 1, + .needs_checkpoints = 1, .forks_child = 1, .needs_root = 1, .setup = setup, .test_all = dirtyc0w_test, + .resource_files = resource_files, }; diff --git a/testcases/kernel/security/dirtyc0w/dirtyc0w_child.c b/testcases/kernel/security/dirtyc0w/dirtyc0w_child.c index 49abdd6ba52e..bb93c62cb979 100644 --- a/testcases/kernel/security/dirtyc0w/dirtyc0w_child.c +++ b/testcases/kernel/security/dirtyc0w/dirtyc0w_child.c @@ -104,7 +104,10 @@ int main(void) int fd; struct stat st; + tst_reinit(); + SAFE_SIGNAL(SIGUSR1, sighandler); + TST_CHECKPOINT_WAKE(0); /* Open it read only and map */ fd = SAFE_OPEN(FNAME, O_RDONLY); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [LTP] [PATCH 2/2] security/dirtyc0w: synchronize parent and child 2017-10-04 10:42 ` [LTP] [PATCH 2/2] security/dirtyc0w: synchronize parent and child Jan Stancek @ 2017-10-04 12:02 ` Cyril Hrubis 2017-10-04 12:25 ` Jan Stancek 0 siblings, 1 reply; 6+ messages in thread From: Cyril Hrubis @ 2017-10-04 12:02 UTC (permalink / raw) To: ltp Hi! > +static const char *const resource_files[] = { > + TEST_APP, > + NULL, > +}; I do not get why we need the test binary to be listed as resource file, the rest seems OK to me. -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH 2/2] security/dirtyc0w: synchronize parent and child 2017-10-04 12:02 ` Cyril Hrubis @ 2017-10-04 12:25 ` Jan Stancek 2017-10-06 11:37 ` Cyril Hrubis 0 siblings, 1 reply; 6+ messages in thread From: Jan Stancek @ 2017-10-04 12:25 UTC (permalink / raw) To: ltp ----- Original Message ----- > Hi! > > +static const char *const resource_files[] = { > > + TEST_APP, > > + NULL, > > +}; > > I do not get why we need the test binary to be listed as resource file, > the rest seems OK to me. Since the test is using tmpdir, we either need exec*p* variant and set PATH or copy binary to tmpdir. I took inspiration from creat07. Copy approach is slightly more convenient when running test from git tree: # ./dirtyc0w But I can replace it with execvpe() - user would just need to setup PATH before running it from git tree: # env PATH=`pwd`:$PATH ./dirtyc0w Regards, Jan ^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH 2/2] security/dirtyc0w: synchronize parent and child 2017-10-04 12:25 ` Jan Stancek @ 2017-10-06 11:37 ` Cyril Hrubis 2017-10-06 11:57 ` Jan Stancek 0 siblings, 1 reply; 6+ messages in thread From: Cyril Hrubis @ 2017-10-06 11:37 UTC (permalink / raw) To: ltp Hi! > > > +static const char *const resource_files[] = { > > > + TEST_APP, > > > + NULL, > > > +}; > > > > I do not get why we need the test binary to be listed as resource file, > > the rest seems OK to me. > > Since the test is using tmpdir, we either need exec*p* variant and > set PATH or copy binary to tmpdir. I took inspiration from creat07. We actually copy the binary for creat07 since are are trying to overwrite it, which is supposed to fail, but I do not want to overwrite the installed files in a case that the kernel is buggy. > Copy approach is slightly more convenient when running test from git tree: > # ./dirtyc0w > > But I can replace it with execvpe() - user would just need to setup PATH > before running it from git tree: > # env PATH=`pwd`:$PATH ./dirtyc0w You are supposed to do that, but maybe we can add the code to change PATH to the test library itself, so it will include $PWD automatically, which would fix this problem for quite a lot of test without a need to modify these. Or add a modified exec function to the test library that locates the binary first. -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH 2/2] security/dirtyc0w: synchronize parent and child 2017-10-06 11:37 ` Cyril Hrubis @ 2017-10-06 11:57 ` Jan Stancek 0 siblings, 0 replies; 6+ messages in thread From: Jan Stancek @ 2017-10-06 11:57 UTC (permalink / raw) To: ltp ----- Original Message ----- > Hi! > > > > +static const char *const resource_files[] = { > > > > + TEST_APP, > > > > + NULL, > > > > +}; > > > > > > I do not get why we need the test binary to be listed as resource file, > > > the rest seems OK to me. > > > > Since the test is using tmpdir, we either need exec*p* variant and > > set PATH or copy binary to tmpdir. I took inspiration from creat07. > > We actually copy the binary for creat07 since are are trying to > overwrite it, which is supposed to fail, but I do not want to overwrite > the installed files in a case that the kernel is buggy. I see, I'll send v2 which drops resource_files. > > > Copy approach is slightly more convenient when running test from git tree: > > # ./dirtyc0w > > > > But I can replace it with execvpe() - user would just need to setup PATH > > before running it from git tree: > > # env PATH=`pwd`:$PATH ./dirtyc0w > > You are supposed to do that, but maybe we can add the code to change > PATH to the test library itself, so it will include $PWD automatically, > which would fix this problem for quite a lot of test without a need to > modify these. We'd also have to make sure all tests use exec*p* variant, right? Regards, Jan ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-06 11:57 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-04 10:42 [LTP] [PATCH 1/2] lib: allow checkpoints to be used by any uid Jan Stancek 2017-10-04 10:42 ` [LTP] [PATCH 2/2] security/dirtyc0w: synchronize parent and child Jan Stancek 2017-10-04 12:02 ` Cyril Hrubis 2017-10-04 12:25 ` Jan Stancek 2017-10-06 11:37 ` Cyril Hrubis 2017-10-06 11:57 ` Jan Stancek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox