From: Peter Maydell <peter.maydell@linaro.org>
To: Alistair Francis <alistair.francis@wdc.com>
Cc: Alistair Francis <alistair23@gmail.com>,
Anup Patel <anup.patel@wdc.com>,
QEMU Developers <qemu-devel@nongnu.org>,
Atish Patra <atish.patra@wdc.com>
Subject: Re: [PULL 05/18] hw/riscv: virt: Allow creating multiple NUMA sockets
Date: Mon, 9 Aug 2021 10:46:16 +0100 [thread overview]
Message-ID: <CAFEAcA94GQMqsRp3tQ7NF7+fA2BK+PdJZScHWgLM5x12RVOiZg@mail.gmail.com> (raw)
In-Reply-To: <20200825184836.1282371-6-alistair.francis@wdc.com>
On Tue, 25 Aug 2020 at 20:03, Alistair Francis <alistair.francis@wdc.com> wrote:
>
> From: Anup Patel <anup.patel@wdc.com>
>
> We extend RISC-V virt machine to allow creating a multi-socket
> machine. Each RISC-V virt machine socket is a NUMA node having
> a set of HARTs, a memory instance, a CLINT instance, and a PLIC
> instance. Other devices are shared between all sockets. We also
> update the generated device tree accordingly.
Hi; Coverity (CID 1460752) points out that this code has
a misunderstanding of the length argument to strncat().
(I think this patch is just doing code-movement of this block of code,
but it seemed like the easiest place to send an email about the issue.)
> + /* Per-socket PLIC hart topology configuration string */
> + plic_hart_config_len =
> + (strlen(VIRT_PLIC_HART_CONFIG) + 1) * hart_count;
> + plic_hart_config = g_malloc0(plic_hart_config_len);
> + for (j = 0; j < hart_count; j++) {
> + if (j != 0) {
> + strncat(plic_hart_config, ",", plic_hart_config_len);
> + }
> + strncat(plic_hart_config, VIRT_PLIC_HART_CONFIG,
> + plic_hart_config_len);
> + plic_hart_config_len -= (strlen(VIRT_PLIC_HART_CONFIG) + 1);
> + }
The length argument to strncat() is here being used as if it were
"do not write more than length bytes", but strncat() will write
length+1 bytes in the "source too long" case (length characters
from the source string plus the trailing NUL). This isn't actually
an issue here because we carefully precalculate the allocation length
to be exactly correct, but it means that the code looks like it has
a guard against accidental miscalculation and overrun but it doesn't.
It might be preferable to write this to use glib string methods
rather than raw strlen/strncat, for example:
const char **vals;
const char *hart_config = VIRT_PLIC_HART_CONFIG;
int i;
vals = g_new(char *, hart_count + 1);
for (i = 0; i < hart_count; i++) {
vals[i] = (gchar *)hart_config;
}
vals[i] = NULL;
/* g_strjoinv() obliges us to discard 'const' here :-( */
plic_hart_config = g_strjoinv(",", (char **)vals);
Untested, but same structure as used in ppc_compat_add_property()
and qemu_rbd_mon_host(). Relieves us of the obligation to
carefully calculate string lengths and makes it clearer that
we're constructing a comma-separated string.
thanks
-- PMM
next prev parent reply other threads:[~2021-08-09 9:49 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-25 18:48 [PULL 00/18] riscv-to-apply queue Alistair Francis
2020-08-25 18:48 ` [PULL 01/18] hw/riscv: Allow creating multiple instances of CLINT Alistair Francis
2020-08-25 18:48 ` [PULL 02/18] hw/riscv: Allow creating multiple instances of PLIC Alistair Francis
2020-08-25 18:48 ` [PULL 03/18] hw/riscv: Add helpers for RISC-V multi-socket NUMA machines Alistair Francis
2020-08-25 18:48 ` [PULL 04/18] hw/riscv: spike: Allow creating multiple NUMA sockets Alistair Francis
2020-08-25 18:48 ` [PULL 05/18] hw/riscv: virt: " Alistair Francis
2021-08-09 9:46 ` Peter Maydell [this message]
2021-08-12 14:57 ` Peter Maydell
2020-08-25 18:48 ` [PULL 06/18] target/riscv: Allow setting a two-stage lookup in the virt status Alistair Francis
2020-08-25 18:48 ` [PULL 07/18] target/riscv: Allow generating hlv/hlvx/hsv instructions Alistair Francis
2020-08-25 18:48 ` [PULL 08/18] target/riscv: Do two-stage lookups on " Alistair Francis
2020-08-25 18:48 ` [PULL 09/18] target/riscv: Don't allow guest to write to htinst Alistair Francis
2020-08-25 18:48 ` [PULL 10/18] target/riscv: Convert MSTATUS MTL to GVA Alistair Francis
2020-08-25 18:48 ` [PULL 11/18] target/riscv: Fix the interrupt cause code Alistair Francis
2020-08-25 18:48 ` [PULL 12/18] target/riscv: Update the Hypervisor trap return/entry Alistair Francis
2020-08-25 18:48 ` [PULL 13/18] target/riscv: Update the CSRs to the v0.6 Hyp extension Alistair Francis
2020-08-25 18:48 ` [PULL 14/18] target/riscv: Only support a single VSXL length Alistair Francis
2020-08-25 18:48 ` [PULL 15/18] target/riscv: Only support little endian guests Alistair Francis
2020-08-25 18:48 ` [PULL 16/18] target/riscv: Support the v0.6 Hypervisor extension CRSs Alistair Francis
2020-08-25 18:48 ` [PULL 17/18] target/riscv: Return the exception from invalid CSR accesses Alistair Francis
2020-08-25 18:48 ` [PULL 18/18] target/riscv: Support the Virtual Instruction fault Alistair Francis
2020-08-25 21:24 ` [PULL 00/18] riscv-to-apply queue Peter Maydell
2020-08-25 21:21 ` Alistair Francis
2020-08-25 21:49 ` Peter Maydell
2020-08-25 22:30 ` Alistair Francis
2020-08-26 3:21 ` Bin Meng
2020-08-26 9:25 ` Peter Maydell
2020-08-26 10:06 ` Bin Meng
2020-08-27 15:44 ` Alistair Francis
2020-08-29 15:49 ` LIU Zhiwei
2020-08-29 17:30 ` Alistair Francis
2020-08-26 9:28 ` 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=CAFEAcA94GQMqsRp3tQ7NF7+fA2BK+PdJZScHWgLM5x12RVOiZg@mail.gmail.com \
--to=peter.maydell@linaro.org \
--cc=alistair.francis@wdc.com \
--cc=alistair23@gmail.com \
--cc=anup.patel@wdc.com \
--cc=atish.patra@wdc.com \
--cc=qemu-devel@nongnu.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).