diff mbox

V4L2: ov7670: fix a wrong index, potentially Oopsing the kernel from user-space

Message ID Pine.LNX.4.64.1404141545280.23631@axis700.grange (mailing list archive)
State New, archived
Headers show

Commit Message

Guennadi Liakhovetski April 14, 2014, 1:49 p.m. UTC
Commit 75e2bdad8901a0b599e01a96229be922eef1e488 "ov7670: allow
configuration of image size, clock speed, and I/O method" uses a wrong
index to iterate an array. Apart from being wrong, it also uses an
unchecked value from user-space, which can cause access to unmapped
memory in the kernel, triggered by a normal desktop user with rights to
use V4L2 devices.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

Jonathan,
I'd prefer to first post it to the lists to maybe have someone test it ;) 
Otherwise - I've got a couple more fixes for 3.15, which I hope to make 
ready and push in a couple of weeks... So, with your ack I can take this 
one too, or, if you prefer to push it earlier - would be good too.

 drivers/media/i2c/ov7670.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jonathan Corbet April 16, 2014, 1:43 p.m. UTC | #1
On Mon, 14 Apr 2014 15:49:34 +0200 (CEST)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> I'd prefer to first post it to the lists to maybe have someone test it ;) 
> Otherwise - I've got a couple more fixes for 3.15, which I hope to make 
> ready and push in a couple of weeks... So, with your ack I can take this 
> one too, or, if you prefer to push it earlier - would be good too.

Unfortunately, my machines that could test this are a couple thousand
miles away, and that situation isn't going to change anytime soon.  It
looks clearly more correct than what was there before, though, so feel
free to add my ack to it.

Thanks,

jon
--
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
Laurent Pinchart May 13, 2014, 12:21 p.m. UTC | #2
Hi Guennadi,

On Monday 14 April 2014 15:49:34 Guennadi Liakhovetski wrote:
> Commit 75e2bdad8901a0b599e01a96229be922eef1e488 "ov7670: allow
> configuration of image size, clock speed, and I/O method" uses a wrong
> index to iterate an array. Apart from being wrong, it also uses an
> unchecked value from user-space, which can cause access to unmapped
> memory in the kernel, triggered by a normal desktop user with rights to
> use V4L2 devices.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> Jonathan,
> I'd prefer to first post it to the lists to maybe have someone test it ;)
> Otherwise - I've got a couple more fixes for 3.15, which I hope to make
> ready and push in a couple of weeks... So, with your ack I can take this
> one too, or, if you prefer to push it earlier - would be good too.

What's your plan for this patch ? Will you send a pull request ? Alternatively 
I can take it in my tree.

>  drivers/media/i2c/ov7670.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> index e8a1ce2..cdd7c1b 100644
> --- a/drivers/media/i2c/ov7670.c
> +++ b/drivers/media/i2c/ov7670.c
> @@ -1109,7 +1109,7 @@ static int ov7670_enum_framesizes(struct v4l2_subdev
> *sd, * windows that fall outside that.
>  	 */
>  	for (i = 0; i < n_win_sizes; i++) {
> -		struct ov7670_win_size *win = &info->devtype->win_sizes[index];
> +		struct ov7670_win_size *win = &info->devtype->win_sizes[i];
>  		if (info->min_width && win->width < info->min_width)
>  			continue;
>  		if (info->min_height && win->height < info->min_height)
Guennadi Liakhovetski May 13, 2014, 12:31 p.m. UTC | #3
Hi Laurent,

On Tue, 13 May 2014, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Monday 14 April 2014 15:49:34 Guennadi Liakhovetski wrote:
> > Commit 75e2bdad8901a0b599e01a96229be922eef1e488 "ov7670: allow
> > configuration of image size, clock speed, and I/O method" uses a wrong
> > index to iterate an array. Apart from being wrong, it also uses an
> > unchecked value from user-space, which can cause access to unmapped
> > memory in the kernel, triggered by a normal desktop user with rights to
> > use V4L2 devices.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> > 
> > Jonathan,
> > I'd prefer to first post it to the lists to maybe have someone test it ;)
> > Otherwise - I've got a couple more fixes for 3.15, which I hope to make
> > ready and push in a couple of weeks... So, with your ack I can take this
> > one too, or, if you prefer to push it earlier - would be good too.
> 
> What's your plan for this patch ? Will you send a pull request ? Alternatively 
> I can take it in my tree.

https://patchwork.linuxtv.org/patch/23815/

Thanks
Guennadi

> 
> >  drivers/media/i2c/ov7670.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> > index e8a1ce2..cdd7c1b 100644
> > --- a/drivers/media/i2c/ov7670.c
> > +++ b/drivers/media/i2c/ov7670.c
> > @@ -1109,7 +1109,7 @@ static int ov7670_enum_framesizes(struct v4l2_subdev
> > *sd, * windows that fall outside that.
> >  	 */
> >  	for (i = 0; i < n_win_sizes; i++) {
> > -		struct ov7670_win_size *win = &info->devtype->win_sizes[index];
> > +		struct ov7670_win_size *win = &info->devtype->win_sizes[i];
> >  		if (info->min_width && win->width < info->min_width)
> >  			continue;
> >  		if (info->min_height && win->height < info->min_height)
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
Laurent Pinchart May 13, 2014, 12:34 p.m. UTC | #4
Hi Guennadi,

On Tuesday 13 May 2014 14:31:25 Guennadi Liakhovetski wrote:
> On Tue, 13 May 2014, Laurent Pinchart wrote:
> > On Monday 14 April 2014 15:49:34 Guennadi Liakhovetski wrote:
> > > Commit 75e2bdad8901a0b599e01a96229be922eef1e488 "ov7670: allow
> > > configuration of image size, clock speed, and I/O method" uses a wrong
> > > index to iterate an array. Apart from being wrong, it also uses an
> > > unchecked value from user-space, which can cause access to unmapped
> > > memory in the kernel, triggered by a normal desktop user with rights to
> > > use V4L2 devices.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > ---
> > > 
> > > Jonathan,
> > > I'd prefer to first post it to the lists to maybe have someone test it
> > > ;)
> > > Otherwise - I've got a couple more fixes for 3.15, which I hope to make
> > > ready and push in a couple of weeks... So, with your ack I can take this
> > > one too, or, if you prefer to push it earlier - would be good too.
> > 
> > What's your plan for this patch ? Will you send a pull request ?
> > Alternatively I can take it in my tree.
> 
> https://patchwork.linuxtv.org/patch/23815/

Sorry for missing that. I'll mark https://patchwork.linuxtv.org/patch/23599/ 
as accepted then.

> > >  drivers/media/i2c/ov7670.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> > > index e8a1ce2..cdd7c1b 100644
> > > --- a/drivers/media/i2c/ov7670.c
> > > +++ b/drivers/media/i2c/ov7670.c
> > > @@ -1109,7 +1109,7 @@ static int ov7670_enum_framesizes(struct
> > > v4l2_subdev
> > > *sd, * windows that fall outside that.
> > > 
> > >  	 */
> > >  	
> > >  	for (i = 0; i < n_win_sizes; i++) {
> > > 
> > > -		struct ov7670_win_size *win = &info->devtype->win_sizes[index];
> > > +		struct ov7670_win_size *win = &info->devtype->win_sizes[i];
> > > 
> > >  		if (info->min_width && win->width < info->min_width)
> > >  		
> > >  			continue;
> > >  		
> > >  		if (info->min_height && win->height < info->min_height)
diff mbox

Patch

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index e8a1ce2..cdd7c1b 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1109,7 +1109,7 @@  static int ov7670_enum_framesizes(struct v4l2_subdev *sd,
 	 * windows that fall outside that.
 	 */
 	for (i = 0; i < n_win_sizes; i++) {
-		struct ov7670_win_size *win = &info->devtype->win_sizes[index];
+		struct ov7670_win_size *win = &info->devtype->win_sizes[i];
 		if (info->min_width && win->width < info->min_width)
 			continue;
 		if (info->min_height && win->height < info->min_height)