From: Peter Maydell <peter.maydell@linaro.org>
To: Alexandra Diupina <adiupina@astralinux.ru>
Cc: "Konrad, Frederic" <Frederic.Konrad@amd.com>,
Alistair Francis <alistair@alistair23.me>,
"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
"qemu-arm@nongnu.org" <qemu-arm@nongnu.org>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
"sdl.qemu@linuxtesting.org" <sdl.qemu@linuxtesting.org>
Subject: Re: [PATCH RFC] prevent overflow in xlnx_dpdma_desc_get_source_address()
Date: Tue, 23 Apr 2024 11:51:27 +0100 [thread overview]
Message-ID: <CAFEAcA9JXOxkbQP6-1uTK+hG5yvYRcO31PYFZSxGjfrPis1nYA@mail.gmail.com> (raw)
In-Reply-To: <aa967494-d3d1-4896-8006-b5fc2252a56e@astralinux.ru>
On Tue, 23 Apr 2024 at 11:23, Alexandra Diupina <adiupina@astralinux.ru> wrote:
> 17/04/24 13:05, Konrad, Frederic пишет:
> > Peter Maydell wrote:
> >> Also, this device looks like it has a host-endianness bug: the
> >> DPDMADescriptor struct is read directly from guest memory in
> >> dma_memory_read(), but the device never does anything to swap
> >> the fields from guest memory order to host memory order. So
> >> this is likely broken on big-endian hosts.
> hw/dma/xlnx_dpdma.c defines a dma_ops structure
> with the endianness field set to DEVICE_NATIVE_ENDIAN.
> Doesn't this guarantee that there will be no endian errors?
The endianness on a MemoryRegionOps struct tells the
memory core how to handle guest accesses to the
*registers* that struct is for. So if the .endianness
is DEVICE_LITTLE_ENDIAN and the guest is big-endian,
the memory core will byteswap the data values. That
means that the read and write functions always take
and return "value" arguments which are what you expect
(i.e. the guest wrote 0x12345678 and the write fn
value argument is 0x12345678).
However, if a device does direct accesses to guest *memory* (like
a DMA-capable device will do), that is something it has to handle
the endianness for itself. (The device's manual should say what
way round it does memory loads.)
There are two basic ways a device can do guest memory access:
(1) we provide functions which say "load from guest memory
a value of this type with this endianness"; for instance
ldq_le_dma() will load a 64-bit little-endian value into a
host C uint64_t, and ldq_be_dma() will do the same for a 64-bit
big-endian value. There are similar functions for stores.
This can be the simplest approach but it's a bit less efficient
because it goes up and down the memory subsystem for each data
item being read.
(2) we provide functions which are "load/store a sequence of bytes
from guest memory". This is what dma_memory_read() does.
Because it's just a sequence of bytes, the device code is
then responsible for interpreting what those bytes mean:
maybe they're just bytes, or maybe they're a sequence of
little-endian 32-bit values, or maybe they're something else.
The device code also has to deal with the fact that the
alignment of these values in memory is not necessarily the
same as what the host CPU requires. These things together
mean you can't simply cast the pointer to a sequence-of-bytes
to a host type, or tell dma_memory_read() to write the
sequence-of-bytes to a host struct type. We provide functions
like ldl_le_p() to say "read a little-endian 32-bit value
from this host pointer location" to assist in pulling
values out of a sequence-of-bytes array. Or if the data is
all of the same type (eg it's an array of 32-bit values)
you can dma_memory_read() it into a host uint32_t[] array
and then use le32_to_cpu() to convert those uint32_t values
to the host's endianness.
(I use the dma_* family of functions here as an example since that's
what xlnx_dpdma.c happens to use; the same general principle applies
for our various other families of guest load/store functions, like
the address_space_* ones.)
thanks
-- PMM
next prev parent reply other threads:[~2024-04-23 10:52 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-12 8:13 [PATCH RFC] prevent overflow in xlnx_dpdma_desc_get_source_address() Alexandra Diupina
2024-04-12 10:06 ` Peter Maydell
2024-04-16 17:56 ` Alexandra Diupina
2024-04-16 18:30 ` Edgar E. Iglesias
2024-04-17 10:05 ` Konrad, Frederic
2024-04-23 10:23 ` Alexandra Diupina
2024-04-23 10:51 ` Peter Maydell [this message]
2024-04-24 12:53 ` [PATCH v2 RFC] fix host-endianness bug and prevent overflow Alexandra Diupina
2024-04-24 16:04 ` Peter Maydell
2024-04-24 18:13 ` [PATCH] fix host-endianness bug Alexandra Diupina
2024-04-25 9:26 ` Peter Maydell
2024-04-25 10:07 ` [PATCH v2] " Alexandra Diupina
2024-04-25 10:42 ` Philippe Mathieu-Daudé
2024-04-25 13:41 ` [PATCH v3] fix endianness bug Alexandra Diupina
2024-04-25 15:24 ` Richard Henderson
2024-04-24 18:13 ` [PATCH] fix bit fields extraction and prevent overflow Alexandra Diupina
2024-04-25 19:25 ` Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFEAcA9JXOxkbQP6-1uTK+hG5yvYRcO31PYFZSxGjfrPis1nYA@mail.gmail.com \
--to=peter.maydell@linaro.org \
--cc=Frederic.Konrad@amd.com \
--cc=adiupina@astralinux.ru \
--cc=alistair@alistair23.me \
--cc=edgar.iglesias@gmail.com \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=sdl.qemu@linuxtesting.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.