Re: [PATCH 1/2] pid: add pidfd_open()

On Wed, May 15, 2019 at 04:38:58PM +0200, Oleg Nesterov wrote:

On 05/15, Christian Brauner wrote:

...

+SYSCALL_DEFINE2(pidfd_open, pid_t, pid, unsigned int, flags) +{

• int fd, ret;
• struct pid *p;
• struct task_struct *tsk;
• if (flags)

• return -EINVAL;
  

• if (pid <= 0)

• return -EINVAL;
  

• p = find_get_pid(pid);
• if (!p)

• return -ESRCH;
  

• rcu_read_lock();
• tsk = pid_task(p, PIDTYPE_PID);

You do not need find_get_pid() before rcu_lock and put_pid() at the end. You can just do find_vpid() under rcu_read_lock().

Will do.

...

• if (!tsk)

• ret = -ESRCH;
  

• else if (unlikely(!thread_group_leader(tsk)))

• ret = -EINVAL;
  

it seems that you can do a single check

tsk = pid_task(p, PIDTYPE_TGID); if (!tsk) ret = -ESRCH;

this even looks more correct if we race with exec changing the leader.

The logic here being that you can only reach the thread_group leader from struct pid if PIDTYPE_PID == PIDTYPE_TGID for this struct pid?

Thanks, Oleg. Christian