Skip to contentSkip to conversation

hw/usb: fix mistaken de-initialization of CCID state

PATCH In review Daniel P. Berrangé 3 participants 3 messages Dec 21, 2018

lore
Share & tools
lore

Daniel P. Berrangé

author
#0 lore raw

PATCH 1/1 hw/usb: fix mistaken de-initialization of CCID state

In previous commit:

commit 7dea29e4af17fc1d27478de9f8ea38144deac54a
Author: Li Qiang <[email protected]>
Date: Fri Oct 19 03:50:36 2018 -0700

hw: ccid-card-emulated: cleanup resource when realize in error path

The emulated_realize method was changed so that it jumps to a cleanup label to de-initialize state upon error. This change failed to ensure the success path exited the method before this point though. So the mutexes are always destroyed even in normal operation. The result is as crashtastic as expected:

$ qemu-system-x86_64 -usb -device usb-ccid,id=ccid0 -device ccid-card-emulated,backend=nss-emulated,id=smartcard0,bus=ccid0.0
qemu-system-x86_64: util/qemu-thread-posix.c:64: qemu_mutex_lock_impl: Assertion `mutex->initialized' failed.
Aborted (core dumped)
Reported-by Michael Tokarev <[email protected]>
Signed-off-by Daniel P. Berrangé <[email protected]>
---
 hw/usb/ccid-card-emulated.c | 2 ++
 1 file changed, 2 insertions(+)
hw/usb/ccid-card-emulated.c +2 -0
index 25976ed84f..e0457d305b 100644
@@ -549,6 +549,8 @@ static void emulated_realize(CCIDCardState *base, Error **errp)
     qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
                        card, QEMU_THREAD_JOINABLE);
 
+    return;
+
 out2:
     clean_event_notifier(card);
 out1:
Signature · 1 line
2.19.2
Reviewed-by on v1 1/1
#1 lore raw
Quoted 5 lines · Daniel P. Berrangé

In previous commit:

   commit 7dea29e4af17fc1d27478de9f8ea38144deac54a
   Author: Li Qiang <[email protected]>
   Date:   Fri Oct 19 03:50:36 2018 -0700

[]

hw/usb/ccid-card-emulated.c static void emulated_realize(CCIDCardState *base, Error **errp) · line 556

Quoted 11 lines
hw/usb/ccid-card-emulated.c +2 -0
@@ -549,6 +549,8 @@ static void emulated_realize(CCIDCardState *base, Error **errp)
      qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
                         card, QEMU_THREAD_JOINABLE);
 
+    return;
+
  out2:
      clean_event_notifier(card);
  out1:

Lovely :)

Reviewed-by Michael Tokarev <[email protected]>

/mjt

Reviewed-by Tested-by on v1 1/1
#2 lore raw
In reply to Daniel P. Berrangé
Quoted 14 lines · Daniel P. Berrangé

In previous commit:

commit 7dea29e4af17fc1d27478de9f8ea38144deac54a
Author: Li Qiang <[email protected]>
Date: Fri Oct 19 03:50:36 2018 -0700

hw: ccid-card-emulated: cleanup resource when realize in error path

The emulated_realize method was changed so that it jumps to a cleanup label to de-initialize state upon error. This change failed to ensure the success path exited the method before this point though. So the mutexes are always destroyed even in normal operation. The result is as crashtastic as expected:

Quoted 5 lines
$ qemu-system-x86_64 -usb -device usb-ccid,id=ccid0 -device ccid-card-emulated,backend=nss-emulated,id=smartcard0,bus=ccid0.0
qemu-system-x86_64: util/qemu-thread-posix.c:64: qemu_mutex_lock_impl: Assertion `mutex->initialized' failed.
Aborted (core dumped)
Reported-by Michael Tokarev <[email protected]>
Fixes 7dea29e4af1
Reviewed-by Philippe Mathieu-Daudé <[email protected]>
Tested-by Philippe Mathieu-Daudé <[email protected]>
Quoted 19 lines
Signed-off-by Daniel P. Berrangé <[email protected]>
---
 hw/usb/ccid-card-emulated.c | 2 ++
 1 file changed, 2 insertions(+)
hw/usb/ccid-card-emulated.c +2 -0
index 25976ed84f..e0457d305b 100644
@@ -549,6 +549,8 @@ static void emulated_realize(CCIDCardState *base, Error **errp)
     qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
                        card, QEMU_THREAD_JOINABLE);
 
+    return;
+
 out2:
     clean_event_notifier(card);
 out1: