Fwd: divide error: 0000 in the gspca_topro

Fwd: divide error: 0000 in the gspca_topro

[Linus Torvalds]

I got this, and it certainly seems relevant,.

It would seem that that whole 'quality' thing needs some range
checking, it should presumably be in the range [1..100] in order to
avoid negative 'sc' values or the divide-by-zero.

Hans, Mauro?

                      Linus

---------- Forwarded message ----------
From: Peter Kovář <peter.kovar@reflexion.tv>
Date: Sun, Feb 8, 2015 at 2:18 PM
Subject: divide error: 0000 in the gspca_topro
To: Linus Torvalds <torvalds@linux-foundation.org>


Hi++ Linus!

There is a trivial bug in the gspca_topro webcam driver.

/* set the JPEG quality for sensor soi763a */
static void jpeg_set_qual(u8 *jpeg_hdr,
                          int quality)
{
        int i, sc;

        if (quality < 50)
                sc = 5000 / quality;
        else
                sc = 200 - quality * 2;



Crash can be reproduced by setting JPEG quality to zero in the guvcview
application.

Cheers,

Peter Kovář
50 65 74 65 72 20 4B 6F 76 C3 A1 C5 99

Re: Fwd: divide error: 0000 in the gspca_topro

[Luis de Bethencourt]

On Sun, Feb 08, 2015 at 06:07:45PM -0800, Linus Torvalds wrote:
> I got this, and it certainly seems relevant,.
> 
> It would seem that that whole 'quality' thing needs some range
> checking, it should presumably be in the range [1..100] in order to
> avoid negative 'sc' values or the divide-by-zero.
> 
> Hans, Mauro?
> 
>                       Linus

Hello Linus,

The case of quality being set to 0 is correctly handled in
drivers/media/usb/gspca/jpeg.h [0], so I have sent a patch to do the same
in topro.c.

Thanks,
Luis

[0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/usb/gspca/jpeg.h#n157

> 
> ---------- Forwarded message ----------
> From: Peter Kovář <peter.kovar@reflexion.tv>
> Date: Sun, Feb 8, 2015 at 2:18 PM
> Subject: divide error: 0000 in the gspca_topro
> To: Linus Torvalds <torvalds@linux-foundation.org>
> 
> 
> Hi++ Linus!
> 
> There is a trivial bug in the gspca_topro webcam driver.
> 
> /* set the JPEG quality for sensor soi763a */
> static void jpeg_set_qual(u8 *jpeg_hdr,
>                           int quality)
> {
>         int i, sc;
> 
>         if (quality < 50)
>                 sc = 5000 / quality;
>         else
>                 sc = 200 - quality * 2;
> 
> 
> 
> Crash can be reproduced by setting JPEG quality to zero in the guvcview
> application.
> 
> Cheers,
> 
> Peter Kovář
> 50 65 74 65 72 20 4B 6F 76 C3 A1 C5 99
> --
> 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

Re: divide error: 0000 in the gspca_topro

[Mauro Carvalho Chehab]

Em Mon, 09 Feb 2015 10:23:48 +0000
Luis de Bethencourt <luis@debethencourt.com> escreveu:

> On Sun, Feb 08, 2015 at 06:07:45PM -0800, Linus Torvalds wrote:
> > I got this, and it certainly seems relevant,.
> > 
> > It would seem that that whole 'quality' thing needs some range
> > checking, it should presumably be in the range [1..100] in order to
> > avoid negative 'sc' values or the divide-by-zero.
> > 
> > Hans, Mauro?
> > 
> >                       Linus
> 
> Hello Linus,
> 
> The case of quality being set to 0 is correctly handled in
> drivers/media/usb/gspca/jpeg.h [0], so I have sent a patch to do the same
> in topro.c.

Patch looks good to me.

I'll double check if some other driver has the same bad handling for
quality set and give a couple days for Hans to take a look.

If he's fine with this approach, I'll add it on a separate pull request.

Regards,
Mauro

> 
> Thanks,
> Luis
> 
> [0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/usb/gspca/jpeg.h#n157
> 
> > 
> > ---------- Forwarded message ----------
> > From: Peter Kovář <peter.kovar@reflexion.tv>
> > Date: Sun, Feb 8, 2015 at 2:18 PM
> > Subject: divide error: 0000 in the gspca_topro
> > To: Linus Torvalds <torvalds@linux-foundation.org>
> > 
> > 
> > Hi++ Linus!
> > 
> > There is a trivial bug in the gspca_topro webcam driver.
> > 
> > /* set the JPEG quality for sensor soi763a */
> > static void jpeg_set_qual(u8 *jpeg_hdr,
> >                           int quality)
> > {
> >         int i, sc;
> > 
> >         if (quality < 50)
> >                 sc = 5000 / quality;
> >         else
> >                 sc = 200 - quality * 2;
> > 
> > 
> > 
> > Crash can be reproduced by setting JPEG quality to zero in the guvcview
> > application.
> > 
> > Cheers,
> > 
> > Peter Kovář
> > 50 65 74 65 72 20 4B 6F 76 C3 A1 C5 99
> > --
> > 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

Re: divide error: 0000 in the gspca_topro

[Luis de Bethencourt]

On Mon, Feb 09, 2015 at 01:56:56PM -0200, Mauro Carvalho Chehab wrote:
> Em Mon, 09 Feb 2015 10:23:48 +0000
> Luis de Bethencourt <luis@debethencourt.com> escreveu:
> 
> > On Sun, Feb 08, 2015 at 06:07:45PM -0800, Linus Torvalds wrote:
> > > I got this, and it certainly seems relevant,.
> > > 
> > > It would seem that that whole 'quality' thing needs some range
> > > checking, it should presumably be in the range [1..100] in order to
> > > avoid negative 'sc' values or the divide-by-zero.
> > > 
> > > Hans, Mauro?
> > > 
> > >                       Linus
> > 
> > Hello Linus,
> > 
> > The case of quality being set to 0 is correctly handled in
> > drivers/media/usb/gspca/jpeg.h [0], so I have sent a patch to do the same
> > in topro.c.
> 
> Patch looks good to me.
> 
> I'll double check if some other driver has the same bad handling for
> quality set and give a couple days for Hans to take a look.
> 
> If he's fine with this approach, I'll add it on a separate pull request.
> 
> Regards,
> Mauro
> 

Hi Mauro,

Thanks for taking the time to look at this.

After sending the patch I searched around for any similar cases, only
finding coda/coda-jpeg.c [0], but in this case the quality is clipped to 5 if
it is < 5.

I might have missed some other case though. Just letting you know to help you
save some time.

Cheers,
Luis

[0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/coda/coda-jpeg.c#n216

> > 
> > Thanks,
> > Luis
> > 
> > [0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/media/usb/gspca/jpeg.h#n157
> > 
> > > 
> > > ---------- Forwarded message ----------
> > > From: Peter Kovář <peter.kovar@reflexion.tv>
> > > Date: Sun, Feb 8, 2015 at 2:18 PM
> > > Subject: divide error: 0000 in the gspca_topro
> > > To: Linus Torvalds <torvalds@linux-foundation.org>
> > > 
> > > 
> > > Hi++ Linus!
> > > 
> > > There is a trivial bug in the gspca_topro webcam driver.
> > > 
> > > /* set the JPEG quality for sensor soi763a */
> > > static void jpeg_set_qual(u8 *jpeg_hdr,
> > >                           int quality)
> > > {
> > >         int i, sc;
> > > 
> > >         if (quality < 50)
> > >                 sc = 5000 / quality;
> > >         else
> > >                 sc = 200 - quality * 2;
> > > 
> > > 
> > > 
> > > Crash can be reproduced by setting JPEG quality to zero in the guvcview
> > > application.
> > > 
> > > Cheers,
> > > 
> > > Peter Kovář
> > > 50 65 74 65 72 20 4B 6F 76 C3 A1 C5 99
> > > --
> > > 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

Re: divide error: 0000 in the gspca_topro

[Hans de Goede]

Hi,

On 09-02-15 16:56, Mauro Carvalho Chehab wrote:
> Em Mon, 09 Feb 2015 10:23:48 +0000
> Luis de Bethencourt <luis@debethencourt.com> escreveu:
>
>> On Sun, Feb 08, 2015 at 06:07:45PM -0800, Linus Torvalds wrote:
>>> I got this, and it certainly seems relevant,.
>>>
>>> It would seem that that whole 'quality' thing needs some range
>>> checking, it should presumably be in the range [1..100] in order to
>>> avoid negative 'sc' values or the divide-by-zero.
>>>
>>> Hans, Mauro?
>>>
>>>                        Linus
>>
>> Hello Linus,
>>
>> The case of quality being set to 0 is correctly handled in
>> drivers/media/usb/gspca/jpeg.h [0], so I have sent a patch to do the same
>> in topro.c.
>
> Patch looks good to me.
>
> I'll double check if some other driver has the same bad handling for
> quality set and give a couple days for Hans to take a look.
>
> If he's fine with this approach, I'll add it on a separate pull request.

Luis' patch for this looks good to me and is:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Mauro, thanks for picking this one up.

Regards,

Hans