[PATCH 0/9] ckpt-v19-rc3 cleanups

[PATCH 0/9] ckpt-v19-rc3 cleanups

[serue-r/Jw6+rmf7HQT0dZR+AlfA]

Sorry, some of these I've sent before, but here is the set I currently
have on top of ckpt-v19-rc3.  Boy is this painful - there are at least
still 32->64 or vice-versa problems with: ipc_shm restore, signals
(I think) and, in 32-bit restore of a x86-64 32-bit image, restore_cpu.
I'll keep pushing at this tomorrow.

[PATCH 1/9] Use CKPT_ARCH_X86_32 for all x86 CKPT_ARCH_IDs

[serue-r/Jw6+rmf7HQT0dZR+AlfA]

From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Sure we can rename it eventually, but I'm still just experimenting...

Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 arch/x86/include/asm/checkpoint_hdr.h |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/checkpoint_hdr.h b/arch/x86/include/asm/checkpoint_hdr.h
index 535a9c6..a1edeb6 100644
--- a/arch/x86/include/asm/checkpoint_hdr.h
+++ b/arch/x86/include/asm/checkpoint_hdr.h
@@ -36,13 +36,7 @@
 #include <asm/processor.h>
 #endif
 
-#ifdef CONFIG_X86_64
-#define CKPT_ARCH_ID	CKPT_ARCH_X86_64
-#endif
-
-#ifdef CONFIG_X86_32
 #define CKPT_ARCH_ID	CKPT_ARCH_X86_32
