lkml.org 
[lkml]   [2019]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v10 1/2] fork: extend clone3() to support setting a PID
On 11/15, Christian Brauner wrote:
>
> +static int set_tid_next(pid_t *set_tid, size_t *size, int idx)
> +{
> + int tid = 0;
> +
> + if (*size) {
> + tid = set_tid[idx];
> + if (tid < 1 || tid >= pid_max)
> + return -EINVAL;
> +
> + /*
> + * Also fail if a PID != 1 is requested and
> + * no PID 1 exists.
> + */
> + if (tid != 1 && !tmp->child_reaper)
> + return -EINVAL;
> +
> + if (!ns_capable(tmp->user_ns, CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + (*size)--;
> + }

this needs more args, struct pid_namespace *tmp + pid_t pid_max

> +
> + return tid;
> +}
> +
> struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid,
> size_t set_tid_size)
> {
> @@ -188,20 +213,10 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid,
> for (i = ns->level; i >= 0; i--) {
> int tid = 0;
>
> - if (set_tid_size) {
> - tid = set_tid[ns->level - i];
> - if (tid < 1 || tid >= pid_max)
> - return ERR_PTR(-EINVAL);
> - /*
> - * Also fail if a PID != 1 is requested and
> - * no PID 1 exists.
> - */
> - if (tid != 1 && !tmp->child_reaper)
> - return ERR_PTR(-EINVAL);
> - if (!ns_capable(tmp->user_ns, CAP_SYS_ADMIN))
> - return ERR_PTR(-EPERM);
> - set_tid_size--;
> - }
> + retval = set_tid_next(set_tid, &set_tid_size, ns->level - i);
> + if (retval < 0)
> + goto out_free;
> + tid = retval;

Well, if we add a helper then

static inline int check_tid(tid, max, ns)
{
if (tid < 1 || tid >= max)
return ERR_PTR(-EINVAL);
/*
* Also fail if a PID != 1 is requested and
* no PID 1 exists.
*/
if (tid != 1 && !ns->child_reaper)
return ERR_PTR(-EINVAL);
if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN))
return ERR_PTR(-EPERM);
return 0;
}

... alloc_pid() ...

if (set_tid_size) {
tid = set_tid[ns->level - i];
retval = check_tid(tid, pid_max, tmp);
if (retval)
goto out_free;
set_tid_size--;
}

looks more clean to me. But still ugly. IMO,

if (set_tid_size) {
tid = set_tid[ns->level - i];

retval = -EINVAL;
if (tid < 1 || tid >= pid_max)
goto out_free;
/*
* Also fail if a PID != 1 is requested and
* no PID 1 exists.
*/
if (tid != 1 && !tmp->child_reaper)
goto out_free;
retval = -EPERM;
if (!ns_capable(tmp->user_ns, CAP_SYS_ADMIN))
goto out_free;
set_tid_size--;
}

makes more sense.

Oleg.

\
 
 \ /
  Last update: 2019-11-15 11:50    [W:0.069 / U:0.348 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site