From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cedric Le Goater Subject: Re: External checkpoint patch Date: Thu, 23 Oct 2008 09:51:13 +0200 Message-ID: <49002CF1.3070505@fr.ibm.com> References: <1224612971.1848.193.camel@nimitz> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Oren Laadan Cc: containers , Dave Hansen List-Id: containers.vger.kernel.org minor issue, > -struct cr_ctx *cr_ctx_alloc(pid_t pid, int fd, unsigned long flags) > +static struct cr_ctx *cr_ctx_alloc(pid_t pid, int fd, unsigned long flags) > { > struct cr_ctx *ctx; > + int err; > > ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); > if (!ctx) > return ERR_PTR(-ENOMEM); with the err goto, you're calling cr_ctx_free() in a few places where ctx is partially initialized. you should at least do : INIT_LIST_HEAD(&ctx->pgarr_list); right after the kzalloc() and probably check the value of ctx->hbuf. There might be other issues. > + ctx->flags = flags; > + ctx->pid = pid; > + > + err = -EBADF; > ctx->file = fget(fd); > - if (!ctx->file) { > - cr_ctx_free(ctx); > - return ERR_PTR(-EBADF); > - } > + if (!ctx->file) > + goto err; > > > + err = -ENOMEM; > ctx->hbuf = kmalloc(CR_HBUF_TOTAL, GFP_KERNEL); > - if (!ctx->hbuf) { > - cr_ctx_free(ctx); > - return ERR_PTR(-ENOMEM); > - } > + if (!ctx->hbuf) > + goto err; > > - if (cr_objhash_alloc(ctx) < 0) { > - cr_ctx_free(ctx); > - return ERR_PTR(-ENOMEM); > - } > + if (cr_objhash_alloc(ctx) < 0) > + goto err; > > /* > * assume checkpointer is in container's root vfs > @@ -206,12 +206,13 @@ struct cr_ctx *cr_ctx_alloc(pid_t pid, int fd, unsigned long flags) > > INIT_LIST_HEAD(&ctx->pgarr_list); > > - ctx->pid = pid; > - ctx->flags = flags; > - > ctx->crid = atomic_inc_return(&cr_ctx_count); > > return ctx; > + > + err: > + cr_ctx_free(ctx); > + return ERR_PTR(err); > } > > /** here's a patch for it. C. Signed-off-by: Cedric Le Goater --- checkpoint/sys.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Index: 2.6.27-lxc/checkpoint/sys.c =================================================================== --- 2.6.27-lxc.orig/checkpoint/sys.c +++ 2.6.27-lxc/checkpoint/sys.c @@ -217,7 +217,8 @@ static void cr_ctx_free(struct cr_ctx *c if (ctx->file) fput(ctx->file); - kfree(ctx->hbuf); + if (ctx->hbuf) + kfree(ctx->hbuf); if (ctx->vfsroot) path_put(ctx->vfsroot); @@ -239,6 +240,7 @@ static struct cr_ctx *cr_ctx_alloc(pid_t if (!ctx) return ERR_PTR(-ENOMEM); + INIT_LIST_HEAD(&ctx->pgarr_list); ctx->flags = flags; err = -EBADF; @@ -265,8 +267,6 @@ static struct cr_ctx *cr_ctx_alloc(pid_t ctx->vfsroot = &ctx->root_task->fs->root; path_get(ctx->vfsroot); - INIT_LIST_HEAD(&ctx->pgarr_list); - ctx->crid = atomic_inc_return(&cr_ctx_count); return ctx;