linux-kernel - Re: [PATCH] v4l: subdev: initialize sd->internal_ops in v4l2_subdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20110401151513.GA2347@herton-IdeaPad-Y430>
Date:	Fri, 1 Apr 2011 12:15:14 -0300
From:	Herton Ronaldo Krzesinski <herton.krzesinski@...onical.com>
To:	Hans Verkuil <hverkuil@...all.nl>
Cc:	Mauro Carvalho Chehab <mchehab@...radead.org>,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] v4l: subdev: initialize sd->internal_ops in
 v4l2_subdev_init

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/