Re: [PATCH] hw/riscv/virt.c: Assemble plic_hart_config string with g_strjoinv()

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

On Fri, 13 Aug 2021 at 01:57, Alistair Francis <alistair23@gmail.com> wrote:
>
> On Fri, Aug 13, 2021 at 2:17 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > Hi Peter,
> >
> > On 8/12/21 4:46 PM, Peter Maydell wrote:
> > > In the riscv virt machine init function, We assemble a string
> > > plic_hart_config which is a comma-separated list of N copies of the
> > > VIRT_PLIC_HART_CONFIG string.  The code that does this has a
> > > misunderstanding of the strncat() length argument.  If the source
> > > string is too large strncat() will write a maximum of length+1 bytes
> > > (length bytes from the source string plus a trailing NUL), but the
> > > code here assumes that it will write only length bytes at most.
> > >
> > > This isn't an actual bug because the code has correctly precalculated
> > > the amount of memory it needs to allocate so that it will never be
> > > too small (i.e.  we could have used plain old strcat()), but it does
> > > mean that the code looks like it has a guard against accidental
> > > overrun when it doesn't.
> > >
> > > Rewrite the string handling here to use the glib g_strjoinv()
> > > function, which means we don't need to do careful accountancy of
> > > string lengths, and makes it clearer that what we're doing is
> > > "create a comma-separated string".
> > >
> > > Fixes: Coverity 1460752
> > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> Thanks for fixing this Peter. Would you like this in for 6.1?

No, this isn't 6.1 material -- as I note in the commit message,
the current code isn't actually buggy, just a bit misleading.

> If you want I can fix the other boards?

That would be great, thanks!

-- PMM