Re: [PATCH] hw/dma: prevent overflow in soc_dma_set_request

[Peter Maydell]

On Tue, 9 Apr 2024 at 14:32, Anastasia Belova <abelova@astralinux.ru> wrote:
>
>
>
> 09/04/24 15:02, Peter Maydell пишет:
> > On Tue, 9 Apr 2024 at 12:54, Anastasia Belova <abelova@astralinux.ru> wrote:
> >> ch->num can reach values up to 31. Add casting to
> >> a larger type before performing left shift to
> >> prevent integer overflow.
> > If ch->num can only reach up to 31, then 1 << ch->num
> > is fine, because QEMU can assume that integers are 32 bits,
> > and we compile with -fwrapv so there isn't a problem with
> > shifting into the sign bit.
>
> Right, thanks for your comments.
> I didn't know about this flag before. It became more clear for me now.

Yep; if you're using a static analyser you probably want to
configure it to accept the behaviours that are
undefined-in-standard-C and which get defined behaviour
with -fwrapv.

This code is definitely a bit dubious, though, because
ch_enable_mask is a uint64_t, so the intention was clearly
to allow up to 64 channels. So I think we should take this
patch anyway, with a slightly adjusted commit message.

All the soc_dma.c code will probably be removed in the
9.2 release, because it's only used by the OMAP board models
which we've just deprecated, so it doesn't seem worth spending
too much time on cleaning up the code, but in this case you've
already written the patch.

I'll put this patch on my list to apply after we've made the
9.0 release and restarted development for 9.1.

thanks
-- PMM