On Wed, Jul 26, 2023 at 04:52:47PM +0200, Denis V. Lunev wrote: > Unfortunately > commit 03b67621445d601c9cdc7dfe25812e9f19b81488 > Author: Denis V. Lunev <[email protected]> > Date: Mon Jul 17 16:55:40 2023 +0200 > qemu-nbd: pass structure into nbd_client_thread instead of plain char* > has introduced a regression. struct NbdClientOpts resides on stack inside > 'if' block. This specifically means that this stack space could be reused > once the execution will leave that block of the code. > > This means that parameters passed into nbd_client_thread could be > overwritten at any moment. > > The patch moves the data to the namespace of main() function effectively > preserving it for the whole process lifetime. > > Signed-off-by: Denis V. Lunev <[email protected]> > CC: Eric Blake <[email protected]> > CC: Vladimir Sementsov-Ogievskiy <[email protected]> > CC: <[email protected]> > --- > qemu-nbd.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/qemu-nbd.c b/qemu-nbd.c > index 5b2757920c..7a15085ade 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -589,6 +589,7 @@ int main(int argc, char **argv) > const char *pid_file_name = NULL; > const char *selinux_label = NULL; > BlockExportOptions *export_opts; > + struct NbdClientOpts opts; > > #ifdef CONFIG_POSIX > os_setup_early_signal_handling(); > @@ -1145,7 +1146,7 @@ int main(int argc, char **argv) > if (device) { > #if HAVE_NBD_DEVICE > int ret; > - struct NbdClientOpts opts = { > + opts = (struct NbdClientOpts) {
Does this case a compiler warning for an unused variable when HAVE_NBD_DEVICE is not set? If so, the solution is to also wrap the declaration in the same #if. I'll see if I can figure out the CI enough to prove (or disprove) my theory on a BSD machine which likely lacks HAVE_NBD_DEVICE. With that addressed, I'm fine with: Reviewed-by: Eric Blake <[email protected]> and I will queue it through my NBD tree in time for 8.1-rc2. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
