Re: [PATCH] v4l: subdev: initialize sd->internal_ops inv4l2_subdev_init

[Herton Ronaldo Krzesinski]

On Fri, Apr 01, 2011 at 05:05:27PM +0200, Hans Verkuil wrote:
> On Friday, April 01, 2011 16:24:17 Herton Ronaldo Krzesinski wrote:
> > Many v4l drivers currently don't initialize their struct v4l2_subdev
> > with zeros, so since the addition of internal_ops in commit 45f6f84, we
> > are at risk of random oopses when code in v4l2_device_register_subdev
> > tries to dereference sd->internal_ops->*, as can be shown by the report
> > at http://bugs.launchpad.net/bugs/745213
> > 
> > So make sure internal_ops is cleared in v4l2_subdev_init.
> 
> NACK: we need to replace those kmalloc's with kzalloc. This patch will just
> fix the internal_ops problem, but sd->entity isn't zeroed. It's going to be
> a neverending problem unless we fix those kmalloc's. It is my fault, I guess:
> I should always have required the use of kzalloc.
> 
> I grepped for this and the number of kmalloc's is quite small:
> 
> drivers/media/video/tda9840.c
> drivers/media/video/upd64031a.c
> drivers/media/video/m52790.c
> drivers/media/video/tea6415c.c
> drivers/media/video/tea6420.c
> drivers/media/video/upd64083.c
> drivers/media/radio/saa7706h.c
> drivers/media/radio/tef6862.c
> 
> If you fix those kmalloc's, then I'll ack those changes. There is just one
> relevant kmalloc in each file.
> 
> The reason this wasn't noticed before is that all these devices are all
> pretty rare. All the more common device drivers use kzalloc. I am really
> surprised to hear of mxb boards still in use! Just for the record: I have
> one myself for testing, although I clearly never realized that I should
> test that particular change with that board...
> 
> Thanks for doing such a great job of tracking this down!

No problem, I'll take a look and send a new patch.

> 
> Regards,
> 
> 	Hans
> 

-- 
[]'s
Herton
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/