Re: [Xen-devel] [PATCH 1/1] xen/gntdev: kmalloc structure gntdev_copy_batch

From: Jan Beulich
Date: Mon May 09 2016 - 02:06:40 EST


>>> Heinrich Schuchardt <xypron.glpk@xxxxxx> 05/08/16 8:13 AM >>>
>--- a/drivers/xen/gntdev.c
>+++ b/drivers/xen/gntdev.c
>@@ -915,36 +915,43 @@ static int gntdev_grant_copy_seg(struct gntdev_copy_batch *batch,
>static long gntdev_ioctl_grant_copy(struct gntdev_priv *priv, void __user *u)
>{
>struct ioctl_gntdev_grant_copy copy;
>- struct gntdev_copy_batch batch;
>+ struct gntdev_copy_batch *batch;
>unsigned int i;
>int ret = 0;
>
>+ batch = kmalloc(sizeof(struct gntdev_copy_batch), GFP_KERNEL);
>+ if (!batch)
>+ return -ENOMEM;
>+
>if (copy_from_user(&copy, u, sizeof(copy)))
>return -EFAULT;

You carefully fix up all other error return paths below, but not the one above,
resulting in a memory leak.

>- out:
>- gntdev_put_pages(&batch);
>+failed:
>+ gntdev_put_pages(batch);
>+out:
>+ kfree(batch);

I'm not sure what the conventions are for label placement in the kernel sources,
but "out" having been indented by one space (which you ditch) avoided diff's -p
option picking up the label instead of the function head as context.

Jan