-#endif
 
 /* arch dependent header types */
 enum {
-- 
1.6.0.4

Re: [PATCH 1/9] Use CKPT_ARCH_X86_32 for all x86 CKPT_ARCH_IDs

[Serge E. Hallyn]

Quoting serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org (serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org):
> From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> 
> Sure we can rename it eventually, but I'm still just experimenting...

Let me elaborate on that before someone calls me on it:

I think the rest of the patches in this set should be applied.  This one should
not be applied until we get x86_32 -> x86_64 32-bit task c/r working.

> Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/x86/include/asm/checkpoint_hdr.h |    6 ------
>  1 files changed, 0 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/include/asm/checkpoint_hdr.h b/arch/x86/include/asm/checkpoint_hdr.h
> index 535a9c6..a1edeb6 100644
> --- a/arch/x86/include/asm/checkpoint_hdr.h
> +++ b/arch/x86/include/asm/checkpoint_hdr.h
> @@ -36,13 +36,7 @@
>  #include <asm/processor.h>
>  #endif
> 
> -#ifdef CONFIG_X86_64
> -#define CKPT_ARCH_ID	CKPT_ARCH_X86_64
> -#endif
> -
> -#ifdef CONFIG_X86_32
>  #define CKPT_ARCH_ID	CKPT_ARCH_X86_32
> -#endif
> 
>  /* arch dependent header types */
>  enum {
> -- 
> 1.6.0.4

Re: [PATCH 1/9] Use CKPT_ARCH_X86_32 for all x86 CKPT_ARCH_IDs

[Oren Laadan]

serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
> From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> 
> Sure we can rename it eventually, but I'm still just experimenting...
> 
> Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/x86/include/asm/checkpoint_hdr.h |    6 ------
>  1 files changed, 0 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/include/asm/checkpoint_hdr.h b/arch/x86/include/asm/checkpoint_hdr.h
> index 535a9c6..a1edeb6 100644
> --- a/arch/x86/include/asm/checkpoint_hdr.h
> +++ b/arch/x86/include/asm/checkpoint_hdr.h
> @@ -36,13 +36,7 @@
>  #include <asm/processor.h>
>  #endif
>  
> -#ifdef CONFIG_X86_64
> -#define CKPT_ARCH_ID	CKPT_ARCH_X86_64
> -#endif
> -
> -#ifdef CONFIG_X86_32
>  #define CKPT_ARCH_ID	CKPT_ARCH_X86_32
> -#endif

In the final version you probably want to get rid of "_32" too...

Oren.

[PATCH 2/9] restart only same bit-ness

[serue-r/Jw6+rmf7HQT0dZR+AlfA]

From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

    Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 arch/x86/kernel/checkpoint.c |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/checkpoint.c b/arch/x86/kernel/checkpoint.c
index 5952e27..b01a2fc 100644
--- a/arch/x86/kernel/checkpoint.c
+++ b/arch/x86/kernel/checkpoint.c
@@ -283,12 +283,21 @@ int restore_thread(struct ckpt_ctx *ctx)
 	load_TLS(thread, cpu);
 	put_cpu();
 
-#if defined(CONFIG_X86_64) && defined(CONFIG_COMPAT)
-	if (h->thread_info_flags & _TIF_IA32)
-		set_thread_flag(TIF_IA32);
-	else
-		clear_thread_flag(TIF_IA32);
-#endif
+	{
+		int pre, post;
+		/*
+		 * Eventually we'd like to support mixed-bit restart, but for
+		 * now don't pretend to.
+		 */
+		pre = test_thread_flag(TIF_IA32);
+		post = h->thread_info_flags & _TIF_IA32;
+		if ((pre && !post) || (post && !pre)) {
+			ret = -EINVAL;
+			ckpt_err(ctx, ret, "%d-bit restarting %d-bit\n",
+				pre ? 32 : 64, post ? 32 : 64);
+			goto out;
+		}
+	}
 
 	/* TODO: restore TIF flags as necessary (e.g. TIF_NOTSC) */
 
-- 
1.6.0.4

[PATCH 3/9] x86_64: keep __u32s in even groups

[serue-r/Jw6+rmf7HQT0dZR+AlfA]

From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Otherwise the struct will be different size on x86-32 and x86-64.

Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 include/linux/checkpoint_hdr.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
index e591fd1..5a68178 100644
--- a/include/linux/checkpoint_hdr.h
+++ b/include/linux/checkpoint_hdr.h
@@ -345,6 +345,9 @@ struct ckpt_hdr_task {
 	__u32 exit_code;
 	__u32 exit_signal;
 	__u32 pdeath_signal;
+	__u32 compat_robust_futex_head_len;
+	__u32 compat_robust_futex_list; /* a compat __user ptr */
+	__u32 robust_futex_head_len;
 
 #ifdef CONFIG_AUDITSYSCALL
 	/* would audit want to track the checkpointed ids,
@@ -354,9 +357,6 @@ struct ckpt_hdr_task {
 	__u64 set_child_tid;
 	__u64 clear_child_tid;
 
-	__u32 compat_robust_futex_head_len;
-	__u32 compat_robust_futex_list; /* a compat __user ptr */
-	__u32 robust_futex_head_len;
 	__u64 robust_futex_list; /* a __user ptr */
 } __attribute__((aligned(8)));
 
-- 
1.6.0.4

[PATCH 4/9] x86_32: add TIF_IA32 to thread flags

[serue-r/Jw6+rmf7HQT0dZR+AlfA]

From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

X86-32 does not set TIF_IA32 on tasks.  But since we now use the
same CKPT_ARCH_ID for X86-32 and X86-64, this means that when we
restart a task from X86-32 on X86-64, we can't tell that it should
have been 32-bit.

So always set TIF_IA32 in thread_info_flags for a task when
checkpointing on X86-32.

Likewise, at restart, when comparing the current task (the running
restart binary) to the checkpointed one (to be restarted) to make
sure both are either 32-bit or 64-bit, we obviously can't count
on TIF_IA32 being set in current_flags on X86-32.

Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 arch/x86/kernel/checkpoint.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/checkpoint.c b/arch/x86/kernel/checkpoint.c
index b01a2fc..961b131 100644
--- a/arch/x86/kernel/checkpoint.c
+++ b/arch/x86/kernel/checkpoint.c
@@ -103,6 +103,9 @@ int checkpoint_thread(struct ckpt_ctx *ctx, struct task_struct *t)
 
 	h->thread_info_flags =
 		task_thread_info(t)->flags & ~CKPT_X86_TIF_UNSUPPORTED;
+#ifndef CONFIG_X86_64
+	h->thread_info_flags |= _TIF_IA32;
+#endif
 	h->gdt_entry_tls_entries = GDT_ENTRY_TLS_ENTRIES;
 	h->sizeof_tls_array = tls_size;
 
@@ -289,7 +292,11 @@ int restore_thread(struct ckpt_ctx *ctx)
 		 * Eventually we'd like to support mixed-bit restart, but for
 		 * now don't pretend to.
 		 */
+#ifndef CONFIG_X86_64
+		pre = 1;
+#else
 		pre = test_thread_flag(TIF_IA32);
+#endif
 		post = h->thread_info_flags & _TIF_IA32;
 		if ((pre && !post) || (post && !pre)) {
 			ret = -EINVAL;
-- 
1.6.0.4

[PATCH 5/9] use ckpt_err for architecture mismatch errors

[serue-r/Jw6+rmf7HQT0dZR+AlfA]

From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 arch/x86/kernel/checkpoint.c |    4 +++-
 checkpoint/restart.c         |   18 ++++++++++++++----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/checkpoint.c b/arch/x86/kernel/checkpoint.c
index 961b131..7c73983 100644
--- a/arch/x86/kernel/checkpoint.c
+++ b/arch/x86/kernel/checkpoint.c
@@ -465,8 +465,10 @@ int restore_read_header_arch(struct ckpt_ctx *ctx)
 	/* verify FPU capabilities */
 	if (h->has_fxsr != cpu_has_fxsr ||
 	    h->has_xsave != cpu_has_xsave ||
-	    h->xstate_size != xstate_size)
+	    h->xstate_size != xstate_size) {
 		ret = -EINVAL;
+		ckpt_err(ctx, ret, "incompatible FPU capabilities");
+	}
 
 	ckpt_hdr_put(ctx, h);
 	return ret;
diff --git a/checkpoint/restart.c b/checkpoint/restart.c
index fcd07fa..9bb17fc 100644
--- a/checkpoint/restart.c
+++ b/checkpoint/restart.c
@@ -569,12 +569,14 @@ static int check_kernel_const(struct ckpt_const *h)
 		return -EINVAL;
 	if (h->uts_nodename_len != sizeof(uts->nodename))
 		return -EINVAL;
+#if 0
 	if (h->uts_release_len != sizeof(uts->release))
 		return -EINVAL;
 	if (h->uts_version_len != sizeof(uts->version))
 		return -EINVAL;
 	if (h->uts_machine_len != sizeof(uts->machine))
 		return -EINVAL;
+#endif
 	if (h->uts_domainname_len != sizeof(uts->domainname))
 		return -EINVAL;
 	/* rlimit */
@@ -601,20 +603,28 @@ static int restore_read_header(struct ckpt_ctx *ctx)
 		return PTR_ERR(h);
 
 	ret = -EINVAL;
-	if (le16_to_cpu(h->arch_id) != CKPT_ARCH_ID)
+	if (le16_to_cpu(h->arch_id) != CKPT_ARCH_ID) {
+		ckpt_err(ctx, ret, "incompatible ARCH_ID");
 		goto out;
+	}
 	if (h->magic != CHECKPOINT_MAGIC_HEAD ||
 	    h->rev != CHECKPOINT_VERSION ||
 	    h->major != ((LINUX_VERSION_CODE >> 16) & 0xff) ||
 	    h->minor != ((LINUX_VERSION_CODE >> 8) & 0xff) ||
-	    h->patch != ((LINUX_VERSION_CODE) & 0xff))
+	    h->patch != ((LINUX_VERSION_CODE) & 0xff)) {
+		ckpt_err(ctx, ret, "incompatible VERSION");
 		goto out;
-	if (h->uflags & ~CHECKPOINT_USER_FLAGS)
+	}
+	if (h->uflags & ~CHECKPOINT_USER_FLAGS) {
+		ckpt_err(ctx, ret, "bad restart user flags");
 		goto out;
+	}
 
 	ret = check_kernel_const(&h->constants);
-	if (ret < 0)
+	if (ret < 0) {
+		ckpt_err(ctx, ret, "incompatible kernel constants");
 		goto out;
+	}
 
 	ret = -ENOMEM;
 	uts = kmalloc(sizeof(*uts), GFP_KERNEL);
-- 
1.6.0.4

Re: [PATCH 5/9] use ckpt_err for architecture mismatch errors

[Oren Laadan]

serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
> From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> 
> Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/x86/kernel/checkpoint.c |    4 +++-
>  checkpoint/restart.c         |   18 ++++++++++++++----
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 

[...]

> diff --git a/checkpoint/restart.c b/checkpoint/restart.c
> index fcd07fa..9bb17fc 100644
> --- a/checkpoint/restart.c
> +++ b/checkpoint/restart.c
> @@ -569,12 +569,14 @@ static int check_kernel_const(struct ckpt_const *h)
>  		return -EINVAL;
>  	if (h->uts_nodename_len != sizeof(uts->nodename))
>  		return -EINVAL;
> +#if 0
>  	if (h->uts_release_len != sizeof(uts->release))
>  		return -EINVAL;
>  	if (h->uts_version_len != sizeof(uts->version))
>  		return -EINVAL;
>  	if (h->uts_machine_len != sizeof(uts->machine))
>  		return -EINVAL;
> +#endif

What is the purpose of this ?  I thought these sizes do not
depends on the kernel bit-ness.

>  	if (h->uts_domainname_len != sizeof(uts->domainname))
>  		return -EINVAL;
>  	/* rlimit */

[...]

Oren.

Re: [PATCH 5/9] use ckpt_err for architecture mismatch errors

[Serge E. Hallyn]

Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
> 
> 
> serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
> > From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> > 
> > Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> > ---
> >  arch/x86/kernel/checkpoint.c |    4 +++-
> >  checkpoint/restart.c         |   18 ++++++++++++++----
> >  2 files changed, 17 insertions(+), 5 deletions(-)
> > 
> 
> [...]
> 
> > diff --git a/checkpoint/restart.c b/checkpoint/restart.c
> > index fcd07fa..9bb17fc 100644
> > --- a/checkpoint/restart.c
> > +++ b/checkpoint/restart.c
> > @@ -569,12 +569,14 @@ static int check_kernel_const(struct ckpt_const *h)
> >  		return -EINVAL;
> >  	if (h->uts_nodename_len != sizeof(uts->nodename))
> >  		return -EINVAL;
> > +#if 0
> >  	if (h->uts_release_len != sizeof(uts->release))
> >  		return -EINVAL;
> >  	if (h->uts_version_len != sizeof(uts->version))
> >  		return -EINVAL;
> >  	if (h->uts_machine_len != sizeof(uts->machine))
> >  		return -EINVAL;
> > +#endif
> 
> What is the purpose of this ?  I thought these sizes do not
> depends on the kernel bit-ness.

Jinkeys - yeah that shouldn't be there.

> >  	if (h->uts_domainname_len != sizeof(uts->domainname))
> >  		return -EINVAL;
> >  	/* rlimit */
> 
> [...]
> 
> Oren.
> 
> _______________________________________________
> Containers mailing list
> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linux-foundation.org/mailman/listinfo/containers

[PATCH 6/9] Make AT_VECTOR_SIZE_ARCH 2 for x86-32

[serue-r/Jw6+rmf7HQT0dZR+AlfA]

From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Both x86-32 and x86-64 with 32-bit compat use ARCH_DLINFO_IA32,
which defines two saved_auxv entries.  But system.h only defines
AT_VECTOR_SIZE_ARCH as 2 for CONFIG_IA32_EMULATION, not for
CONFIG_X86_32.  Fix that.

Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 arch/x86/include/asm/system.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/system.h b/arch/x86/include/asm/system.h
index ecb544e..e04740f 100644
--- a/arch/x86/include/asm/system.h
+++ b/arch/x86/include/asm/system.h
@@ -11,9 +11,9 @@
 #include <linux/irqflags.h>
 
 /* entries in ARCH_DLINFO: */
-#ifdef CONFIG_IA32_EMULATION
+#if defined(CONFIG_IA32_EMULATION) || !defined(CONFIG_X86_64)
 # define AT_VECTOR_SIZE_ARCH 2
-#else
+#else /* else it's non-compat x86-64 */
 # define AT_VECTOR_SIZE_ARCH 1
 #endif
 
-- 
1.6.0.4

Re: [PATCH 6/9] Make AT_VECTOR_SIZE_ARCH 2 for x86-32

[Oren Laadan]

Are you going to send this to mainline ?

serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
> From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> 
> Both x86-32 and x86-64 with 32-bit compat use ARCH_DLINFO_IA32,
> which defines two saved_auxv entries.  But system.h only defines
> AT_VECTOR_SIZE_ARCH as 2 for CONFIG_IA32_EMULATION, not for
> CONFIG_X86_32.  Fix that.
> 
> Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/x86/include/asm/system.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/system.h b/arch/x86/include/asm/system.h
> index ecb544e..e04740f 100644
> --- a/arch/x86/include/asm/system.h
> +++ b/arch/x86/include/asm/system.h
> @@ -11,9 +11,9 @@
>  #include <linux/irqflags.h>
>  
>  /* entries in ARCH_DLINFO: */
> -#ifdef CONFIG_IA32_EMULATION
> +#if defined(CONFIG_IA32_EMULATION) || !defined(CONFIG_X86_64)
>  # define AT_VECTOR_SIZE_ARCH 2
> -#else
> +#else /* else it's non-compat x86-64 */
>  # define AT_VECTOR_SIZE_ARCH 1
>  #endif
>  

Re: [PATCH 6/9] Make AT_VECTOR_SIZE_ARCH 2 for x86-32

[Serge E. Hallyn]

Sent it a few hours ago.

http://lkml.org/lkml/2010/2/8/388


Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
> 
> Are you going to send this to mainline ?
> 
> serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
> > From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> > 
> > Both x86-32 and x86-64 with 32-bit compat use ARCH_DLINFO_IA32,
> > which defines two saved_auxv entries.  But system.h only defines
> > AT_VECTOR_SIZE_ARCH as 2 for CONFIG_IA32_EMULATION, not for
> > CONFIG_X86_32.  Fix that.
> > 
> > Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> > ---
> >  arch/x86/include/asm/system.h |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/system.h b/arch/x86/include/asm/system.h
> > index ecb544e..e04740f 100644
> > --- a/arch/x86/include/asm/system.h
> > +++ b/arch/x86/include/asm/system.h
> > @@ -11,9 +11,9 @@
> >  #include <linux/irqflags.h>
> >  
> >  /* entries in ARCH_DLINFO: */
> > -#ifdef CONFIG_IA32_EMULATION
> > +#if defined(CONFIG_IA32_EMULATION) || !defined(CONFIG_X86_64)
> >  # define AT_VECTOR_SIZE_ARCH 2
> > -#else
> > +#else /* else it's non-compat x86-64 */
> >  # define AT_VECTOR_SIZE_ARCH 1
> >  #endif
> >  
> _______________________________________________
> Containers mailing list
> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linux-foundation.org/mailman/listinfo/containers

[PATCH 7/9] cr: checkpoint saved_auxv as u64s

[serue-r/Jw6+rmf7HQT0dZR+AlfA]

From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

unsigned longs are not a good value to checkpoint between
x86-32 and x86-64 32-bit tasks :)

Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 checkpoint/checkpoint.c        |    5 +--
 checkpoint/memory.c            |   53 +++++++++++++++++++++++++++++++++++++--
 checkpoint/restart.c           |    6 ++--
 include/linux/checkpoint_hdr.h |    2 +-
 4 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/checkpoint/checkpoint.c b/checkpoint/checkpoint.c
