From 8f8366eb1898a137c5ea15be3767c22b7348de77 Mon Sep 17 00:00:00 2001 From: Sakari Ailus Date: Thu, 18 Mar 2021 07:41:07 +0000 Subject: [PATCH] media: v4l: ioctl: Fix memory leak in video_usercopy stable inclusion from stable-5.10.21 commit 5400770e31e8b80efc25b4c1d619361255174d11 bugzilla: 50609 -------------------------------- commit fb18802a338b36f675a388fc03d2aa504a0d0899 upstream. When an IOCTL with argument size larger than 128 that also used array arguments were handled, two memory allocations were made but alas, only the latter one of them was released. This happened because there was only a single local variable to hold such a temporary allocation. Fix this by adding separate variables to hold the pointers to the temporary allocations. Reported-by: Arnd Bergmann Reported-by: syzbot+1115e79c8df6472c612b@syzkaller.appspotmail.com Fixes: d14e6d76ebf7 ("[media] v4l: Add multi-planar ioctl handling code") Cc: stable@vger.kernel.org Signed-off-by: Sakari Ailus Acked-by: Arnd Bergmann Acked-by: Hans Verkuil Reviewed-by: Laurent Pinchart Signed-off-by: Mauro Carvalho Chehab Signed-off-by: Greg Kroah-Hartman Signed-off-by: Chen Jun Signed-off-by: Zheng Zengkai --- drivers/media/v4l2-core/v4l2-ioctl.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index eeff398fbdcc..9eda8b91d17a 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -3251,7 +3251,7 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg, v4l2_kioctl func) { char sbuf[128]; - void *mbuf = NULL; + void *mbuf = NULL, *array_buf = NULL; void *parg = (void *)arg; long err = -EINVAL; bool has_array_args; @@ -3286,20 +3286,14 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg, has_array_args = err; if (has_array_args) { - /* - * When adding new types of array args, make sure that the - * parent argument to ioctl (which contains the pointer to the - * array) fits into sbuf (so that mbuf will still remain - * unused up to here). - */ - mbuf = kvmalloc(array_size, GFP_KERNEL); + array_buf = kvmalloc(array_size, GFP_KERNEL); err = -ENOMEM; - if (NULL == mbuf) + if (array_buf == NULL) goto out_array_args; err = -EFAULT; - if (copy_from_user(mbuf, user_ptr, array_size)) + if (copy_from_user(array_buf, user_ptr, array_size)) goto out_array_args; - *kernel_ptr = mbuf; + *kernel_ptr = array_buf; } /* Handles IOCTL */ @@ -3318,7 +3312,7 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg, if (has_array_args) { *kernel_ptr = (void __force *)user_ptr; - if (copy_to_user(user_ptr, mbuf, array_size)) + if (copy_to_user(user_ptr, array_buf, array_size)) err = -EFAULT; goto out_array_args; } @@ -3333,6 +3327,7 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg, if (video_put_user((void __user *)arg, parg, orig_cmd)) err = -EFAULT; out: + kvfree(array_buf); kvfree(mbuf); return err; } -- GitLab