Hello, Stefan
The problem occurs between steps 2 and 3.
Let's say packet arrives after step 2 is done by driver.
Head and tail are 0 because of step 1
Check_rxov is 0 because of two reasons:
1. On startup it is 0 by default
2. It is zeroed by setting ring tail to 0 on first step
Then first check ( __if (!(s->mac_reg[RCTL] & E1000_RCTL_EN))__ )
passes because RX enabled on step 2.
e1000_has_rxbufs() returs true because it treats equal head and tail
as fully filled ring when check_rxov is 0:
static bool e1000_has_rxbufs(E1000State *s, size_t total_size)
{
[...]
if (total_size <= s->rxbuf_size) {
return s->mac_reg[RDH] != s->mac_reg[RDT] || !s->check_rxov;
[...]
} else if (s->mac_reg[RDH] > s->mac_reg[RDT] || !s->check_rxov) {
[...]
}
So QEMU reads uninitialized descriptor and tries to perform "DMA" to
arbitrary address from descriptor.
Depending on address value it corrupts guest memory or abort()'s here:
void *qemu_get_ram_ptr(ram_addr_t addr)
{
[...]
fprintf(stderr, "Bad ram offset %" PRIx64 "\n", (uint64_t)addr);
abort();
[...]
}
Thanks for review,
Dmitry.
On Thu, Oct 18, 2012 at 9:31 AM, Stefan Hajnoczi <stefa...@gmail.com> wrote:
>
> On Wed, Oct 17, 2012 at 08:31:46PM +0200, Dmitry Fleytman wrote:
> > Device RX initization from driver's side consists of following steps:
> > 1. Initialize head and tail of RX ring to 0
> > 2. Enable Rx (set bit in RCTL register)
> > 3. Allocate buffers, fill descriptors
> > 4. Write ring tail
> >
> > Forth operation signals hardware that RX buffers available
> > and it may start packets indication.
> >
> > Current implementation treats first operation (write 0 to ring tail)
> > as signal of buffers availability and starts data transfers as soon
> > as RX enable indicaton arrives.
> >
> > This is not correct because there is a chance that ring is still
> > empty (third action not performed yet) and then memory corruption
> > occures.
>
> The existing code tries to prevent this:
>
> e1000_receive(NetClientState *nc, const uint8_t *buf, size_t size)
> {
> [...]
>
> if (!(s->mac_reg[RCTL] & E1000_RCTL_EN))
> return -1;
>
> [...]
> total_size = size + fcs_len(s);
> if (!e1000_has_rxbufs(s, total_size)) {
> set_ics(s, 0, E1000_ICS_RXO);
> return -1;
> }
>
> Why are these checks not enough?
>
> Which memory gets corrupted?
>
> Stefan
--
Dmitry Fleytman
Technology Expert and Consultant,
Daynix Computing Ltd.
Cell: +972-54-2819481
Skype: dmitry.fleytman
