On Wed, Apr 30, 2025 at 07:27:29AM +0200, Mario Fleischmann wrote:
> This commit implements the necessary operations required to establish
> a connection with the MCD server:
>
> * query information about the server
> * connect to "
> * disconnect from "
>
> Signed-off-by: Mario Fleischmann <mario.fleischm...@lauterbach.com>
> ---
> mcd/mcd_qapi.c | 13 +++
> mcd/mcd_qapi.h | 2 +
> mcd/mcd_server.c | 110 +++++++++++++++++++++-
> mcd/mcd_stub.c | 98 ++++++++++++++++++++
> qapi/mcd.json | 205 +++++++++++++++++++++++++++++++++++++++++
> tests/qtest/mcd-test.c | 96 +++++++++++++++++++
> tests/qtest/mcd-util.c | 60 ++++++++++++
> tests/qtest/mcd-util.h | 9 ++
> 8 files changed, 588 insertions(+), 5 deletions(-)
>
> diff --git a/mcd/mcd_qapi.c b/mcd/mcd_qapi.c
> index 9a99866..d2a2926 100644
> --- a/mcd/mcd_qapi.c
> +++ b/mcd/mcd_qapi.c
> +MCDQryServersResult *qmp_mcd_qry_servers(const char *host, bool running,
> + uint32_t start_index,
> + uint32_t num_servers, Error **errp)
> +{
> + MCDServerInfoList **tailp;
> + MCDServerInfo *info;
> + mcd_server_info_st *server_info = NULL;
> + bool query_num_only = num_servers == 0;
> + MCDQryServersResult *result = g_malloc0(sizeof(*result));
> +
> + if (!query_num_only) {
> + server_info = g_malloc0(num_servers * sizeof(*server_info));
This multiplication is (theoretically) subject to overflow. To eliminate
this risk, this should use
g_new0(mcd_server_info_st, num_servers)
which will validate overflow & abort if hit.
There are many more instances of this code pattern in the series
$ git diff -r master | grep g_malloc | grep ' \* '
+ .tx = g_malloc(txlist->num_tx * sizeof(mcd_tx_st)),
+ server_info = g_malloc0(num_servers * sizeof(*server_info));
+ system_con_info = g_malloc0(num_systems * sizeof(*system_con_info));
+ device_con_info = g_malloc0(num_devices * sizeof(*device_con_info));
+ core_con_info = g_malloc0(num_cores * sizeof(*core_con_info));
+ memspaces = g_malloc0(num_mem_spaces * sizeof(*memspaces));
+ reg_groups = g_malloc0(num_reg_groups * sizeof(*reg_groups));
+ regs = g_malloc0(num_regs * sizeof(*regs));
+ ctrig_info = g_malloc0(num_ctrigs * sizeof(*ctrig_info));
+ trig_ids = g_malloc0(num_trigs * sizeof(*trig_ids));
QEMU is a bit inconsistent, but we have a slight bias in favour
of using g_new0, even for single struct allocations.
IMHO being in the habit of always using g_new0 instead of g_malloc
makes it less likely for people to inadvertantly introduce the
multiplication overflow code pattern with g_malloc.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
