yannick.naumann/RT-Task-Generator-QEMU
1
0
Fork 0
forked from alwin.berger/FRET-qemu
Code Pull Requests Activity

nbd/server: Clean up abuse of BlockExportOptionsNbd member @arg

Browse Source 
block-export-add argument @name defaults to the value of argument
@node-name.

nbd_export_create() implements this by copying @node_name to @name.
It leaves @has_node_name false, violating the "has_node_name ==
!!node_name" invariant.  Unclean.  Falls apart when we elide
@has_node_name (next commit): then QAPI frees the same value twice,
once for @node_name and once @name.  iotest 307 duly explodes.

Goes back to commit c62d24e906 "blockdev-nbd: Boxed argument type for
nbd-server-add" (v5.0.0).  Got moved from qmp_nbd_server_add() to
nbd_export_create() (commit 56ee86261e), then copied back (commit
b6076afcab).  Commit 8675cbd68b "nbd: Utilize QAPI_CLONE for type
conversion" (v5.2.0) cleaned up the copy in qmp_nbd_server_add()
noting

    Second, our assignment to arg->name is fishy: the generated QAPI code
    for qapi_free_NbdServerAddOptions does not visit arg->name if
    arg->has_name is false, but if it DID visit it, we would have
    introduced a double-free situation when arg is finally freed.

Exactly.  However, the copy in nbd_export_create() remained dirty.

Clean it up.  Since the value stored in member @name is not actually
used outside this function, use a local variable instead of modifying
the QAPI object.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: qemu-block@nongnu.org
Message-Id: <20221104160712.3005652-10-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
This commit is contained in:
Markus Armbruster 2022-11-04 17:06:51 +01:00
parent 04658a5b90
commit 8461b4d601
1 changed files with 6 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
nbd/server.c
View File

@ -1638,6 +1638,7 @@ static int nbd_export_create(BlockExport *blk_exp, BlockExportOptions *exp_args,
{ {
NBDExport *exp = container_of(blk_exp, NBDExport, common); NBDExport *exp = container_of(blk_exp, NBDExport, common);
BlockExportOptionsNbd *arg = &exp_args->u.nbd; BlockExportOptionsNbd *arg = &exp_args->u.nbd;
const char *name = arg->name ?: exp_args->node_name;
BlockBackend *blk = blk_exp->blk; BlockBackend *blk = blk_exp->blk;
int64_t size; int64_t size;
uint64_t perm, shared_perm; uint64_t perm, shared_perm;
@ -1653,12 +1654,8 @@ static int nbd_export_create(BlockExport *blk_exp, BlockExportOptions *exp_args,
return -EINVAL; return -EINVAL;
} }
if (!arg->has_name) { if (strlen(name) > NBD_MAX_STRING_SIZE) {
arg->name = exp_args->node_name; error_setg(errp, "export name '%s' too long", name);
}
if (strlen(arg->name) > NBD_MAX_STRING_SIZE) {
error_setg(errp, "export name '%s' too long", arg->name);
return -EINVAL; return -EINVAL;
} }
@ -1667,8 +1664,8 @@ static int nbd_export_create(BlockExport *blk_exp, BlockExportOptions *exp_args,
return -EINVAL; return -EINVAL;
} }
if (nbd_export_find(arg->name)) { if (nbd_export_find(name)) {
error_setg(errp, "NBD server already has export named '%s'", arg->name); error_setg(errp, "NBD server already has export named '%s'", name);
return -EEXIST; return -EEXIST;
} }
@ -1688,7 +1685,7 @@ static int nbd_export_create(BlockExport *blk_exp, BlockExportOptions *exp_args,
} }
QTAILQ_INIT(&exp->clients); QTAILQ_INIT(&exp->clients);
exp->name = g_strdup(arg->name); exp->name = g_strdup(name);
exp->description = g_strdup(arg->description); exp->description = g_strdup(arg->description);
exp->nbdflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_FLUSH | exp->nbdflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_FLUSH |
NBD_FLAG_SEND_FUA | NBD_FLAG_SEND_CACHE); NBD_FLAG_SEND_FUA | NBD_FLAG_SEND_CACHE);