[GIT PULL FOR v3.7] Add missing vidioc-subdev-g-edid.xml.

[GIT PULL FOR v3.7] Add missing vidioc-subdev-g-edid.xml.

[Hans Verkuil]

Hi Mauro,

As requested!

Regards,

	Hans

The following changes since commit 4313902ebe33155209472215c62d2f29d117be29:

  [media] ivtv-alsa, ivtv: Connect ivtv PCM capture stream to ivtv-alsa interface driver (2012-09-18 13:29:07 -0300)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git docfix

for you to fetch changes up to 369832c0cb2cd8df37d4854997d31978a286348e:

  DocBook: add missing vidioc-subdev-g-edid.xml. (2012-09-25 13:54:34 +0200)

----------------------------------------------------------------
Hans Verkuil (1):
      DocBook: add missing vidioc-subdev-g-edid.xml.

 Documentation/DocBook/media/v4l/v4l2.xml                 |    1 +
 Documentation/DocBook/media/v4l/vidioc-subdev-g-edid.xml |  152 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 153 insertions(+)
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-subdev-g-edid.xml

Re: [GIT PULL FOR v3.7] Add missing vidioc-subdev-g-edid.xml.

[Hans Verkuil]

On Tue 25 September 2012 13:56:34 Hans Verkuil wrote:
> Hi Mauro,
> 
> As requested!

I've respun this tree, fixing one documentation bug (the max value for
'blocks' is 256, not 255) and adding an overflow check in v4l2-ioctl.c as
reported by Dan Carpenter:

http://www.mail-archive.com/linux-media@vger.kernel.org/msg52640.html

Regards,

	Hans

> 
> Regards,
> 
> 	Hans
> 
> The following changes since commit 4313902ebe33155209472215c62d2f29d117be29:
> 
>   [media] ivtv-alsa, ivtv: Connect ivtv PCM capture stream to ivtv-alsa interface driver (2012-09-18 13:29:07 -0300)
> 
> are available in the git repository at:
> 
>   git://linuxtv.org/hverkuil/media_tree.git docfix
> 
> for you to fetch changes up to 369832c0cb2cd8df37d4854997d31978a286348e:
> 
>   DocBook: add missing vidioc-subdev-g-edid.xml. (2012-09-25 13:54:34 +0200)
> 
> ----------------------------------------------------------------
> Hans Verkuil (1):
>       DocBook: add missing vidioc-subdev-g-edid.xml.
> 
>  Documentation/DocBook/media/v4l/v4l2.xml                 |    1 +
>  Documentation/DocBook/media/v4l/vidioc-subdev-g-edid.xml |  152 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 153 insertions(+)
>  create mode 100644 Documentation/DocBook/media/v4l/vidioc-subdev-g-edid.xml
> --
> 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: [GIT PULL FOR v3.7] Add missing vidioc-subdev-g-edid.xml.

[Mauro Carvalho Chehab]

Em Wed, 26 Sep 2012 10:33:51 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On Tue 25 September 2012 13:56:34 Hans Verkuil wrote:
> > Hi Mauro,
> > 
> > As requested!
> 
> I've respun this tree, fixing one documentation bug (the max value for
> 'blocks' is 256, not 255) and adding an overflow check in v4l2-ioctl.c as
> reported by Dan Carpenter:
> 
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg52640.html

It seems you forgot to send the patches for review at the ML (at least, I'm
not seeing it on my linux-media local inbox).

Also, please document it better. Only after reading Dan's email I was able
to understand *why* you wrote such patch, as your patch description is bogus:

> Subject: Return -EINVAL if blocks > 256.
>
>...
>
>@@ -2205,6 +2205,10 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size,
>                struct v4l2_subdev_edid *edid = parg;
> 
>                if (edid->blocks) {
>+                       if (edid->blocks > 256) {
>+                               ret = -EINVAL;
>+                               break;

Well, Kernel developers are generally able to read C, so you don't need to repeat
what's written at the code as the patch subject ;)

Dan's comment provides the reason why this patch is needed:

>  2207                          *array_size = edid->blocks * 128;
>                                              ^^^^^^^^^^^^^^^^^^
> This can overflow.

So, the patch subject should be saying, instead:

v4l2-ioctl: limit the max amount of edid blocks to avoid overflow

and putting Dan's comments in the body of the patch description.

Thanks!
Mauro

Re: [GIT PULL FOR v3.7] Add missing vidioc-subdev-g-edid.xml.

[Hans Verkuil]

On Mon October 1 2012 20:24:56 Mauro Carvalho Chehab wrote:
> Em Wed, 26 Sep 2012 10:33:51 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
> > On Tue 25 September 2012 13:56:34 Hans Verkuil wrote:
> > > Hi Mauro,
> > > 
> > > As requested!
> > 
> > I've respun this tree, fixing one documentation bug (the max value for
> > 'blocks' is 256, not 255) and adding an overflow check in v4l2-ioctl.c as
> > reported by Dan Carpenter:
> > 
> > http://www.mail-archive.com/linux-media@vger.kernel.org/msg52640.html
> 
> It seems you forgot to send the patches for review at the ML (at least, I'm
> not seeing it on my linux-media local inbox).

Posted them (after rebasing to the latest for_3.7).

> Also, please document it better. Only after reading Dan's email I was able
> to understand *why* you wrote such patch, as your patch description is bogus:
> 
> > Subject: Return -EINVAL if blocks > 256.

Hmm, the patch description I see is:

	v4l2-ioctl: add overflow check for VIDIOC_SUBDEV_G/S_EDID

	Return -EINVAL if blocks > 256.

Which I thought was clear enough. Anyway, I've improved it. Strictly speaking
this isn't an overflow check, it's a check for insane memory allocations.

Regards,

	Hans

> >
> >...
> >
> >@@ -2205,6 +2205,10 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size,
> >                struct v4l2_subdev_edid *edid = parg;
> > 
> >                if (edid->blocks) {
> >+                       if (edid->blocks > 256) {
> >+                               ret = -EINVAL;
> >+                               break;
> 
> Well, Kernel developers are generally able to read C, so you don't need to repeat
> what's written at the code as the patch subject ;)
> 
> Dan's comment provides the reason why this patch is needed:
> 
> >  2207                          *array_size = edid->blocks * 128;
> >                                              ^^^^^^^^^^^^^^^^^^
> > This can overflow.
> 
> So, the patch subject should be saying, instead:
> 
> v4l2-ioctl: limit the max amount of edid blocks to avoid overflow
> 
> and putting Dan's comments in the body of the patch description.
> 
> Thanks!
> Mauro
>