clone_extended() syscall

Subject: [RFC][v4][PATCH 7/7]: Define clone_extended() syscall

Container restart requires that a task have the same pid it had when it was checkpointed. When containers are nested the tasks within the containers exist in multiple pid namespaces and hence have multiple pids to specify during restart.

This patch defines, a new system call, clone_extended() which is like clone(), but takes a new 'pid_set' parameter. This parameter lets caller choose specific pid numbers for the child process, in the process's active and ancestor pid namespaces. (Descendant pid namespaces in general don't matter since processes don't have pids in them anyway, but see comments in copy_target_pids() regarding CLONE_NEWPID).

Unlike clone(), however, clone_extended() needs CAP_SYS_ADMIN, at least for now, to prevent unprivileged processes from misusing this interface.

While the main motivation for this interface is the need to let a process choose its 'pid numbers', the clone_extended() interface uses 64-bit clone flags. The 'higher' portion of the clone flags are unused and are only included to preclude yet another version of clone when a new clone flag is needed.

===== Interface:

Compared to clone(), clone_extended() needs to pass in three more pieces of information:

- additional 32-bit of clone_flags
- number of pids in the set
- user buffer containing the list of pids.

But since clone() already takes 5 parameters and some (all ?) architectures are restricted to 6 parameters to a system-call, additional data-structures (and copy_from_user()) are needed.

The proposed interface for clone_extended() is:

	struct clone_tid_info {
		void *parent_tid; 	/* parent_tid_ptr parameter */
		void *child_tid; 	/* child_tid_ptr parameter */
	};

	struct pid_set {
		int num_pids;
		pid_t *pids;
	};

	int clone_extended(int flags_low, int flags_high, void *child_stack,
			void *unused, struct clone_tid_info *tid_ptrs,
			struct pid_set *pid_setp);

Quoted 14 lines · Sukadev Bhattiprolu

Subject: [RFC][v4][PATCH 7/7]: Define clone_extended() syscall

Container restart requires that a task have the same pid it had when it was checkpointed. When containers are nested the tasks within the containers exist in multiple pid namespaces and hence have multiple pids to specify during restart.

This patch defines, a new system call, clone_extended() which is like clone(), but takes a new 'pid_set' parameter. This parameter lets caller choose specific pid numbers for the child process, in the process's active and ancestor pid namespaces. (Descendant pid namespaces in general don't matter since processes don't have pids in them anyway, but see comments in copy_target_pids() regarding CLONE_NEWPID).

Quoted 2 lines

Unlike clone(), however, clone_extended() needs CAP_SYS_ADMIN, at least for now, to prevent unprivileged processes from misusing this interface.

It only needs that when specifying pids.

Quoted 31 lines

While the main motivation for this interface is the need to let a process choose its 'pid numbers', the clone_extended() interface uses 64-bit clone flags. The 'higher' portion of the clone flags are unused and are only included to preclude yet another version of clone when a new clone flag is needed.

===== Interface:

Compared to clone(), clone_extended() needs to pass in three more pieces of information:

- additional 32-bit of clone_flags
- number of pids in the set
- user buffer containing the list of pids.

But since clone() already takes 5 parameters and some (all ?) architectures are restricted to 6 parameters to a system-call, additional data-structures (and copy_from_user()) are needed.

The proposed interface for clone_extended() is:

	struct clone_tid_info {
		void *parent_tid; 	/* parent_tid_ptr parameter */
		void *child_tid; 	/* child_tid_ptr parameter */
	};

	struct pid_set {
		int num_pids;
		pid_t *pids;
	};

Quoted 3 lines

int clone_extended(int flags_low, int flags_high, void *child_stack,
void *unused, struct clone_tid_info *tid_ptrs, struct pid_set *pid_setp);

I was thinking additional flags would be passed in the (renamed) struct pid_set.

Quoted 51 lines · Serge E. Hallyn

Quoted 14 lines · Sukadev Bhattiprolu

Subject: [RFC][v4][PATCH 7/7]: Define clone_extended() syscall

Container restart requires that a task have the same pid it had when it was checkpointed. When containers are nested the tasks within the containers exist in multiple pid namespaces and hence have multiple pids to specify during restart.

