Thread (28 messages) 28 messages, 8 authors, 2010-02-18

Re: [PATCH RFC] r8169: straighten out overlength frame detection (v3)

From: David Miller <davem@davemloft.net>
Date: 2010-01-07 01:15:09

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Thu, 7 Jan 2010 02:01:22 +0100
entry 024 opts1 0x20803ff0 opts2 0x00000000 size 1536 <- problem starts here.
entry 025 opts1 0x00803ff0 opts2 0x00000000 size 1536    The lines below are
entry 026 opts1 0x00803ff0 opts2 0x00000000 size 1536    not meaningful.
entry 027 opts1 0x00803ff0 opts2 0x00000000 size 1536

The driver will mess things up because it needs the desc->opts1.RxRes bit
to be set before it claims that the packet is broken. Since this condition
is not fulfilled when opts1 = 0x20803ff0, the problem goes unnoticed and
it can be further exploited.

AFAIUI, if the driver does not lose sync (to use the paper's words), i.e.
if it notices the condition above and stops / resets the chipset, there
is not much to exploit.
Thanks for doing this test.

Also note the FirstFrag and LastFrag bits.  Here we see FirstFrag set,
and this indicates a fragmented frame.

The fix seems to be to simply check the error bits unconditionally, or
alternatively validate the FirstFrag and LastFrag bits
unconditionally.

I don't even think we need to reset the chip, just do what the code
does now and recycle the buffer back to the chip as we currently do
when RxRes is set.  We can keep a state bit around, showing that we
saw the beginning of a fragmented frame, and we recycle buffers back
to the chip when in state state until we see LastFrag.

That should work, right?

In your trace, we're merely seeing the >1536 frame being chopped
into a fragmented frame by the device.  The first one has FirstFrag
set and the remaining (that you've shown) have neither bit set.

Did you eventually get a descriptor with LastFrag (bit 28) set?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help