Message ID | 20220722120501.28670-3-andrea.cervesato@suse.com |
---|---|
State | Superseded |
Headers | show |
Series | Refactor mqns testing suite | expand |
Hello, Andrea Cervesato via ltp <ltp@lists.linux.it> writes: > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> > --- > runtest/containers | 3 +- > testcases/kernel/containers/mqns/common.h | 101 +++++++++++ > testcases/kernel/containers/mqns/mqns_01.c | 193 +++++++-------------- > 3 files changed, 166 insertions(+), 131 deletions(-) > create mode 100644 testcases/kernel/containers/mqns/common.h > > diff --git a/runtest/containers b/runtest/containers > index 2637b62fe..863a964ad 100644 > --- a/runtest/containers > +++ b/runtest/containers > @@ -16,7 +16,8 @@ pidns31 pidns31 > pidns32 pidns32 > > mqns_01 mqns_01 > -mqns_01_clone mqns_01 -clone > +mqns_01_clone mqns_01 -m clone > +mqns_01_unshare mqns_01 -m unshare > mqns_02 mqns_02 > mqns_02_clone mqns_02 -clone > mqns_03 mqns_03 > diff --git a/testcases/kernel/containers/mqns/common.h b/testcases/kernel/containers/mqns/common.h > new file mode 100644 > index 000000000..92a77b566 > --- /dev/null > +++ b/testcases/kernel/containers/mqns/common.h > @@ -0,0 +1,101 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> > + */ > + > +#ifndef MQNS_H > +#define MQNS_H > + > +#include <stdlib.h> > +#include "lapi/namespaces_constants.h" > +#include "tst_test.h" > +#include "tst_safe_posix_ipc.h" > + > +enum { > + T_CLONE, > + T_UNSHARE, > + T_NONE, > +}; > + > +static int dummy_child1(void *v) > +{ > + (void)v; > + return 0; > +} > + > +static inline void check_newipc(void) > +{ > + int pid, status; > + > + pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child1, > NULL); ltp_clone_quick is still part of the old API and only uses clone2. I think it should be replaced with tst_clone. This may require extending tst_clone. In fact we probably need a test variant to switch between the clone2 and clone3 syscalls when using tst_clone. I'll leave it to you whether you want to try that and rebase this patch set on it.
Hi! On 8/11/22 11:53, Richard Palethorpe wrote: > Hello, > > Andrea Cervesato via ltp <ltp@lists.linux.it> writes: > >> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> >> --- >> runtest/containers | 3 +- >> testcases/kernel/containers/mqns/common.h | 101 +++++++++++ >> testcases/kernel/containers/mqns/mqns_01.c | 193 +++++++-------------- >> 3 files changed, 166 insertions(+), 131 deletions(-) >> create mode 100644 testcases/kernel/containers/mqns/common.h >> >> diff --git a/runtest/containers b/runtest/containers >> index 2637b62fe..863a964ad 100644 >> --- a/runtest/containers >> +++ b/runtest/containers >> @@ -16,7 +16,8 @@ pidns31 pidns31 >> pidns32 pidns32 >> >> mqns_01 mqns_01 >> -mqns_01_clone mqns_01 -clone >> +mqns_01_clone mqns_01 -m clone >> +mqns_01_unshare mqns_01 -m unshare >> mqns_02 mqns_02 >> mqns_02_clone mqns_02 -clone >> mqns_03 mqns_03 >> diff --git a/testcases/kernel/containers/mqns/common.h b/testcases/kernel/containers/mqns/common.h >> new file mode 100644 >> index 000000000..92a77b566 >> --- /dev/null >> +++ b/testcases/kernel/containers/mqns/common.h >> @@ -0,0 +1,101 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> >> + */ >> + >> +#ifndef MQNS_H >> +#define MQNS_H >> + >> +#include <stdlib.h> >> +#include "lapi/namespaces_constants.h" >> +#include "tst_test.h" >> +#include "tst_safe_posix_ipc.h" >> + >> +enum { >> + T_CLONE, >> + T_UNSHARE, >> + T_NONE, >> +}; >> + >> +static int dummy_child1(void *v) >> +{ >> + (void)v; >> + return 0; >> +} >> + >> +static inline void check_newipc(void) >> +{ >> + int pid, status; >> + >> + pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child1, >> NULL); > ltp_clone_quick is still part of the old API and only uses clone2. I > think it should be replaced with tst_clone. This may require extending > tst_clone. In fact we probably need a test variant to switch between the > clone2 and clone3 syscalls when using tst_clone. > > I'll leave it to you whether you want to try that and rebase this patch > set on it. > I see ltp_clone_quick as wrapper of ltp_clone, since it's using ltp_alloc_stack without calling it explicitly all the times before ltp_clone.
Hello, Andrea Cervesato <andrea.cervesato@suse.com> writes: > Hi! > > On 8/11/22 11:53, Richard Palethorpe wrote: >> Hello, >> >> Andrea Cervesato via ltp <ltp@lists.linux.it> writes: >> >>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> >>> --- >>> runtest/containers | 3 +- >>> testcases/kernel/containers/mqns/common.h | 101 +++++++++++ >>> testcases/kernel/containers/mqns/mqns_01.c | 193 +++++++-------------- >>> 3 files changed, 166 insertions(+), 131 deletions(-) >>> create mode 100644 testcases/kernel/containers/mqns/common.h >>> >>> diff --git a/runtest/containers b/runtest/containers >>> index 2637b62fe..863a964ad 100644 >>> --- a/runtest/containers >>> +++ b/runtest/containers >>> @@ -16,7 +16,8 @@ pidns31 pidns31 >>> pidns32 pidns32 >>> mqns_01 mqns_01 >>> -mqns_01_clone mqns_01 -clone >>> +mqns_01_clone mqns_01 -m clone >>> +mqns_01_unshare mqns_01 -m unshare >>> mqns_02 mqns_02 >>> mqns_02_clone mqns_02 -clone >>> mqns_03 mqns_03 >>> diff --git a/testcases/kernel/containers/mqns/common.h b/testcases/kernel/containers/mqns/common.h >>> new file mode 100644 >>> index 000000000..92a77b566 >>> --- /dev/null >>> +++ b/testcases/kernel/containers/mqns/common.h >>> @@ -0,0 +1,101 @@ >>> +// SPDX-License-Identifier: GPL-2.0-or-later >>> +/* >>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> >>> + */ >>> + >>> +#ifndef MQNS_H >>> +#define MQNS_H >>> + >>> +#include <stdlib.h> >>> +#include "lapi/namespaces_constants.h" >>> +#include "tst_test.h" >>> +#include "tst_safe_posix_ipc.h" >>> + >>> +enum { >>> + T_CLONE, >>> + T_UNSHARE, >>> + T_NONE, >>> +}; >>> + >>> +static int dummy_child1(void *v) >>> +{ >>> + (void)v; >>> + return 0; >>> +} >>> + >>> +static inline void check_newipc(void) >>> +{ >>> + int pid, status; >>> + >>> + pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child1, >>> NULL); >> ltp_clone_quick is still part of the old API and only uses clone2. I >> think it should be replaced with tst_clone. This may require extending >> tst_clone. In fact we probably need a test variant to switch between the >> clone2 and clone3 syscalls when using tst_clone. >> >> I'll leave it to you whether you want to try that and rebase this patch >> set on it. >> > I see ltp_clone_quick as wrapper of ltp_clone, since it's using > ltp_alloc_stack without calling it explicitly all the times before > ltp_clone. ltp_clone is also part of the old API. At some point we should remove that.
Hello, Richard Palethorpe <rpalethorpe@suse.de> writes: > Hello, > > Andrea Cervesato <andrea.cervesato@suse.com> writes: > >> Hi! >> >> On 8/11/22 11:53, Richard Palethorpe wrote: >>> Hello, >>> >>> Andrea Cervesato via ltp <ltp@lists.linux.it> writes: >>> >>>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> >>>> --- >>>> runtest/containers | 3 +- >>>> testcases/kernel/containers/mqns/common.h | 101 +++++++++++ >>>> testcases/kernel/containers/mqns/mqns_01.c | 193 +++++++-------------- >>>> 3 files changed, 166 insertions(+), 131 deletions(-) >>>> create mode 100644 testcases/kernel/containers/mqns/common.h >>>> >>>> diff --git a/runtest/containers b/runtest/containers >>>> index 2637b62fe..863a964ad 100644 >>>> --- a/runtest/containers >>>> +++ b/runtest/containers >>>> @@ -16,7 +16,8 @@ pidns31 pidns31 >>>> pidns32 pidns32 >>>> mqns_01 mqns_01 >>>> -mqns_01_clone mqns_01 -clone >>>> +mqns_01_clone mqns_01 -m clone >>>> +mqns_01_unshare mqns_01 -m unshare >>>> mqns_02 mqns_02 >>>> mqns_02_clone mqns_02 -clone >>>> mqns_03 mqns_03 >>>> diff --git a/testcases/kernel/containers/mqns/common.h b/testcases/kernel/containers/mqns/common.h >>>> new file mode 100644 >>>> index 000000000..92a77b566 >>>> --- /dev/null >>>> +++ b/testcases/kernel/containers/mqns/common.h >>>> @@ -0,0 +1,101 @@ >>>> +// SPDX-License-Identifier: GPL-2.0-or-later >>>> +/* >>>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> >>>> + */ >>>> + >>>> +#ifndef MQNS_H >>>> +#define MQNS_H >>>> + >>>> +#include <stdlib.h> >>>> +#include "lapi/namespaces_constants.h" >>>> +#include "tst_test.h" >>>> +#include "tst_safe_posix_ipc.h" >>>> + >>>> +enum { >>>> + T_CLONE, >>>> + T_UNSHARE, >>>> + T_NONE, >>>> +}; >>>> + >>>> +static int dummy_child1(void *v) >>>> +{ >>>> + (void)v; >>>> + return 0; >>>> +} >>>> + >>>> +static inline void check_newipc(void) >>>> +{ >>>> + int pid, status; >>>> + >>>> + pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child1, >>>> NULL); >>> ltp_clone_quick is still part of the old API and only uses clone2. I >>> think it should be replaced with tst_clone. This may require extending >>> tst_clone. In fact we probably need a test variant to switch between the >>> clone2 and clone3 syscalls when using tst_clone. >>> >>> I'll leave it to you whether you want to try that and rebase this patch >>> set on it. >>> >> I see ltp_clone_quick as wrapper of ltp_clone, since it's using >> ltp_alloc_stack without calling it explicitly all the times before >> ltp_clone. > > ltp_clone is also part of the old API. At some point we should remove > that. I'm marking this as changes requested. tst_clone should be made to support this scenario.
Hi Are we sure that we don't need this modification before adding tst_clone? We can add the patch and then starting to think how to replace tst_clone_quick with tst_clone in all tests. Andrea On 10/11/22 11:17, Richard Palethorpe wrote: > Hello, > > Richard Palethorpe <rpalethorpe@suse.de> writes: > >> Hello, >> >> Andrea Cervesato <andrea.cervesato@suse.com> writes: >> >>> Hi! >>> >>> On 8/11/22 11:53, Richard Palethorpe wrote: >>>> Hello, >>>> >>>> Andrea Cervesato via ltp <ltp@lists.linux.it> writes: >>>> >>>>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> >>>>> --- >>>>> runtest/containers | 3 +- >>>>> testcases/kernel/containers/mqns/common.h | 101 +++++++++++ >>>>> testcases/kernel/containers/mqns/mqns_01.c | 193 +++++++-------------- >>>>> 3 files changed, 166 insertions(+), 131 deletions(-) >>>>> create mode 100644 testcases/kernel/containers/mqns/common.h >>>>> >>>>> diff --git a/runtest/containers b/runtest/containers >>>>> index 2637b62fe..863a964ad 100644 >>>>> --- a/runtest/containers >>>>> +++ b/runtest/containers >>>>> @@ -16,7 +16,8 @@ pidns31 pidns31 >>>>> pidns32 pidns32 >>>>> mqns_01 mqns_01 >>>>> -mqns_01_clone mqns_01 -clone >>>>> +mqns_01_clone mqns_01 -m clone >>>>> +mqns_01_unshare mqns_01 -m unshare >>>>> mqns_02 mqns_02 >>>>> mqns_02_clone mqns_02 -clone >>>>> mqns_03 mqns_03 >>>>> diff --git a/testcases/kernel/containers/mqns/common.h b/testcases/kernel/containers/mqns/common.h >>>>> new file mode 100644 >>>>> index 000000000..92a77b566 >>>>> --- /dev/null >>>>> +++ b/testcases/kernel/containers/mqns/common.h >>>>> @@ -0,0 +1,101 @@ >>>>> +// SPDX-License-Identifier: GPL-2.0-or-later >>>>> +/* >>>>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> >>>>> + */ >>>>> + >>>>> +#ifndef MQNS_H >>>>> +#define MQNS_H >>>>> + >>>>> +#include <stdlib.h> >>>>> +#include "lapi/namespaces_constants.h" >>>>> +#include "tst_test.h" >>>>> +#include "tst_safe_posix_ipc.h" >>>>> + >>>>> +enum { >>>>> + T_CLONE, >>>>> + T_UNSHARE, >>>>> + T_NONE, >>>>> +}; >>>>> + >>>>> +static int dummy_child1(void *v) >>>>> +{ >>>>> + (void)v; >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static inline void check_newipc(void) >>>>> +{ >>>>> + int pid, status; >>>>> + >>>>> + pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child1, >>>>> NULL); >>>> ltp_clone_quick is still part of the old API and only uses clone2. I >>>> think it should be replaced with tst_clone. This may require extending >>>> tst_clone. In fact we probably need a test variant to switch between the >>>> clone2 and clone3 syscalls when using tst_clone. >>>> >>>> I'll leave it to you whether you want to try that and rebase this patch >>>> set on it. >>>> >>> I see ltp_clone_quick as wrapper of ltp_clone, since it's using >>> ltp_alloc_stack without calling it explicitly all the times before >>> ltp_clone. >> ltp_clone is also part of the old API. At some point we should remove >> that. > I'm marking this as changes requested. tst_clone should be made to > support this scenario. >
Hello, Andrea Cervesato <andrea.cervesato@suse.com> writes: > Hi > > Are we sure that we don't need this modification before adding > tst_clone? We can add the patch and then starting to think how to > replace tst_clone_quick with tst_clone in all tests. You're not the first person to use this argument. So it's actually important for precisely the reason you don't want to do it. The next person wont' want to do it either and we'll sleep walk into never replacing it. Meanwhile there are solid reasons why clone3 exists and why we should test it. That's possibly more important than the API conversion. BTW I could take over one of these patches and do the work on tst_clone? I'm pretty familiar with it. > > Andrea > > On 10/11/22 11:17, Richard Palethorpe wrote: >> Hello, >> >> Richard Palethorpe <rpalethorpe@suse.de> writes: >> >>> Hello, >>> >>> Andrea Cervesato <andrea.cervesato@suse.com> writes: >>> >>>> Hi! >>>> >>>> On 8/11/22 11:53, Richard Palethorpe wrote: >>>>> Hello, >>>>> >>>>> Andrea Cervesato via ltp <ltp@lists.linux.it> writes: >>>>> >>>>>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> >>>>>> --- >>>>>> runtest/containers | 3 +- >>>>>> testcases/kernel/containers/mqns/common.h | 101 +++++++++++ >>>>>> testcases/kernel/containers/mqns/mqns_01.c | 193 +++++++-------------- >>>>>> 3 files changed, 166 insertions(+), 131 deletions(-) >>>>>> create mode 100644 testcases/kernel/containers/mqns/common.h >>>>>> >>>>>> diff --git a/runtest/containers b/runtest/containers >>>>>> index 2637b62fe..863a964ad 100644 >>>>>> --- a/runtest/containers >>>>>> +++ b/runtest/containers >>>>>> @@ -16,7 +16,8 @@ pidns31 pidns31 >>>>>> pidns32 pidns32 >>>>>> mqns_01 mqns_01 >>>>>> -mqns_01_clone mqns_01 -clone >>>>>> +mqns_01_clone mqns_01 -m clone >>>>>> +mqns_01_unshare mqns_01 -m unshare >>>>>> mqns_02 mqns_02 >>>>>> mqns_02_clone mqns_02 -clone >>>>>> mqns_03 mqns_03 >>>>>> diff --git a/testcases/kernel/containers/mqns/common.h b/testcases/kernel/containers/mqns/common.h >>>>>> new file mode 100644 >>>>>> index 000000000..92a77b566 >>>>>> --- /dev/null >>>>>> +++ b/testcases/kernel/containers/mqns/common.h >>>>>> @@ -0,0 +1,101 @@ >>>>>> +// SPDX-License-Identifier: GPL-2.0-or-later >>>>>> +/* >>>>>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> >>>>>> + */ >>>>>> + >>>>>> +#ifndef MQNS_H >>>>>> +#define MQNS_H >>>>>> + >>>>>> +#include <stdlib.h> >>>>>> +#include "lapi/namespaces_constants.h" >>>>>> +#include "tst_test.h" >>>>>> +#include "tst_safe_posix_ipc.h" >>>>>> + >>>>>> +enum { >>>>>> + T_CLONE, >>>>>> + T_UNSHARE, >>>>>> + T_NONE, >>>>>> +}; >>>>>> + >>>>>> +static int dummy_child1(void *v) >>>>>> +{ >>>>>> + (void)v; >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +static inline void check_newipc(void) >>>>>> +{ >>>>>> + int pid, status; >>>>>> + >>>>>> + pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child1, >>>>>> NULL); >>>>> ltp_clone_quick is still part of the old API and only uses clone2. I >>>>> think it should be replaced with tst_clone. This may require extending >>>>> tst_clone. In fact we probably need a test variant to switch between the >>>>> clone2 and clone3 syscalls when using tst_clone. >>>>> >>>>> I'll leave it to you whether you want to try that and rebase this patch >>>>> set on it. >>>>> >>>> I see ltp_clone_quick as wrapper of ltp_clone, since it's using >>>> ltp_alloc_stack without calling it explicitly all the times before >>>> ltp_clone. >>> ltp_clone is also part of the old API. At some point we should remove >>> that. >> I'm marking this as changes requested. tst_clone should be made to >> support this scenario. >>
Hi On 10/11/22 12:49, Richard Palethorpe wrote: > Hello, > > Andrea Cervesato <andrea.cervesato@suse.com> writes: > >> Hi >> >> Are we sure that we don't need this modification before adding >> tst_clone? We can add the patch and then starting to think how to >> replace tst_clone_quick with tst_clone in all tests. > You're not the first person to use this argument. So it's actually > important for precisely the reason you don't want to do it. The next > person wont' want to do it either and we'll sleep walk into never > replacing it. > > Meanwhile there are solid reasons why clone3 exists and why we should > test it. That's possibly more important than the API conversion. > > BTW I could take over one of these patches and do the work on > tst_clone? I'm pretty familiar with it. I can take care to change all tst_quick_clone after pushing these patches, no problem. If you have an example how to use tst_clone, please do so. I will repeat the same patter to all remaining tests. Thanks! Andrea >> Andrea >> >> On 10/11/22 11:17, Richard Palethorpe wrote: >>> Hello, >>> >>> Richard Palethorpe <rpalethorpe@suse.de> writes: >>> >>>> Hello, >>>> >>>> Andrea Cervesato <andrea.cervesato@suse.com> writes: >>>> >>>>> Hi! >>>>> >>>>> On 8/11/22 11:53, Richard Palethorpe wrote: >>>>>> Hello, >>>>>> >>>>>> Andrea Cervesato via ltp <ltp@lists.linux.it> writes: >>>>>> >>>>>>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> >>>>>>> --- >>>>>>> runtest/containers | 3 +- >>>>>>> testcases/kernel/containers/mqns/common.h | 101 +++++++++++ >>>>>>> testcases/kernel/containers/mqns/mqns_01.c | 193 +++++++-------------- >>>>>>> 3 files changed, 166 insertions(+), 131 deletions(-) >>>>>>> create mode 100644 testcases/kernel/containers/mqns/common.h >>>>>>> >>>>>>> diff --git a/runtest/containers b/runtest/containers >>>>>>> index 2637b62fe..863a964ad 100644 >>>>>>> --- a/runtest/containers >>>>>>> +++ b/runtest/containers >>>>>>> @@ -16,7 +16,8 @@ pidns31 pidns31 >>>>>>> pidns32 pidns32 >>>>>>> mqns_01 mqns_01 >>>>>>> -mqns_01_clone mqns_01 -clone >>>>>>> +mqns_01_clone mqns_01 -m clone >>>>>>> +mqns_01_unshare mqns_01 -m unshare >>>>>>> mqns_02 mqns_02 >>>>>>> mqns_02_clone mqns_02 -clone >>>>>>> mqns_03 mqns_03 >>>>>>> diff --git a/testcases/kernel/containers/mqns/common.h b/testcases/kernel/containers/mqns/common.h >>>>>>> new file mode 100644 >>>>>>> index 000000000..92a77b566 >>>>>>> --- /dev/null >>>>>>> +++ b/testcases/kernel/containers/mqns/common.h >>>>>>> @@ -0,0 +1,101 @@ >>>>>>> +// SPDX-License-Identifier: GPL-2.0-or-later >>>>>>> +/* >>>>>>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> >>>>>>> + */ >>>>>>> + >>>>>>> +#ifndef MQNS_H >>>>>>> +#define MQNS_H >>>>>>> + >>>>>>> +#include <stdlib.h> >>>>>>> +#include "lapi/namespaces_constants.h" >>>>>>> +#include "tst_test.h" >>>>>>> +#include "tst_safe_posix_ipc.h" >>>>>>> + >>>>>>> +enum { >>>>>>> + T_CLONE, >>>>>>> + T_UNSHARE, >>>>>>> + T_NONE, >>>>>>> +}; >>>>>>> + >>>>>>> +static int dummy_child1(void *v) >>>>>>> +{ >>>>>>> + (void)v; >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> +static inline void check_newipc(void) >>>>>>> +{ >>>>>>> + int pid, status; >>>>>>> + >>>>>>> + pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child1, >>>>>>> NULL); >>>>>> ltp_clone_quick is still part of the old API and only uses clone2. I >>>>>> think it should be replaced with tst_clone. This may require extending >>>>>> tst_clone. In fact we probably need a test variant to switch between the >>>>>> clone2 and clone3 syscalls when using tst_clone. >>>>>> >>>>>> I'll leave it to you whether you want to try that and rebase this patch >>>>>> set on it. >>>>>> >>>>> I see ltp_clone_quick as wrapper of ltp_clone, since it's using >>>>> ltp_alloc_stack without calling it explicitly all the times before >>>>> ltp_clone. >>>> ltp_clone is also part of the old API. At some point we should remove >>>> that. >>> I'm marking this as changes requested. tst_clone should be made to >>> support this scenario. >>> >
diff --git a/runtest/containers b/runtest/containers index 2637b62fe..863a964ad 100644 --- a/runtest/containers +++ b/runtest/containers @@ -16,7 +16,8 @@ pidns31 pidns31 pidns32 pidns32 mqns_01 mqns_01 -mqns_01_clone mqns_01 -clone +mqns_01_clone mqns_01 -m clone +mqns_01_unshare mqns_01 -m unshare mqns_02 mqns_02 mqns_02_clone mqns_02 -clone mqns_03 mqns_03 diff --git a/testcases/kernel/containers/mqns/common.h b/testcases/kernel/containers/mqns/common.h new file mode 100644 index 000000000..92a77b566 --- /dev/null +++ b/testcases/kernel/containers/mqns/common.h @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> + */ + +#ifndef MQNS_H +#define MQNS_H + +#include <stdlib.h> +#include "lapi/namespaces_constants.h" +#include "tst_test.h" +#include "tst_safe_posix_ipc.h" + +enum { + T_CLONE, + T_UNSHARE, + T_NONE, +}; + +static int dummy_child1(void *v) +{ + (void)v; + return 0; +} + +static inline void check_newipc(void) +{ + int pid, status; + + pid = ltp_clone_quick(CLONE_NEWIPC | SIGCHLD, dummy_child1, NULL); + if (pid < 0) + tst_brk(TCONF | TERRNO, "CLONE_NEWIPC not supported"); + + SAFE_WAITPID(pid, &status, 0); +} + +static inline int get_clone_unshare_enum(const char* str_op) +{ + if (!str_op) + return T_NONE; + else if (!strcmp(str_op, "clone")) + return T_CLONE; + else if (!strcmp(str_op, "unshare")) + return T_UNSHARE; + + return T_NONE; +} + +static void clone_test(unsigned long clone_flags, int (*fn1)(void *arg), void *arg1) +{ + int pid; + + pid = ltp_clone_quick(clone_flags | SIGCHLD, fn1, arg1); + if (pid < 0) + tst_brk(TBROK | TERRNO, "ltp_clone_quick error"); +} + +static void unshare_test(unsigned long clone_flags, int (*fn1)(void *arg), void *arg1) +{ + int pid; + + pid = SAFE_FORK(); + if (!pid) { + SAFE_UNSHARE(clone_flags); + + fn1(arg1); + exit(0); + } +} + +static void plain_test(int (*fn1)(void *arg), void *arg1) +{ + int pid; + + pid = SAFE_FORK(); + if (!pid) { + fn1(arg1); + exit(0); + } +} + +static void clone_unshare_test(int use_clone, unsigned long clone_flags, + int (*fn1)(void *arg), void *arg1) +{ + switch (use_clone) { + case T_NONE: + plain_test(fn1, arg1); + break; + case T_CLONE: + clone_test(clone_flags, fn1, arg1); + break; + case T_UNSHARE: + unshare_test(clone_flags, fn1, arg1); + break; + default: + tst_brk(TBROK, "%s: bad use_clone option: %d", __FUNCTION__, use_clone); + break; + } +} + +#endif /* MQNS_H */ diff --git a/testcases/kernel/containers/mqns/mqns_01.c b/testcases/kernel/containers/mqns/mqns_01.c index 1d109e020..a34dc4f66 100644 --- a/testcases/kernel/containers/mqns/mqns_01.c +++ b/testcases/kernel/containers/mqns/mqns_01.c @@ -1,148 +1,81 @@ +// SPDX-License-Identifier: GPL-2.0 /* -* Copyright (c) International Business Machines Corp., 2009 -* Copyright (c) Nadia Derbey, 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: Nadia Derbey <Nadia.Derbey@bull.net> -* -* Check mqns isolation: father mqns cannot be accessed from newinstance -* -* Mount mqueue fs -* Create a posix mq -->mq1 -* unshare -* In unshared process: -* Mount newinstance mqueuefs -* Check that mq1 is not readable from new ns - -***************************************************************************/ - -#ifndef _GNU_SOURCE -#define _GNU_SOURCE -#endif -#include <sys/wait.h> -#include <errno.h> -#include <stdio.h> -#include <stdlib.h> -#include <string.h> -#include <unistd.h> -#include "mqns.h" -#include "mqns_helper.h" - -char *TCID = "posixmq_namespace_01"; -int TST_TOTAL = 1; - -int p1[2]; -int p2[2]; - -int check_mqueue(void *vtest) + * Copyright (c) International Business Machines Corp., 2009 + * Copyright (c) Nadia Derbey, 2009 <Nadia.Derbey@bull.net> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> + */ + +/*\ + * [Description] + * + * Create a mqueue inside the parent and check if it can be accessed from + * the isolated/forked child namespace. + */ + +#include "common.h" + +#define MQNAME "/MQ1" + +static mqd_t mqd; +static char *str_op; +static int use_clone; + +static int check_mqueue(LTP_ATTRIBUTE_UNUSED void *vtest) { - char buf[30]; - mqd_t mqd; + mqd_t mqd1; - (void) vtest; + mqd1 = mq_open(MQNAME, O_RDONLY); - close(p1[1]); - close(p2[0]); + if (use_clone == T_NONE) { + if (mqd1 == -1) + tst_res(TFAIL, "Queue has been accessed form plain cloned process"); + else + tst_res(TPASS, "Can't access to queue from plain cloned process"); - if (read(p1[0], buf, strlen("go") + 1) < 0) { - printf("read(p1[0], ...) failed: %s\n", strerror(errno)); - exit(1); - } - mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDONLY); - if (mqd == -1) { - if (write(p2[1], "notfnd", strlen("notfnd") + 1) < 0) { - perror("write(p2[1], ...) failed"); - exit(1); - } - } else { - if (write(p2[1], "exists", strlen("exists") + 1) < 0) { - perror("write(p2[1], \"exists\", 7) failed"); - exit(1); - } else if (mq_close(mqd) < 0) { - perror("mq_close(mqd) failed"); - exit(1); - } + return 0; } - exit(0); + if (mqd1 == -1) + tst_res(TPASS, "Can't access to queue from isolated process"); + else + tst_res(TFAIL, "Queue has been accessed from isolated process"); + + return 0; } -static void setup(void) +static void run(void) { - tst_require_root(); - check_mqns(); + tst_res(TINFO, "Checking namespaces isolation from parent to child"); + + clone_unshare_test(use_clone, CLONE_NEWIPC, check_mqueue, NULL); } -int main(int argc, char *argv[]) +static void setup(void) { - int r; - mqd_t mqd; - 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"); - } + use_clone = get_clone_unshare_enum(str_op); - mqd = tst_syscall(__NR_mq_open, NOSLASH_MQ1, O_RDWR | O_CREAT | O_EXCL, - 0777, NULL); - if (mqd == -1) { - perror("mq_open"); - tst_brkm(TFAIL, NULL, "mq_open failed"); - } + if (use_clone != T_NONE) + check_newipc(); - tst_resm(TINFO, "Checking namespaces isolation from parent to child"); - /* fire off the test */ - r = do_clone_unshare_test(use_clone, CLONE_NEWIPC, check_mqueue, NULL); - if (r < 0) { - tst_resm(TFAIL, "failed clone/unshare"); - mq_close(mqd); - tst_syscall(__NR_mq_unlink, NOSLASH_MQ1); - tst_exit(); - } - - close(p1[0]); - close(p2[1]); - if (write(p1[1], "go", strlen("go") + 1) < 0) - tst_resm(TBROK | TERRNO, "write(p1[1], \"go\", ...) failed"); - else if (read(p2[0], buf, 7) < 0) - tst_resm(TBROK | TERRNO, "read(p2[0], buf, ...) failed"); - else { - if (!strcmp(buf, "exists")) { - tst_resm(TFAIL, "child process found mqueue"); - } else if (!strcmp(buf, "notfnd")) { - tst_resm(TPASS, "child process didn't find mqueue"); - } else { - tst_resm(TFAIL, "UNKNOWN RESULT"); - } - } + mqd = SAFE_MQ_OPEN(MQNAME, O_RDWR | O_CREAT | O_EXCL, 0777, NULL); +} - /* destroy the mqueue */ - if (mq_close(mqd) == -1) { - tst_brkm(TBROK | TERRNO, NULL, "mq_close failed"); +static void cleanup(void) +{ + if (mqd != -1) { + SAFE_MQ_CLOSE(mqd); + SAFE_MQ_UNLINK(MQNAME); } - tst_syscall(__NR_mq_unlink, NOSLASH_MQ1); - - tst_exit(); } + +static struct tst_test test = { + .test_all = run, + .setup = setup, + .cleanup = cleanup, + .needs_root = 1, + .forks_child = 1, + .options = (struct tst_option[]) { + { "m:", &str_op, "Test execution mode <clone|unshare>" }, + {}, + }, +};
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> --- runtest/containers | 3 +- testcases/kernel/containers/mqns/common.h | 101 +++++++++++ testcases/kernel/containers/mqns/mqns_01.c | 193 +++++++-------------- 3 files changed, 166 insertions(+), 131 deletions(-) create mode 100644 testcases/kernel/containers/mqns/common.h