[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/riscv/virt.c: Assemble plic_hart_config string with g_str
Re: [PATCH] hw/riscv/virt.c: Assemble plic_hart_config string with g_strjoinv()
Fri, 13 Aug 2021 10:19:55 +0100
On Fri, 13 Aug 2021 at 01:57, Alistair Francis <email@example.com> wrote:
> On Fri, Aug 13, 2021 at 2:17 AM Philippe Mathieu-Daudé <firstname.lastname@example.org>
> > 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 <email@example.com>
> 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!