Messages in this thread | | | Date | Tue, 15 Sep 2015 16:20:52 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH RFC] pidns: introduce syscall getvpid |
| |
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.
| |