linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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.


  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).