All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
To: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
Cc: Containers
	<containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"Eric W. Biederman"
	<ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>,
	hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org,
	mingo-X9Un+BFzKDI@public.gmane.org,
	Sukadev Bhattiprolu
	<sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>,
	torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org,
	Alexey Dobriyan
	<adobriyan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Pavel Emelyanov <xemul-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
Subject: Re: [RFC][v5][PATCH 8/8]: Define clone_with_pids() syscall
Date: Wed, 9 Sep 2009 14:19:50 +0200	[thread overview]
Message-ID: <200909091419.50496.arnd@arndb.de> (raw)
In-Reply-To: <m3zl95l2oz.fsf-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>

On Tuesday 08 September 2009, Nathan Lynch wrote:

> This doesn't work on a 64-bit kernel when the process is 32-bit and uses
> the definition of struct pid_set provided in types.h:
> 
> +struct pid_set {
> +       int num_pids;
> +       pid_t *pids;
> +};
> 
> Shouldn't the pids field be u64 or some other type of fixed size?

This is a complex problem. The structure above would need a conversion
for the pointer size that you can avoid by using a u64, but that introduces
another problem:

struct pid_set {
	int num_pids;
	u64 pidp;
};

Has implicit padding between the two members on all 64 bit architectures,
but not on i386, so you would still need a conversion (not for s390, power,
mips, sparc or parisc though, only for x86).

I can see two solutions for this:

1. use separate system call arguments for num_pids and pidp.
This avoids the data structure and saves one copy_from_user call,
at the cost of adding another argument to the syscall. syscalls with
more than 6 arguments are somewhat problematic as well.

2. use a single pointer, with variable length data structures:

struct pid_set {
	int num_pids;
	pid_t pids[0];
};

Since pid_t is always an int, you have no problem with padding or
incompatible types, but rely on a data structure definition that
is not in C89 (not sure about C99).

	Arnd <><

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Nathan Lynch <ntl@pobox.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org,
	Containers <containers@lists.linux-foundation.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	hpa@zytor.com, mingo@elte.hu, torvalds@linux-foundation.org,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Pavel Emelyanov <xemul@openvz.org>
Subject: Re: [RFC][v5][PATCH 8/8]: Define clone_with_pids() syscall
Date: Wed, 9 Sep 2009 14:19:50 +0200	[thread overview]
Message-ID: <200909091419.50496.arnd@arndb.de> (raw)
In-Reply-To: <m3zl95l2oz.fsf@pobox.com>

On Tuesday 08 September 2009, Nathan Lynch wrote:

> This doesn't work on a 64-bit kernel when the process is 32-bit and uses
> the definition of struct pid_set provided in types.h:
> 
> +struct pid_set {
> +       int num_pids;
> +       pid_t *pids;
> +};
> 
> Shouldn't the pids field be u64 or some other type of fixed size?

This is a complex problem. The structure above would need a conversion
for the pointer size that you can avoid by using a u64, but that introduces
another problem:

struct pid_set {
	int num_pids;
	u64 pidp;
};

Has implicit padding between the two members on all 64 bit architectures,
but not on i386, so you would still need a conversion (not for s390, power,
mips, sparc or parisc though, only for x86).

I can see two solutions for this:

1. use separate system call arguments for num_pids and pidp.
This avoids the data structure and saves one copy_from_user call,
at the cost of adding another argument to the syscall. syscalls with
more than 6 arguments are somewhat problematic as well.

2. use a single pointer, with variable length data structures:

struct pid_set {
	int num_pids;
	pid_t pids[0];
};

