Skip to contentSkip to conversation

allocate checkpoint headers with kzalloc

PATCH In review Nathan Lynch 3 participants 4 messages May 13, 2009 – May 14, 2009 2 messages not in the archive

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 allocate checkpoint headers with kzalloc

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.

---
 checkpoint/sys.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
checkpoint/sys.c +1 -1
index 255ebc1..34a226c 100644
@@ -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);
 }
 
 /**
Signature · 1 line
1.6.0.6

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw
Quoted 6 lines · 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.

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

Quoted 29 lines

[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.

---
 checkpoint/sys.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
checkpoint/sys.c +1 -1
index 255ebc1..34a226c 100644
@@ -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);
 }
 
 /**
Signature · 6 lines
1.6.0.6

_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linux-foundation.org/mailman/listinfo/containers

A message that is not in the archive (sent off-list or not yet received): 20090513170257.GA18675-r/[email protected]

#4 lore raw
Quoted 6 lines · Serge E. Hallyn
Quoted 5 lines · 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].

Quoted 7 lines
Quoted 1 line

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.

#5 lore raw
In reply to a message not in the archive

Pushed on ckpt-v15-dev.

Thanks,

Oren.

Quoted 29 lines · 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.

---
 checkpoint/sys.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
checkpoint/sys.c +1 -1
index 255ebc1..34a226c 100644
@@ -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);
 }
 
 /**