From: Yuan Sun <sunyuan3@huawei.com>
To: Jan Stancek <jstancek@redhat.com>
Cc: ltp-list@lists.sourceforge.net
Subject: Re: [LTP] [PATCH V3] containers: new testcase userns03
Date: Thu, 25 Jun 2015 10:05:11 +0800 [thread overview]
Message-ID: <558B61D7.4000900@huawei.com> (raw)
In-Reply-To: <55841B5D.5050608@redhat.com>
Hi Jan,
I didn't find the synchronization error. The comment added by you
is great.
You are very careful and professional. Thank you very much.
Could you please merge the patch into master? Or I will send a V4
patch
according to your modification.
Regards.
Yuan
On 2015/6/19 21:38, Jan Stancek wrote:
> On 06/16/2015 11:28 PM, Yuan Sun wrote:
>> ID-outside-ns is interpreted according to which process is opening
>> the file. If the process opening the file is in the same user namespace
>> as the process PID, then ID-outside-ns is defined with respect to the
>> parent user namespace. If the process opening the file is in a different
>> user namespace, then ID-outside-ns is defined with respect to the user
>> namespace of the process opening the file.
>> If kernel version >= 3.19.0, the case will ignore the git check.
>>
>> Signed-off-by: Yuan Sun <sunyuan3@huawei.com>
> Hi,
>
> there is one problem in v3 with synchronization.
> Problem is that child2 can start checking it's uid/gid sooner than
> main sets the mapping.
>
> I have attached my idea for possible fix, comments about changes I made are below.
>
>> ---
>> runtest/containers | 1 +
>> testcases/kernel/containers/.gitignore | 1 +
>> testcases/kernel/containers/userns/userns03.c | 254 ++++++++++++++++++++++++++
>> 3 files changed, 256 insertions(+)
>> create mode 100644 testcases/kernel/containers/userns/userns03.c
>>
>> diff --git a/runtest/containers b/runtest/containers
>> index bb1beb6..720d0f2 100644
>> --- a/runtest/containers
>> +++ b/runtest/containers
>> @@ -70,3 +70,4 @@ mountns04 mountns04
>>
>> userns01 userns01
>> userns02 userns02
>> +userns03 userns03
>> diff --git a/testcases/kernel/containers/.gitignore b/testcases/kernel/containers/.gitignore
>> index e3c92c9..bd3cb9d 100644
>> --- a/testcases/kernel/containers/.gitignore
>> +++ b/testcases/kernel/containers/.gitignore
>> @@ -5,3 +5,4 @@ mountns/mountns03
>> mountns/mountns04
>> userns/userns01
>> userns/userns02
>> +userns/userns03
>> diff --git a/testcases/kernel/containers/userns/userns03.c b/testcases/kernel/containers/userns/userns03.c
>> new file mode 100644
>> index 0000000..b5596fc
>> --- /dev/null
>> +++ b/testcases/kernel/containers/userns/userns03.c
>> @@ -0,0 +1,254 @@
>> +/*
>> +* 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.
>> +*/
>> +
>> +/*
>> +* Verify that:
>> +* /proc/PID/uid_map and /proc/PID/gid_map contains three values separated by
>> +* white space:
>> +* ID-inside-ns ID-outside-ns length
>> +*
>> +* ID-outside-ns is interpreted according to which process is opening the file.
>> +* If the process opening the file is in the same user namespace as the process
>> +* PID, then ID-outside-ns is defined with respect to the parent user namespace.
>> +* If the process opening the file is in a different user namespace, then
>> +* ID-outside-ns is defined with respect to the user namespace of the process
>> +* opening the file.
>> +*/
> I added comment here about when we set/test gid map.
>
>> +
>> +#define _GNU_SOURCE
>> +#include <sys/wait.h>
>> +#include <assert.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <stdbool.h>
>> +#include <unistd.h>
>> +#include <string.h>
>> +#include <errno.h>
>> +#include "test.h"
>> +#include "libclone.h"
>> +#include "userns_helper.h"
>> +
>> +char *TCID = "user_namespace3";
>> +int TST_TOTAL = 1;
>> +static int cpid1, parentuid, parentgid;
>> +static bool setgroupstag = true;
>> +
>> +#define CHILD1UID 0
>> +#define CHILD1GID 0
>> +#define CHILD2UID 200
>> +#define CHILD2GID 200
>> +#define UID_MAP 0
>> +#define GID_MAP 1
>> +#define UID_MAP 0
>> +#define GID_MAP 1
> UID_MAP, GID_MAP is defined twice, I deleted one.
>
>> +
>> +/*
>> + * child_fn1() - Inside a new user namespace
>> + */
>> +static int child_fn1(void)
>> +{
>> + TST_SAFE_CHECKPOINT_WAKE_AND_WAIT(NULL, 0);
>> + return 0;
>> +}
>> +
>> +
>> +/*
>> + * child_fn2() - Inside a new user namespace
>> + */
>> +static int child_fn2(void)
>> +{
>> + int exit_val;
>> + int uid, gid;
>> + char cpid1uidpath[BUFSIZ];
>> + char cpid1gidpath[BUFSIZ];
>> + int idinsidens, idoutsidens, length;
>> +
> I added WAIT here, so child doesn't start before main sets the mapping.
>
>> + uid = geteuid();
>> + gid = getegid();
>> +
>> + if (uid == CHILD2UID) {
>> + printf("Got expected uid.\n");
> I removed this printf.
>
>> + exit_val = 0;
>> + } else {
>> + printf("unexpected uid=%d\n", uid);
>> + exit_val = 1;
>> + }
>> +
>> + if (setgroupstag == false) {
>> + if (gid == CHILD2GID) {
>> + printf("Got expected gid.\n");
>> + exit_val = 0;
>> + } else {
>> + printf("unexpected: gid=%d\n", gid);
>> + exit_val = 1;
>> + }
>> + }
>> + /*Get the uid parameters of the child_fn2 process.*/
>> + SAFE_FILE_SCANF(NULL, "/proc/self/uid_map", "%d %d %d", &idinsidens,
>> + &idoutsidens, &length);
>> +
>> + /* map file format:ID-inside-ns ID-outside-ns length
>> + If the process opening the file is in the same user namespace as
>> + the process PID, then ID-outside-ns is defined with respect to the
>> + parent user namespace.*/
>> + if (idinsidens != CHILD2UID || idoutsidens != parentuid) {
>> + printf("child_fn2 checks /proc/cpid2/uid_map:\n");
>> + printf("unexpected: idinsidens=%d idoutsidens=%d\n",
>> + idinsidens, idoutsidens);
>> + exit_val = 1;
>> + }
>> +
>> + sprintf(cpid1uidpath, "/proc/%d/uid_map", cpid1);
>> + SAFE_FILE_SCANF(NULL, cpid1uidpath, "%d %d %d", &idinsidens,
>> + &idoutsidens, &length);
>> +
>> + /* If the process opening the file is in a different user namespace,
>> + then ID-outside-ns is defined with respect to the user namespace
>> + of the process opening the file.*/
>> + if (idinsidens != CHILD1UID || idoutsidens != CHILD2UID) {
>> + printf("child_fn2 checks /proc/cpid1/uid_map:\n");
>> + printf("unexpected: idinsidens=%d idoutsidens=%d\n",
>> + idinsidens, idoutsidens);
>> + exit_val = 1;
>> + }
>> +
>> + if (setgroupstag == false) {
>> + sprintf(cpid1gidpath, "/proc/%d/gid_map", cpid1);
>> + SAFE_FILE_SCANF(NULL, "/proc/self/gid_map", "%d %d %d",
>> + &idinsidens, &idoutsidens, &length);
>> +
>> + if (idinsidens != CHILD2GID || idoutsidens != parentgid) {
>> + printf("child_fn2 checks /proc/cpid2/gid_map:\n");
>> + printf("unexpected: idinsidens=%d idoutsidens=%d\n",
>> + idinsidens, idoutsidens);
>> + exit_val = 1;
>> + }
>> +
>> + SAFE_FILE_SCANF(NULL, cpid1gidpath, "%d %d %d", &idinsidens,
>> + &idoutsidens, &length);
>> +
>> + if (idinsidens != CHILD1GID || idoutsidens != CHILD2GID) {
>> + printf("child_fn1 checks /proc/cpid1/gid_map:\n");
>> + printf("unexpected: idinsidens=%d idoutsidens=%d\n",
>> + idinsidens, idoutsidens);
>> + exit_val = 1;
>> + }
>> + }
>> + TST_SAFE_CHECKPOINT_WAKE_AND_WAIT(NULL, 1);
>> + return exit_val;
>> +}
>> +
>> +
>> +static void setup(void)
>> +{
>> + char read_buf[BUFSIZ];
>> +
>> + check_newuser();
>> + tst_tmpdir();
>> + TST_CHECKPOINT_INIT(NULL);
>> + if (access("/proc/self/setgroups", F_OK) == 0) {
>> + SAFE_FILE_SCANF(NULL, "/proc/self/setgroups", "%s", read_buf);
> added cleanup
>
>> + if (strcmp(read_buf, "deny") == 0)
>> + setgroupstag = false;
>> + }
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> + tst_rmdir();
>> +}
>> +
>> +static int updatemap(int cpid, bool type, int idnum, int parentmappid)
>> +{
>> + char path[BUFSIZ];
>> + char content[BUFSIZ];
>> + int fd;
>> +
>> + if (type == UID_MAP)
>> + sprintf(path, "/proc/%d/uid_map", cpid);
>> + else if (type == GID_MAP)
>> + sprintf(path, "/proc/%d/gid_map", cpid);
>> + else
>> + tst_brkm(TFAIL, cleanup, "invalid type parameter");
>> +
>> + sprintf(content, "%d %d 1", idnum, parentmappid);
>> + fd = SAFE_OPEN(NULL, path, O_WRONLY, 0644);
>> + SAFE_WRITE(cleanup, 1, fd, content, strlen(content));
>> + SAFE_CLOSE(cleanup, fd);
>> + return 0;
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + pid_t cpid2;
>> + int cpid1status, cpid2status;
>> + int lc;
>> +
>> + tst_parse_opts(argc, argv, NULL, NULL);
>> + setup();
>> +
>> + for (lc = 0; TEST_LOOPING(lc); lc++) {
>> + tst_count = 0;
>> +
>> + parentuid = geteuid();
>> + parentgid = getegid();
>> +
>> + cpid1 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD,
>> + (void *)child_fn1, NULL);
>> + if (cpid1 < 0)
>> + tst_brkm(TFAIL | TERRNO, cleanup,
>> + "cpid1 clone failed");
>> +
>> + cpid2 = ltp_clone_quick(CLONE_NEWUSER | SIGCHLD,
>> + (void *)child_fn2, NULL);
>> + if (cpid2 < 0)
>> + tst_brkm(TFAIL | TERRNO, cleanup,
>> + "cpid2 clone failed");
>> +
>> + updatemap(cpid1, 0, CHILD1UID, parentuid);
>> + updatemap(cpid2, 0, CHILD2UID, parentuid);
>> +
>> + if (setgroupstag == false) {
>> + updatemap(cpid1, 1, CHILD1GID, parentuid);
>> + updatemap(cpid2, 1, CHILD2GID, parentuid);
>> + }
> calls to updatemap changed to use defines rather than 0/1
>
> Regards,
> Jan
>
>
------------------------------------------------------------------------------
Monitor 25 network devices or servers for free with OpManager!
OpManager is web-based network management software that monitors
network devices and physical & virtual servers, alerts via email & sms
for fault. Monitor 25 devices for free with no restriction. Download now
http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
next prev parent reply other threads:[~2015-06-25 2:05 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-16 21:28 [LTP] [PATCH V3] containers: new testcase userns03 Yuan Sun
2015-06-19 13:38 ` Jan Stancek
2015-06-25 2:05 ` Yuan Sun [this message]
2015-06-26 9:57 ` Jan Stancek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=558B61D7.4000900@huawei.com \
--to=sunyuan3@huawei.com \
--cc=jstancek@redhat.com \
--cc=ltp-list@lists.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.