Hi everybody,
Márton Németh found an integer overflow bug in the extended control ioctl
handling code. This affects both video_usercopy and video_ioctl2. See
http://bugzilla.kernel.org/show_bug.cgi?id=13357 for a detailed description of
the problem.
v4l2_ext_controls::count is not checked explicitly by
video_usercopy/video_ioctl2. Instead the code tries to allocate
v4l2_ext_controls::count * sizeof(struct v4l2_ext_control) to copy
v4l2_ext_controls::controls from userspace to kernelspace, and return an error
if the memory can't be allocated or if the user pointer is invalid.
The v4l2_ext_controls::count * sizeof(struct v4l2_ext_control) value is stored
in a 32 bits integer, resulting in an overflow if v4l2_ext_controls::count is
too high. If the result is smaller than the maximum kmalloc'able size, the
ioctl call will make it to the device driver, which will likely crash.
The following patch (copied from bugzilla) fixes the problem.
diff -r e0d881b21bc9 linux/drivers/media/video/v4l2-ioctl.c
--- a/linux/drivers/media/video/v4l2-ioctl.c Tue May 19 15:12:17 2009 +0200
+++ b/linux/drivers/media/video/v4l2-ioctl.c Sun May 24 18:26:29 2009 +0200
@@ -402,6 +402,10 @@
a specific control that caused it. */
p->error_idx = p->count;
user_ptr = (void __user *)p->controls;
+ if (p->count > KMALLOC_MAX_SIZE / sizeof(p->controls[0])) {
+ err = -ENOMEM;
+ goto out_ext_ctrl;
+ }
if (p->count) {
ctrls_size = sizeof(struct v4l2_ext_control) * p->count;
/* Note: v4l2_ext_controls fits in sbuf[] so mbuf is
still NULL. */
@@ -1859,6 +1863,10 @@
a specific control that caused it. */
p->error_idx = p->count;
user_ptr = (void __user *)p->controls;
+ if (p->count > KMALLOC_MAX_SIZE / sizeof(p->controls[0])) {
+ err = -ENOMEM;
+ goto out_ext_ctrl;
+ }
if (p->count) {
ctrls_size = sizeof(struct v4l2_ext_control) * p->count;
/* Note: v4l2_ext_controls fits in sbuf[] so mbuf is
still NULL. */
Restricting v4l2_ext_controls::count to values smaller than KMALLOC_MAX_SIZE /
sizeof(struct v4l2_ext_control) should be enough, but we might want to
restrict the value even further. I'd like opinions on this.
Best regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
