linux-kernel - Re: [v4l-dvb-maintainer] Re: [PATCH] V4L: struct video

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] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 20 Jul 2006 14:57:31 -0700 (PDT)
From:	Trent Piepho <xyzzy@...akeasy.org>
To:	Mauro Carvalho Chehab <mchehab@...radead.org>
cc:	Robert Fitzsimons <robfitz@...k.net>,
	Andrew Morton <akpm@...l.org>,
	Linux and Kernel Video <video4linux-list@...hat.com>,
	76306.1226@...puserve.com, fork0@...nline.de, greg@...ah.com,
	linux-kernel@...r.kernel.org,
	"Randy.Dunlap" <rdunlap@...otime.net>,
	v4l-dvb maintainer list <v4l-dvb-maintainer@...uxtv.org>,
	shemminger@...l.org
Subject: Re: [v4l-dvb-maintainer] Re: [PATCH] V4L: struct video_device
 corruption

On Wed, 19 Jul 2006, Mauro Carvalho Chehab wrote:
> Em Seg, 2006-07-17 às 17:25 -0700, Trent Piepho escreveu:
> > On Sat, 15 Jul 2006, Mauro Carvalho Chehab wrote:
> > > Em Sb, 2006-07-15 s 23:08 +0000, Robert Fitzsimons escreveu:
> > > > The layout of struct video_device would change depending on whether
> > > > videodev.h (V4L1) was include or not before v4l2-dev.h, which caused
> > > > the structure to get corrupted.

> > I think that the real problem is that many drivers include the V4L1 API
> > file videodev.h when V4L1 is NOT on.  Should drivers be providing V4L1 API
> > functions, or need anything from videodev.h, if V4L1 is not on?
> >
> > It seems like they either need to depend on VIDEO_V4L1 or only include the
> > V4L1 API header file when V4L1 is turned on.  Which also means they would
> > need to #ifdef out any V4L1 code when V4L1 is turned off.  The bttv driver
> > for example does not do this.  It includes a bunch of V4L1 functions even
> > when V4L1 (and V4L1_COMPAT) are turned off.

I've looked into this more, and there is still a serious bug here.  If you
turn off V4L1 and V4L1_COMPAT, many drivers will a big issue with struct
video_device.

In some files (saa7134-tvaudio.c, saa6752hs.c, v4l2-common.c, dozens
more), V4L1 will be off and HAVE_V4L1 will not be defined.  This gives you
the struct video_dev _without_ vidiocgmbuf.

In other files (tveeprom.c, tvaudio.c, bttv-driver.c, and many more), V4L1
will still be off (of course) by HAVE_V4L1 _will_ be defined.  This gives
you the struct viddeo_dev _with_ vidiocgmbuf.

The first thing to solve this that HAVE_V4L1 should die.  Why have a define
that is supposed to mirror a Kconfig variable?  If everyone used
CONFIG_VIDEO_V4L1_COMPAT then there wouldn't be this problem, which some
code things V4L1 is on, and some code thinks it's off.

The second thing, is that many drivers don't respect
CONFIG_VIDEO_V4L1_COMPAT.  They include V4L1 code when V4L1 is turned off.

To fix this completely:
1. Find all unnecessary includes of videodev.h and remove them.

2. Any remaining includes of videodef.h in drivers which don't depend on
   VIDEO_V4L1_COMPAT or VIDEO_V4L1 in Kconfig should be protected with
   #ifdef CONFIG_VIDEO_V4L1_COMPAT.  This will break many drivers.

3. Replace HAVE_V4L1 with CONFIG_VIDEO_V4L1_COMPAT everywhere.
   This will break many drivers.

4. Any drivers broken by steps 2 and 3 should be fixed by either:
   A.  Protecting all V4L1 code with #ifdef CONFIG_VIDEO_V4L1_COMPAT
   B.  Making the drivers require V4L1 in Kconfig
-
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/