| Message ID | 20230510124206.19627-4-andrea.cervesato@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Refactor mqns testing suite | expand |
Hi! > +#include "tst_test.h" > +#include "lapi/sched.h" > +#include "tst_safe_posix_ipc.h" > +#include "tst_safe_stdio.h" > + > +#define CHECK_MQ_OPEN_RET(x) ((x) >= 0 || ((x) == -1 && errno != EMFILE)) > + > +#define MQNAME1 "/MQ1" > +#define MQNAME2 "/MQ2" > + > +static char *str_op; > +static char *devdir; > +static char *mqueue1; > +static char *mqueue2; > +static int *mq_freed1; > +static int *mq_freed2; > + > +static void check_mqueue(void) > { > - char buf[30]; > - mqd_t mqd; > int rc; > + mqd_t mqd; > struct stat statbuf; > > - (void) vtest; > + tst_res(TINFO, "Creating %s mqueue from within child process", MQNAME1); > > - close(p1[1]); > - close(p2[0]); > + mqd = TST_RETRY_FUNC( > + mq_open(MQNAME1, O_RDWR | O_CREAT | O_EXCL, 0777, NULL), > + CHECK_MQ_OPEN_RET); > + if (mqd == -1) > + tst_brk(TBROK | TERRNO, "mq_open failed"); > > - if (read(p1[0], buf, 3) != 3) { /* go */ > - perror("read failed"); > - exit(1); > - } > + SAFE_MQ_CLOSE(mqd); > + tst_atomic_inc(mq_freed1); I do not think that we need atomicity here, the cleanup code does not run concurently at all as the cleanup in the parent is triggered after the child did exit. I suppose that instead we need to set the mq_freed to be volatile because it's shared memory which may change at any change, so we need to tell that to the compiler. > - mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDWR | O_CREAT | O_EXCL, > - 0755, NULL); > - if (mqd == -1) { > - write(p2[1], "mqfail", 7); > - exit(1); > - } > + tst_res(TINFO, "Mount %s from within child process", devdir); > > - mq_close(mqd); > + SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL); > > - rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL); > - if (rc == -1) { > - write(p2[1], "mount1", 7); > - exit(1); > - } > + SAFE_STAT(mqueue1, &statbuf); > + tst_res(TPASS, "%s exists at first mount", mqueue1); > > - rc = stat(FNAM1, &statbuf); > - if (rc == -1) { > - write(p2[1], "stat1", 6); > - exit(1); > - } > + tst_res(TINFO, "Creating %s from within child process", mqueue2); > > - rc = creat(FNAM2, 0755); > - if (rc == -1) { > - write(p2[1], "creat", 6); > - exit(1); > - } > + rc = SAFE_CREAT(mqueue2, 0755); > + SAFE_CLOSE(rc); > + tst_atomic_inc(mq_freed2); > > - close(rc); > + tst_res(TINFO, "Mount %s from within child process a second time", devdir); > > - rc = umount(DEV_MQUEUE2); > - if (rc == -1) { > - write(p2[1], "umount", 7); > - exit(1); > - } > + SAFE_UMOUNT(devdir); > + SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL); > > - rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL); > - if (rc == -1) { > - write(p2[1], "mount2", 7); > - exit(1); > - } > + SAFE_STAT(mqueue1, &statbuf); > + tst_res(TPASS, "%s exists at second mount", mqueue1); > > - rc = stat(FNAM1, &statbuf); > - if (rc == -1) { > - write(p2[1], "stat2", 7); > - exit(1); > - } > + SAFE_STAT(mqueue2, &statbuf); > + tst_res(TPASS, "%s exists at second mount", mqueue2); > > - rc = stat(FNAM2, &statbuf); > - if (rc == -1) { > - write(p2[1], "stat3", 7); > - exit(1); > - } > + SAFE_UMOUNT(devdir); > + > + SAFE_MQ_UNLINK(MQNAME1); > + tst_atomic_store(0, mq_freed1); > > - write(p2[1], "done", 5); > + SAFE_MQ_UNLINK(MQNAME2); > + tst_atomic_store(0, mq_freed2); > +} > > - exit(0); > +static void run(void) > +{ > + const struct tst_clone_args clone_args = { CLONE_NEWIPC, SIGCHLD }; > + > + if (str_op && !strcmp(str_op, "clone")) { > + tst_res(TINFO, "Spawning isolated process"); > + > + if (!SAFE_CLONE(&clone_args)) { > + check_mqueue(); > + return; > + } > + } else if (str_op && !strcmp(str_op, "unshare")) { > + tst_res(TINFO, "Spawning unshared process"); > + > + if (!SAFE_FORK()) { > + SAFE_UNSHARE(CLONE_NEWIPC); > + check_mqueue(); > + return; > + } > + } > } > > static void setup(void) > { > - tst_require_root(); > - check_mqns(); > + char *tmpdir; > + > + if (!str_op) > + tst_brk(TCONF, "Please specify clone|unshare child isolation"); > + > + tmpdir = tst_get_tmpdir(); > + > + SAFE_ASPRINTF(&devdir, "%s/mqueue", tmpdir); > + SAFE_MKDIR(devdir, 0755); > + > + SAFE_ASPRINTF(&mqueue1, "%s" MQNAME1, devdir); > + SAFE_ASPRINTF(&mqueue2, "%s" MQNAME2, devdir); > + > + mq_freed1 = SAFE_MMAP(NULL, > + sizeof(int), > + PROT_READ | PROT_WRITE, > + MAP_SHARED | MAP_ANONYMOUS, > + -1, 0); > + > + mq_freed2 = SAFE_MMAP(NULL, > + sizeof(int), > + PROT_READ | PROT_WRITE, > + MAP_SHARED | MAP_ANONYMOUS, > + -1, 0); So here you are allocating two pages of memory for something that is basically two bitflags. Can you at least change this to a single mmap() something as: static int *mq_freed; mq_freed = SAFE_MMAP(NULL, 2 * sizeof(int), ...) mq_freed[0] = 1; ... Moreover since we can actually stat()/access() the mqueue we can as well check for the existence of the devdir in the cleanup and only remove it if it exists in the filesystem. Also I would be more afraid of the mqueue filesystem being mounted in the temp directory if we trigger a failure between one of the mount()/umount() calls, so we should as well check if it's mounted in the cleanup and attempt to umount it.
Hi Cyril, On 5/22/23 16:41, Cyril Hrubis wrote: > Hi! >> +#include "tst_test.h" >> +#include "lapi/sched.h" >> +#include "tst_safe_posix_ipc.h" >> +#include "tst_safe_stdio.h" >> + >> +#define CHECK_MQ_OPEN_RET(x) ((x) >= 0 || ((x) == -1 && errno != EMFILE)) >> + >> +#define MQNAME1 "/MQ1" >> +#define MQNAME2 "/MQ2" >> + >> +static char *str_op; >> +static char *devdir; >> +static char *mqueue1; >> +static char *mqueue2; >> +static int *mq_freed1; >> +static int *mq_freed2; >> + >> +static void check_mqueue(void) >> { >> - char buf[30]; >> - mqd_t mqd; >> int rc; >> + mqd_t mqd; >> struct stat statbuf; >> >> - (void) vtest; >> + tst_res(TINFO, "Creating %s mqueue from within child process", MQNAME1); >> >> - close(p1[1]); >> - close(p2[0]); >> + mqd = TST_RETRY_FUNC( >> + mq_open(MQNAME1, O_RDWR | O_CREAT | O_EXCL, 0777, NULL), >> + CHECK_MQ_OPEN_RET); >> + if (mqd == -1) >> + tst_brk(TBROK | TERRNO, "mq_open failed"); >> >> - if (read(p1[0], buf, 3) != 3) { /* go */ >> - perror("read failed"); >> - exit(1); >> - } >> + SAFE_MQ_CLOSE(mqd); >> + tst_atomic_inc(mq_freed1); > I do not think that we need atomicity here, the cleanup code does not > run concurently at all as the cleanup in the parent is triggered after > the child did exit. I suppose that instead we need to set the mq_freed > to be volatile because it's shared memory which may change at any > change, so we need to tell that to the compiler. That's fine, but I followed suggestions in the reviews. I think that having 3 people reviewing the same patch doesn't help the development process. Now I'm not sure who I should follow :-) >> - mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDWR | O_CREAT | O_EXCL, >> - 0755, NULL); >> - if (mqd == -1) { >> - write(p2[1], "mqfail", 7); >> - exit(1); >> - } >> + tst_res(TINFO, "Mount %s from within child process", devdir); >> >> - mq_close(mqd); >> + SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL); >> >> - rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL); >> - if (rc == -1) { >> - write(p2[1], "mount1", 7); >> - exit(1); >> - } >> + SAFE_STAT(mqueue1, &statbuf); >> + tst_res(TPASS, "%s exists at first mount", mqueue1); >> >> - rc = stat(FNAM1, &statbuf); >> - if (rc == -1) { >> - write(p2[1], "stat1", 6); >> - exit(1); >> - } >> + tst_res(TINFO, "Creating %s from within child process", mqueue2); >> >> - rc = creat(FNAM2, 0755); >> - if (rc == -1) { >> - write(p2[1], "creat", 6); >> - exit(1); >> - } >> + rc = SAFE_CREAT(mqueue2, 0755); >> + SAFE_CLOSE(rc); >> + tst_atomic_inc(mq_freed2); >> >> - close(rc); >> + tst_res(TINFO, "Mount %s from within child process a second time", devdir); >> >> - rc = umount(DEV_MQUEUE2); >> - if (rc == -1) { >> - write(p2[1], "umount", 7); >> - exit(1); >> - } >> + SAFE_UMOUNT(devdir); >> + SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL); >> >> - rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL); >> - if (rc == -1) { >> - write(p2[1], "mount2", 7); >> - exit(1); >> - } >> + SAFE_STAT(mqueue1, &statbuf); >> + tst_res(TPASS, "%s exists at second mount", mqueue1); >> >> - rc = stat(FNAM1, &statbuf); >> - if (rc == -1) { >> - write(p2[1], "stat2", 7); >> - exit(1); >> - } >> + SAFE_STAT(mqueue2, &statbuf); >> + tst_res(TPASS, "%s exists at second mount", mqueue2); >> >> - rc = stat(FNAM2, &statbuf); >> - if (rc == -1) { >> - write(p2[1], "stat3", 7); >> - exit(1); >> - } >> + SAFE_UMOUNT(devdir); >> + >> + SAFE_MQ_UNLINK(MQNAME1); >> + tst_atomic_store(0, mq_freed1); >> >> - write(p2[1], "done", 5); >> + SAFE_MQ_UNLINK(MQNAME2); >> + tst_atomic_store(0, mq_freed2); >> +} >> >> - exit(0); >> +static void run(void) >> +{ >> + const struct tst_clone_args clone_args = { CLONE_NEWIPC, SIGCHLD }; >> + >> + if (str_op && !strcmp(str_op, "clone")) { >> + tst_res(TINFO, "Spawning isolated process"); >> + >> + if (!SAFE_CLONE(&clone_args)) { >> + check_mqueue(); >> + return; >> + } >> + } else if (str_op && !strcmp(str_op, "unshare")) { >> + tst_res(TINFO, "Spawning unshared process"); >> + >> + if (!SAFE_FORK()) { >> + SAFE_UNSHARE(CLONE_NEWIPC); >> + check_mqueue(); >> + return; >> + } >> + } >> } >> >> static void setup(void) >> { >> - tst_require_root(); >> - check_mqns(); >> + char *tmpdir; >> + >> + if (!str_op) >> + tst_brk(TCONF, "Please specify clone|unshare child isolation"); >> + >> + tmpdir = tst_get_tmpdir(); >> + >> + SAFE_ASPRINTF(&devdir, "%s/mqueue", tmpdir); >> + SAFE_MKDIR(devdir, 0755); >> + >> + SAFE_ASPRINTF(&mqueue1, "%s" MQNAME1, devdir); >> + SAFE_ASPRINTF(&mqueue2, "%s" MQNAME2, devdir); >> + >> + mq_freed1 = SAFE_MMAP(NULL, >> + sizeof(int), >> + PROT_READ | PROT_WRITE, >> + MAP_SHARED | MAP_ANONYMOUS, >> + -1, 0); >> + >> + mq_freed2 = SAFE_MMAP(NULL, >> + sizeof(int), >> + PROT_READ | PROT_WRITE, >> + MAP_SHARED | MAP_ANONYMOUS, >> + -1, 0); > So here you are allocating two pages of memory for something that is > basically two bitflags. Can you at least change this to a single mmap() > something as: > > static int *mq_freed; > > mq_freed = SAFE_MMAP(NULL, 2 * sizeof(int), ...) > > > mq_freed[0] = 1; > ... > > Moreover since we can actually stat()/access() the mqueue we can as well > check for the existence of the devdir in the cleanup and only remove it > if it exists in the filesystem. > > Also I would be more afraid of the mqueue filesystem being mounted in > the temp directory if we trigger a failure between one of the > mount()/umount() calls, so we should as well check if it's mounted in > the cleanup and attempt to umount it. > > Andrea
Hi! > > I do not think that we need atomicity here, the cleanup code does not > > run concurently at all as the cleanup in the parent is triggered after > > the child did exit. I suppose that instead we need to set the mq_freed > > to be volatile because it's shared memory which may change at any > > change, so we need to tell that to the compiler. > That's fine, but I followed suggestions in the reviews. I think that > having 3 people reviewing the same patch doesn't help the development > process. Now I'm not sure who I should follow :-) It's actually the other way around, the more people look at the code the better, at least that way we have potential to catch more problems earlier. And if the reviewers disagree, let them fight for the right answer. I think that in this case this all can be actually simplified and we can get rid of the mq_freed flag as I tried to outline below. > >> - mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDWR | O_CREAT | O_EXCL, > >> - 0755, NULL); > >> - if (mqd == -1) { > >> - write(p2[1], "mqfail", 7); > >> - exit(1); > >> - } > >> + tst_res(TINFO, "Mount %s from within child process", devdir); > >> > >> - mq_close(mqd); > >> + SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL); > >> > >> - rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL); > >> - if (rc == -1) { > >> - write(p2[1], "mount1", 7); > >> - exit(1); > >> - } > >> + SAFE_STAT(mqueue1, &statbuf); > >> + tst_res(TPASS, "%s exists at first mount", mqueue1); > >> > >> - rc = stat(FNAM1, &statbuf); > >> - if (rc == -1) { > >> - write(p2[1], "stat1", 6); > >> - exit(1); > >> - } > >> + tst_res(TINFO, "Creating %s from within child process", mqueue2); > >> > >> - rc = creat(FNAM2, 0755); > >> - if (rc == -1) { > >> - write(p2[1], "creat", 6); > >> - exit(1); > >> - } > >> + rc = SAFE_CREAT(mqueue2, 0755); > >> + SAFE_CLOSE(rc); > >> + tst_atomic_inc(mq_freed2); > >> > >> - close(rc); > >> + tst_res(TINFO, "Mount %s from within child process a second time", devdir); > >> > >> - rc = umount(DEV_MQUEUE2); > >> - if (rc == -1) { > >> - write(p2[1], "umount", 7); > >> - exit(1); > >> - } > >> + SAFE_UMOUNT(devdir); > >> + SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL); > >> > >> - rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL); > >> - if (rc == -1) { > >> - write(p2[1], "mount2", 7); > >> - exit(1); > >> - } > >> + SAFE_STAT(mqueue1, &statbuf); > >> + tst_res(TPASS, "%s exists at second mount", mqueue1); > >> > >> - rc = stat(FNAM1, &statbuf); > >> - if (rc == -1) { > >> - write(p2[1], "stat2", 7); > >> - exit(1); > >> - } > >> + SAFE_STAT(mqueue2, &statbuf); > >> + tst_res(TPASS, "%s exists at second mount", mqueue2); > >> > >> - rc = stat(FNAM2, &statbuf); > >> - if (rc == -1) { > >> - write(p2[1], "stat3", 7); > >> - exit(1); > >> - } > >> + SAFE_UMOUNT(devdir); > >> + > >> + SAFE_MQ_UNLINK(MQNAME1); > >> + tst_atomic_store(0, mq_freed1); > >> > >> - write(p2[1], "done", 5); > >> + SAFE_MQ_UNLINK(MQNAME2); > >> + tst_atomic_store(0, mq_freed2); > >> +} > >> > >> - exit(0); > >> +static void run(void) > >> +{ > >> + const struct tst_clone_args clone_args = { CLONE_NEWIPC, SIGCHLD }; > >> + > >> + if (str_op && !strcmp(str_op, "clone")) { > >> + tst_res(TINFO, "Spawning isolated process"); > >> + > >> + if (!SAFE_CLONE(&clone_args)) { > >> + check_mqueue(); > >> + return; > >> + } > >> + } else if (str_op && !strcmp(str_op, "unshare")) { > >> + tst_res(TINFO, "Spawning unshared process"); > >> + > >> + if (!SAFE_FORK()) { > >> + SAFE_UNSHARE(CLONE_NEWIPC); > >> + check_mqueue(); > >> + return; > >> + } > >> + } > >> } > >> > >> static void setup(void) > >> { > >> - tst_require_root(); > >> - check_mqns(); > >> + char *tmpdir; > >> + > >> + if (!str_op) > >> + tst_brk(TCONF, "Please specify clone|unshare child isolation"); > >> + > >> + tmpdir = tst_get_tmpdir(); > >> + > >> + SAFE_ASPRINTF(&devdir, "%s/mqueue", tmpdir); > >> + SAFE_MKDIR(devdir, 0755); > >> + > >> + SAFE_ASPRINTF(&mqueue1, "%s" MQNAME1, devdir); > >> + SAFE_ASPRINTF(&mqueue2, "%s" MQNAME2, devdir); > >> + > >> + mq_freed1 = SAFE_MMAP(NULL, > >> + sizeof(int), > >> + PROT_READ | PROT_WRITE, > >> + MAP_SHARED | MAP_ANONYMOUS, > >> + -1, 0); > >> + > >> + mq_freed2 = SAFE_MMAP(NULL, > >> + sizeof(int), > >> + PROT_READ | PROT_WRITE, > >> + MAP_SHARED | MAP_ANONYMOUS, > >> + -1, 0); > > So here you are allocating two pages of memory for something that is > > basically two bitflags. Can you at least change this to a single mmap() > > something as: > > > > static int *mq_freed; > > > > mq_freed = SAFE_MMAP(NULL, 2 * sizeof(int), ...) > > > > > > mq_freed[0] = 1; > > ... > > > > Moreover since we can actually stat()/access() the mqueue we can as well > > check for the existence of the devdir in the cleanup and only remove it > > if it exists in the filesystem. > > > > Also I would be more afraid of the mqueue filesystem being mounted in > > the temp directory if we trigger a failure between one of the > > mount()/umount() calls, so we should as well check if it's mounted in > > the cleanup and attempt to umount it. > > > > > Andrea >
> Hi! > > > I do not think that we need atomicity here, the cleanup code does not > > > run concurently at all as the cleanup in the parent is triggered after > > > the child did exit. I suppose that instead we need to set the mq_freed > > > to be volatile because it's shared memory which may change at any > > > change, so we need to tell that to the compiler. > > That's fine, but I followed suggestions in the reviews. I think that > > having 3 people reviewing the same patch doesn't help the development > > process. Now I'm not sure who I should follow :-) > It's actually the other way around, the more people look at the code the > better, at least that way we have potential to catch more problems > earlier. And if the reviewers disagree, let them fight for the right > answer. +1 Kind regards, Petr > I think that in this case this all can be actually simplified and we can > get rid of the mq_freed flag as I tried to outline below.
diff --git a/runtest/containers b/runtest/containers index dbb4c5fa6..07bcc8a19 100644 --- a/runtest/containers +++ b/runtest/containers @@ -21,8 +21,8 @@ mqns_01_unshare mqns_01 -m unshare mqns_02 mqns_02 mqns_02_clone mqns_02 -m clone mqns_02_unshare mqns_02 -m unshare -mqns_03 mqns_03 -mqns_03_clone mqns_03 -clone +mqns_02_clone mqns_03 -m clone +mqns_02_unshare mqns_03 -m unshare mqns_04 mqns_04 mqns_04_clone mqns_04 -clone diff --git a/testcases/kernel/containers/mqns/mqns_03.c b/testcases/kernel/containers/mqns/mqns_03.c index a7452b970..81dea7d38 100644 --- a/testcases/kernel/containers/mqns/mqns_03.c +++ b/testcases/kernel/containers/mqns/mqns_03.c @@ -1,207 +1,173 @@ +// SPDX-License-Identifier: GPL-2.0 /* -* Copyright (c) International Business Machines Corp., 2009 -* This program is free software; you can redistribute it and/or modify -* it under the terms of the GNU General Public License as published by -* the Free Software Foundation; either version 2 of the License, or -* (at your option) any later version. -* -* This program is distributed in the hope that it will be useful, -* but WITHOUT ANY WARRANTY; without even the implied warranty of -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See -* the GNU General Public License for more details. -* You should have received a copy of the GNU General Public License -* along with this program; if not, write to the Free Software -* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA -* -* Author: Serge Hallyn <serue@us.ibm.com> -* -* Check ipcns+sb longevity -* -* Mount mqueue fs -* unshare -* In unshared process: -* Create "/mq1" with mq_open() -* Mount mqueuefs -* Check that /mq1 exists -* Create /dev/mqueue/mq2 through vfs (create(2)) -* Umount /dev/mqueue -* Remount /dev/mqueue -* Check that both /mq1 and /mq2 exist - -***************************************************************************/ - -#ifndef _GNU_SOURCE -#define _GNU_SOURCE -#endif -#include <sys/types.h> -#include <sys/stat.h> -#include <sys/wait.h> -#include <assert.h> -#include <stdio.h> -#include <stdlib.h> -#include <unistd.h> -#include <string.h> -#include <errno.h> -#include "mqns.h" -#include "mqns_helper.h" - -char *TCID = "posixmq_namespace_03"; -int TST_TOTAL = 1; - -int p1[2]; -int p2[2]; - -#define FNAM1 DEV_MQUEUE2 SLASH_MQ1 -#define FNAM2 DEV_MQUEUE2 SLASH_MQ2 - -int check_mqueue(void *vtest) + * Copyright (c) International Business Machines Corp., 2009 + * Copyright (c) Serge Hallyn <serue@us.ibm.com> + * Copyright (C) 2023 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> + */ + +/*\ + * [Description] + * + * Test mqueuefs from an isolated/forked process namespace. + * + * [Algorithm] + * + * - create /MQ1 + * - mount mqueue inside the temporary folder + * - check for /MQ1 existance + * - create /MQ2 inside the temporary folder + * - umount + * - mount mqueue inside the temporary folder + * - check /MQ1 existance + * - check /MQ2 existance + * - umount + */ + +#include "tst_test.h" +#include "lapi/sched.h" +#include "tst_safe_posix_ipc.h" +#include "tst_safe_stdio.h" + +#define CHECK_MQ_OPEN_RET(x) ((x) >= 0 || ((x) == -1 && errno != EMFILE)) + +#define MQNAME1 "/MQ1" +#define MQNAME2 "/MQ2" + +static char *str_op; +static char *devdir; +static char *mqueue1; +static char *mqueue2; +static int *mq_freed1; +static int *mq_freed2; + +static void check_mqueue(void) { - char buf[30]; - mqd_t mqd; int rc; + mqd_t mqd; struct stat statbuf; - (void) vtest; + tst_res(TINFO, "Creating %s mqueue from within child process", MQNAME1); - close(p1[1]); - close(p2[0]); + mqd = TST_RETRY_FUNC( + mq_open(MQNAME1, O_RDWR | O_CREAT | O_EXCL, 0777, NULL), + CHECK_MQ_OPEN_RET); + if (mqd == -1) + tst_brk(TBROK | TERRNO, "mq_open failed"); - if (read(p1[0], buf, 3) != 3) { /* go */ - perror("read failed"); - exit(1); - } + SAFE_MQ_CLOSE(mqd); + tst_atomic_inc(mq_freed1); - mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDWR | O_CREAT | O_EXCL, - 0755, NULL); - if (mqd == -1) { - write(p2[1], "mqfail", 7); - exit(1); - } + tst_res(TINFO, "Mount %s from within child process", devdir); - mq_close(mqd); + SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL); - rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL); - if (rc == -1) { - write(p2[1], "mount1", 7); - exit(1); - } + SAFE_STAT(mqueue1, &statbuf); + tst_res(TPASS, "%s exists at first mount", mqueue1); - rc = stat(FNAM1, &statbuf); - if (rc == -1) { - write(p2[1], "stat1", 6); - exit(1); - } + tst_res(TINFO, "Creating %s from within child process", mqueue2); - rc = creat(FNAM2, 0755); - if (rc == -1) { - write(p2[1], "creat", 6); - exit(1); - } + rc = SAFE_CREAT(mqueue2, 0755); + SAFE_CLOSE(rc); + tst_atomic_inc(mq_freed2); - close(rc); + tst_res(TINFO, "Mount %s from within child process a second time", devdir); - rc = umount(DEV_MQUEUE2); - if (rc == -1) { - write(p2[1], "umount", 7); - exit(1); - } + SAFE_UMOUNT(devdir); + SAFE_MOUNT("mqueue", devdir, "mqueue", 0, NULL); - rc = mount("mqueue", DEV_MQUEUE2, "mqueue", 0, NULL); - if (rc == -1) { - write(p2[1], "mount2", 7); - exit(1); - } + SAFE_STAT(mqueue1, &statbuf); + tst_res(TPASS, "%s exists at second mount", mqueue1); - rc = stat(FNAM1, &statbuf); - if (rc == -1) { - write(p2[1], "stat2", 7); - exit(1); - } + SAFE_STAT(mqueue2, &statbuf); + tst_res(TPASS, "%s exists at second mount", mqueue2); - rc = stat(FNAM2, &statbuf); - if (rc == -1) { - write(p2[1], "stat3", 7); - exit(1); - } + SAFE_UMOUNT(devdir); + + SAFE_MQ_UNLINK(MQNAME1); + tst_atomic_store(0, mq_freed1); - write(p2[1], "done", 5); + SAFE_MQ_UNLINK(MQNAME2); + tst_atomic_store(0, mq_freed2); +} - exit(0); +static void run(void) +{ + const struct tst_clone_args clone_args = { CLONE_NEWIPC, SIGCHLD }; + + if (str_op && !strcmp(str_op, "clone")) { + tst_res(TINFO, "Spawning isolated process"); + + if (!SAFE_CLONE(&clone_args)) { + check_mqueue(); + return; + } + } else if (str_op && !strcmp(str_op, "unshare")) { + tst_res(TINFO, "Spawning unshared process"); + + if (!SAFE_FORK()) { + SAFE_UNSHARE(CLONE_NEWIPC); + check_mqueue(); + return; + } + } } static void setup(void) { - tst_require_root(); - check_mqns(); + char *tmpdir; + + if (!str_op) + tst_brk(TCONF, "Please specify clone|unshare child isolation"); + + tmpdir = tst_get_tmpdir(); + + SAFE_ASPRINTF(&devdir, "%s/mqueue", tmpdir); + SAFE_MKDIR(devdir, 0755); + + SAFE_ASPRINTF(&mqueue1, "%s" MQNAME1, devdir); + SAFE_ASPRINTF(&mqueue2, "%s" MQNAME2, devdir); + + mq_freed1 = SAFE_MMAP(NULL, + sizeof(int), + PROT_READ | PROT_WRITE, + MAP_SHARED | MAP_ANONYMOUS, + -1, 0); + + mq_freed2 = SAFE_MMAP(NULL, + sizeof(int), + PROT_READ | PROT_WRITE, + MAP_SHARED | MAP_ANONYMOUS, + -1, 0); } -int main(int argc, char *argv[]) +static void cleanup(void) { - int r; - char buf[30]; - int use_clone = T_UNSHARE; - - setup(); - - if (argc == 2 && strcmp(argv[1], "-clone") == 0) { - tst_resm(TINFO, "Testing posix mq namespaces through clone(2)"); - use_clone = T_CLONE; - } else - tst_resm(TINFO, - "Testing posix mq namespaces through unshare(2)"); - - if (pipe(p1) == -1 || pipe(p2) == -1) - tst_brkm(TBROK | TERRNO, NULL, "pipe failed"); - - /* fire off the test */ - r = do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_mqueue, NULL); - if (r < 0) { - tst_brkm(TBROK | TERRNO, NULL, "failed clone/unshare"); - } + if (!devdir) + return; - tst_resm(TINFO, "Checking correct umount+remount of mqueuefs"); - - mkdir(DEV_MQUEUE2, 0755); - - close(p1[0]); - close(p2[1]); - write(p1[1], "go", 3); - - read(p2[0], buf, 7); - r = TFAIL; - if (!strcmp(buf, "mqfail")) { - tst_resm(TFAIL, "child process could not create mqueue"); - goto fail; - } else if (!strcmp(buf, "mount1")) { - tst_resm(TFAIL, "child process could not mount mqueue"); - goto fail; - } else if (!strcmp(buf, "stat1x")) { - tst_resm(TFAIL, "mq created by child is not in mqueuefs"); - goto fail; - } else if (!strcmp(buf, "creat")) { - tst_resm(TFAIL, "child couldn't creat mq through mqueuefs"); - goto fail; - } else if (!strcmp(buf, "umount")) { - tst_resm(TFAIL, "child couldn't umount mqueuefs"); - goto fail; - } else if (!strcmp(buf, "mount2")) { - tst_resm(TFAIL, "child couldn't remount mqueuefs"); - goto fail; - } else if (!strcmp(buf, "stat2")) { - tst_resm(TFAIL, - "mq_open()d file gone after remount of mqueuefs"); - goto fail; - } else if (!strcmp(buf, "stat3")) { - tst_resm(TFAIL, - "creat(2)'d file gone after remount of mqueuefs"); - goto fail; - } + if (tst_is_mounted(devdir)) + SAFE_UMOUNT(devdir); - tst_resm(TPASS, "umount+remount of mqueuefs remounted the right fs"); + if (*mq_freed1) + SAFE_MQ_UNLINK(MQNAME1); - r = 0; -fail: - umount(DEV_MQUEUE2); - rmdir(DEV_MQUEUE2); - tst_exit(); + if (*mq_freed2) + SAFE_MQ_UNLINK(MQNAME2); } + +static struct tst_test test = { + .test_all = run, + .setup = setup, + .cleanup = cleanup, + .needs_root = 1, + .forks_child = 1, + .needs_tmpdir = 1, + .options = (struct tst_option[]) { + { "m:", &str_op, "Child process isolation <clone|unshare>" }, + {}, + }, + .needs_kconfigs = (const char *[]) { + "CONFIG_USER_NS", + NULL + }, +};