[PATCH ckpt-v15] allocate checkpoint headers with kzalloc

[PATCH ckpt-v15] allocate checkpoint headers with kzalloc

[Nathan Lynch]

In a checkpoint image I noticed a lot of 0x6b (POISON_FREE) bytes
corresponding to checkpoint_restart_block -- this indicates that we
would write uninitialized kernel memory to the image in cases where
slab allocator debugging is not enabled[1].

Use kzalloc in ckpt_hdr_get.

[1] slub's debug mode apparently initializes allocated buffers to
POISON_FREE instead of POISON_INUSE, which confused me for a bit.
Maybe I don't understand the intended meanings of the poison values.

Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
---
 checkpoint/sys.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/checkpoint/sys.c b/checkpoint/sys.c
index 255ebc1..34a226c 100644
--- a/checkpoint/sys.c
+++ b/checkpoint/sys.c
@@ -117,7 +117,7 @@ int ckpt_kread(struct ckpt_ctx *ctx, void *addr, int count)
  */
 void *ckpt_hdr_get(struct ckpt_ctx *ctx, int len)
 {
-	return kmalloc(len, GFP_KERNEL);
+	return kzalloc(len, GFP_KERNEL);
 }
 
 /**
-- 
1.6.0.6

Re: [PATCH ckpt-v15] allocate checkpoint headers with kzalloc

[Serge E. Hallyn]

Quoting Nathan Lynch (ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org):
> In a checkpoint image I noticed a lot of 0x6b (POISON_FREE) bytes
> corresponding to checkpoint_restart_block -- this indicates that we
> would write uninitialized kernel memory to the image in cases where
> slab allocator debugging is not enabled[1].
> 
> Use kzalloc in ckpt_hdr_get.

Yikes, yes, that's on par with a recent ecryptfs bug that did the
same thing.

Have you audited for other such potential privileged data leaks?
(besides "all sysvipc" which we know about :)

thanks,
-serge

> [1] slub's debug mode apparently initializes allocated buffers to
> POISON_FREE instead of POISON_INUSE, which confused me for a bit.
> Maybe I don't understand the intended meanings of the poison values.
> 
> Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
> ---
>  checkpoint/sys.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/checkpoint/sys.c b/checkpoint/sys.c
> index 255ebc1..34a226c 100644
> --- a/checkpoint/sys.c
> +++ b/checkpoint/sys.c
> @@ -117,7 +117,7 @@ int ckpt_kread(struct ckpt_ctx *ctx, void *addr, int count)
>   */
>  void *ckpt_hdr_get(struct ckpt_ctx *ctx, int len)
>  {
> -	return kmalloc(len, GFP_KERNEL);
> +	return kzalloc(len, GFP_KERNEL);
>  }
> 
>  /**
> -- 
> 1.6.0.6
> 
> _______________________________________________
> Containers mailing list
> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [PATCH ckpt-v15] allocate checkpoint headers with kzalloc

[Nathan Lynch]

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

> Quoting Nathan Lynch (ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org):
>> In a checkpoint image I noticed a lot of 0x6b (POISON_FREE) bytes
>> corresponding to checkpoint_restart_block -- this indicates that we
>> would write uninitialized kernel memory to the image in cases where
>> slab allocator debugging is not enabled[1].
>> 
>> Use kzalloc in ckpt_hdr_get.
>
> Yikes, yes, that's on par with a recent ecryptfs bug that did the
> same thing.
>
> Have you audited for other such potential privileged data leaks?
> (besides "all sysvipc" which we know about :)

No, haven't audited other things; the strings of 0x6b just caught my eye
and this looked like the obvious fix.  As general rule, I'd say the
checkpoint code should be zeroing any intermediate buffers that could be
written to the image.

Re: [PATCH ckpt-v15] allocate checkpoint headers with kzalloc

[Oren Laadan]

Pushed on ckpt-v15-dev.

Thanks,

Oren.


Nathan Lynch wrote:
> In a checkpoint image I noticed a lot of 0x6b (POISON_FREE) bytes
> corresponding to checkpoint_restart_block -- this indicates that we
> would write uninitialized kernel memory to the image in cases where
> slab allocator debugging is not enabled[1].
> 
> Use kzalloc in ckpt_hdr_get.
> 
> [1] slub's debug mode apparently initializes allocated buffers to
> POISON_FREE instead of POISON_INUSE, which confused me for a bit.
> Maybe I don't understand the intended meanings of the poison values.
> 
> Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
> ---
>  checkpoint/sys.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/checkpoint/sys.c b/checkpoint/sys.c
> index 255ebc1..34a226c 100644
> --- a/checkpoint/sys.c
> +++ b/checkpoint/sys.c
> @@ -117,7 +117,7 @@ int ckpt_kread(struct ckpt_ctx *ctx, void *addr, int count)
>   */
>  void *ckpt_hdr_get(struct ckpt_ctx *ctx, int len)
>  {
> -	return kmalloc(len, GFP_KERNEL);
> +	return kzalloc(len, GFP_KERNEL);
>  }
>  
>  /**