Message ID | 1462609046-10559-1-git-send-email-xypron.glpk@gmx.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 07/05/16 10:17, Heinrich Schuchardt wrote: > Commit a4cdb556cae0 ("xen/gntdev: add ioctl for grant copy") > leads to a warning > xen/gntdev.c: In function ‘gntdev_ioctl_grant_copy’: > xen/gntdev.c:949:1: warning: the frame size of 1248 bytes > is larger than 1024 bytes [-Wframe-larger-than=] > > This can be avoided by using kmalloc instead of the stack. > > Testing requires CONFIG_XEN_GNTDEV. > > Fixes: a4cdb556cae0 ("xen/gntdev: add ioctl for grant copy") > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Acked-by: Juergen Gross <jgross@suse.com>
>>> Heinrich Schuchardt <xypron.glpk@gmx.de> 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(©, 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
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index dc49538..e8aec48 100644 --- 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(©, u, sizeof(copy))) return -EFAULT; - batch.nr_ops = 0; - batch.nr_pages = 0; + batch->nr_ops = 0; + batch->nr_pages = 0; for (i = 0; i < copy.count; i++) { struct gntdev_grant_copy_segment seg; if (copy_from_user(&seg, ©.segments[i], sizeof(seg))) { ret = -EFAULT; - goto out; + goto failed; } - ret = gntdev_grant_copy_seg(&batch, &seg, ©.segments[i].status); + ret = gntdev_grant_copy_seg(batch, &seg, + ©.segments[i].status); if (ret < 0) - goto out; + goto failed; cond_resched(); } - if (batch.nr_ops) - ret = gntdev_copy(&batch); - return ret; + if (batch->nr_ops) + ret = gntdev_copy(batch); + goto out; - out: - gntdev_put_pages(&batch); +failed: + gntdev_put_pages(batch); +out: + kfree(batch); return ret; }
Commit a4cdb556cae0 ("xen/gntdev: add ioctl for grant copy") leads to a warning xen/gntdev.c: In function ‘gntdev_ioctl_grant_copy’: xen/gntdev.c:949:1: warning: the frame size of 1248 bytes is larger than 1024 bytes [-Wframe-larger-than=] This can be avoided by using kmalloc instead of the stack. Testing requires CONFIG_XEN_GNTDEV. Fixes: a4cdb556cae0 ("xen/gntdev: add ioctl for grant copy") Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- drivers/xen/gntdev.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)