Re: [PATCH 1/1] qemu-nbd: regression with arguments passing into nbd_client_thread()

[Eric Blake]

On Wed, Jul 26, 2023 at 04:52:47PM +0200, Denis V. Lunev wrote:
> Unfortunately
>     commit 03b67621445d601c9cdc7dfe25812e9f19b81488
>     Author: Denis V. Lunev <den@openvz.org>
>     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 <den@openvz.org>
> CC: Eric Blake <eblake@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: <qemu-stable@nongnu.org>
> ---
>  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 <eblake@redhat.com>

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