Message ID | 1443817993-32406-1-git-send-email-ao2@ao2.it (mailing list archive) |
---|---|
State | Accepted, archived |
Delegated to: | Hans Verkuil |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1Zi779-0008JZ-HG; Fri, 02 Oct 2015 22:39:35 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.76/mailfrontend-5) with esmtp id 1Zi777-0000BS-75; Fri, 02 Oct 2015 22:39:35 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751090AbbJBUja (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Fri, 2 Oct 2015 16:39:30 -0400 Received: from smtp207.alice.it ([82.57.200.103]:8541 "EHLO smtp207.alice.it" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750983AbbJBUj3 (ORCPT <rfc822;linux-media@vger.kernel.org>); Fri, 2 Oct 2015 16:39:29 -0400 X-Greylist: delayed 321 seconds by postgrey-1.27 at vger.kernel.org; Fri, 02 Oct 2015 16:39:29 EDT Received: from jcn (82.57.82.166) by smtp207.alice.it (8.6.060.28) id 55BB66530B6916C1; Fri, 2 Oct 2015 22:33:59 +0200 Received: from ao2 by jcn with local (Exim 4.86) (envelope-from <ao2@ao2.it>) id 1Zi71F-0008RK-BP; Fri, 02 Oct 2015 22:33:29 +0200 From: Antonio Ospite <ao2@ao2.it> To: Linux Media Mailing List <linux-media@vger.kernel.org> Cc: Hans de Goede <hdegoede@redhat.com>, moinejf@free.fr, Anders Blomdell <anders.blomdell@control.lth.se>, Thomas Champagne <lafeuil@gmail.com>, Antonio Ospite <ao2@ao2.it>, stable@vger.kernel.org Subject: [PATCH] [media] gspca: ov534/topro: prevent a division by 0 Date: Fri, 2 Oct 2015 22:33:13 +0200 Message-Id: <1443817993-32406-1-git-send-email-ao2@ao2.it> X-Mailer: git-send-email 2.6.0 X-Face: z*RaLf`X<@C75u6Ig9}{oW$H; 1_\2t5)({*|jhM<pyWR#k60!#=#>/Vb; ]yA5<GWI5`6u&+ ; 6b'@y|8w"wB; 4/e!7wYYrcqdJFY,~%Gk_4]cq$Ei/7<j&N3ah(m`ku?pX.&+~:_/wC~dwn^)MizBG !pE^+iDQQ1yC6^,)YDKkxDd!T>\I~93>J<_`<4)A{':UrE Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2015.10.2.203016 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODY_SIZE_5000_5999 0, BODY_SIZE_7000_LESS 0, NO_URI_HTTPS 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __FRAUD_CONTACT_NAME 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __MULTIPLE_URI_TEXT 0, __SANE_MSGID 0, __STOCK_PHRASE_24 0, __TO_MALFORMED_2 0, __URI_IN_BODY 0, __URI_NS ' |
Commit Message
Antonio Ospite
Oct. 2, 2015, 8:33 p.m. UTC
v4l2-compliance sends a zeroed struct v4l2_streamparm in
v4l2-test-formats.cpp::testParmType(), and this results in a division by
0 in some gspca subdrivers:
divide error: 0000 [#1] SMP
Modules linked in: gspca_ov534 gspca_main ...
CPU: 0 PID: 17201 Comm: v4l2-compliance Not tainted 4.3.0-rc2-ao2 #1
Hardware name: System manufacturer System Product Name/M2N-E SLI, BIOS
ASUS M2N-E SLI ACPI BIOS Revision 1301 09/16/2010
task: ffff8800818306c0 ti: ffff880095c4c000 task.ti: ffff880095c4c000
RIP: 0010:[<ffffffffa079bd62>] [<ffffffffa079bd62>] sd_set_streamparm+0x12/0x60 [gspca_ov534]
RSP: 0018:ffff880095c4fce8 EFLAGS: 00010296
RAX: 0000000000000000 RBX: ffff8800c9522000 RCX: ffffffffa077a140
RDX: 0000000000000000 RSI: ffff880095e0c100 RDI: ffff8800c9522000
RBP: ffff880095e0c100 R08: ffffffffa077a100 R09: 00000000000000cc
R10: ffff880067ec7740 R11: 0000000000000016 R12: ffffffffa07bb400
R13: 0000000000000000 R14: ffff880081b6a800 R15: 0000000000000000
FS: 00007fda0de78740(0000) GS:ffff88012fc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000014630f8 CR3: 00000000cf349000 CR4: 00000000000006f0
Stack:
ffffffffa07a6431 ffff8800c9522000 ffffffffa077656e 00000000c0cc5616
ffff8800c9522000 ffffffffa07a5e20 ffff880095e0c100 0000000000000000
ffff880067ec7740 ffffffffa077a140 ffff880067ec7740 0000000000000016
Call Trace:
[<ffffffffa07a6431>] ? v4l_s_parm+0x21/0x50 [videodev]
[<ffffffffa077656e>] ? vidioc_s_parm+0x4e/0x60 [gspca_main]
[<ffffffffa07a5e20>] ? __video_do_ioctl+0x280/0x2f0 [videodev]
[<ffffffffa07a5ba0>] ? video_ioctl2+0x20/0x20 [videodev]
[<ffffffffa07a59b9>] ? video_usercopy+0x319/0x4e0 [videodev]
[<ffffffff81182dc1>] ? page_add_new_anon_rmap+0x71/0xa0
[<ffffffff811afb92>] ? mem_cgroup_commit_charge+0x52/0x90
[<ffffffff81179b18>] ? handle_mm_fault+0xc18/0x1680
[<ffffffffa07a15cc>] ? v4l2_ioctl+0xac/0xd0 [videodev]
[<ffffffff811c846f>] ? do_vfs_ioctl+0x28f/0x480
[<ffffffff811c86d4>] ? SyS_ioctl+0x74/0x80
[<ffffffff8154a8b6>] ? entry_SYSCALL_64_fastpath+0x16/0x75
Code: c7 93 d9 79 a0 5b 5d e9 f1 f3 9a e0 0f 1f 00 66 2e 0f 1f 84 00
00 00 00 00 66 66 66 66 90 53 31 d2 48 89 fb 48 83 ec 08 8b 46 10 <f7>
76 0c 80 bf ac 0c 00 00 00 88 87 4e 0e 00 00 74 09 80 bf 4f
RIP [<ffffffffa079bd62>] sd_set_streamparm+0x12/0x60 [gspca_ov534]
RSP <ffff880095c4fce8>
---[ end trace 279710c2c6c72080 ]---
Following what the doc says about a zeroed timeperframe (see
http://www.linuxtv.org/downloads/v4l-dvb-apis/vidioc-g-parm.html):
...
To reset manually applications can just set this field to zero.
fix the issue by resetting the frame rate to a default value in case of
an unusable timeperframe.
The fix is done in the subdrivers instead of gspca.c because only the
subdrivers have notion of a default frame rate to reset the camera to.
Signed-off-by: Antonio Ospite <ao2@ao2.it>
Cc: stable@vger.kernel.org
---
Hi,
I think the problem in the gspca subdrivers has always been there, so the
patch could be applied to any relevant stable releases.
After the fix, v4l2-compliance runs fine but it gets two failures, I'll send
another mail about those.
After this change gets merged I will also send a patch to use defines for the
default framerates used below as the same value is used in multiple places.
Ah, I ran the patch through scripts/checkpatch.pl from 4.3-rc2 and it
complained about the commit message but I think it may be a false positive.
Ciao ciao,
Antonio
drivers/media/usb/gspca/ov534.c | 9 +++++++--
drivers/media/usb/gspca/topro.c | 6 +++++-
2 files changed, 12 insertions(+), 3 deletions(-)
Comments
Hi, On 02-10-15 22:33, Antonio Ospite wrote: > v4l2-compliance sends a zeroed struct v4l2_streamparm in > v4l2-test-formats.cpp::testParmType(), and this results in a division by > 0 in some gspca subdrivers: > > divide error: 0000 [#1] SMP > Modules linked in: gspca_ov534 gspca_main ... > CPU: 0 PID: 17201 Comm: v4l2-compliance Not tainted 4.3.0-rc2-ao2 #1 > Hardware name: System manufacturer System Product Name/M2N-E SLI, BIOS > ASUS M2N-E SLI ACPI BIOS Revision 1301 09/16/2010 > task: ffff8800818306c0 ti: ffff880095c4c000 task.ti: ffff880095c4c000 > RIP: 0010:[<ffffffffa079bd62>] [<ffffffffa079bd62>] sd_set_streamparm+0x12/0x60 [gspca_ov534] > RSP: 0018:ffff880095c4fce8 EFLAGS: 00010296 > RAX: 0000000000000000 RBX: ffff8800c9522000 RCX: ffffffffa077a140 > RDX: 0000000000000000 RSI: ffff880095e0c100 RDI: ffff8800c9522000 > RBP: ffff880095e0c100 R08: ffffffffa077a100 R09: 00000000000000cc > R10: ffff880067ec7740 R11: 0000000000000016 R12: ffffffffa07bb400 > R13: 0000000000000000 R14: ffff880081b6a800 R15: 0000000000000000 > FS: 00007fda0de78740(0000) GS:ffff88012fc00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00000000014630f8 CR3: 00000000cf349000 CR4: 00000000000006f0 > Stack: > ffffffffa07a6431 ffff8800c9522000 ffffffffa077656e 00000000c0cc5616 > ffff8800c9522000 ffffffffa07a5e20 ffff880095e0c100 0000000000000000 > ffff880067ec7740 ffffffffa077a140 ffff880067ec7740 0000000000000016 > Call Trace: > [<ffffffffa07a6431>] ? v4l_s_parm+0x21/0x50 [videodev] > [<ffffffffa077656e>] ? vidioc_s_parm+0x4e/0x60 [gspca_main] > [<ffffffffa07a5e20>] ? __video_do_ioctl+0x280/0x2f0 [videodev] > [<ffffffffa07a5ba0>] ? video_ioctl2+0x20/0x20 [videodev] > [<ffffffffa07a59b9>] ? video_usercopy+0x319/0x4e0 [videodev] > [<ffffffff81182dc1>] ? page_add_new_anon_rmap+0x71/0xa0 > [<ffffffff811afb92>] ? mem_cgroup_commit_charge+0x52/0x90 > [<ffffffff81179b18>] ? handle_mm_fault+0xc18/0x1680 > [<ffffffffa07a15cc>] ? v4l2_ioctl+0xac/0xd0 [videodev] > [<ffffffff811c846f>] ? do_vfs_ioctl+0x28f/0x480 > [<ffffffff811c86d4>] ? SyS_ioctl+0x74/0x80 > [<ffffffff8154a8b6>] ? entry_SYSCALL_64_fastpath+0x16/0x75 > Code: c7 93 d9 79 a0 5b 5d e9 f1 f3 9a e0 0f 1f 00 66 2e 0f 1f 84 00 > 00 00 00 00 66 66 66 66 90 53 31 d2 48 89 fb 48 83 ec 08 8b 46 10 <f7> > 76 0c 80 bf ac 0c 00 00 00 88 87 4e 0e 00 00 74 09 80 bf 4f > RIP [<ffffffffa079bd62>] sd_set_streamparm+0x12/0x60 [gspca_ov534] > RSP <ffff880095c4fce8> > ---[ end trace 279710c2c6c72080 ]--- > > Following what the doc says about a zeroed timeperframe (see > http://www.linuxtv.org/downloads/v4l-dvb-apis/vidioc-g-parm.html): > > ... > To reset manually applications can just set this field to zero. > > fix the issue by resetting the frame rate to a default value in case of > an unusable timeperframe. > > The fix is done in the subdrivers instead of gspca.c because only the > subdrivers have notion of a default frame rate to reset the camera to. > > Signed-off-by: Antonio Ospite <ao2@ao2.it> > Cc: stable@vger.kernel.org Good catch: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Mauro can you pick this one up directly, and include it in your next pull-req for 4.3 please ? Regards, Hans > --- > > Hi, > > I think the problem in the gspca subdrivers has always been there, so the > patch could be applied to any relevant stable releases. > > After the fix, v4l2-compliance runs fine but it gets two failures, I'll send > another mail about those. > > After this change gets merged I will also send a patch to use defines for the > default framerates used below as the same value is used in multiple places. > > Ah, I ran the patch through scripts/checkpatch.pl from 4.3-rc2 and it > complained about the commit message but I think it may be a false positive. > > Ciao ciao, > Antonio > > > drivers/media/usb/gspca/ov534.c | 9 +++++++-- > drivers/media/usb/gspca/topro.c | 6 +++++- > 2 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/usb/gspca/ov534.c b/drivers/media/usb/gspca/ov534.c > index 146071b..bfff1d1 100644 > --- a/drivers/media/usb/gspca/ov534.c > +++ b/drivers/media/usb/gspca/ov534.c > @@ -1491,8 +1491,13 @@ static void sd_set_streamparm(struct gspca_dev *gspca_dev, > struct v4l2_fract *tpf = &cp->timeperframe; > struct sd *sd = (struct sd *) gspca_dev; > > - /* Set requested framerate */ > - sd->frame_rate = tpf->denominator / tpf->numerator; > + if (tpf->numerator == 0 || tpf->denominator == 0) > + /* Set default framerate */ > + sd->frame_rate = 30; > + else > + /* Set requested framerate */ > + sd->frame_rate = tpf->denominator / tpf->numerator; > + > if (gspca_dev->streaming) > set_frame_rate(gspca_dev); > > diff --git a/drivers/media/usb/gspca/topro.c b/drivers/media/usb/gspca/topro.c > index c70ff40..c028a5c 100644 > --- a/drivers/media/usb/gspca/topro.c > +++ b/drivers/media/usb/gspca/topro.c > @@ -4802,7 +4802,11 @@ static void sd_set_streamparm(struct gspca_dev *gspca_dev, > struct v4l2_fract *tpf = &cp->timeperframe; > int fr, i; > > - sd->framerate = tpf->denominator / tpf->numerator; > + if (tpf->numerator == 0 || tpf->denominator == 0) > + sd->framerate = 30; > + else > + sd->framerate = tpf->denominator / tpf->numerator; > + > if (gspca_dev->streaming) > setframerate(gspca_dev, v4l2_ctrl_g_ctrl(gspca_dev->exposure)); > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 3 Oct 2015 10:14:17 +0200 Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > Hi HdG, > On 02-10-15 22:33, Antonio Ospite wrote: [...] > > Signed-off-by: Antonio Ospite <ao2@ao2.it> > > Cc: stable@vger.kernel.org > > Good catch: > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > > Mauro can you pick this one up directly, and include it in your > next pull-req for 4.3 please ? > Ping. On https://patchwork.linuxtv.org/patch/31561/ it says: Delegated to: Hans de Goede Who is going to handle this? Thanks, Antonio
Em Fri, 23 Oct 2015 11:13:56 +0200 Antonio Ospite <ao2@ao2.it> escreveu: > On Sat, 3 Oct 2015 10:14:17 +0200 > Hans de Goede <hdegoede@redhat.com> wrote: > > > Hi, > > > > Hi HdG, > > > On 02-10-15 22:33, Antonio Ospite wrote: > [...] > > > Signed-off-by: Antonio Ospite <ao2@ao2.it> > > > Cc: stable@vger.kernel.org > > > > Good catch: > > > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > > > > Mauro can you pick this one up directly, and include it in your > > next pull-req for 4.3 please ? Hans, I'm removing the delegation at patchwork. Next time, please don't forget to remove the delegation there, as I generally don't bother about delegated patches, as my scripts just ignore them. I'm afraid that it is too late for 4.3, though, as we'll all be next week at KS. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/usb/gspca/ov534.c b/drivers/media/usb/gspca/ov534.c index 146071b..bfff1d1 100644 --- a/drivers/media/usb/gspca/ov534.c +++ b/drivers/media/usb/gspca/ov534.c @@ -1491,8 +1491,13 @@ static void sd_set_streamparm(struct gspca_dev *gspca_dev, struct v4l2_fract *tpf = &cp->timeperframe; struct sd *sd = (struct sd *) gspca_dev; - /* Set requested framerate */ - sd->frame_rate = tpf->denominator / tpf->numerator; + if (tpf->numerator == 0 || tpf->denominator == 0) + /* Set default framerate */ + sd->frame_rate = 30; + else + /* Set requested framerate */ + sd->frame_rate = tpf->denominator / tpf->numerator; + if (gspca_dev->streaming) set_frame_rate(gspca_dev); diff --git a/drivers/media/usb/gspca/topro.c b/drivers/media/usb/gspca/topro.c index c70ff40..c028a5c 100644 --- a/drivers/media/usb/gspca/topro.c +++ b/drivers/media/usb/gspca/topro.c @@ -4802,7 +4802,11 @@ static void sd_set_streamparm(struct gspca_dev *gspca_dev, struct v4l2_fract *tpf = &cp->timeperframe; int fr, i; - sd->framerate = tpf->denominator / tpf->numerator; + if (tpf->numerator == 0 || tpf->denominator == 0) + sd->framerate = 30; + else + sd->framerate = tpf->denominator / tpf->numerator; + if (gspca_dev->streaming) setframerate(gspca_dev, v4l2_ctrl_g_ctrl(gspca_dev->exposure));