From: Sakari Ailus sakari.ailus@linux.intel.com
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 arnd@kernel.org 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 sakari.ailus@linux.intel.com Acked-by: Arnd Bergmann arnd@arndb.de Acked-by: Hans Verkuil hverkuil-cisco@xs4all.nl Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Mauro Carvalho Chehab mchehab+huawei@kernel.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- drivers/media/v4l2-core/v4l2-ioctl.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-)
--- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -3283,7 +3283,7 @@ video_usercopy(struct file *file, unsign 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; @@ -3318,27 +3318,21 @@ video_usercopy(struct file *file, unsign 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 (in_compat_syscall()) - err = v4l2_compat_get_array_args(file, mbuf, user_ptr, - array_size, orig_cmd, - parg); + err = v4l2_compat_get_array_args(file, array_buf, + user_ptr, array_size, + orig_cmd, parg); else - err = copy_from_user(mbuf, user_ptr, array_size) ? + err = copy_from_user(array_buf, user_ptr, array_size) ? -EFAULT : 0; if (err) goto out_array_args; - *kernel_ptr = mbuf; + *kernel_ptr = array_buf; }
/* Handles IOCTL */ @@ -3360,12 +3354,13 @@ video_usercopy(struct file *file, unsign if (in_compat_syscall()) { int put_err;
- put_err = v4l2_compat_put_array_args(file, user_ptr, mbuf, - array_size, orig_cmd, - parg); + put_err = v4l2_compat_put_array_args(file, user_ptr, + array_buf, + array_size, + orig_cmd, parg); if (put_err) err = put_err; - } else if (copy_to_user(user_ptr, mbuf, array_size)) { + } else if (copy_to_user(user_ptr, array_buf, array_size)) { err = -EFAULT; } goto out_array_args; @@ -3381,6 +3376,7 @@ out_array_args: if (video_put_user((void __user *)arg, parg, cmd, orig_cmd)) err = -EFAULT; out: + kvfree(array_buf); kvfree(mbuf); return err; }