qemu-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 97b1d8: hw/9pfs: avoid 'path' copy in v9fs_wa


From: Peter Maydell
Subject: [Qemu-commits] [qemu/qemu] 97b1d8: hw/9pfs: avoid 'path' copy in v9fs_walk()
Date: Fri, 03 Sep 2021 00:33:09 -0700

  Branch: refs/heads/staging
  Home:   https://github.com/qemu/qemu
  Commit: 97b1d8fdf6c923203968f44805e25dc92b11a317
      
https://github.com/qemu/qemu/commit/97b1d8fdf6c923203968f44805e25dc92b11a317
  Author: Christian Schoenebeck <qemu_oss@crudebyte.com>
  Date:   2021-09-02 (Thu, 02 Sep 2021)

  Changed paths:
    M hw/9pfs/9p.c

  Log Message:
  -----------
  hw/9pfs: avoid 'path' copy in v9fs_walk()

The v9fs_walk() function resolves all client submitted path nodes to the
local 'pathes' array. Using a separate string scalar variable 'path'
inside the background worker thread loop and copying that local 'path'
string scalar variable subsequently to the 'pathes' array (at the end of
each loop iteration) is not necessary.

Instead simply resolve each path directly to the 'pathes' array and
don't use the string scalar variable 'path' inside the fs worker thread
loop at all.

The only advantage of the 'path' scalar was that in case of an error
the respective 'pathes' element would not be filled. Right now this is
not an issue as the v9fs_walk() function returns as soon as any error
occurs.

Suggested-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: 
<7dacbecf25b2c9b4a0ce12d689a8a535f09a31e3.1629208359.git.qemu_oss@crudebyte.com>


  Commit: 869605b5a076e231ae36c54866f348b9bdf18f76
      
https://github.com/qemu/qemu/commit/869605b5a076e231ae36c54866f348b9bdf18f76
  Author: Christian Schoenebeck <qemu_oss@crudebyte.com>
  Date:   2021-09-02 (Thu, 02 Sep 2021)

  Changed paths:
    M hw/9pfs/9p.c

  Log Message:
  -----------
  hw/9pfs: use g_autofree in v9fs_walk() where possible

Suggested-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: 
<b51670d2a39399535a035f6bc77c3cbeed85edae.1629208359.git.qemu_oss@crudebyte.com>


  Commit: f83df00900816476cca41bb536e4d532b297d76e
      
https://github.com/qemu/qemu/commit/f83df00900816476cca41bb536e4d532b297d76e
  Author: Christian Schoenebeck <qemu_oss@crudebyte.com>
  Date:   2021-09-02 (Thu, 02 Sep 2021)

  Changed paths:
    M hw/9pfs/coth.h

  Log Message:
  -----------
  9pfs: fix crash in v9fs_walk()

v9fs_walk() utilizes the v9fs_co_run_in_worker({...}) macro to run the
supplied fs driver code block on a background worker thread.

When either the 'Twalk' client request was interrupted or if the client
requested fid for that 'Twalk' request caused a stat error then that
fs driver code block was left by 'break' keyword, with the intention to
return from worker thread back to main thread as well:

    v9fs_co_run_in_worker({
        if (v9fs_request_cancelled(pdu)) {
            err = -EINTR;
            break;
        }
        err = s->ops->lstat(&s->ctx, &dpath, &fidst);
        if (err < 0) {
            err = -errno;
            break;
        }
        ...
    });

However that 'break;' statement also skipped the v9fs_co_run_in_worker()
macro's final and mandatory

    /* re-enter back to qemu thread */
    qemu_coroutine_yield();

call and thus caused the rest of v9fs_walk() to be continued being
executed on the worker thread instead of main thread, eventually
leading to a crash in the transport virtio transport driver.

To fix this issue and to prevent the same error from happening again by
other users of v9fs_co_run_in_worker() in future, auto wrap the supplied
code block into its own

    do { } while (0);

loop inside the 'v9fs_co_run_in_worker' macro definition.

Full discussion and backtrace:
https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg05209.html
https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg00174.html

Fixes: 8d6cb100731c4d28535adbf2a3c2d1f29be3fef4
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Cc: qemu-stable@nongnu.org
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <E1mLTBg-0002Bh-2D@lizzy.crudebyte.com>


  Commit: 8880cc4362fde4ecdac0b2092318893118206fcf
      
https://github.com/qemu/qemu/commit/8880cc4362fde4ecdac0b2092318893118206fcf
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2021-09-03 (Fri, 03 Sep 2021)

  Changed paths:
    M hw/9pfs/9p.c
    M hw/9pfs/coth.h

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/cschoenebeck/tags/pull-9p-20210902' 
into staging

9pfs: misc patches

* Fix an occasional crash when handling 'Twalk' requests.

* Two code cleanup patches.

# gpg: Signature made Thu 02 Sep 2021 12:42:32 BST
# gpg:                using RSA key 96D8D110CF7AF8084F88590134C2B58765A47395
# gpg:                issuer "qemu_oss@crudebyte.com"
# gpg: Good signature from "Christian Schoenebeck <qemu_oss@crudebyte.com>" 
[unknown]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: ECAB 1A45 4014 1413 BA38  4926 30DB 47C3 A012 D5F4
#      Subkey fingerprint: 96D8 D110 CF7A F808 4F88  5901 34C2 B587 65A4 7395

* remotes/cschoenebeck/tags/pull-9p-20210902:
  9pfs: fix crash in v9fs_walk()
  hw/9pfs: use g_autofree in v9fs_walk() where possible
  hw/9pfs: avoid 'path' copy in v9fs_walk()

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Compare: https://github.com/qemu/qemu/compare/8664d30a30fd...8880cc4362fd



reply via email to

[Prev in Thread] Current Thread [Next in Thread]