This patch defines, a new system call, clone_extended() which is like clone(), but takes a new 'pid_set' parameter. This parameter lets caller choose specific pid numbers for the child process, in the process's active and ancestor pid namespaces. (Descendant pid namespaces in general don't matter since processes don't have pids in them anyway, but see comments in copy_target_pids() regarding CLONE_NEWPID).

Quoted 2 lines

Unlike clone(), however, clone_extended() needs CAP_SYS_ADMIN, at least for now, to prevent unprivileged processes from misusing this interface.

It only needs that when specifying pids.

Quoted 31 lines

While the main motivation for this interface is the need to let a process choose its 'pid numbers', the clone_extended() interface uses 64-bit clone flags. The 'higher' portion of the clone flags are unused and are only included to preclude yet another version of clone when a new clone flag is needed.

===== Interface:

Compared to clone(), clone_extended() needs to pass in three more pieces of information:

- additional 32-bit of clone_flags
- number of pids in the set
- user buffer containing the list of pids.

But since clone() already takes 5 parameters and some (all ?) architectures are restricted to 6 parameters to a system-call, additional data-structures (and copy_from_user()) are needed.

The proposed interface for clone_extended() is:

	struct clone_tid_info {
		void *parent_tid; 	/* parent_tid_ptr parameter */
		void *child_tid; 	/* child_tid_ptr parameter */
	};

	struct pid_set {
		int num_pids;
		pid_t *pids;
	};

Quoted 6 lines

Quoted 3 lines

int clone_extended(int flags_low, int flags_high, void *child_stack,
void *unused, struct clone_tid_info *tid_ptrs, struct pid_set *pid_setp);

I was thinking additional flags would be passed in the (renamed) struct pid_set.

Yes.

But maybe in (renamed) 'struct clone_info' instead of 'struct pid_set' ?

I vaguely recall a strong preference to not require copy-from-user during a fast-path clone, because it may hurt performance.

*If* this is the case, then maybe place extra flags among the "base" args, or at least a CLONE_EXTRA would indicate that more arguments need to be pulled from user-space ?

Do you intend to get feedback from LKML too ?

Oren.

Quoted 61 lines · Oren Laadan

Quoted 50 lines · Serge E. Hallyn

Quoted 14 lines · Sukadev Bhattiprolu

Subject: [RFC][v4][PATCH 7/7]: Define clone_extended() syscall

Container restart requires that a task have the same pid it had when it was checkpointed. When containers are nested the tasks within the containers exist in multiple pid namespaces and hence have multiple pids to specify during restart.

This patch defines, a new system call, clone_extended() which is like clone(), but takes a new 'pid_set' parameter. This parameter lets caller choose specific pid numbers for the child process, in the process's active and ancestor pid namespaces. (Descendant pid namespaces in general don't matter since processes don't have pids in them anyway, but see comments in copy_target_pids() regarding CLONE_NEWPID).

Quoted 2 lines

Unlike clone(), however, clone_extended() needs CAP_SYS_ADMIN, at least for now, to prevent unprivileged processes from misusing this interface.

It only needs that when specifying pids.

Quoted 31 lines

While the main motivation for this interface is the need to let a process choose its 'pid numbers', the clone_extended() interface uses 64-bit clone flags. The 'higher' portion of the clone flags are unused and are only included to preclude yet another version of clone when a new clone flag is needed.

===== Interface:

Compared to clone(), clone_extended() needs to pass in three more pieces of information:

- additional 32-bit of clone_flags
- number of pids in the set
- user buffer containing the list of pids.

But since clone() already takes 5 parameters and some (all ?) architectures are restricted to 6 parameters to a system-call, additional data-structures (and copy_from_user()) are needed.

The proposed interface for clone_extended() is:

	struct clone_tid_info {
		void *parent_tid; 	/* parent_tid_ptr parameter */
		void *child_tid; 	/* child_tid_ptr parameter */
	};

	struct pid_set {
		int num_pids;
		pid_t *pids;
	};

Quoted 5 lines

Quoted 3 lines

int clone_extended(int flags_low, int flags_high, void *child_stack,
void *unused, struct clone_tid_info *tid_ptrs, struct pid_set *pid_setp);

