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

[Peter Maydell <peter.maydell@linaro.org>]

On Tue, 9 Apr 2024 at 14:38, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> 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.

Now applied to target-arm.next for 9.1 (with adjustments
to the commit message); thanks.

-- PMM