index b4e0021..b3c1c4f 100644
--- a/checkpoint/checkpoint.c
+++ b/checkpoint/checkpoint.c
@@ -103,13 +103,12 @@ int ckpt_write_string(struct ckpt_ctx *ctx, char *str, int len)
 static void fill_kernel_const(struct ckpt_const *h)
 {
 	struct task_struct *tsk;
-	struct mm_struct *mm;
 	struct new_utsname *uts;
 
 	/* task */
 	h->task_comm_len = sizeof(tsk->comm);
-	/* mm */
-	h->mm_saved_auxv_len = sizeof(mm->saved_auxv);
+	/* mm->saved_auxv size */
+	h->at_vector_size = AT_VECTOR_SIZE;
 	/* signal */
 	h->signal_nsig = _NSIG;
 	/* uts */
diff --git a/checkpoint/memory.c b/checkpoint/memory.c
index d51f94b..5058ab3 100644
--- a/checkpoint/memory.c
+++ b/checkpoint/memory.c
@@ -661,6 +661,25 @@ static int checkpoint_vmas(struct ckpt_ctx *ctx, struct mm_struct *mm)
 	return ret < 0 ? ret : map_count;
 }
 
+#define CKPT_AT_SZ (AT_VECTOR_SIZE * sizeof(u64))
+/*
+ * We always write saved_auxv out as an array of u64s, though it is
+ * an array of u32s on 32-bit arch.
+ */
+static int ckpt_write_auxv(struct ckpt_ctx *ctx, struct mm_struct *mm)
+{
+	int i, ret;
+	u64 *buf = kzalloc(CKPT_AT_SZ, GFP_KERNEL);
+
+	if (!buf)
+		return -ENOMEM;
+	for (i=0; i<AT_VECTOR_SIZE; i++)
+		buf[i] = mm->saved_auxv[i];
+	ret = ckpt_write_buffer(ctx, buf, CKPT_AT_SZ);
+	kfree(buf);
+	return ret;
+}
+
 static int do_checkpoint_mm(struct ckpt_ctx *ctx, struct mm_struct *mm)
 {
 	struct ckpt_hdr_mm *h;
@@ -718,7 +737,7 @@ static int do_checkpoint_mm(struct ckpt_ctx *ctx, struct mm_struct *mm)
 	if (ret < 0)
 		goto out;
 
-	ret = ckpt_write_buffer(ctx, mm->saved_auxv, sizeof(mm->saved_auxv));
+	ret = ckpt_write_auxv(ctx, mm);
 	if (ret < 0)
 		return ret;
 
@@ -1205,6 +1224,32 @@ static int restore_vma(struct ckpt_ctx *ctx, struct mm_struct *mm)
 	return ret;
 }
 