Since pid_t is always an int, you have no problem with padding or
incompatible types, but rely on a data structure definition that
is not in C89 (not sure about C99).

	Arnd <><

  parent reply	other threads:[~2009-09-09 12:19 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-07 21:13 [RFC][v5] clone_with_pids() system call Sukadev Bhattiprolu
2009-09-07 21:13 ` Sukadev Bhattiprolu
2009-09-07 21:13 ` [RFC][v5][PATCH 1/8]: Factor out code to allocate pidmap page Sukadev Bhattiprolu
2009-09-07 21:14 ` [RFC][v5][PATCH 3/8] Make pid_max a pid_ns property Sukadev Bhattiprolu
2009-09-07 21:15 ` [RFC][v5][PATCH 5/8]: Add target_pids parameter to alloc_pid() Sukadev Bhattiprolu
2009-09-07 21:16 ` [RFC][v5][PATCH 7/8]: Define do_fork_with_pids() Sukadev Bhattiprolu
2009-09-07 21:17 ` [RFC][v5][PATCH 8/8]: Define clone_with_pids() syscall Sukadev Bhattiprolu
     [not found]   ` <20090907211700.GH6685-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2009-09-08 18:19     ` Nathan Lynch
2009-09-08 18:19   ` Nathan Lynch
     [not found]     ` <m3zl95l2oz.fsf-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
2009-09-09 12:19       ` Arnd Bergmann [this message]
2009-09-09 12:19         ` Arnd Bergmann
     [not found]         ` <200909091419.50496.arnd-r2nGTMty4D4@public.gmane.org>
2009-09-09 15:51           ` H. Peter Anvin
2009-09-09 15:51             ` H. Peter Anvin
     [not found]             ` <4AA7CF1F.3020408-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
2009-09-09 18:03               ` Sukadev Bhattiprolu
2009-09-09 18:03             ` Sukadev Bhattiprolu
2009-09-09 18:01               ` H. Peter Anvin
     [not found]               ` <20090909180303.GA21048-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2009-09-09 18:01                 ` H. Peter Anvin
2009-09-09 18:34                 ` Linus Torvalds
2009-09-09 18:34               ` Linus Torvalds
     [not found] ` <20090907211302.GA5892-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2009-09-07 21:13   ` [RFC][v5][PATCH 1/8]: Factor out code to allocate pidmap page Sukadev Bhattiprolu
2009-09-07 21:14   ` [RFC][v5][PATCH 2/8]: Have alloc_pidmap() return actual error code Sukadev Bhattiprolu
2009-09-07 21:14     ` Sukadev Bhattiprolu
2009-09-07 21:14   ` [RFC][v5][PATCH 3/8] Make pid_max a pid_ns property Sukadev Bhattiprolu
2009-09-07 21:15   ` [RFC][v5][PATCH 4/8]: Add target_pid parameter to alloc_pidmap() Sukadev Bhattiprolu
2009-09-07 21:15     ` Sukadev Bhattiprolu
     [not found]     ` <20090907211506.GD6685-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2009-09-08 14:16       ` Serge E. Hallyn
2009-09-08 14:16     ` Serge E. Hallyn
2009-09-07 21:15   ` [RFC][v5][PATCH 5/8]: Add target_pids parameter to alloc_pid() Sukadev Bhattiprolu
2009-09-07 21:15   ` [RFC][v5][PATCH 6/8]: Add target_pids parameter to copy_process() Sukadev Bhattiprolu
2009-09-07 21:15     ` Sukadev Bhattiprolu
2009-09-07 21:16   ` [RFC][v5][PATCH 7/8]: Define do_fork_with_pids() Sukadev Bhattiprolu
2009-09-07 21:17   ` [RFC][v5][PATCH 8/8]: Define clone_with_pids() syscall Sukadev Bhattiprolu

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=200909091419.50496.arnd@arndb.de \
    --to=arnd-r2ngtmty4d4@public.gmane.org \
    --cc=adobriyan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org \
    --cc=hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mingo-X9Un+BFzKDI@public.gmane.org \
    --cc=ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org \
    --cc=sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org \
    --cc=torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=xemul-GEFAQzZX7r8dnm+yROfE0A@public.gmane.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.