Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
hw/net/msf2-emac: Don't modify descriptor in-place in emac_store_desc()
The msf2-emac ethernet controller has functions emac_load_desc() and
emac_store_desc() which read and write the in-memory descriptor
blocks and handle conversion between guest and host endianness.

As currently written, emac_store_desc() does the endianness
conversion in-place; this means that it effectively consumes the
input EmacDesc struct, because on a big-endian host the fields will
be overwritten with the little-endian versions of their values.
Unfortunately, in all the callsites the code continues to access
fields in the EmacDesc struct after it has called emac_store_desc()
-- specifically, it looks at the d.next field.

The effect of this is that on a big-endian host networking doesn't
work because the address of the next descriptor is corrupted.

We could fix this by making the callsite avoid using the struct; but
it's more robust to have emac_store_desc() leave its input alone.

(emac_load_desc() also does an in-place conversion, but here this is
fine, because the function is supposed to be initializing the
struct.)

Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 20230424151919.1333299-1-peter.maydell@linaro.org
(cherry picked from commit d565f58)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
  • Loading branch information
pm215 authored and Michael Tokarev committed May 30, 2023
1 parent ea6ce34 commit dc3e9aa
Showing 1 changed file with 11 additions and 7 deletions.
18 changes: 11 additions & 7 deletions hw/net/msf2-emac.c
Expand Up @@ -118,14 +118,18 @@ static void emac_load_desc(MSF2EmacState *s, EmacDesc *d, hwaddr desc)
d->next = le32_to_cpu(d->next);
}

static void emac_store_desc(MSF2EmacState *s, EmacDesc *d, hwaddr desc)
static void emac_store_desc(MSF2EmacState *s, const EmacDesc *d, hwaddr desc)
{
/* Convert from host endianness into LE. */
d->pktaddr = cpu_to_le32(d->pktaddr);
d->pktsize = cpu_to_le32(d->pktsize);
d->next = cpu_to_le32(d->next);

address_space_write(&s->dma_as, desc, MEMTXATTRS_UNSPECIFIED, d, sizeof *d);
EmacDesc outd;
/*
* Convert from host endianness into LE. We use a local struct because
* calling code may still want to look at the fields afterwards.
*/
outd.pktaddr = cpu_to_le32(d->pktaddr);
outd.pktsize = cpu_to_le32(d->pktsize);
outd.next = cpu_to_le32(d->next);

address_space_write(&s->dma_as, desc, MEMTXATTRS_UNSPECIFIED, &outd, sizeof outd);
}

static void msf2_dma_tx(MSF2EmacState *s)
Expand Down

0 comments on commit dc3e9aa

Please sign in to comment.