2006-01-25 10:24:34

by Duncan Sands

[permalink] [raw]
Subject: [PATCH] bttv: correct bttv_risc_packed buffer size

This patch fixes the strange crashes I was seeing after using
my bttv card to watch television. They were caused by a
buffer overflow in bttv_risc_packed.

The instruction buffer size calculation contains two errors:
(a) a non-zero padding value can push the start of the next bpl
section to just before a page border, leading to more scanline
splits and thus additional instructions.
(b) the first DMA region can be smaller than one page, so there can
be a scanline split even if bpl*lines is smaller than PAGE_SIZE.

For example, consider the case where offset is 0, bpl is 2, padding
is 4094, lines is smaller than 2048, the first DMA region has size 1
and all others have size PAGE_SIZE, assumed to equal 4096. Then
all bpl regions cross page borders and the number of instructions
written is 2*lines+2, rather than lines+2 (the current estimate).
With this patch the number of instructions for this example is
estimated to be 2*lines+3.

Also, the BUG_ON that was supposed to catch buffer overflows contained
a thinko causing it fire only if the buffer was overrun by a factor of
16 or more.

I didn't check whether similar mistakes exist elsewhere in the bttv
code.

Signed-off-by: Duncan Sands <[email protected]>

PS: I'm sending the patch as an attachment because for some reason my
mailer crashes if I try to insert it into the email.


Attachments:
(No filename) (1.31 kB)
bttv (1.03 kB)
Download all attachments

2006-01-25 18:41:49

by Bodo Eggert

[permalink] [raw]
Subject: Re: [PATCH] bttv: correct bttv_risc_packed buffer size

Duncan Sands <[email protected]> wrote:

> This patch fixes the strange crashes I was seeing after using
> my bttv card to watch television. They were caused by a
> buffer overflow in bttv_risc_packed.

<snip>

Would these errors e.g. cause a corruption of exactly four bytes at the start
of random pages?
--
Ich danke GMX daf?r, die Verwendung meiner Adressen mittels per SPF
verbreiteten L?gen zu sabotieren.

2006-01-26 08:02:33

by Duncan Sands

[permalink] [raw]
Subject: Re: [PATCH] bttv: correct bttv_risc_packed buffer size

Hi Bodo,

> > This patch fixes the strange crashes I was seeing after using
> > my bttv card to watch television. They were caused by a
> > buffer overflow in bttv_risc_packed.
>
> <snip>
>
> Would these errors e.g. cause a corruption of exactly four bytes at the start
> of random pages?

I don't think so. It should cause either no corruption or at least 8 bytes worth
(it does pairs of 4 byte writes). What you might see is an Oops when it tries to
write the first 4 bytes at the start of a page, because of a page fault, but then
the write doesn't happen and there is no corruption...

Best wishes,

Duncan.