[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20081121131452.24508d6e@pedra.chehab.org>
Date: Fri, 21 Nov 2008 13:14:52 -0200
From: Mauro Carvalho Chehab <mchehab@...hat.com>
To: "Eugene Teo" <eugeneteo@...nel.sg>
Cc: oss-security@...ts.openwall.com,
"Steven M. Christey"
<coley@...us.mitre.org>
Subject: Re: CVE request: kernel: V4L/DVB (9621): Avoid
writing outside shadow.bytes[] array
Eugene/Steve,
On Fri, 21 Nov 2008 10:26:26 +0800
"Eugene Teo" <eugeneteo@...nel.sg> wrote:
> Hi Steve,
>
> On Fri, Nov 21, 2008 at 9:59 AM, Steven M. Christey
> <coley@...us.mitre.org> wrote:
> >
> > On Wed, 19 Nov 2008, Eugene Teo wrote:
> >
> >> If the write operation fails, the device won't be able to decode audio
> >> signals properly, so on further analysis, we probably don't need a CVE
> >> name for this. Take note.
> >
> > Does this mean, roughly, that this write only occurs into a different
> > portion of a larger contiguous buffer, so it affects audio processing
> > (e.g. throwing an error) or parsing, but otherwise can't be used to affect
> > other memory locations outside that buffer?
No. This buffer is just a shadow buffer for storing the previous register value.
> To be honest, I'm not entirely familiar with the bug. I have Cc'ed
> Mauro who is the maintainer of the driver, and he should be able to
> share with us more about it.
>
> Mauro, can you explain to us the implications of not including the
> "V4L/DVB (9621): Avoid writing outside shadow.bytes[] array" fix, and
> if it has a security consequence?
This patch doesn't fix any security breach I'm aware.
Basically, tvaudio is one of the oldest modules at the multimedia subsystem.
The driver were written back on 2000 by a developer that is not maintaining it
anymore. Since then, most updates were just API changes that happened on other
parts of the kernel that required changes at the module.
When CVE-2008-5033 were discovered, I did a deep analysis at the driver, since
the proposed fix didn't seem to solve the bug [1].
On that analysis, I noticed that, on several places, there weren't proper
checks before calling callbacks, no array limit checks, and other miscellaneous
troubles. So, I added 10 patches to add additional checks to minimize the
potential problems (since CVE-2008-5033 might be caused by some buffer
overflow) plus one patch that actually fixed CVE-2008-5033.
In the case of this specific patch, it fixes the routines that access the
audio chip registers. Those routines are used only internally, and maintains a
local cache of the register values (shadow.bytes).
I saw two troubles fixed by the patch:
1) negative calls with values different from -1;
2) calls above the array size (64).
There were no place at the driver that would warrant that the above potential
risks wouldn't happen.
Yet, the register values come from a static table that it is internal to the
driver and from explicit register calls, using register aliases.
A deep inspection was done later at the driver, looking for every call to the
register write routines. On every register value currently used inside the
driver, the values were non-negative and bellow the maximum size.
So, I couldn't see any real case where it is writing outside the array
limits.
The test is still important to be at the driver, since a future patch could
eventually be applied, introducing some bug at the driver that could cause a
buffer overflow. So, it is a preventive control added at the source code.
[1] I did some tests later with a real hardware. Even applying the original
upstream patch that were suppose to fix the bug, I could reproduce the bug
locally, since the bug were on another place of the driver. The upstream patch
I sent for CVE-2008-5033 properly fixed it.
Cheers,
Mauro
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