+static int ckpt_read_auxv(struct ckpt_ctx *ctx, struct mm_struct *mm)
+{
+	int i, ret;
+	u64 *buf = kmalloc(CKPT_AT_SZ, GFP_KERNEL);
+
+	if (!buf)
+		return -ENOMEM;
+	ret = _ckpt_read_buffer(ctx, buf, CKPT_AT_SZ);
+	if (ret < 0) {
+		kfree(buf);
+		return ret;
+	}
+
+	for (i=0; i<AT_VECTOR_SIZE; i++)
+		if (buf[i] > (u64) ULONG_MAX) {
+			kfree(buf);
+			return -E2BIG;
+		}
+
+	for (i=0; i<AT_VECTOR_SIZE; i++)
+		mm->saved_auxv[i] = buf[i];
+
+	kfree(buf);
+	return 0;
+}
+
 static struct mm_struct *do_restore_mm(struct ckpt_ctx *ctx)
 {
 	struct ckpt_hdr_mm *h;
@@ -1270,9 +1315,11 @@ static struct mm_struct *do_restore_mm(struct ckpt_ctx *ctx)
 	}
 	up_write(&mm->mmap_sem);
 
-	ret = _ckpt_read_buffer(ctx, mm->saved_auxv, sizeof(mm->saved_auxv));
-	if (ret < 0)
+	ret = ckpt_read_auxv(ctx, mm);
+	if (ret < 0) {
+		ckpt_err(ctx, ret, "Error restoring auxv\n");
 		goto out;
+	}
 
 	for (nr = h->map_count; nr; nr--) {
 		ret = restore_vma(ctx, mm);
diff --git a/checkpoint/restart.c b/checkpoint/restart.c
index 9bb17fc..a5f30f2 100644
--- a/checkpoint/restart.c
+++ b/checkpoint/restart.c
@@ -552,15 +552,15 @@ int ckpt_read_consume(struct ckpt_ctx *ctx, int len, int type)
 static int check_kernel_const(struct ckpt_const *h)
 {
 	struct task_struct *tsk;
-	struct mm_struct *mm;
 	struct new_utsname *uts;
 
 	/* task */
 	if (h->task_comm_len != sizeof(tsk->comm))
 		return -EINVAL;
-	/* mm */
-	if (h->mm_saved_auxv_len != sizeof(mm->saved_auxv))
+	/* mm->saved_auxv size */
+	if (h->at_vector_size != AT_VECTOR_SIZE)
 		return -EINVAL;
+
 	/* signal */
 	if (h->signal_nsig != _NSIG)
 		return -EINVAL;
diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
index 5a68178..b5dd95c 100644
--- a/include/linux/checkpoint_hdr.h
+++ b/include/linux/checkpoint_hdr.h
@@ -262,7 +262,7 @@ struct ckpt_const {
 	/* task */
 	__u16 task_comm_len;
 	/* mm */
-	__u16 mm_saved_auxv_len;
+	__u16 at_vector_size;
 	/* signal */
 	__u16 signal_nsig;
 	/* uts */
-- 
1.6.0.4

Re: [PATCH 7/9] cr: checkpoint saved_auxv as u64s

[Oren Laadan]

serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
> From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> 
> unsigned longs are not a good value to checkpoint between
> x86-32 and x86-64 32-bit tasks :)
> 
> Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> ---
>  checkpoint/checkpoint.c        |    5 +--
>  checkpoint/memory.c            |   53 +++++++++++++++++++++++++++++++++++++--
>  checkpoint/restart.c           |    6 ++--
>  include/linux/checkpoint_hdr.h |    2 +-
>  4 files changed, 56 insertions(+), 10 deletions(-)

[...]

Sketch for a sanity check:

> +static int ckpt_read_auxv(struct ckpt_ctx *ctx, struct mm_struct *mm)
> +{
> +	int i, ret;
> +	u64 *buf = kmalloc(CKPT_AT_SZ, GFP_KERNEL);
> +
> +	if (!buf)
> +		return -ENOMEM;
> +	ret = _ckpt_read_buffer(ctx, buf, CKPT_AT_SZ);
> +	if (ret < 0) {
> +		kfree(buf);
> +		return ret;
> +	}
> +

	ret = -E2BIG;
> +	for (i=0; i<AT_VECTOR_SIZE; i++)
> +		if (buf[i] > (u64) ULONG_MAX)
			goto out;

	ret = -EINVAL;
	for (i=0; i<AT_VECTOR_SIZE; i++)
		if (mm->saved_auxv[i] == AT_NULL)
			ret = 0;
	if (ret < 0)
		goto out;

> +
> +	for (i=0; i<AT_VECTOR_SIZE; i++)
> +		mm->saved_auxv[i] = buf[i];
> +
 out:
> +	kfree(buf);
> +	return 0;
	       ret;

[...]

Oren.

Re: [PATCH 7/9] cr: checkpoint saved_auxv as u64s

[Serge E. Hallyn]

Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
> 
> 
> serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
> > From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> > 
> > unsigned longs are not a good value to checkpoint between
> > x86-32 and x86-64 32-bit tasks :)
> > 
> > Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> > ---
> >  checkpoint/checkpoint.c        |    5 +--
> >  checkpoint/memory.c            |   53 +++++++++++++++++++++++++++++++++++++--
> >  checkpoint/restart.c           |    6 ++--
> >  include/linux/checkpoint_hdr.h |    2 +-
> >  4 files changed, 56 insertions(+), 10 deletions(-)
> 
> [...]
> 
> Sketch for a sanity check:
> 
> > +static int ckpt_read_auxv(struct ckpt_ctx *ctx, struct mm_struct *mm)
> > +{
> > +	int i, ret;
> > +	u64 *buf = kmalloc(CKPT_AT_SZ, GFP_KERNEL);
> > +
> > +	if (!buf)
> > +		return -ENOMEM;
> > +	ret = _ckpt_read_buffer(ctx, buf, CKPT_AT_SZ);
> > +	if (ret < 0) {
> > +		kfree(buf);
> > +		return ret;
> > +	}
> > +
> 
> 	ret = -E2BIG;
> > +	for (i=0; i<AT_VECTOR_SIZE; i++)
> > +		if (buf[i] > (u64) ULONG_MAX)
> 			goto out;
> 
> 	ret = -EINVAL;
> 	for (i=0; i<AT_VECTOR_SIZE; i++)
> 		if (mm->saved_auxv[i] == AT_NULL)
> 			ret = 0;
> 	if (ret < 0)
> 		goto out;

Yup, that would work.

Alternatively, do you think it would be safe to just
always set the last entry to AT_NULL?

> > +
> > +	for (i=0; i<AT_VECTOR_SIZE; i++)
> > +		mm->saved_auxv[i] = buf[i];
> > +
>  out:
> > +	kfree(buf);
> > +	return 0;
> 	       ret;
> 
> [...]
> 
> Oren.

Re: [PATCH 7/9] cr: checkpoint saved_auxv as u64s

[Oren Laadan]

Serge E. Hallyn wrote:
> Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
>>
>> serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
>>> From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>>>
>>> unsigned longs are not a good value to checkpoint between
>>> x86-32 and x86-64 32-bit tasks :)
>>>
>>> Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  checkpoint/checkpoint.c        |    5 +--
>>>  checkpoint/memory.c            |   53 +++++++++++++++++++++++++++++++++++++--
>>>  checkpoint/restart.c           |    6 ++--
>>>  include/linux/checkpoint_hdr.h |    2 +-
>>>  4 files changed, 56 insertions(+), 10 deletions(-)
>> [...]
>>
>> Sketch for a sanity check:
>>
>>> +static int ckpt_read_auxv(struct ckpt_ctx *ctx, struct mm_struct *mm)
>>> +{
>>> +	int i, ret;
>>> +	u64 *buf = kmalloc(CKPT_AT_SZ, GFP_KERNEL);
>>> +
>>> +	if (!buf)
>>> +		return -ENOMEM;
>>> +	ret = _ckpt_read_buffer(ctx, buf, CKPT_AT_SZ);
>>> +	if (ret < 0) {
>>> +		kfree(buf);
>>> +		return ret;
>>> +	}
>>> +
>> 	ret = -E2BIG;
>>> +	for (i=0; i<AT_VECTOR_SIZE; i++)
>>> +		if (buf[i] > (u64) ULONG_MAX)
>> 			goto out;
>>
>> 	ret = -EINVAL;
>> 	for (i=0; i<AT_VECTOR_SIZE; i++)
>> 		if (mm->saved_auxv[i] == AT_NULL)
>> 			ret = 0;
>> 	if (ret < 0)
>> 		goto out;
> 
> Yup, that would work.
> 
> Alternatively, do you think it would be safe to just
> always set the last entry to AT_NULL?

A bit hacky, but should work.
... so I'll do just that :)

Oren.

> 
>>> +
>>> +	for (i=0; i<AT_VECTOR_SIZE; i++)
>>> +		mm->saved_auxv[i] = buf[i];
>>> +
>>  out:
>>> +	kfree(buf);
>>> +	return 0;
>> 	       ret;
>>
>> [...]
>>
>> Oren.
> 

[PATCH 8/9] fix potential use-before-set ret in arch/x86/vdso/vma.c

[serue-r/Jw6+rmf7HQT0dZR+AlfA]

From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

(should be folded into
	"c/r: extend arch_setup_additional_pages()"
)

Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 arch/x86/vdso/vma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/vdso/vma.c b/arch/x86/vdso/vma.c
index 393b22a..b10ed32 100644
--- a/arch/x86/vdso/vma.c
+++ b/arch/x86/vdso/vma.c
@@ -104,7 +104,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
 {
 	struct mm_struct *mm = current->mm;
 	unsigned long addr;
-	int ret;
+	int ret = -EINVAL;
 
 	if (!vdso_enabled)
 		return 0;
-- 
1.6.0.4

[PATCH 9/9] mm/mmap.c:do_munmap(): remove unused local vars

[serue-r/Jw6+rmf7HQT0dZR+AlfA]

From: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 mm/mmap.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 89c3d9c..75f5de3 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2014,9 +2014,6 @@ int do_munmap_nocheck(struct mm_struct *mm, unsigned long start, size_t len)
 
 int do_munmap(struct mm_struct *mm, unsigned long start, size_t len)
 {
-	unsigned long end;
-	struct vm_area_struct *vma, *prev, *last;
-
 	if ((start & ~PAGE_MASK) || start > TASK_SIZE || len > TASK_SIZE-start)
 		return -EINVAL;
 
-- 
1.6.0.4

Re: [PATCH 0/9] ckpt-v19-rc3 cleanups

[Oren Laadan]

I applied patches 2,4,5,7,8,9.

Patches 1,3,6 are related to mixed bit-ness restart -> for v20.

Thanks,

Oren.

serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
> Sorry, some of these I've sent before, but here is the set I currently
> have on top of ckpt-v19-rc3.  Boy is this painful - there are at least
> still 32->64 or vice-versa problems with: ipc_shm restore, signals
> (I think) and, in 32-bit restore of a x86-64 32-bit image, restore_cpu.
> I'll keep pushing at this tomorrow.
> 
>