All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: Maxim Levitsky <mlevitsk@redhat.com>
Subject: [PULL 10/30] qemu-pr-helper: fix crash in mpath_reconstruct_sense
Date: Wed,  2 Oct 2019 18:51:33 +0200	[thread overview]
Message-ID: <1570035113-56848-11-git-send-email-pbonzini@redhat.com> (raw)
In-Reply-To: <1570035113-56848-1-git-send-email-pbonzini@redhat.com>

From: Maxim Levitsky <mlevitsk@redhat.com>

The 'r' variable was accidently shadowed, and because of this
we were always passing 0 to mpath_generic_sense, instead of original
return value, which triggers an abort()

This is an attempt to fix the
https://bugzilla.redhat.com/show_bug.cgi?id=1720047
although there might be other places in the code
that trigger qemu-pr-helper crash, and this fix might
not be the root cause.

The crash was reproduced by creating an iscsi target on a test machine,
and passing it twice to the guest like that:

-blockdev node-name=idisk0,driver=iscsi,transport=...,target=...
-device scsi-block,drive=idisk0,bus=scsi0.0,bootindex=-1,scsi-id=1,lun=0,share-rw=on
-device scsi-block,drive=idisk0,bus=scsi0.0,bootindex=-1,scsi-id=1,lun=1,share-rw=on

Then in the guest, both /dev/sda and /dev/sdb were aggregated by multipath to /dev/mpatha,
which was passed to a nested guest like that

-object pr-manager-helper,id=qemu_pr_helper,path=/root/work/vm/testvm/.run/pr_helper.socket
-blockdev node-name=test,driver=host_device,filename=/dev/mapper/mpatha,pr-manager=qemu_pr_helper
-device scsi-block,drive=test,bus=scsi0.0,bootindex=-1,scsi-id=0,lun=0

The nested guest run:

sg_persist --no-inquiry  -v --out --register --param-sark 0x1234 /dev/sda

Strictly speaking this is wrong configuration since qemu is where
the multipath was split, and thus the iscsi target was not aware of
multipath, and thus when libmpathpersist code rightfully tried to register
the PR key on all paths, it failed to do so.

However qemu-pr-helper should not crash in this case.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scsi/qemu-pr-helper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index a8a74d1..debb18f 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -323,10 +323,10 @@ static int mpath_reconstruct_sense(int fd, int r, uint8_t *sense)
              */
             uint8_t cdb[6] = { TEST_UNIT_READY };
             int sz = 0;
-            int r = do_sgio(fd, cdb, sense, NULL, &sz, SG_DXFER_NONE);
+            int ret = do_sgio(fd, cdb, sense, NULL, &sz, SG_DXFER_NONE);
 
-            if (r != GOOD) {
-                return r;
+            if (ret != GOOD) {
+                return ret;
             }
             scsi_build_sense(sense, mpath_generic_sense(r));
             return CHECK_CONDITION;
-- 
1.8.3.1




  parent reply	other threads:[~2019-10-02 17:03 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02 16:51 [PULL 00/30] Misc patches for 2010-10-02 Paolo Bonzini
2019-10-02 16:51 ` [PULL 01/30] tests/migration: Add a test for auto converge Paolo Bonzini
2019-10-02 16:51 ` [PULL 02/30] target/i386: handle filtered_features in a new function mark_unavailable_features Paolo Bonzini
2019-10-02 16:51 ` [PULL 03/30] target/i386: introduce generic feature dependency mechanism Paolo Bonzini
2019-10-02 16:51 ` [PULL 04/30] target/i386: expand feature words to 64 bits Paolo Bonzini
2019-10-02 16:51 ` [PULL 05/30] target/i386: add VMX definitions Paolo Bonzini
2019-10-02 16:51 ` [PULL 06/30] vmxcap: correct the name of the variables Paolo Bonzini
2019-10-02 16:51 ` [PULL 07/30] target/i386: add VMX features Paolo Bonzini
2019-10-02 16:51 ` [PULL 08/30] target/i386: work around KVM_GET_MSRS bug for secondary execution controls Paolo Bonzini
2019-10-02 16:51 ` [PULL 09/30] target/i386/kvm: Silence warning from Valgrind about uninitialized bytes Paolo Bonzini
2019-10-02 16:51 ` Paolo Bonzini [this message]
2019-10-02 16:51 ` [PULL 11/30] replay: don't synchronize memory operations in replay mode Paolo Bonzini
2019-10-02 16:51 ` [PULL 12/30] Makefile: Remove generated files when doing 'distclean' Paolo Bonzini
2019-10-04 12:20   ` Peter Maydell
2019-10-04 16:48     ` Paolo Bonzini
2019-10-07  6:28       ` Thomas Huth
2019-10-07  7:13         ` Aleksandar Markovic
2019-10-02 16:51 ` [PULL 13/30] hw/isa: Introduce a CONFIG_ISA_SUPERIO switch for isa-superio.c Paolo Bonzini
2019-10-02 16:51 ` [PULL 14/30] ide: fix leak from qemu_allocate_irqs Paolo Bonzini
2019-10-02 16:51 ` [PULL 15/30] microblaze: fix leak of fdevice tree blob Paolo Bonzini
2019-10-02 16:51 ` [PULL 16/30] mcf5208: fix leak from qemu_allocate_irqs Paolo Bonzini
2019-10-02 16:51 ` [PULL 17/30] hppa: fix leak from g_strdup_printf Paolo Bonzini
2019-10-02 16:51 ` [PULL 18/30] mips: fix memory leaks in board initialization Paolo Bonzini
2019-10-02 16:51 ` [PULL 19/30] cris: do not leak struct cris_disasm_data Paolo Bonzini
2019-10-02 16:51 ` [PULL 20/30] lm32: do not leak memory on object_new/object_unref Paolo Bonzini
2019-10-02 16:51 ` [PULL 21/30] docker: test-debug: disable LeakSanitizer Paolo Bonzini
2019-10-02 16:51 ` [PULL 22/30] i386: Add CPUID bit for CLZERO and XSAVEERPTR Paolo Bonzini
2019-10-02 16:51 ` [PULL 23/30] vfio: Turn the container error into an Error handle Paolo Bonzini
2019-10-02 16:51 ` [PULL 24/30] memory: allow memory_region_register_iommu_notifier() to fail Paolo Bonzini
2019-10-02 16:51 ` [PULL 25/30] Fix wrong behavior of cpu_memory_rw_debug() function in SMM Paolo Bonzini
2019-10-07  8:29   ` Laszlo Ersek
2019-10-02 16:51 ` [PULL 26/30] util: WSAEWOULDBLOCK on connect should map to EINPROGRESS Paolo Bonzini
2019-10-02 16:51 ` [PULL 27/30] tests: skip serial test on windows Paolo Bonzini
2019-10-02 16:51 ` [PULL 28/30] win32: work around main-loop busy loop on socket/fd event Paolo Bonzini
2019-10-02 16:51 ` [PULL 29/30] tests/docker: only enable ubsan for test-clang Paolo Bonzini
2019-10-02 16:51 ` [PULL 30/30] accel/kvm: ensure ret always set Paolo Bonzini
2019-10-02 18:29 ` [PULL 00/30] Misc patches for 2010-10-02 no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1570035113-56848-11-git-send-email-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=mlevitsk@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.