cr: fix ckpt_obj_fetch return values

PATCH In review Serge E. Hallyn 1 participant 2 messages May 13, 2009 1 message not in the archive VFS

Share & tools

Serge E. Hallyn

author

2009-05-13 16:52 UTC #0 lore raw

PATCH 1/1 cr: fix ckpt_obj_fetch return values

ckpt_obj_fetch returned ERR_PTR(error) on some failures, NULL on others. Not all of its callers were checking for NULL, which would lead to NULL dereferences.

Return -EINVAL if the object is not in the hash table. Fix up pipe_file_restore to do the right thing.

Signed-off-by Serge E. Hallyn <serue-r/[email protected]>

---
 checkpoint/files.c   |    4 +---
 checkpoint/memory.c  |    6 +-----
 checkpoint/objhash.c |    2 +-
 checkpoint/process.c |    4 +---
 fs/pipe.c            |    9 ++++-----
 5 files changed, 8 insertions(+), 17 deletions(-)

checkpoint/files.c +1 -3

index 22c8bb9..b8b4197 100644
@@ -496,9 +496,7 @@ static int restore_fd_ent(struct ckpt_ctx *ctx)
 		goto out;
 
 	file = ckpt_obj_fetch(ctx, h->fd_objref, CKPT_OBJ_FILE);
-	if (!file)
-		goto out;
-	else if (IS_ERR(file)) {
+	if (IS_ERR(file)) {
 		ret = PTR_ERR(file);
 		goto out;
 	}

checkpoint/memory.c +1 -5

index 92d4485..5f2930f 100644
@@ -1207,8 +1207,6 @@ static struct mm_struct *do_restore_mm(struct ckpt_ctx *ctx)
 	/* restore the ->exe_file */
 	if (h->exefile_objref) {
 		file = ckpt_obj_fetch(ctx, h->exefile_objref, CKPT_OBJ_FILE);
-		if (!file)
-			file = ERR_PTR(-EINVAL);
 		if (IS_ERR(file)) {
 			up_write(&mm->mmap_sem);
 			ret = PTR_ERR(file);

@@ -1246,9 +1244,7 @@ int restore_mm_obj(struct ckpt_ctx *ctx, int mm_objref)
 	int ret;
 
 	mm = ckpt_obj_fetch(ctx, mm_objref, CKPT_OBJ_MM);
-	if (!mm)
-		return -EINVAL;
-	else if (IS_ERR(mm))
+	if (IS_ERR(mm))
 		return -EINVAL;
 
 	if (mm == current->mm)

checkpoint/objhash.c +1 -1

index 0ed7cac..7b26005 100644
@@ -692,7 +692,7 @@ void *ckpt_obj_fetch(struct ckpt_ctx *ctx, int objref, enum obj_type type)
 
 	obj = obj_find_by_objref(ctx, objref);
 	if (!obj)
-		return NULL;
+		ERR_PTR(-EINVAL);
 	ckpt_debug("%s ref %d\n", obj->ops->obj_name, obj->objref);
 	return (obj->ops->obj_type == type ? obj->ptr : ERR_PTR(-EINVAL));
 }

checkpoint/process.c +1 -3

index 63a6c99..79b593d 100644
@@ -1009,9 +1009,7 @@ static int restore_ns_obj(struct ckpt_ctx *ctx, int ns_objref)
 	struct nsproxy *nsproxy;
 
 	nsproxy = ckpt_obj_fetch(ctx, ns_objref, CKPT_OBJ_NS);
-	if (!nsproxy)
-		return -EINVAL;
-	else if (IS_ERR(nsproxy))
+	if (IS_ERR(nsproxy))
 		return PTR_ERR(nsproxy);
 
 	if (nsproxy != task_nsproxy(current))

fs/pipe.c +4 -5

index ab2de3c..b284dcb 100644
@@ -982,14 +982,12 @@ struct file *pipe_file_restore(struct ckpt_ctx *ctx, struct ckpt_hdr_file *ptr)
 		return ERR_PTR(-EINVAL);
 
 	file = ckpt_obj_fetch(ctx, h->pipe_objref, CKPT_OBJ_FILE);
-	if (IS_ERR(file))
-		return file;
 	/*
-	 * If ckpt_obj_fetch() returned NULL, then this is the first
+	 * If ckpt_obj_fetch() returned -EINVAL, then this is the first
 	 * time we see this pipe so need to restore the contents.
 	 * Otherwise, use the file pointer skip forward.
 	 */
-	if (!file) {
+	if (PTR_ERR(file) == -EINVAL) {
 		/* first encounter of this pipe: create it */
 		ret = do_pipe_flags(fds, 0);
 		if (ret < 0)

@@ -1025,7 +1023,8 @@ struct file *pipe_file_restore(struct ckpt_ctx *ctx, struct ckpt_hdr_file *ptr)
 		/* get rid of the file descriptors (caller sets that) */
 		sys_close(fds[which]);
 		sys_close(fds[1-which]);
-	}
+	} else if (IS_ERR(file))
+		return file;
 
 	ret = restore_file_common(ctx, file, ptr);
 	if (ret < 0) {

Signature · 1 line

1.6.1

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

Serge E. Hallyn

author

2009-05-13 18:43 UTC #2 lore raw

Quoted 21 lines · Serge E. Hallyn

ckpt_obj_fetch returned ERR_PTR(error) on some failures, NULL on others. Not all of its callers were checking for NULL, which would lead to NULL dereferences.
Return -EINVAL if the object is not in the hash table. Fix up pipe_file_restore to do the right thing.
Signed-off-by Serge E. Hallyn <serue-r/[email protected]>
---
 checkpoint/files.c   |    4 +---
 checkpoint/memory.c  |    6 +-----
 checkpoint/objhash.c |    2 +-
 checkpoint/process.c |    4 +---
 fs/pipe.c            |    9 ++++-----
 5 files changed, 8 insertions(+), 17 deletions(-)
checkpoint/objhash.c +0 -0
index 0ed7cac..7b26005 100644

checkpoint/objhash.c void *ckpt_obj_fetch(struct ckpt_ctx *ctx, int objref, enum obj_type type) · line 695

Quoted 5 lines

checkpoint/objhash.c +1 -1

@@ -692,7 +692,7 @@ void *ckpt_obj_fetch(struct ckpt_ctx *ctx, int objref, enum obj_type type)
 
 	obj = obj_find_by_objref(ctx, objref);
 	if (!obj)
-		return NULL;
+		ERR_PTR(-EINVAL);

obvious bug here :)

-serge