* Re: [LTP] [PATCH] containers: new testcase userns02 The user ID and group ID, which are inside a container, can be modified by its parent process.
2015-05-25 11:24 [LTP] [PATCH] containers: new testcase userns02 The user ID and group ID, which are inside a container, can be modified by its parent process Yuan Sun
@ 2015-05-26 13:22 ` Jan Stancek
0 siblings, 0 replies; 2+ messages in thread
From: Jan Stancek @ 2015-05-26 13:22 UTC (permalink / raw)
To: Yuan Sun; +Cc: ltp-list
Hi,
----- Original Message -----
> From: "Yuan Sun" <sunyuan3@huawei.com>
> To: jstancek@redhat.com
> Cc: ltp-list@lists.sourceforge.net
> Sent: Monday, 25 May, 2015 1:24:44 PM
> Subject: [PATCH] containers: new testcase userns02 The user ID and group ID, which are inside a container, can be
> modified by its parent process.
This is quite long for single line, please add new line after "The user ID",
>
> ---
> runtest/containers | 1 +
> testcases/kernel/containers/.gitignore | 1 +
> testcases/kernel/containers/userns/userns02.c | 94 +++++++++++++++++++++++++++
> 3 files changed, 96 insertions(+)
> create mode 100644 testcases/kernel/containers/userns/userns02.c
>
> diff --git a/runtest/containers b/runtest/containers
> index ca10372..bb1beb6 100644
> --- a/runtest/containers
> +++ b/runtest/containers
> @@ -69,3 +69,4 @@ mountns03 mountns03
> mountns04 mountns04
>
> userns01 userns01
> +userns02 userns02
> diff --git a/testcases/kernel/containers/.gitignore b/testcases/kernel/containers/.gitignore
> index 4478b53..e3c92c9 100644
> --- a/testcases/kernel/containers/.gitignore
> +++ b/testcases/kernel/containers/.gitignore
> @@ -4,3 +4,4 @@ mountns/mountns02
> mountns/mountns03
> mountns/mountns04
> userns/userns01
> +userns/userns02
> diff --git a/testcases/kernel/containers/userns/userns02.c b/testcases/kernel/containers/userns/userns02.c
> new file mode 100644
> index 0000000..078439e
> --- /dev/null
> +++ b/testcases/kernel/containers/userns/userns02.c
> @@ -0,0 +1,94 @@
> +/*
> +* Copyright (c) Huawei Technologies Co., Ltd., 2015
> +* 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 it's not too much trouble, could you please align those stars? :-)
Take a look at userns01.c
> +*
> +* Verify that:
> +* The user ID and group ID, which are inside a container, can be modified by
> +* its parent process.
> +*/
> +
> +#define _GNU_SOURCE
> +#include <sys/wait.h>
> +#include <assert.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <errno.h>
> +#include "test.h"
> +#include "libclone.h"
I think you're not using libclone.h for anything in this test.
> +#include "userns_helper.h"
> +
> +char *TCID = "user_namespace2";
> +int TST_TOTAL = 1;
> +
> +char fullpath[BUFSIZ];
Seems unused.
> +pid_t childpid;
> +int parentuid;
> +int parentgid;
> +char cmd[BUFSIZ];
> +FILE *fp;
Why global? These are used only in main.
> +/*
> + * child_fn1() - Inside a new user namespace
> + */
> +static int child_fn1(void *arg)
> +{
> + int exit_val;
> + int uid, gid;
> +
> + sleep(1);
No sleeps for synchronization please. You can use checkpoints, see include/tst_checkpoint.h.
> + uid = geteuid();
> + gid = getegid();
> +
> + tst_resm(TINFO, "USERNS test is running in a new user namespace.");
Style guide discourages from using tst_* functions in child processes
> + if (uid == 100 && gid == 100) {
> + tst_resm(TINFO, "Got expected uid and gid.");
> + exit_val = 0;
> + } else {
> + tst_brkm(TFAIL | TTERRNO, NULL, "Got unexpected result of uid=%d gid=%d\n",
> + uid, gid);
This line is too long
It's tst_* function in child process
It will end execution of child - making lines below irrelevant
TTERRNO doesn't make sense, since you haven't used TEST() macro
printf would suffice here
> + exit_val = 1;
> + }
> +
> + return exit_val;
> +}
> +
> +static void setup(void)
> +{
> + check_newuser();
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + int status;
> +
> + tst_parse_opts(argc, argv, NULL, NULL);
> + setup();
This is something I missed in userns01 - TEST_LOOPING is missing.
Running "./userns02 -i 5" should run the case 5 times.
> +
> + childpid = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD, child_fn1, NULL);
> +
> + if (childpid < 0)
> + tst_brkm(TFAIL | TTERRNO, NULL, "clone failed");
TTERRNO will print errno from TEST_ERRNO, TERRNO from errno
You can either wrap call with TEST() and use TEST_RETURN instead of childpid
or you can change TTERRNO to TERRNO.
> +
> + parentuid = geteuid();
> + sprintf(cmd, "echo 100 %d 1 > /proc/%d/uid_map", parentuid, childpid);
> + system(cmd);
There is SAFE_WRITE macro, which also checks that write succeeded.
> + parentgid = getegid();
> + sprintf(cmd, "echo 100 %d 1 > /proc/%d/gid_map", parentgid, childpid);
> + system(cmd);
> +
> + if (waitpid(childpid, &status, 0) < 0)
> + tst_resm(TWARN, "parent: waitpid failed.");
TBROK | TERRNO would be better here - in case it fails, knowing errno
gives you more data.
You're missing a check that exit code was actually 0.
Regards,
Jan
> +
> + tst_resm(TPASS, "the uid and the gid are right inside the container");
> + tst_exit();
> +}
> +
>
------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 2+ messages in thread