Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 80c22132 authored by Sakari Ailus's avatar Sakari Ailus Committed by Greg Kroah-Hartman
Browse files

media: v4l: ioctl: Fix memory leak in video_usercopy 



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: default avatarArnd Bergmann <arnd@kernel.org>
Reported-by: default avatar <syzbot+1115e79c8df6472c612b@syzkaller.appspotmail.com>
Fixes: d14e6d76 ("[media] v4l: Add multi-planar ioctl handling code")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarSakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: default avatarArnd Bergmann <arnd@arndb.de>
Acked-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent da4e1e3f

drivers/media/v4l2-core/v4l2-ioctl.c

+7 −12
Original line number Diff line number Diff line
@@ -2804,7 +2804,7 @@ video_usercopy(struct file *file, unsigned int 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;
@@ -2859,20 +2859,14 @@ video_usercopy(struct file *file, unsigned int 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 = kmalloc(array_size, GFP_KERNEL);

		array_buf = kmalloc(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 */
@@ -2891,7 +2885,7 @@ video_usercopy(struct file *file, unsigned int 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;

	}
@@ -2911,6 +2905,7 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg, 
	}



out:

	kfree(array_buf);

	kfree(mbuf);

	return err;

}