FAILED: patch "[PATCH] media: v4l: ioctl: Fix memory leak in video_usercopy" failed to apply to 4.4-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From fb18802a338b36f675a388fc03d2aa504a0d0899 Mon Sep 17 00:00:00 2001
From: Sakari Ailus <[email protected]>
Date: Sat, 19 Dec 2020 23:29:58 +0100
Subject: [PATCH] media: v4l: ioctl: Fix memory leak in video_usercopy
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.
drivers/media/v4l2-core/v4l2-ioctl.c +14 -18
@@ -3283,7 +3283,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;
@@ -3318,27 +3318,21 @@ 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 (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, unsigned int orig_cmd, unsigned long arg,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 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,if (video_put_user ((void __user *)arg, parg, cmd, orig_cmd)) err = -EFAULT ; out: +kvfree (array_buf);kvfree (mbuf);return err; }
Hi Greg,
Quoted 5 lines · On Mon, Mar 01, 2021 at 12:17:20PM +0100, [email protected] wrote:
The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
I've manually backported patches for the stable trees as there have been changes in the upstream between the stable trees and the fix. Small changes were also needed between different stable kernels.
Signature · 3 lines
Kind regards, Sakari Ailus
PATCH 1/1 media: v4l: ioctl: Fix memory leak in video_usercopy
[ Upstream commit fb18802a338b36f675a388fc03d2aa504a0d0899 ]
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.
--- drivers/media/v4l2-core/v4l2-ioctl.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
drivers/media/v4l2-core/v4l2-ioctl.c +7 -12
@@ -2836,7 +2836,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;
@@ -2894,20 +2894,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 =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 */
@@ -2926,7 +2920,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; }
@@ -2948,6 +2942,7 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg, } out: +kvfree (array_buf);kvfree (mbuf);return err; }
Signature · 1 line
2.20.1
PATCH 1/1 media: v4l: ioctl: Fix memory leak in video_usercopy
In reply to Sakari Ailus[ Upstream commit fb18802a338b36f675a388fc03d2aa504a0d0899 ]
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.
--- drivers/media/v4l2-core/v4l2-ioctl.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
drivers/media/v4l2-core/v4l2-ioctl.c +7 -12
@@ -2939,7 +2939,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;
@@ -2998,20 +2998,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 =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 */
@@ -3030,7 +3024,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; }
@@ -3052,6 +3046,7 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg, } out: +kvfree (array_buf);kvfree (mbuf);return err; }
Signature · 1 line
2.20.1
PATCH 1/1 media: v4l: ioctl: Fix memory leak in video_usercopy
In reply to Sakari Ailus[ Upstream commit fb18802a338b36f675a388fc03d2aa504a0d0899 ]
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.
--- drivers/media/v4l2-core/v4l2-ioctl.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
drivers/media/v4l2-core/v4l2-ioctl.c +7 -12
@@ -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; }
Signature · 1 line
2.20.1
PATCH 1/1 media: v4l: ioctl: Fix memory leak in video_usercopy
In reply to Sakari Ailus[ Upstream commit fb18802a338b36f675a388fc03d2aa504a0d0899 ]
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.
--- drivers/media/v4l2-core/v4l2-ioctl.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
drivers/media/v4l2-core/v4l2-ioctl.c +7 -12
@@ -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; }
Signature · 1 line
2.20.1
PATCH 1/1 media: v4l: ioctl: Fix memory leak in video_usercopy
In reply to Sakari Ailus[ Upstream commit fb18802a338b36f675a388fc03d2aa504a0d0899 ]
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.
--- drivers/media/v4l2-core/v4l2-ioctl.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
drivers/media/v4l2-core/v4l2-ioctl.c +7 -12
@@ -3016,7 +3016,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;
@@ -3075,20 +3075,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 =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 */
@@ -3107,7 +3101,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; }
@@ -3129,6 +3123,7 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg, } out: +kvfree (array_buf);kvfree (mbuf);return err; }
Signature · 1 line
2.20.1
PATCH 1/1 media: v4l: ioctl: Fix memory leak in video_usercopy
In reply to Sakari Ailus[ Upstream commit fb18802a338b36f675a388fc03d2aa504a0d0899 ]
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.
--- drivers/media/v4l2-core/v4l2-ioctl.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
drivers/media/v4l2-core/v4l2-ioctl.c +7 -12
@@ -2710,7 +2710,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;
@@ -2765,20 +2765,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 */
@@ -2797,7 +2791,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; }
@@ -2817,6 +2811,7 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg, } out: +kfree (array_buf);kfree (mbuf);return err; }
Signature · 1 line
2.20.1
Greg KH
maintainerQuoted 4 lines · Sakari Ailus
Hi Greg,
Quoted 1 line · On Mon, Mar 01, 2021 at 12:17:20PM +0100, [email protected] wrote:
Quoted 8 lines
Quoted 4 lines
The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.I've manually backported patches for the stable trees as there have been changes in the upstream between the stable trees and the fix. Small changes were also needed between different stable kernels.
Thanks for these, all now queued up.
greg k-h