All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: Alexandra Diupina <adiupina@astralinux.ru>,
	Alistair Francis <alistair@alistair23.me>
Cc: "Konrad, Frederic" <Frederic.Konrad@amd.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	qemu-arm@nongnu.org, qemu-devel@nongnu.org,
	sdl.qemu@linuxtesting.org
Subject: Re: [PATCH v2] fix host-endianness bug
Date: Thu, 25 Apr 2024 12:42:21 +0200	[thread overview]
Message-ID: <074c9cbb-939e-4fb1-87a9-235626bf3a94@linaro.org> (raw)
In-Reply-To: <20240425100718.20184-1-adiupina@astralinux.ru>

Hi Alexandra,

On 25/4/24 12:07, Alexandra Diupina wrote:
> Add a function xlnx_dpdma_read_descriptor() that
> combines reading the descriptor from desc_addr
> by calling dma_memory_read() and swapping desc
> fields from guest memory order to host memory order.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: d3c6369a96 ("introduce xlnx-dpdma")
> Signed-off-by: Alexandra Diupina <adiupina@astralinux.ru>
> ---
> v2:minor changes in xlnx_dpdma_read_descriptor()
>   hw/dma/xlnx_dpdma.c | 31 +++++++++++++++++++++++++++++--
>   1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c
> index dd66be5265..62a0952377 100644
> --- a/hw/dma/xlnx_dpdma.c
> +++ b/hw/dma/xlnx_dpdma.c
> @@ -614,6 +614,34 @@ static void xlnx_dpdma_register_types(void)
>       type_register_static(&xlnx_dpdma_info);
>   }
>   
> +static MemTxResult xlnx_dpdma_read_descriptor(XlnxDPDMAState *s,
> +                                    uint64_t desc_addr, DPDMADescriptor *desc)
> +{
> +    if (dma_memory_read(&address_space_memory, desc_addr, &desc,
> +                            sizeof(DPDMADescriptor), MEMTXATTRS_UNSPECIFIED))
> +        return MEMTX_ERROR;
> +
> +    /* Convert from LE into host endianness.  */
> +    desc->control = le32_to_cpu(desc->control);
> +    desc->descriptor_id = le32_to_cpu(desc->descriptor_id);
> +    desc->xfer_size = le32_to_cpu(desc->xfer_size);
> +    desc->line_size_stride = le32_to_cpu(desc->line_size_stride);
> +    desc->timestamp_lsb = le32_to_cpu(desc->timestamp_lsb);
> +    desc->timestamp_msb = le32_to_cpu(desc->timestamp_msb);
> +    desc->address_extension = le32_to_cpu(desc->address_extension);
> +    desc->next_descriptor = le32_to_cpu(desc->next_descriptor);
> +    desc->source_address = le32_to_cpu(desc->source_address);
> +    desc->address_extension_23 = le32_to_cpu(desc->address_extension_23);
> +    desc->address_extension_45 = le32_to_cpu(desc->address_extension_45);
> +    desc->source_address2 = le32_to_cpu(desc->source_address2);
> +    desc->source_address3 = le32_to_cpu(desc->source_address3);
> +    desc->source_address4 = le32_to_cpu(desc->source_address4);
> +    desc->source_address5 = le32_to_cpu(desc->source_address5);
> +    desc->crc = le32_to_cpu(desc->crc);
> +
> +    return MEMTX_OK;
> +}
> +
>   size_t xlnx_dpdma_start_operation(XlnxDPDMAState *s, uint8_t channel,
>                                       bool one_desc)
>   {
> @@ -651,8 +679,7 @@ size_t xlnx_dpdma_start_operation(XlnxDPDMAState *s, uint8_t channel,
>               desc_addr = xlnx_dpdma_descriptor_next_address(s, channel);
>           }
>   
> -        if (dma_memory_read(&address_space_memory, desc_addr, &desc,
> -                            sizeof(DPDMADescriptor), MEMTXATTRS_UNSPECIFIED)) {
> +        if (xlnx_dpdma_read_descriptor(s, desc_addr, &desc)) {

Correct, but this is incomplete, because we have the same problem
on the write descriptor back path, few lines later in the
xlnx_dpdma_desc_update_enabled() block.

>               s->registers[DPDMA_EISR] |= ((1 << 1) << channel);
>               xlnx_dpdma_update_irq(s);
>               s->operation_finished[channel] = true;



  reply	other threads:[~2024-04-25 10:46 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
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é [this message]
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=074c9cbb-939e-4fb1-87a9-235626bf3a94@linaro.org \
    --to=philmd@linaro.org \
    --cc=Frederic.Konrad@amd.com \
    --cc=adiupina@astralinux.ru \
    --cc=alistair@alistair23.me \
    --cc=edgar.iglesias@gmail.com \
    --cc=peter.maydell@linaro.org \
    --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.