I was thinking additional flags would be passed in the (renamed) struct pid_set.

Yes.

But maybe in (renamed) 'struct clone_info' instead of 'struct pid_set' ?

Quoted 6 lines

I vaguely recall a strong preference to not require copy-from-user during a fast-path clone, because it may hurt performance.

*If* this is the case, then maybe place extra flags among the "base" args, or at least a CLONE_EXTRA would indicate that more arguments need to be pulled from user-space ?

Wouldn't passing NULL for struct clone_info suffice?

Quoted 4 lines

Do you intend to get feedback from LKML too ?

Oren.

Quoted 28 lines

Quoted 27 lines

Quoted 12 lines

Quoted 12 lines

The proposed interface for clone_extended() is:

	struct clone_tid_info {
		void *parent_tid; 	/* parent_tid_ptr parameter */
		void *child_tid; 	/* child_tid_ptr parameter */
	};

	struct pid_set {
		int num_pids;
		pid_t *pids;
	};

Quoted 5 lines

Quoted 3 lines

int clone_extended(int flags_low, int flags_high, void *child_stack,
void *unused, struct clone_tid_info *tid_ptrs, struct pid_set *pid_setp);

I was thinking additional flags would be passed in the (renamed) struct pid_set.

Yes.

But maybe in (renamed) 'struct clone_info' instead of 'struct pid_set' ?

I vaguely recall a strong preference to not require copy-from-user during a fast-path clone, because it may hurt performance.

*If* this is the case, then maybe place extra flags among the "base" args, or at least a CLONE_EXTRA would indicate that more arguments need to be pulled from user-space ?

Quoted 1 line

Wouldn't passing NULL for struct clone_info suffice?

:o

Actually, I misread the original prototype, and I prefer Suka's current suggestion.

| I think Suka's suggestion is again inherently limited (in # of
| clone flags) and will force even uglier syscalls for each arch than the
| previous one already does.
| 

Yes other architectures are forced to ignore the flags_high and copy-in the tid pointers.

But if we want more than 64 bit flags, we may need follow the sigset_t model ?

Also, I am listing three approaches below. Do you prefer #2 below ?

1. =====

        struct clone_tid_info {
                void *parent_tid;       /* parent_tid_ptr parameter */
                void *child_tid;        /* child_tid_ptr parameter */
        };

        struct pid_set {
                int num_pids;
                pid_t *pids;
        };

        int clone_extended(int flags_low, int flags_high, void *child_stack,
                        void *unused, struct clone_tid_info *tid_ptrs,
                        struct pid_set *pid_setp);

2. ======

	struct clone_info {
		int flags_high;
		struct pid_set pid_set;
	}

        int clone_extended(int flags_low, void *child_stack, void *unused,
		int *parent_tid, int *child_tid, struct clone_info *clone_info);


	Pros:
		- copy_from_user() needed only for new flags and pid_set

	Cons:
		- splitting the high and low clone-flags is awkward ?

3. =====

typedef struct {
unsigned long flags[N_CLONE_FLAGS];
} clone_flags_t;

int clone_extended(clone_flags_t *flags, void *child_stack, int *unused,
int *parent_tid, int *child_tid, struct pid_set *pid_set);

Pros:
- extendible clone_flags (like sigset_t)
- no copy_from_user() when we have 32 clone-flags
- no copy_from_user for tids

Cons:
- copy_from_user() needed on 32-bit architectures for all flags when they exceed 32.

Sukadev

Subject: [RFC][v4][PATCH 7/7]: Define clone_extended() syscall

Container restart requires that a task have the same pid it had when it was checkpointed. When containers are nested the tasks within the containers exist in multiple pid namespaces and hence have multiple pids to specify during restart.

This patch defines, a new system call, clone_extended() which is like clone(), but takes a new 'pid_set' parameter. This parameter lets caller choose specific pid numbers for the child process, in the process's active and ancestor pid namespaces. (Descendant pid namespaces in general don't matter since processes don't have pids in them anyway, but see comments in copy_target_pids() regarding CLONE_NEWPID).

Unlike clone(), however, clone_extended() needs CAP_SYS_ADMIN, at least for now, to prevent unprivileged processes from misusing this interface.