From: Oleg Nesterov <oleg@redhat.com>
To: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Cc: linux-api@vger.kernel.org, containers@lists.linux-foundation.org,
linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
"Eric W. Biederman" <ebiederm@xmission.com>
Subject: Re: [PATCH RFC] pidns: introduce syscall getvpid
Date: Tue, 15 Sep 2015 16:20:52 +0200 [thread overview]
Message-ID: <20150915142052.GA14820@redhat.com> (raw)
In-Reply-To: <20150915120924.14818.49490.stgit@buzz>
On 09/15, Konstantin Khlebnikov wrote:
>
> +SYSCALL_DEFINE3(getvpid, pid_t, pid, pid_t, source, pid_t, target)
> +{
> +#ifdef CONFIG_PID_NS
> + struct pid_namespace *current_ns = task_active_pid_ns(current);
> + struct pid_namespace *source_ns = current_ns, *target_ns = current_ns;
> + struct pid *task_pid;
> + pid_t result = -ESRCH;
> +
> + rcu_read_lock();
> + if (source)
> + source_ns = ns_of_pid(find_pid_ns(source, current_ns));
> + if (target)
> + target_ns = ns_of_pid(find_pid_ns(target, current_ns));
> + if (source_ns && target_ns) {
> + task_pid = find_pid_ns(pid, source_ns);
> + if (task_pid)
> + result = pid_nr_ns(task_pid, target_ns);
> + }
> + rcu_read_unlock();
> +
> + return result;
> +#else
> + return pid;
> +#endif /* CONFIG_PID_NS */
> +}
Not sure we actually want ifdef(CONFIG_PID_NS). If this is just optimization
I'd suggest to simply add
if (!IS_ENABLED(CONFIG_PID_NS))
return pid;
at the start.
But. Either way this unconditional "return pid" doesn't look right imho.
I think we should return -ESRCH if this pid number is not valid to ensure
we have the same semantics with-or-without CONFIG_PID_NS. So it seems that
you should remove this ifdef, this will also ensure that we return -ESRCH
if (say) source != 0 and find_pid_ns(source) fails.
Oleg.
next prev parent reply other threads:[~2015-09-15 14:23 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-15 12:09 [PATCH RFC] pidns: introduce syscall getvpid Konstantin Khlebnikov
2015-09-15 14:20 ` Oleg Nesterov [this message]
2015-09-15 14:27 ` Eric W. Biederman
2015-09-15 15:01 ` Konstantin Khlebnikov
2015-09-15 15:17 ` Stéphane Graber
2015-09-15 15:51 ` Konstantin Khlebnikov
2015-09-15 17:41 ` Serge Hallyn
2015-09-16 7:37 ` Konstantin Khlebnikov
2015-09-16 14:39 ` Serge E. Hallyn
2015-09-16 14:49 ` Eric W. Biederman
2015-09-16 16:31 ` Serge E. Hallyn
2015-09-21 2:49 ` Chen Fan
2015-09-21 14:22 ` Serge E. Hallyn
2015-09-22 7:42 ` Konstantin Khlebnikov
2015-09-22 21:00 ` Eric W. Biederman
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=20150915142052.GA14820@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=containers@lists.linux-foundation.org \
--cc=ebiederm@xmission.com \
--cc=khlebnikov@yandex-team.ru \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).