Re: [v4l-dvb-maintainer] Re: [PATCH] V4L: struct video_device corruption

From: Trent Piepho <xyzzy@speakeasy.org>
To: Andrew Morton <akpm@osdl.org>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>,
	robfitz@273k.net,
	Linux and Kernel Video <video4linux-list@redhat.com>,
	76306.1226@compuserve.com, fork0@t-online.de, greg@kroah.com,
	linux-kernel@vger.kernel.org, rdunlap@xenotime.net,
	v4l-dvb maintainer list <v4l-dvb-maintainer@linuxtv.org>,
	shemminger@osdl.org
Subject: Re: [v4l-dvb-maintainer] Re: [PATCH] V4L: struct video_device corruption
Date: Tue, 25 Jul 2006 01:42:19 -0700 (PDT)	[thread overview]
Message-ID: <Pine.LNX.4.58.0607250054410.18397@shell3.speakeasy.net> (raw)
In-Reply-To: <20060724200855.603be3bb.akpm@osdl.org>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=X-UNKNOWN, Size: 2026 bytes --]

On Mon, 24 Jul 2006, Andrew Morton wrote:
> On Sat, 15 Jul 2006 22:31:04 -0300
> Mauro Carvalho Chehab <mchehab@infradead.org> wrote:
>
> > Em Sáb, 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.
> > Hmm... good point! However, I the proper solution would be to trust on
> > CONFIG_VIDEO_V4L1_COMPAT or CONFIG_VIDEO_V4L1 instead. it makes no sense
> > to keep a pointer to an unsupported callback, when V4L1 is not selected.
> >
>
> So I've lost the plot with all of this.  Does the current git-dvb contain
> the desired fixes?

The problem was that the v4l code in general was not using
CONFIG_VIDEO_V4L1* like it should, but was detecting V4L1 support based on
whether on not the V4L1 header file had been included.

Some drivers didn't depend on V4L1 in Kconfig, and would include the V4L1
header when V4L1 was off.  This caused some code to think V4L1 was off and
some code to think it was on.

This caused a serious bug with inconsistent defintions of struct
video_device, as well as other problems.

I posted a patch that fixed the video_device problem, then Mauro made a
comprehensive patch that incorporated this fix and well as all the other
code that was using V4L1 when it shouldn't.  That patch is in the v4l-dvb
Mercurial repository, but not moved on to git yet.

> Do we expect this will fix the various DVB crashes which people (including
> Alex) have reported?

This problem would only appear if VIDEO_V4L1 was turned off.  If it was on,
then all the code would agree it was on, and there would be no problems.
If the crash is still there when VIDEO_V4L1 = y, then it's not related to
this bug.

If VIDEO_V4L1 was turned off, then some drivers (one of which is bttv)
would have a different struct video_device than the video core code.  This
would break things so completely that it could crash just about anywhere.