Re: [Qemu-devel] [PATCH 3/3] libcacard: don't free sign buffer while sign op is pending
On 10/19/2014 05:12 AM, Ray Strode wrote:
From: Ray Strode rstr...@redhat.com
commit 57f97834efe0c208ffadc9d2959f3d3d55580e52 cleaned up
the cac_applet_pki_process_apdu function to have a single
exit point. Unfortunately, that commit introduced a bug
where the sign buffer can get free'd and nullified while
it's still being used.
This commit corrects the bug by introducing a boolean to
track whether or not the sign buffer should be freed in
the function exit path.
My bad, thanks for catching this.
Reviewed-by: Alon Levy a...@pobox.com
Signed-off-by: Ray Strode rstr...@redhat.com
---
libcacard/cac.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/libcacard/cac.c b/libcacard/cac.c
index ae8c378..f38fdce 100644
--- a/libcacard/cac.c
+++ b/libcacard/cac.c
@@ -88,60 +88,61 @@ cac_common_process_apdu(VCard *card, VCardAPDU *apdu,
VCardResponse **response)
}
/*
* reset the inter call state between applet selects
*/
static VCardStatus
cac_applet_pki_reset(VCard *card, int channel)
{
VCardAppletPrivate *applet_private;
CACPKIAppletData *pki_applet;
applet_private = vcard_get_current_applet_private(card, channel);
assert(applet_private);
pki_applet = (applet_private-u.pki_data);
pki_applet-cert_buffer = NULL;
g_free(pki_applet-sign_buffer);
pki_applet-sign_buffer = NULL;
pki_applet-cert_buffer_len = 0;
pki_applet-sign_buffer_len = 0;
return VCARD_DONE;
}
static VCardStatus
cac_applet_pki_process_apdu(VCard *card, VCardAPDU *apdu,
VCardResponse **response)
{
CACPKIAppletData *pki_applet;
VCardAppletPrivate *applet_private;
int size, next;
unsigned char *sign_buffer;
+bool retain_sign_buffer = FALSE;
vcard_7816_status_t status;
VCardStatus ret = VCARD_FAIL;
applet_private = vcard_get_current_applet_private(card, apdu-a_channel);
assert(applet_private);
pki_applet = (applet_private-u.pki_data);
switch (apdu-a_ins) {
case CAC_UPDATE_BUFFER:
*response = vcard_make_response(
VCARD7816_STATUS_ERROR_CONDITION_NOT_SATISFIED);
ret = VCARD_DONE;
break;
case CAC_GET_CERTIFICATE:
if ((apdu-a_p2 != 0) || (apdu-a_p1 != 0)) {
*response = vcard_make_response(
VCARD7816_STATUS_ERROR_P1_P2_INCORRECT);
break;
}
assert(pki_applet-cert != NULL);
size = apdu-a_Le;
if (pki_applet-cert_buffer == NULL) {
pki_applet-cert_buffer = pki_applet-cert;
pki_applet-cert_buffer_len = pki_applet-cert_len;
}
size = MIN(size, pki_applet-cert_buffer_len);
next = MIN(255, pki_applet-cert_buffer_len - size);
*response = vcard_response_new_bytes(
card, pki_applet-cert_buffer, size,
apdu-a_Le, next ?
@@ -151,85 +152,88 @@ cac_applet_pki_process_apdu(VCard *card, VCardAPDU
*apdu,
pki_applet-cert_buffer += size;
pki_applet-cert_buffer_len -= size;
if ((*response == NULL) || (next == 0)) {
pki_applet-cert_buffer = NULL;
}
if (*response == NULL) {
*response = vcard_make_response(
VCARD7816_STATUS_EXC_ERROR_MEMORY_FAILURE);
}
ret = VCARD_DONE;
break;
case CAC_SIGN_DECRYPT:
if (apdu-a_p2 != 0) {
*response = vcard_make_response(
VCARD7816_STATUS_ERROR_P1_P2_INCORRECT);
break;
}
size = apdu-a_Lc;
sign_buffer = g_realloc(pki_applet-sign_buffer,
pki_applet-sign_buffer_len + size);
memcpy(sign_buffer+pki_applet-sign_buffer_len, apdu-a_body, size);
size += pki_applet-sign_buffer_len;
switch (apdu-a_p1) {
case 0x80:
/* p1 == 0x80 means we haven't yet sent the whole buffer, wait
for
* the rest */
pki_applet-sign_buffer = sign_buffer;
pki_applet-sign_buffer_len = size;
*response = vcard_make_response(VCARD7816_STATUS_SUCCESS);
+retain_sign_buffer = TRUE;
break;
case 0x00:
/* we now have the whole buffer, do the operation, result will be
* in the sign_buffer */
status = vcard_emul_rsa_op(card, pki_applet-key,
sign_buffer, size);
if (status != VCARD7816_STATUS_SUCCESS) {
*response = vcard_make_response(status);
break;
}
*response = vcard_response_new(card, sign_buffer, size,
apdu-a_Le
Re: [Qemu-devel] [PATCH] ccid-card-emulated: use EventNotifier
Shut up Coverity's complaint about unchecked fcntl return values,
and especially make the code simpler and more efficient.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Reviewed-by: Alon Levy al...@redhat.com
one question below.
---
hw/usb/ccid-card-emulated.c | 29 ++---
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index 7213c89..aa1c37a 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -126,7 +126,7 @@ struct EmulatedState {
QemuMutex vreader_mutex; /* and guest_apdu_list mutex */
QemuMutex handle_apdu_mutex;
QemuCond handle_apdu_cond;
-int pipe[2];
+EventNotifier notifier;
int quit_apdu_thread;
QemuThread apdu_thread_id;
};
@@ -162,9 +162,7 @@ static void emulated_push_event(EmulatedState *card,
EmulEvent *event)
qemu_mutex_lock(card-event_list_mutex);
QSIMPLEQ_INSERT_TAIL((card-event_list), event, entry);
qemu_mutex_unlock(card-event_list_mutex);
-if (write(card-pipe[1], card, 1) != 1) {
-DPRINTF(card, 1, write to pipe failed\n);
-}
+event_notifier_set(card-notifier);
}
static void emulated_push_type(EmulatedState *card, uint32_t type)
@@ -358,16 +356,12 @@ static void *event_thread(void *arg)
return NULL;
}
-static void pipe_read(void *opaque)
+static void card_event_handler(EventNotifier *notifier)
{
-EmulatedState *card = opaque;
+EmulatedState *card = container_of(notifier, EmulatedState, notifier);
EmulEvent *event, *next;
-char dummy;
-int len;
-do {
-len = read(card-pipe[0], dummy, sizeof(dummy));
-} while (len == sizeof(dummy));
+event_notifier_test_and_clear(card-notifier);
Shouldn't the ignored return value be marked somehow?
qemu_mutex_lock(card-event_list_mutex);
QSIMPLEQ_FOREACH_SAFE(event, card-event_list, entry, next) {
DPRINTF(card, 2, event %s\n,
emul_event_to_string(event-p.gen.type));
@@ -404,16 +398,13 @@ static void pipe_read(void *opaque)
qemu_mutex_unlock(card-event_list_mutex);
}
-static int init_pipe_signaling(EmulatedState *card)
+static int init_event_notifier(EmulatedState *card)
{
-if (pipe(card-pipe) 0) {
-DPRINTF(card, 2, pipe creation failed\n);
+if (event_notifier_init(card-notifier, false) 0) {
+DPRINTF(card, 2, event notifier creation failed\n);
return -1;
}
-fcntl(card-pipe[0], F_SETFL, O_NONBLOCK);
-fcntl(card-pipe[1], F_SETFL, O_NONBLOCK);
-fcntl(card-pipe[0], F_SETOWN, getpid());
-qemu_set_fd_handler(card-pipe[0], pipe_read, NULL, card);
+event_notifier_set_handler(card-notifier, card_event_handler);
return 0;
}
@@ -500,7 +491,7 @@ static int emulated_initfn(CCIDCardState *base)
qemu_cond_init(card-handle_apdu_cond);
card-reader = NULL;
card-quit_apdu_thread = 0;
-if (init_pipe_signaling(card) 0) {
+if (init_event_notifier(card) 0) {
return -1;
}
--
1.9.3
Re: [Qemu-devel] [PATCH] libcacard: improve documentation
On 05/28/2014 07:15 PM, Paolo Bonzini wrote: Using the file-backed smartcard backend is black magic, but it can be useful if your only smartcard bricks itself if it is accessed the wrong way too many times. Complete the documentation to include the art of creating certificates and using them with QEMU, based on Ray Strode's useful tutorial at https://blogs.gnome.org/halfline/2013/09/08/another-smartcard-post/ but with ccid-card-emulated or vscclient instead of SPICE. Cc: Alon Levy al...@redhat.com Cc: Ray Strode rstr...@redhat.com Did you mean Robert Relyea rrel...@redhat.com ? Thanks, looks good. Reviewed-by: Alon Levy al...@redhat.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- docs/ccid.txt | 80 ++- 1 file changed, 63 insertions(+), 17 deletions(-) diff --git a/docs/ccid.txt b/docs/ccid.txt index 83c174d..c7fda6d 100644 --- a/docs/ccid.txt +++ b/docs/ccid.txt @@ -47,6 +47,7 @@ In ubuntu/debian: Configuring and building: ./configure --enable-smartcard make + 3. Using ccid-card-emulated with hardware Assuming you have a working smartcard on the host with the current @@ -54,19 +55,55 @@ user, using NSS, qemu acts as another NSS client using ccid-card-emulated: qemu -usb -device usb-ccid -device ccid-card-emulated -4. Using ccid-card-emulated with certificates -You must create the certificates. This is a one time process. We use NSS -certificates: +4. Using ccid-card-emulated with certificates stored in files + +You must create the CA and card certificates. This is a one time process. +We use NSS certificates: -certutil -d /etc/pki/nssdb -x -t CT,CT,CT -S -s CN=cert1 -n cert1 +mkdir fake-smartcard +cd fake-smartcard +certutil -N -d sql:$PWD +certutil -S -d sql:$PWD -s CN=Fake Smart Card CA -x -t TC,TC,TC -n fake-smartcard-ca +certutil -S -d sql:$PWD -t ,, -s CN=John Doe -n id-cert -c fake-smartcard-ca +certutil -S -d sql:$PWD -t ,, -s CN=John Doe (signing) --nsCertType smime -n signing-cert -c fake-smartcard-ca +certutil -S -d sql:$PWD -t ,, -s CN=John Doe (encryption) --nsCertType sslClient -n encryption-cert -c fake-smartcard-ca Note: you must have exactly three certificates. -Assuming the current user can access the certificates (use certutil -L to -verify), you can use the emulated card type with the certificates backend: +You can use the emulated card type with the certificates backend: + +qemu -usb -device usb-ccid -device ccid-card-emulated,backend=certificates,db=sql:$PWD,cert1=id-cert,cert2=signing-cert,cert3=encryption-cert + +To use the certificates in the guest, export the CA certificate: + +certutil -L -r -d sql:$PWD -o fake-smartcard-ca.cer -n fake-smartcard-ca + +and import it in the guest: + +certutil -A -d /etc/pki/nssdb -i fake-smartcard-ca.cer -t TC,TC,TC -n fake-smartcard-ca + +In a Linux guest you can then use the CoolKey PKCS #11 module to access +the card: + +certutil -d /etc/pki/nssdb -L -h all + +It will prompt you for the PIN (which is the password you assigned to the +certificate database early on), and then show you all three certificates +together with the manually imported CA cert: + +Certificate NicknameTrust Attributes +fake-smartcard-ca CT,C,C +John Doe:CAC ID Certificate u,u,u +John Doe:CAC Email Signature Certificateu,u,u +John Doe:CAC Email Encryption Certificate u,u,u + +If this does not happen, CoolKey is not installed or not registered with +NSS. Registration can be done from Firefox or the command line: + +modutil -dbdir /etc/pki/nssdb -add CAC Module -libfile /usr/lib64/pkcs11/libcoolkeypk11.so +modutil -dbdir /etc/pki/nssdb -list -qemu -usb -device usb-ccid -device ccid-card-emulated,backend=certificates,cert1=cert1,cert2=cert2,cert3=cert3 5. Using ccid-card-passthru with client side hardware @@ -74,15 +111,23 @@ on the host specify the ccid-card-passthru device with a suitable chardev: qemu -chardev socket,server,host=0.0.0.0,port=2001,id=ccid,nowait -usb -device usb-ccid -device ccid-card-passthru,chardev=ccid -on the client run vscclient, built when you built the libcacard library: -libcacard/vscclient qemu-host 2001 +on the client run vscclient, built when you built QEMU: + +vscclient qemu-host 2001 + 6. Using ccid-card-passthru with client side certificates -Run qemu as per #5, and run vscclient as follows: -(Note: vscclient command line interface is in a state of change) +This case is not particularly useful, but you can use it to debug +your setup if #4 works but #5 does not. + +Follow instructions as per #4, except run QEMU and vscclient as follows: +Run qemu as per #5, and run vscclient from the fake-smartcard +directory as follows
Re: [Qemu-devel] [Qemu-trivial] [PATCH] libcacard: remove useless initializers
On 05/23/2014 11:59 PM, Michael Tokarev wrote: So, should we apply this or not? It's been waiting for quite some time, and during this time we've found a very good example of why it should be applied (I think anyway). I'm fine with applying it, I changed my mind. Thanks, /mjt 12.05.2014 13:20, Markus Armbruster wrote: Michael Tokarev m...@tls.msk.ru writes: 11.05.2014 11:58, Alon Levy wrote: On 05/08/2014 08:19 PM, Michael Tokarev wrote: libcacard has many functions which initializes local variables at declaration time, which are always assigned some values later (often right after declaration). Clean up these initializers. How is this an improvement? Doesn't the compiler ignore this anyhow? Just less code. To me, when I see something like Type *var = NULL; in a function, it somehow translates to a construct like Type *found = NULL; That is -- so this variable will be used either as an accumulator or a search result, so that initial value is really important. So when I see the same variable receives its initial value in the next line, I start wondering what's missed in the code which should be there. Or why I don't read the code correctly. Or something like this. So, basically, this is a cleanup patch just to avoid confusion, it most likely not needed for current compiler who can figure it out by its own. And for consistency - why not initialize other variables too? I hate redundant initializers for yet another reason: when I change the code, and accidentally add a path bypassing the *real* initialization, I don't get a may be used uninitialized warning, I get the stupid redundant initialization and quite possibly a crash to debug some time later. Maybe that's just my old-scool mind works this way. At any rate you can just ignore this patch. Please consider it.
Re: [Qemu-devel] [PATCH 0/7] libcacard: A few simple fixes and cleanups
On 05/22/2014 05:57 PM, Markus Armbruster wrote: Makes Coverity happy with libcacard/ (for now). Markus Armbruster (7): libcacard/vscclient: Bury some dead code libcacard: Plug memory leaks around vreader_get_reader_list() libcacard/vreader: Drop broken recovery from failed assertion libcacard/vreader: Tighten assertion to clarify intent libcacard: Convert two leftover realloc() to GLib libcacard/vcard_emul_nss: Assert vreaderOpt isn't null libcacard/vcard_emul_nss: Drop a redundant conditional libcacard/cac.c| 13 ++--- libcacard/vcard_emul_nss.c | 16 ++-- libcacard/vreader.c| 10 -- libcacard/vscclient.c | 7 +++ 4 files changed, 15 insertions(+), 31 deletions(-) Reviewed-by: Alon Levy al...@redhat.com
Re: [Qemu-devel] [PATCH] libcacard: remove useless initializers
On 05/08/2014 08:19 PM, Michael Tokarev wrote:
libcacard has many functions which initializes local variables
at declaration time, which are always assigned some values later
(often right after declaration). Clean up these initializers.
How is this an improvement? Doesn't the compiler ignore this anyhow?
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
libcacard/cac.c| 14 +++---
libcacard/card_7816.c |5 ++---
libcacard/vcard.c |4 ++--
libcacard/vcard_emul_nss.c |6 +++---
libcacard/vreader.c| 10 +-
libcacard/vscclient.c |4 ++--
6 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/libcacard/cac.c b/libcacard/cac.c
index 122129e..d1d9ee2 100644
--- a/libcacard/cac.c
+++ b/libcacard/cac.c
@@ -93,8 +93,8 @@ cac_common_process_apdu(VCard *card, VCardAPDU *apdu,
VCardResponse **response)
static VCardStatus
cac_applet_pki_reset(VCard *card, int channel)
{
-VCardAppletPrivate *applet_private = NULL;
-CACPKIAppletData *pki_applet = NULL;
+VCardAppletPrivate *applet_private;
+CACPKIAppletData *pki_applet;
applet_private = vcard_get_current_applet_private(card, channel);
assert(applet_private);
pki_applet = (applet_private-u.pki_data);
@@ -113,8 +113,8 @@ static VCardStatus
cac_applet_pki_process_apdu(VCard *card, VCardAPDU *apdu,
VCardResponse **response)
{
-CACPKIAppletData *pki_applet = NULL;
-VCardAppletPrivate *applet_private = NULL;
+CACPKIAppletData *pki_applet;
+VCardAppletPrivate *applet_private;
int size, next;
unsigned char *sign_buffer;
vcard_7816_status_t status;
@@ -288,7 +288,7 @@ cac_applet_container_process_apdu(VCard *card, VCardAPDU
*apdu,
static void
cac_delete_pki_applet_private(VCardAppletPrivate *applet_private)
{
-CACPKIAppletData *pki_applet_data = NULL;
+CACPKIAppletData *pki_applet_data;
if (applet_private == NULL) {
return;
@@ -336,8 +336,8 @@ static VCardApplet *
cac_new_pki_applet(int i, const unsigned char *cert,
int cert_len, VCardKey *key)
{
-VCardAppletPrivate *applet_private = NULL;
-VCardApplet *applet = NULL;
+VCardAppletPrivate *applet_private;
+VCardApplet *applet;
unsigned char pki_aid[] = { 0xa0, 0x00, 0x00, 0x00, 0x79, 0x01, 0x00 };
int pki_aid_len = sizeof(pki_aid);
diff --git a/libcacard/card_7816.c b/libcacard/card_7816.c
index bca8c4a..a54f880 100644
--- a/libcacard/card_7816.c
+++ b/libcacard/card_7816.c
@@ -416,7 +416,7 @@
VCARD_RESPONSE_NEW_STATIC_STATUS(VCARD7816_STATUS_ERROR_GENERAL)
VCardResponse *
vcard_make_response(vcard_7816_status_t status)
{
-VCardResponse *response = NULL;
+VCardResponse *response;
switch (status) {
/* known 7816 response codes */
@@ -543,9 +543,8 @@ vcard_make_response(vcard_7816_status_t status)
return VCARD_RESPONSE_GET_STATIC(
VCARD7816_STATUS_EXC_ERROR_MEMORY_FAILURE);
}
+return response;
}
-assert(response);
-return response;
}
/*
diff --git a/libcacard/vcard.c b/libcacard/vcard.c
index 227e477..6aaf085 100644
--- a/libcacard/vcard.c
+++ b/libcacard/vcard.c
@@ -166,8 +166,8 @@ vcard_reference(VCard *vcard)
void
vcard_free(VCard *vcard)
{
-VCardApplet *current_applet = NULL;
-VCardApplet *next_applet = NULL;
+VCardApplet *current_applet;
+VCardApplet *next_applet;
if (vcard == NULL) {
return;
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index 75b9d79..3f38a4c 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -367,7 +367,7 @@ vcard_7816_status_t
vcard_emul_login(VCard *card, unsigned char *pin, int pin_len)
{
PK11SlotInfo *slot;
-unsigned char *pin_string = NULL;
+unsigned char *pin_string;
int i;
SECStatus rv;
@@ -423,7 +423,7 @@ static VReader *
vcard_emul_find_vreader_from_slot(PK11SlotInfo *slot)
{
VReaderList *reader_list = vreader_get_reader_list();
-VReaderListEntry *current_entry = NULL;
+VReaderListEntry *current_entry;
if (reader_list == NULL) {
return NULL;
@@ -1050,7 +1050,7 @@ void
vcard_emul_replay_insertion_events(void)
{
VReaderListEntry *current_entry;
-VReaderListEntry *next_entry = NULL;
+VReaderListEntry *next_entry;
VReaderList *list = vreader_get_reader_list();
for (current_entry = vreader_list_get_first(list); current_entry;
diff --git a/libcacard/vreader.c b/libcacard/vreader.c
index 9304a28..22dfe43 100644
--- a/libcacard/vreader.c
+++ b/libcacard/vreader.c
@@ -341,7 +341,7 @@ void
vreader_list_delete(VReaderList *list)
{
VReaderListEntry *current_entry;
-VReaderListEntry *next_entry = NULL;
+VReaderListEntry *next_entry;
Re: [Qemu-devel] [PATCH] libcacard: g_malloc cleanups
On 05/08/2014 06:54 PM, Michael Tokarev wrote:
This patch replaces g_malloc() in libcacard into g_new()
or g_new0() where appropriate (removing some init-to-zero
surrounding code), g_malloc+memcpy into g_memdup() and the
like.
Reviewed-by: Alon Levy al...@redhat.com
just one comment below.
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
libcacard/cac.c| 11 +++
libcacard/card_7816.c | 11 +--
libcacard/event.c |2 +-
libcacard/vcard.c | 22 +-
libcacard/vcard_emul_nss.c | 12 ++--
libcacard/vreader.c| 11 +++
6 files changed, 23 insertions(+), 46 deletions(-)
diff --git a/libcacard/cac.c b/libcacard/cac.c
index 74ef3e3..122129e 100644
--- a/libcacard/cac.c
+++ b/libcacard/cac.c
@@ -310,16 +310,11 @@ static VCardAppletPrivate *
cac_new_pki_applet_private(const unsigned char *cert,
int cert_len, VCardKey *key)
{
-CACPKIAppletData *pki_applet_data = NULL;
-VCardAppletPrivate *applet_private = NULL;
These two lines above (and the corresponding ones beloow) seem unrelated
to this patch.
-applet_private = (VCardAppletPrivate
*)g_malloc(sizeof(VCardAppletPrivate));
+CACPKIAppletData *pki_applet_data;
+VCardAppletPrivate *applet_private;
+applet_private = g_new0(VCardAppletPrivate, 1);
pki_applet_data = (applet_private-u.pki_data);
-pki_applet_data-cert_buffer = NULL;
-pki_applet_data-cert_buffer_len = 0;
-pki_applet_data-sign_buffer = NULL;
-pki_applet_data-sign_buffer_len = 0;
-pki_applet_data-key = NULL;
pki_applet_data-cert = (unsigned char *)g_malloc(cert_len+1);
/*
* if we want to support compression, then we simply change the 0 to a 1
diff --git a/libcacard/card_7816.c b/libcacard/card_7816.c
index c28bb60..bca8c4a 100644
--- a/libcacard/card_7816.c
+++ b/libcacard/card_7816.c
@@ -51,7 +51,7 @@ vcard_response_new_data(unsigned char *buf, int len)
{
VCardResponse *new_response;
-new_response = (VCardResponse *)g_malloc(sizeof(VCardResponse));
+new_response = g_new(VCardResponse, 1);
new_response-b_data = g_malloc(len + 2);
memcpy(new_response-b_data, buf, len);
new_response-b_total_len = len+2;
@@ -132,7 +132,7 @@ vcard_response_new_status(vcard_7816_status_t status)
{
VCardResponse *new_response;
-new_response = (VCardResponse *)g_malloc(sizeof(VCardResponse));
+new_response = g_new(VCardResponse, 1);
new_response-b_data = new_response-b_sw1;
new_response-b_len = 0;
new_response-b_total_len = 2;
@@ -149,7 +149,7 @@ vcard_response_new_status_bytes(unsigned char sw1,
unsigned char sw2)
{
VCardResponse *new_response;
-new_response = (VCardResponse *)g_malloc(sizeof(VCardResponse));
+new_response = g_new(VCardResponse, 1);
new_response-b_data = new_response-b_sw1;
new_response-b_len = 0;
new_response-b_total_len = 2;
@@ -336,9 +336,8 @@ vcard_apdu_new(unsigned char *raw_apdu, int len,
vcard_7816_status_t *status)
return NULL;
}
-new_apdu = (VCardAPDU *)g_malloc(sizeof(VCardAPDU));
-new_apdu-a_data = g_malloc(len);
-memcpy(new_apdu-a_data, raw_apdu, len);
+new_apdu = g_new(VCardAPDU, 1);
+new_apdu-a_data = g_memdup(raw_apdu, len);
new_apdu-a_len = len;
*status = vcard_apdu_set_class(new_apdu);
if (*status != VCARD7816_STATUS_SUCCESS) {
diff --git a/libcacard/event.c b/libcacard/event.c
index 2d7500f..a2e6c7d 100644
--- a/libcacard/event.c
+++ b/libcacard/event.c
@@ -17,7 +17,7 @@ vevent_new(VEventType type, VReader *reader, VCard *card)
{
VEvent *new_vevent;
-new_vevent = (VEvent *)g_malloc(sizeof(VEvent));
+new_vevent = g_new(VEvent, 1);
new_vevent-next = NULL;
new_vevent-type = type;
new_vevent-reader = vreader_reference(reader);
diff --git a/libcacard/vcard.c b/libcacard/vcard.c
index 539177b..227e477 100644
--- a/libcacard/vcard.c
+++ b/libcacard/vcard.c
@@ -37,9 +37,8 @@ vcard_buffer_response_new(unsigned char *buffer, int size)
{
VCardBufferResponse *new_buffer;
-new_buffer = (VCardBufferResponse
*)g_malloc(sizeof(VCardBufferResponse));
-new_buffer-buffer = (unsigned char *)g_malloc(size);
-memcpy(new_buffer-buffer, buffer, size);
+new_buffer = g_new(VCardBufferResponse, 1);
+new_buffer-buffer = (unsigned char *)g_memdup(buffer, size);
new_buffer-buffer_len = size;
new_buffer-current = new_buffer-buffer;
new_buffer-len = size;
@@ -102,15 +101,11 @@ vcard_new_applet(VCardProcessAPDU
applet_process_function,
{
VCardApplet *applet;
-applet = (VCardApplet *)g_malloc(sizeof(VCardApplet));
-applet-next = NULL;
-applet-applet_private = NULL;
-applet-applet_private_free = NULL;
+applet = g_new0(VCardApplet, 1);
applet
Re: [Qemu-devel] [PATCH 0/5] glib thread interface and libcacard cleanups
On 05/05/2014 07:36 PM, Michael Tokarev wrote: 05.05.2014 18:49, Alon Levy wrote: On 04/29/2014 09:02 AM, Michael Tokarev wrote: Basically libgthread has been rewritten in glib version 2.31, and old ways to use thread primitives stopped working while new ways appeared. The two interfaces were sufficiently different to warrant large ifdeffery across all code using it. [...] [] Reviewed-by: Alon Levy al...@redhat.com Tested-by: Alon Levy al...@redhat.com Hmm. Now I'm a bit confused. Which version did you test? :) I posted a v2 patch which splits one of the changes into two (pstrcpy to memcpy conversion in libcacard), added some more (minor) changes (which should not affect libcacard code in any way), and adjusted commit messages. The main code is not affected (or should not be), so Tested-by probably may stay, except of the pstrcpy to memcpy patch (http://patchwork.ozlabs.org/patch/345002/) which may affect libcacard. Here's the v2: http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg00286.html If you tested the git branch which I referred to, that's the v2, not original submission which you're replying to. I've tested and reviewed 7191b2c43eecc52994924245720c534ea1a0dc84 so v2, my bad. This would be nice to have too (it has nothing to do with your patchset, but it would save me a pull request): commit 2fc95f8bc1912e2de243389d9d102a5a28267f31 Author: Alon Levy al...@redhat.com Date: Mon May 5 17:41:32 2014 +0300 libcacard: remove unnecessary EOL from debug prints Well, this can easily go to -trivial, as I'm planning to send a pull request for it soon anyway. Thank you! /mjt
Re: [Qemu-devel] [PATCH 0/5] glib thread interface and libcacard cleanups
to be strict, exporting only really necessary
symbols, omitting internal symbols. Previously, libcacard used and
exported many of qemu internals, including thread functions. Now
it not only stopped exporting them, but also stopped using them.
The whole thing has been compile-tested with both new and old glib
versions on linux and FreeBSD, and runtime-tested on linux (again,
both old and new versions) with --with-coroutine=gthread. I didn't
test libcacard much, because I found no testcases for it, but at
least it _appears_ to work.
The diffstat below does not look like a diffstat of a cleanup, because
the patchset adds about 2 times more lines than it removes. This is
because of large addition to glib-compat.h, plus addition of compat
code to vscclient, to make it independent of qemu.
and a few others.
Thanks,
Reviewed-by: Alon Levy al...@redhat.com
Tested-by: Alon Levy al...@redhat.com
This would be nice to have too (it has nothing to do with your patchset,
but it would save me a pull request):
commit 2fc95f8bc1912e2de243389d9d102a5a28267f31
Author: Alon Levy al...@redhat.com
Date: Mon May 5 17:41:32 2014 +0300
libcacard: remove unnecessary EOL from debug prints
Signed-off-by: Alon Levy al...@redhat.com
diff --git a/libcacard/vreader.c b/libcacard/vreader.c
index 91799b4..d1e05af 100644
--- a/libcacard/vreader.c
+++ b/libcacard/vreader.c
@@ -272,12 +272,12 @@ vreader_xfr_bytes(VReader *reader,
response = vcard_make_response(status);
card_status = VCARD_DONE;
} else {
-g_debug(%s: CLS=0x%x,INS=0x%x,P1=0x%x,P2=0x%x,Lc=%d,Le=%d %s\n,
+g_debug(%s: CLS=0x%x,INS=0x%x,P1=0x%x,P2=0x%x,Lc=%d,Le=%d %s,
__func__, apdu-a_cla, apdu-a_ins, apdu-a_p1, apdu-a_p2,
apdu-a_Lc, apdu-a_Le, apdu_ins_to_string(apdu-a_ins));
card_status = vcard_process_apdu(card, apdu, response);
if (response) {
-g_debug(%s: status=%d sw1=0x%x sw2=0x%x len=%d (total=%d)\n,
+g_debug(%s: status=%d sw1=0x%x sw2=0x%x len=%d (total=%d),
__func__, response-b_status, response-b_sw1,
response-b_sw2, response-b_len, response-b_total_len);
}
/mjt
Michael Tokarev (5):
do not call g_thread_init() for glib = 2.31
glib-compat.h: add new thread API emulation on top of pre-2.31 API
vscclient: use glib thread primitives not qemu
libcacard: replace qemu thread primitives with glib ones
libcacard: actually use symbols file
coroutine-gthread.c| 37 ++--
include/glib-compat.h | 103
libcacard/Makefile | 10 +
libcacard/event.c | 25 ++-
libcacard/vcard_emul_nss.c |3 +-
libcacard/vreader.c| 19
libcacard/vscclient.c | 75 +++-
trace/simple.c | 50 ++---
util/osdep.c | 21 -
9 files changed, 206 insertions(+), 137 deletions(-)
Re: [Qemu-devel] [PATCH] libcacard: actually use the symbols file
On 04/21/2014 12:09 PM, Michael Tokarev wrote: 21.04.2014 13:01, Michael Tokarev wrote: libtool has an argument for .syms file, which is -export-symbols. There's no argument `-export-syms', and it looks like at least on linux, -export-syms is just ignored. Use the correct argument, -export-symbols, to actually get the right export list. Note: with this patch applied, linking vscclient fails due to the following symbols missing: qemu_mutex_lock qemu_mutex_unlock qemu_cond_wait qemu_cond_signal qemu_thread_create socket_init qemu_socket qemu_mutex_init qemu_cond_init So it looks like a preparation patch is needed before this one, to ensure bisectability. Should those symbols be exported by libcacard (hence listed in the .syms file), or should vscclient link line use more objects? They should not be exported by libcacard. I'm not sure what is happening here except to note that the list of objects being linked must satisfy all missing symbols since it results in a loadable and executable executable. Thanks, /mjt Signed-off-by: Michael Tokarev m...@tls.msk.ru --- libcacard/Makefile |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcacard/Makefile b/libcacard/Makefile index 6b06448..ca08991 100644 --- a/libcacard/Makefile +++ b/libcacard/Makefile @@ -24,7 +24,7 @@ vscclient$(EXESUF): libcacard/vscclient.o libcacard.la # Rules for building libcacard standalone library libcacard.la: LDFLAGS += -rpath $(libdir) -no-undefined \ --export-syms $(SRC_PATH)/libcacard/libcacard.syms +-export-symbols $(SRC_PATH)/libcacard/libcacard.syms libcacard.la: LIBS = $(libcacard_libs) libcacard.la: $(libcacard-lobj-y) $(call LINK,$^)
Re: [Qemu-devel] [PATCH] libcacard: actually use symbols file
On 04/27/2014 05:37 PM, Michael Tokarev wrote: libtool has an argument for .syms file, which is -export-symbols. There's no argument `-export-syms', and it looks like at least on linux, -export-syms is just ignored. Use the correct argument, -export-symbols, to actually get the right export list. But it turns out that vscclient binary, which also uses qemu privitives for sockets, mutexes and some other stuff, but only linked with libcacard, does not link after hiding extra symbols previously exported by libcacard. So while at it, link it with libqemuutil.a and libqemustub.a. This makes the binary as twice as large, but allows to have cleaner export table for libcacard.so. Reviewed-by: Alon Levy al...@redhat.com Any chance to get this through the trivial patch queue? Signed-off-by: Michael Tokarev m...@tls.msk.ru --- libcacard/Makefile |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libcacard/Makefile b/libcacard/Makefile index 6b06448..bb00c94 100644 --- a/libcacard/Makefile +++ b/libcacard/Makefile @@ -17,14 +17,14 @@ $(libcacard-obj-y): | $(libcacard-lobj-y) all: libcacard.la libcacard.pc -vscclient$(EXESUF): libcacard/vscclient.o libcacard.la +vscclient$(EXESUF): libcacard/vscclient.o libcacard.la libqemuutil.a libqemustub.a $(call LINK,$^) # # Rules for building libcacard standalone library libcacard.la: LDFLAGS += -rpath $(libdir) -no-undefined \ - -export-syms $(SRC_PATH)/libcacard/libcacard.syms + -export-symbols $(SRC_PATH)/libcacard/libcacard.syms libcacard.la: LIBS = $(libcacard_libs) libcacard.la: $(libcacard-lobj-y) $(call LINK,$^)
[Qemu-devel] [PATCH] display/qxl: don't abort on reset with non empty rings
The command ring and cursor rings are pushed to by the guest, and
cleared asynchronously by qemu's spice thread. It is easy to have
them non empty by bad guest behaviour, and we must never abort on bad
guest behaviour.
Signed-off-by: Alon Levy al...@redhat.com
---
hw/display/qxl.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 47bbf1f..abe7a18 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -,8 +,12 @@ static void qxl_check_state(PCIQXLDevice *d)
QXLRam *ram = d-ram;
int spice_display_running = qemu_spice_display_is_running(d-ssd);
-assert(!spice_display_running || SPICE_RING_IS_EMPTY(ram-cmd_ring));
-assert(!spice_display_running || SPICE_RING_IS_EMPTY(ram-cursor_ring));
+if (spice_display_running !SPICE_RING_IS_EMPTY(ram-cmd_ring)) {
+fprintf(stderr, qxl: cmd ring not empty on reset\n);
+}
+if (spice_display_running !SPICE_RING_IS_EMPTY(ram-cursor_ring)) {
+fprintf(stderr, qxl: cursor ring not empty on reset\n);
+}
}
static void qxl_reset_state(PCIQXLDevice *d)
--
1.8.5.3
[Qemu-devel] [PULL 0/1 v2] libcacard glusterfs fix
changes from v1: Added Signed-of by me. The following changes since commit 1f6b12f75f2c22f861d0202374033a7594c91707: Merge remote-tracking branch 'remotes/mwalle/tags/lm32-fixes/20140204' into staging (2014-02-08 15:57:51 +) are available in the git repository at: git://people.freedesktop.org/~alon/qemu pull-libcacard.glusterfs for you to fetch changes up to 73db416ae7941f8ffeabc060ec87402b97314b6d: libcacard: Don't link with all libraries QEMU links to (2014-02-09 13:06:02 +0200) Christophe Fergeau (1): libcacard: Don't link with all libraries QEMU links to libcacard/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 1.8.5.3
[Qemu-devel] [PULL 1/1] libcacard: Don't link with all libraries QEMU links to
From: Christophe Fergeau cferg...@redhat.com As described in https://bugzilla.redhat.com/show_bug.cgi?id=987441 , libcacard currently links to all the libraries QEMU is linking to, including glusterfs libraries, libiscsi, ... libcacard does not need all of these. This patch ensures it's only linked with the libraries it needs. Signed-off-by: Christophe Fergeau cferg...@redhat.com Signed-off-by: Alon Levy al...@redhat.com --- libcacard/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcacard/Makefile b/libcacard/Makefile index 4d15da4..6b06448 100644 --- a/libcacard/Makefile +++ b/libcacard/Makefile @@ -25,7 +25,7 @@ vscclient$(EXESUF): libcacard/vscclient.o libcacard.la libcacard.la: LDFLAGS += -rpath $(libdir) -no-undefined \ -export-syms $(SRC_PATH)/libcacard/libcacard.syms -libcacard.la: LIBS += $(libcacard_libs) +libcacard.la: LIBS = $(libcacard_libs) libcacard.la: $(libcacard-lobj-y) $(call LINK,$^) -- 1.8.5.3
Re: [Qemu-devel] [PATCH] libcacard: Don't link with all libraries QEMU links to
On 01/30/2014 03:56 PM, Christophe Fergeau wrote: Acked-by: Alon Levy al...@redhat.com As described in https://bugzilla.redhat.com/show_bug.cgi?id=987441 , libcacard currently links to all the libraries QEMU is linking to, including glusterfs libraries, libiscsi, ... libcacard does not need all of these. This patch ensures it's only linked with the libraries it needs. Signed-off-by: Christophe Fergeau cferg...@redhat.com --- libcacard/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcacard/Makefile b/libcacard/Makefile index 4d15da4..6b06448 100644 --- a/libcacard/Makefile +++ b/libcacard/Makefile @@ -25,7 +25,7 @@ vscclient$(EXESUF): libcacard/vscclient.o libcacard.la libcacard.la: LDFLAGS += -rpath $(libdir) -no-undefined \ -export-syms $(SRC_PATH)/libcacard/libcacard.syms -libcacard.la: LIBS += $(libcacard_libs) +libcacard.la: LIBS = $(libcacard_libs) libcacard.la: $(libcacard-lobj-y) $(call LINK,$^)
[Qemu-devel] [PULL 1/1] libcacard: Don't link with all libraries QEMU links to
From: Christophe Fergeau cferg...@redhat.com As described in https://bugzilla.redhat.com/show_bug.cgi?id=987441 , libcacard currently links to all the libraries QEMU is linking to, including glusterfs libraries, libiscsi, ... libcacard does not need all of these. This patch ensures it's only linked with the libraries it needs. Signed-off-by: Christophe Fergeau cferg...@redhat.com --- libcacard/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcacard/Makefile b/libcacard/Makefile index 4d15da4..6b06448 100644 --- a/libcacard/Makefile +++ b/libcacard/Makefile @@ -25,7 +25,7 @@ vscclient$(EXESUF): libcacard/vscclient.o libcacard.la libcacard.la: LDFLAGS += -rpath $(libdir) -no-undefined \ -export-syms $(SRC_PATH)/libcacard/libcacard.syms -libcacard.la: LIBS += $(libcacard_libs) +libcacard.la: LIBS = $(libcacard_libs) libcacard.la: $(libcacard-lobj-y) $(call LINK,$^) -- 1.8.4.2
[Qemu-devel] [PULL 0/1] libcacard glusterfs fix
The following changes since commit 0706f7c85b3c0783f92d44b551f362884db0f4bd: Merge remote-tracking branch 'mjt/tags/trivial-patches-2014-01-16' into staging (2014-01-30 13:56:00 +) are available in the git repository at: git://people.freedesktop.org/~alon/qemu pull-libcacard.glusterfs for you to fetch changes up to 02c783d9d356f3e6410e46fc3f96114024e3fc32: libcacard: Don't link with all libraries QEMU links to (2014-01-30 16:54:06 +0200) Christophe Fergeau (1): libcacard: Don't link with all libraries QEMU links to libcacard/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 1.8.4.2
[Qemu-devel] [PATCH 1/2] hw/display/qxl: improve framebuffer error message
Signed-off-by: Alon Levy al...@redhat.com
---
hw/display/qxl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index e4f172e..f6af470 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1367,7 +1367,8 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl,
int loadvm,
size = abs(requested_stride) * requested_height;
if (size qxl-vgamem_size) {
qxl_set_guest_bug(qxl, %s: requested primary larger then framebuffer
-size, __func__);
+size %d %d, __func__, size,
+ qxl-vgamem_size);
return;
}
--
1.8.4.2
[Qemu-devel] [PATCH 2/2] qxl: clear irq on reset
Without this we occasionally trigger an assert at hw/pci/pci.c:pcibus_reset that asserts the irq_count is zero on reset. This has become a problem with the new drm driver for linux, since doing a reboot from console causes a race between console updates that set the irq and the reset assertion that the irq is clear. Signed-off-by: Alon Levy al...@redhat.com --- hw/display/qxl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/display/qxl.c b/hw/display/qxl.c index f6af470..e164d96 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -1126,6 +1126,7 @@ static void qxl_reset_state(PCIQXLDevice *d) d-num_free_res = 0; d-last_release = NULL; memset(d-ssd.dirty, 0, sizeof(d-ssd.dirty)); +qxl_update_irq(d); } static void qxl_soft_reset(PCIQXLDevice *d) -- 1.8.4.2
[Qemu-devel] [PATCH v2] hw/display/qxl: fix signed to unsigned comparison
Fix signed to unsigned comparison in qxl_create_guest_primary and add
the size of the framebuffer to the error message used when setting the
guest bug state (which causes a complete guess blackout until reset, so
it helps if it is verbose).
Signed-off-by: Alon Levy al...@redhat.com
---
hw/display/qxl.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index e4f172e..b799b51 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1360,14 +1360,15 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl,
int loadvm,
{
QXLDevSurfaceCreate surface;
QXLSurfaceCreate *sc = qxl-guest_primary.surface;
-int size;
+uint32_t size;
int requested_height = le32_to_cpu(sc-height);
int requested_stride = le32_to_cpu(sc-stride);
size = abs(requested_stride) * requested_height;
if (size qxl-vgamem_size) {
-qxl_set_guest_bug(qxl, %s: requested primary larger then framebuffer
-size, __func__);
+qxl_set_guest_bug(qxl, %s: requested primary larger than framebuffer
+size %u %u, __func__, size,
+ qxl-vgamem_size);
return;
}
--
1.8.4.2
[Qemu-devel] [PATCH v3] hw/display/qxl: fix signed to unsigned comparison
Several small signedness / overflow corrections to qxl_create_guest_primary:
1. use 64 bit unsigned for size to avoid overflow possible from two 32
bit multiplicants.
2. correct sign for requested_height
3. add a more verbose error message when setting guest bug state (which
causes a complete guess blackout until reset, so it helps if it is
verbose).
Signed-off-by: Alon Levy al...@redhat.com
---
hw/display/qxl.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index e4f172e..ba752b5 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1360,14 +1360,15 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl,
int loadvm,
{
QXLDevSurfaceCreate surface;
QXLSurfaceCreate *sc = qxl-guest_primary.surface;
-int size;
-int requested_height = le32_to_cpu(sc-height);
+uint64_t size;
+uint32_t requested_height = le32_to_cpu(sc-height);
int requested_stride = le32_to_cpu(sc-stride);
size = abs(requested_stride) * requested_height;
if (size qxl-vgamem_size) {
-qxl_set_guest_bug(qxl, %s: requested primary larger then framebuffer
-size, __func__);
+qxl_set_guest_bug(qxl, %s: requested primary larger than framebuffer
+size % PRIu64 % PRIu32, __func__, size,
+ qxl-vgamem_size);
return;
}
--
1.8.4.2
[Qemu-devel] [PATCH v4] hw/display/qxl: fix signed to unsigned comparison
Several small signedness / overflow corrections to qxl_create_guest_primary:
1. use 64 bit unsigned for size to avoid overflow possible from two 32
bit multiplicants.
2. correct sign for requested_height
3. add a more verbose error message when setting guest bug state (which
causes a complete guess blackout until reset, so it helps if it is
verbose).
Signed-off-by: Alon Levy al...@redhat.com
---
hw/display/qxl.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index e4f172e..ceae1d9 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -19,6 +19,7 @@
*/
#include zlib.h
+#include stdint.h
#include qemu-common.h
#include qemu/timer.h
@@ -1360,14 +1361,16 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl,
int loadvm,
{
QXLDevSurfaceCreate surface;
QXLSurfaceCreate *sc = qxl-guest_primary.surface;
-int size;
-int requested_height = le32_to_cpu(sc-height);
+uint32_t requested_height = le32_to_cpu(sc-height);
int requested_stride = le32_to_cpu(sc-stride);
-size = abs(requested_stride) * requested_height;
-if (size qxl-vgamem_size) {
-qxl_set_guest_bug(qxl, %s: requested primary larger then framebuffer
-size, __func__);
+if (requested_stride == INT32_MIN ||
+abs(requested_stride) * (uint64_t)requested_height
+ qxl-vgamem_size) {
+qxl_set_guest_bug(qxl, %s: requested primary larger than framebuffer
+stride %d x height % PRIu32 % PRIu32,
+ __func__, requested_stride, requested_height,
+ qxl-vgamem_size);
return;
}
--
1.8.4.2
[Qemu-devel] [PULL] libcacard fixes
Anthony, The following changes since commit a1d22a367d5780c9553b2cd5a24f665534ce6ed6: target-cris: Use new qemu_ld/st opcodes (2013-12-08 09:36:02 +0100) are available in the git repository at: git://people.freedesktop.org/~alon/qemu libcacard_ccid.4 for you to fetch changes up to 5ad04fb6f112cf2917909be4c22109dbb65fed18: libcacard: Fix compilation for older versions of glib (bug #1258168) (2013-12-09 12:19:05 +0200) Please pull, Thanks, Alon Stefan Weil (1): libcacard: Fix compilation for older versions of glib (bug #1258168) libcacard/vscclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 1.8.4.2
[Qemu-devel] [PULL] libcacard: Fix compilation for older versions of glib (bug #1258168)
From: Stefan Weil s...@weilnetz.de See https://bugs.launchpad.net/bugs/1258168 libcacard/vscclient.c: In function 'do_socket_read': libcacard/vscclient.c:410: warning: implicit declaration of function 'g_warn_if_reached' libcacard/vscclient.c:410: warning: nested extern declaration of 'g_warn_if_reached' libcacard/vscclient.c: In function 'main': libcacard/vscclient.c:763: warning: implicit declaration of function 'g_byte_array_unref' libcacard/vscclient.c:763: warning: nested extern declaration of 'g_byte_array_unref' ... libcacard/vscclient.o: In function `do_socket_read': libcacard/vscclient.c:410: undefined reference to `g_warn_if_reached' libcacard/vscclient.o: In function `main': libcacard/vscclient.c:763: undefined reference to `g_byte_array_unref' g_warn_if_reached was added in glib 2.16, and g_byte_array_unref is supported since glib 2.22. QEMU requires glib 2.12, so both names must not be used. Instead of showing a warning for code which should not be reached, vscclient better stop running, so g_warn_if_reached is not useful for vscclient. In libcacard/vsclient.c, g_byte_array_unref can be replaced by g_byte_array_free. This is not generally true, so adding a compatibility layer in include/glib-compat.h is no option here. Reported-by: Laurent Desnogues laurent.desnog...@gmail.com Reported-by: Don Slutz dsl...@verizon.com Signed-off-by: Stefan Weil s...@weilnetz.de --- libcacard/vscclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c index a3cb776..f1d46d3 100644 --- a/libcacard/vscclient.c +++ b/libcacard/vscclient.c @@ -407,7 +407,7 @@ do_socket_read(GIOChannel *source, } break; default: -g_warn_if_reached(); +g_assert_not_reached(); return FALSE; } @@ -760,7 +760,7 @@ main( g_io_channel_unref(channel_stdin); g_io_channel_unref(channel_socket); -g_byte_array_unref(socket_to_send); +g_byte_array_free(socket_to_send, TRUE); closesocket(sock); return 0; -- 1.8.4.2
Re: [Qemu-devel] [PATCH v2] libcacard: Fix compilation for older versions of glib (bug #1258168)
On 12/05/2013 08:41 PM, Stefan Weil wrote: See https://bugs.launchpad.net/bugs/1258168 libcacard/vscclient.c: In function 'do_socket_read': libcacard/vscclient.c:410: warning: implicit declaration of function 'g_warn_if_reached' libcacard/vscclient.c:410: warning: nested extern declaration of 'g_warn_if_reached' libcacard/vscclient.c: In function 'main': libcacard/vscclient.c:763: warning: implicit declaration of function 'g_byte_array_unref' libcacard/vscclient.c:763: warning: nested extern declaration of 'g_byte_array_unref' ... libcacard/vscclient.o: In function `do_socket_read': libcacard/vscclient.c:410: undefined reference to `g_warn_if_reached' libcacard/vscclient.o: In function `main': libcacard/vscclient.c:763: undefined reference to `g_byte_array_unref' g_warn_if_reached was added in glib 2.16, and g_byte_array_unref is supported since glib 2.22. QEMU requires glib 2.12, so both names must not be used. Instead of showing a warning for code which should not be reached, QEMU better stops running, so g_warn_if_reached is not useful for QEMU. Just note that this fix is in vscclient, so this code is not part of qemu executable. If you are using QEMU (all caps) as the whole project, the commit message is ok, but it should clarify this is about the separate vscclient executable. Other then that ACK, I can send a pull request with the commit message fixed as I suggested. In libcacard/vsclient.c, g_byte_array_unref can be replaced by g_byte_array_free. This is not generally true, so adding a compatibility layer in include/glib-compat.h is no option here. Reported-by: Laurent Desnogues laurent.desnog...@gmail.com Reported-by: Don Slutz dsl...@verizon.com Signed-off-by: Stefan Weil s...@weilnetz.de --- v2: Fix commit message and add missing parameter for g_byte_array_free. libcacard/vscclient.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c index a3cb776..f1d46d3 100644 --- a/libcacard/vscclient.c +++ b/libcacard/vscclient.c @@ -407,7 +407,7 @@ do_socket_read(GIOChannel *source, } break; default: -g_warn_if_reached(); +g_assert_not_reached(); return FALSE; } @@ -760,7 +760,7 @@ main( g_io_channel_unref(channel_stdin); g_io_channel_unref(channel_socket); -g_byte_array_unref(socket_to_send); +g_byte_array_free(socket_to_send, TRUE); closesocket(sock); return 0;
Re: [Qemu-devel] [PATCH] spice: flip streaming video mode to off by default
On 12/02/2013 12:27 PM, Gerd Hoffmann wrote:
Video streaming detection heuristics in spice-server have problems
keeping modern desktop animations (as done by gnome shell) and real
video playback apart. This leads to jpeg compression artefacts on
your desktop, due to spice using mjpeg to send what it thinks is
a video stream.
Turn off video detection by default to avoid these artifacts.
Reviewed-by: Alon Levy al...@redhat.com
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
ui/spice-core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index e4d533d..9fb9544 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -776,6 +776,8 @@ void qemu_spice_init(void)
if (str) {
int streaming_video = parse_stream_video(str);
spice_server_set_streaming_video(spice_server, streaming_video);
+} else {
+spice_server_set_streaming_video(spice_server,
SPICE_STREAM_VIDEO_OFF);
}
spice_server_set_agent_mouse
Re: [Qemu-devel] [PATCH 05/21] char: add qemu_chr_fe_event()
On 11/18/2013 02:25 PM, Marc-André Lureau wrote:
From: Marc-André Lureau marcandre.lur...@redhat.com
The patch description is incomplete, or the patch should be split - this
patch also implements qemu_chr_fe_event for spiceport.
Signed-off-by: Marc-André Lureau marcandre.lur...@redhat.com
---
include/sysemu/char.h | 10 ++
qemu-char.c | 7 +++
spice-qemu-char.c | 10 ++
3 files changed, 27 insertions(+)
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index ad101d9..d23c8f1 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -69,6 +69,7 @@ struct CharDriverState {
void (*chr_accept_input)(struct CharDriverState *chr);
void (*chr_set_echo)(struct CharDriverState *chr, bool echo);
void (*chr_set_fe_open)(struct CharDriverState *chr, int fe_open);
+void (*chr_fe_event)(struct CharDriverState *chr, int event);
void *opaque;
char *label;
char *filename;
@@ -138,6 +139,15 @@ void qemu_chr_fe_set_echo(struct CharDriverState *chr,
bool echo);
void qemu_chr_fe_set_open(struct CharDriverState *chr, int fe_open);
/**
+ * @qemu_chr_fe_event:
+ *
+ * Send an event from the back end to the front end.
+ *
+ * @event the event to send
+ */
+void qemu_chr_fe_event(CharDriverState *s, int event);
+
+/**
* @qemu_chr_fe_printf:
*
* Write to a character backend using a printf style interface.
diff --git a/qemu-char.c b/qemu-char.c
index e00f84c..418dc69 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3353,6 +3353,13 @@ void qemu_chr_fe_set_open(struct CharDriverState *chr,
int fe_open)
}
}
+void qemu_chr_fe_event(struct CharDriverState *chr, int event)
+{
+if (chr-chr_fe_event) {
+chr-chr_fe_event(chr, event);
+}
+}
+
int qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond,
GIOFunc func, void *user_data)
{
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index e074d9e..16439c5 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -222,6 +222,15 @@ static void spice_chr_set_fe_open(struct CharDriverState
*chr, int fe_open)
}
}
+static void spice_chr_fe_event(struct CharDriverState *chr, int event)
+{
+#if SPICE_SERVER_VERSION = 0x000c02
+SpiceCharDriver *s = chr-opaque;
+
+spice_server_port_event(s-sin, event);
+#endif
+}
+
static void print_allowed_subtypes(void)
{
const char** psubtype;
@@ -255,6 +264,7 @@ static CharDriverState *chr_open(const char *subtype)
chr-chr_close = spice_chr_close;
chr-chr_set_fe_open = spice_chr_set_fe_open;
chr-explicit_be_open = true;
+chr-chr_fe_event = spice_chr_fe_event;
QLIST_INSERT_HEAD(spice_chars, s, next);
Re: [Qemu-devel] [Spice-devel] [PATCH 20/21] spice-core: allow an interface to be in AIO context
On 11/18/2013 02:25 PM, Marc-André Lureau wrote:
The Spice block driver must be able complete operations within a AIO
context only.
Spice is currently only running within the main loop, and doesn't allow
the block driver to complete operations, such as flush during migration.
This patch allows a Spice interface to be associated with a different
context. Currently, the interface user_data is simply used to
differentiate main loop from AIO, but could later be used to associate
an interface with a particular thread.
Signed-off-by: Marc-André Lureau marcandre.lur...@gmail.com
---
include/ui/qemu-spice.h | 2 +-
qemu-char.c | 2 +-
spice-qemu-char.c | 9 +++
ui/spice-core.c | 62
+++--
4 files changed, 62 insertions(+), 13 deletions(-)
diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index a93b4b2..d5ba702 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -48,7 +48,7 @@ int qemu_spice_migrate_info(const char *hostname, int port,
int tls_port,
void do_info_spice_print(Monitor *mon, const QObject *data);
void do_info_spice(Monitor *mon, QObject **ret_data);
-CharDriverState *qemu_chr_open_spice_vmc(const char *type);
+CharDriverState *qemu_chr_open_spice_vmc(const char *type, bool aio);
#if SPICE_SERVER_VERSION = 0x000c02
CharDriverState *qemu_chr_open_spice_port(const char *name);
void qemu_spice_register_ports(void);
diff --git a/qemu-char.c b/qemu-char.c
index 418dc69..bfac7bf 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3747,7 +3747,7 @@ ChardevReturn *qmp_chardev_add(const char *id,
ChardevBackend *backend,
#endif
#ifdef CONFIG_SPICE
case CHARDEV_BACKEND_KIND_SPICEVMC:
-chr = qemu_chr_open_spice_vmc(backend-spicevmc-type);
+chr = qemu_chr_open_spice_vmc(backend-spicevmc-type, false);
break;
case CHARDEV_BACKEND_KIND_SPICEPORT:
chr = qemu_chr_open_spice_port(backend-spiceport-fqdn);
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 16439c5..421f7de 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -248,7 +248,7 @@ static void print_allowed_subtypes(void)
fprintf(stderr, \n);
}
-static CharDriverState *chr_open(const char *subtype)
+static CharDriverState *chr_open(const char *subtype, bool aio)
{
CharDriverState *chr;
SpiceCharDriver *s;
@@ -257,6 +257,7 @@ static CharDriverState *chr_open(const char *subtype)
s = g_malloc0(sizeof(SpiceCharDriver));
s-chr = chr;
s-active = false;
+s-sin.base.user_data = (void*)aio;
s-sin.subtype = g_strdup(subtype);
chr-opaque = s;
chr-chr_write = spice_chr_write;
@@ -271,7 +272,7 @@ static CharDriverState *chr_open(const char *subtype)
return chr;
}
-CharDriverState *qemu_chr_open_spice_vmc(const char *type)
+CharDriverState *qemu_chr_open_spice_vmc(const char *type, bool aio)
{
const char **psubtype = spice_server_char_device_recognized_subtypes();
@@ -291,7 +292,7 @@ CharDriverState *qemu_chr_open_spice_vmc(const char *type)
return NULL;
}
-return chr_open(type);
+return chr_open(type, aio);
}
#if SPICE_SERVER_VERSION = 0x000c02
@@ -305,7 +306,7 @@ CharDriverState *qemu_chr_open_spice_port(const char
*name)
return NULL;
}
-chr = chr_open(port);
+chr = chr_open(port, false);
s = chr-opaque;
s-sin.portname = g_strdup(name);
diff --git a/ui/spice-core.c b/ui/spice-core.c
index e4d533d..0f69630 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -53,34 +53,64 @@ static QemuThread me;
struct SpiceTimer {
QEMUTimer *timer;
+QEMUBH *bh;
QTAILQ_ENTRY(SpiceTimer) next;
};
static QTAILQ_HEAD(, SpiceTimer) timers = QTAILQ_HEAD_INITIALIZER(timers);
+#if SPICE_INTERFACE_CORE_MAJOR = 2
+static SpiceTimer *timer_add(SpiceTimerFunc func, void *opaque,
SpiceBaseInstance *sin)
+#else
static SpiceTimer *timer_add(SpiceTimerFunc func, void *opaque)
+#endif
{
SpiceTimer *timer;
timer = g_malloc0(sizeof(*timer));
-timer-timer = timer_new_ms(QEMU_CLOCK_REALTIME, func, opaque);
+
+#if SPICE_INTERFACE_CORE_MAJOR = 2
+bool aio = sin ? !!sin-user_data : false;
Shouldn't there be a cast there:
(bool)sin-user_data
?
+if (aio) {
+fprintf(stderr, AIO doesn't have timers yet, using BH\n);
+timer-bh = qemu_bh_new(func, opaque);
+} else
+#endif
+{
+timer-timer = timer_new_ms(QEMU_CLOCK_REALTIME, func, opaque);
+}
+
QTAILQ_INSERT_TAIL(timers, timer, next);
+
return timer;
}
static void timer_start(SpiceTimer *timer, uint32_t ms)
{
-timer_mod(timer-timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + ms);
+if (timer-bh) {
+qemu_bh_schedule_idle(timer-bh); /* at least every 10ms, see
async.c */
+} else {
+
Re: [Qemu-devel] [PATCH] qxl: replace pipe signaling with bottom half
On 10/30/2013 10:17 AM, Gerd Hoffmann wrote:
qxl creates a pipe, then writes something to it to wake up the iothread
from the spice server thread to raise an irq. These days qemu bottom
halves can be scheduled from threads and signals, so there is no reason
to do this any more. Time to clean it up.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
Reviewed-by: Alon Levy al...@redhat.com
---
hw/display/qxl.c | 33 +++--
hw/display/qxl.h | 3 +--
2 files changed, 4 insertions(+), 32 deletions(-)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index de835d6..41c34b1 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1701,15 +1701,9 @@ static const MemoryRegionOps qxl_io_ops = {
},
};
-static void pipe_read(void *opaque)
+static void qxl_update_irq_bh(void *opaque)
{
PCIQXLDevice *d = opaque;
-char dummy;
-int len;
-
-do {
-len = read(d-pipe[0], dummy, sizeof(dummy));
-} while (len == sizeof(dummy));
qxl_update_irq(d);
}
@@ -1730,28 +1724,7 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t
events)
if ((old_pending le_events) == le_events) {
return;
}
-if (qemu_thread_is_self(d-main)) {
-qxl_update_irq(d);
-} else {
-if (write(d-pipe[1], d, 1) != 1) {
-dprint(d, 1, %s: write to pipe failed\n, __func__);
-}
-}
-}
-
-static void init_pipe_signaling(PCIQXLDevice *d)
-{
-if (pipe(d-pipe) 0) {
-fprintf(stderr, %s:%s: qxl pipe creation failed\n,
-__FILE__, __func__);
-exit(1);
-}
-fcntl(d-pipe[0], F_SETFL, O_NONBLOCK);
-fcntl(d-pipe[1], F_SETFL, O_NONBLOCK);
-fcntl(d-pipe[0], F_SETOWN, getpid());
-
-qemu_thread_get_self(d-main);
-qemu_set_fd_handler(d-pipe[0], pipe_read, NULL, d);
+qemu_bh_schedule(d-update_irq);
}
/* graphics console */
@@ -2044,7 +2017,7 @@ static int qxl_init_common(PCIQXLDevice *qxl)
}
qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
-init_pipe_signaling(qxl);
+qxl-update_irq = qemu_bh_new(qxl_update_irq_bh, qxl);
qxl_reset_state(qxl);
qxl-update_area_bh = qemu_bh_new(qxl_render_update_area_bh, qxl);
diff --git a/hw/display/qxl.h b/hw/display/qxl.h
index 84f0182..c5de3d7 100644
--- a/hw/display/qxl.h
+++ b/hw/display/qxl.h
@@ -81,8 +81,7 @@ typedef struct PCIQXLDevice {
QemuMutex track_lock;
/* thread signaling */
-QemuThread main;
-intpipe[2];
+QEMUBH *update_irq;
/* ram pci bar */
QXLRam *ram;
Re: [Qemu-devel] [PATCH 0/2] Try to fix problem with emulated smartcards where invalid PIN succeeds
I started writing a blog post yesterday about virtualized smartcards here: https://blogs.gnome.org/halfline/2013/09/08/another-smartcard-post/ and while testing what I was writing I noticed an invalid PIN worked when it shouldn't have. It turns out that typing a valid PIN once in one program in the guest, is enough to make all future programs asking for the PIN to succeed regardless of what gets typed in for the PIN. I did some digging through the libcacard code, and noticed it uses the NSS PK11_Authenticate function which calls a function that has this comment above it: If we're already logged in and this function is called we will still prompt for a password, but we will probably succeed no matter what the password was. Also, PK11_Authenticate short-circuits to an early return SECSuccess if the token is already logged in. The two patches in this series attempt to correct this problem by calling PK11_Logout. I'm not 100% certain I've placed the PK11_Logout call in the best place, but it does seeming to fix the issue. Hi Ray, Thanks for the patches! It looks good to me but I'll defer to Robert, Alon
Re: [Qemu-devel] [PATCH v3 RESEND] libxl: Spice vdagent support for upstream qemu
Il 23/07/2013 15:42, Fabio Fantoni ha scritto:
Il 03/07/2013 15:54, fantonifa...@tiscali.it ha scritto:
Usage: spicevdagent=1|0 (default=0)
Enables spice vdagent. The Spice vdagent is an optional component for
enhancing user experience and performing guest-oriented management
tasks. Its features includes: client mouse mode (no need to grab mouse
by client, no mouse lag), automatic adjustment of screen resolution,
copy and paste (text and image) between client and domU. It also
requires vdagent service installed on domU o.s. to work.
Signed-off-by: Fabio Fantoni fabio.fant...@m2r.biz
---
docs/man/xl.cfg.pod.5 |9 +
tools/libxl/libxl_create.c |1 +
tools/libxl/libxl_dm.c |6 ++
tools/libxl/libxl_types.idl |1 +
tools/libxl/xl_cmdimpl.c|2 ++
5 files changed, 19 insertions(+)
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index f8b4576..766862d 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1123,6 +1123,15 @@ Specify the ticket password which is used by a
client for connection.
Whether SPICE agent is used for client mouse mode. The default is true
(turn on)
+=item Bspicevdagent=BOOLEAN
+
+Enables spice vdagent. The Spice vdagent is an optional component for
+enhancing user experience and performing guest-oriented management
+tasks. Its features includes: client mouse mode (no need to grab mouse
+by client, no mouse lag), automatic adjustment of screen resolution,
+copy and paste (text and image) between client and domU. It also
+requires vdagent service installed on domU o.s. to work. The default
is 0.
+
=back
=head3 Miscellaneous Emulated Hardware
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index cb9c822..8db5460 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -288,6 +288,7 @@ int libxl__domain_build_info_setdefault(libxl__gc
*gc,
libxl_defbool_setdefault(b_info-u.hvm.spice.disable_ticketing,
false);
libxl_defbool_setdefault(b_info-u.hvm.spice.agent_mouse, true);
+ libxl_defbool_setdefault(b_info-u.hvm.spice.vdagent, false);
}
libxl_defbool_setdefault(b_info-u.hvm.nographic, false);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index d10a58f..bc605e4 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -465,6 +465,12 @@ static char **
libxl__build_device_model_args_new(libxl__gc *gc,
flexarray_append(dm_args, -spice);
flexarray_append(dm_args, spiceoptions);
+if (libxl_defbool_val(b_info-u.hvm.spice.vdagent)) {
+flexarray_vappend(dm_args, -device, virtio-serial,
+-chardev, spicevmc,id=vdagent,name=vdagent,
-device,
+ virtserialport,chardev=vdagent,name=com.redhat.spice.0,
+NULL);
+}
}
switch (b_info-u.hvm.vga.kind) {
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index ecf1f0b..14425d1 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -172,6 +172,7 @@ libxl_spice_info = Struct(spice_info, [
(disable_ticketing, libxl_defbool),
(passwd, string),
(agent_mouse, libxl_defbool),
+(vdagent, libxl_defbool),
])
libxl_sdl_info = Struct(sdl_info, [
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index c1a969b..44a632c 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1491,6 +1491,8 @@ skip_vfb:
b_info-u.hvm.spice.passwd, 0);
xlu_cfg_get_defbool(config, spiceagent_mouse,
b_info-u.hvm.spice.agent_mouse, 0);
+xlu_cfg_get_defbool(config, spicevdagent,
+ b_info-u.hvm.spice.vdagent, 0);
xlu_cfg_get_defbool(config, nographic,
b_info-u.hvm.nographic, 0);
xlu_cfg_get_defbool(config, gfx_passthru,
b_info-u.hvm.gfx_passthru, 0);
I haven't seen any more replies about that patch.
The complete Ian question was:
What are the security implications ?
I don't know the details about security implications but I think risks
are minimal. vdagent is disabled by default, therefore you must enable
it if you want to use it, and you need the spice client and vdagent
must be installed on the domU os. Furthermore spice can be protected
with password and/or ssl.
In particular, does it mean that when the user has a spice client
connected to a guest, the guest can spy on the user's clipboard all
the time ?
I don't know, do you think we should ask for details about security
implications to qemu or spice experts? (added qemu-devel and
spice-devel on cc)
Ping...
Yes, the guest can see the users clipboard, it could automatically copy
anything the client makes available. There is no extra have you
[Qemu-devel] [PATCH] libcacard/vreader: remove never used define
Signed-off-by: Alon Levy al...@redhat.com
---
libcacard/vreader.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/libcacard/vreader.c b/libcacard/vreader.c
index 60eb43b..fd1a9ac 100644
--- a/libcacard/vreader.c
+++ b/libcacard/vreader.c
@@ -21,8 +21,6 @@
#include vevent.h
#include cac.h /* just for debugging defines */
-#define LIBCACARD_LOG_DOMAIN libcacard
-
struct VReaderStruct {
intreference_count;
VCard *card;
--
1.8.3.1
Re: [Qemu-devel] [PATCH 1/1] spice: fix display initialization
Spice has two display interface implementations: One integrated into
the qxl graphics card, and one generic which can operate with every
qemu-emulated graphics card.
The generic one is activated in case spice is used without qxl. The
logic for that only caught the -vga qxl case, -device qxl-vga goes
unnoticed. Fix that by adding a check in the spice interface
registration so we'll notice the qxl card no matter how it is created.
https://bugzilla.redhat.com/show_bug.cgi?id=981094
Reviewed-by: Alon Levy al...@redhat.com
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
include/sysemu/sysemu.h |1 -
include/ui/qemu-spice.h |2 ++
ui/spice-core.c |5 +
vl.c|2 +-
4 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 3caeb66..d7a77b6 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -103,7 +103,6 @@ typedef enum {
extern int vga_interface_type;
#define xenfb_enabled (vga_interface_type == VGA_XENFB)
-#define qxl_enabled (vga_interface_type == VGA_QXL)
extern int graphic_width;
extern int graphic_height;
diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index eba6d77..c6c756b 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -27,6 +27,7 @@
#include monitor/monitor.h
extern int using_spice;
+extern int spice_displays;
void qemu_spice_init(void);
void qemu_spice_input_init(void);
@@ -57,6 +58,7 @@ static inline CharDriverState
*qemu_chr_open_spice_port(const char *name)
#include monitor/monitor.h
#define using_spice 0
+#define spice_displays 0
static inline int qemu_spice_set_passwd(const char *passwd,
bool fail_if_connected,
bool disconnect_if_connected)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 033fd89..bd7a248 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -48,6 +48,7 @@ static char *auth_passwd;
static time_t auth_expires = TIME_MAX;
static int spice_migration_completed;
int using_spice = 0;
+int spice_displays;
static QemuThread me;
@@ -836,6 +837,10 @@ int qemu_spice_add_interface(SpiceBaseInstance *sin)
qemu_add_vm_change_state_handler(vm_change_state_handler, NULL);
}
+if (strcmp(sin-sif-type, SPICE_INTERFACE_QXL) == 0) {
+spice_displays++;
+}
+
return spice_server_add_interface(spice_server, sin);
}
diff --git a/vl.c b/vl.c
index 25b8f2f..f422a1c 100644
--- a/vl.c
+++ b/vl.c
@@ -4387,7 +4387,7 @@ int main(int argc, char **argv, char **envp)
}
#endif
#ifdef CONFIG_SPICE
-if (using_spice !qxl_enabled) {
+if (using_spice !spice_displays) {
qemu_spice_display_init(ds);
}
#endif
--
1.7.9.7
Re: [Qemu-devel] [PATCH] make screendump an async command
On Mon, 2013-06-17 at 08:06 +0200, Gerd Hoffmann wrote:
Hi,
Why is QXL unable to do a synchronous screendump?
Lazy rendering. By default spice-server doesn't render anything, it
just sends over all drawing ops to the client who does the actual
rendering for the user.
qemu can kick spice-server and ask it to render stuff locally if needed.
Happens when either the guest asks for it or when qemu needs it for its
own purposes. One reason is screendump, the other is qxl being used
with gtk/sdl/vnc ui instead of spice.
Today we kick the spice server worker thread to do the rendering, but we
don't wait for it completing the rendering before writing out the
screendump. That isn't perfect of course because you are not guaranteed
to get a up-to-date dump. But works good enougth for some use cases
like autotest, which does screendumps each second anyway, so the actual
screen dump is at most one second old.
Hmm, while thinking about it: There is another screendump change in the
pipeline: allow screen dumps from *any* device. So, I think this is
actually a very good reason to implement a new screendump command as I
think we can kill two birds with one stone then:
First we can add a new parameter to specify the device we want dump
from. And second we are not bound to the behavior of the existing
screendump command. So we could simply send out a qmp event as
completion (or error) notification.
Comments?
I personally think having an event is not really required, it
complicates things in qemu by requiring the command to return before it
issues any events, i.e. this would be illegal (and breaks libvirt):
Libvirt: { execute: screendump, arguments: { filename:
/tmp/image } }
Qemu: { event: SCREENDUMP_COMPLETE, data : { filename:
screenshot.ppm} }
Qemu: { return: {} }
But on the other hand if this is something that would be acceptable now
and having proper Async error handling is not forthcoming (why btw? is
anyone working on it) . But it would become baggage later on..
cheers,
Gerd
Regards,
Anthony Liguori
---
hmp.c | 2 +-
hw/display/qxl-render.c | 1 +
hw/display/vga.c | 1 +
include/qapi/qmp/qerror.h | 6 +
include/ui/console.h | 10
qapi-schema.json | 13 ---
qmp-commands.hx | 3 ++-
ui/console.c | 58
++-
8 files changed, 78 insertions(+), 16 deletions(-)
diff --git a/hmp.c b/hmp.c
index 494a9aa..2a37975 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1346,7 +1346,7 @@ void hmp_screen_dump(Monitor *mon, const QDict
*qdict)
const char *filename = qdict_get_str(qdict, filename);
Error *err = NULL;
-qmp_screendump(filename, err);
+hmp_screen_dump_helper(filename, err);
hmp_handle_error(mon, err);
}
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index f511a62..d719448 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -139,6 +139,7 @@ static void
qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
qxl-dirty[i].bottom - qxl-dirty[i].top);
}
qxl-num_dirty_rects = 0;
+console_screendump_complete(vga-con);
}
/*
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 21a108d..1fc52eb 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1922,6 +1922,7 @@ static void vga_update_display(void *opaque)
break;
}
}
+console_screendump_complete(s-con);
}
/* force a full display refresh */
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 6c0a18d..1bd7efa 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -237,6 +237,12 @@ void assert_no_error(Error *err);
#define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
ERROR_CLASS_GENERIC_ERROR, Migration is disabled when VirtFS export
path '%s' is mounted in the guest using mount_tag '%s'
+#define QERR_SCREENDUMP_NOT_AVAILABLE \
+ERROR_CLASS_GENERIC_ERROR, There is no QemuConsole I can screendump
from
+
+#define QERR_SCREENDUMP_IN_PROGRESS \
+ERROR_CLASS_GENERIC_ERROR, Existing screendump in progress
+
#define QERR_SOCKET_CONNECT_FAILED \
ERROR_CLASS_GENERIC_ERROR, Failed to connect to socket
diff --git a/include/ui/console.h b/include/ui/console.h
index 4307b5f..643da45 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -341,4 +341,14 @@ int index_from_keycode(int code);
void early_gtk_display_init(void);
void gtk_display_init(DisplayState *ds);
+/* hw/display/vga.c hw/display/qxl.c */
+void console_screendump_complete(QemuConsole *con);
+
+/* monitor.c */
+int qmp_screendump(Monitor *mon, const QDict *qdict,
+ MonitorCompletion cb, void *opaque);
+
+/* hmp.c */
+void
Re: [Qemu-devel] [PATCH] make screendump an async command
Hi,
Note: due to QAPI not generating async commands yet I had to remove the
schema screendump definition.
Hmm, that will break libvirt I suspect. Guess this one has to wait
until QAPI gained proper async command support.
It doesn't break anything. I've tested, and here is the QMP transcript, the
only difference is internal to qemu, from libvirt point of view it issues the
same command and gets the same response:
$ ./x86_64-softmmu/qemu-system-x86_64 -qmp stdio
{QMP: {version: {qemu: {micro: 50, minor: 5, major: 1}, package:
}, capabilities: []}}
{execute:qmp_capabilities}
{return: {}}
{execute:screendump,arguments:{filename:test2.ppm}}
{return: {}}
cheers,
Gerd
Re: [Qemu-devel] [PATCH] make screendump an async command
Hi,
Note: due to QAPI not generating async commands yet I had to remove the
schema screendump definition.
Hmm, that will break libvirt I suspect. Guess this one has to wait
until QAPI gained proper async command support.
It doesn't break anything. I've tested,
To clarify, I tested with virsh screenshot as well.
and here is the QMP transcript, the
only difference is internal to qemu, from libvirt point of view it issues
the same command and gets the same response:
$ ./x86_64-softmmu/qemu-system-x86_64 -qmp stdio
{QMP: {version: {qemu: {micro: 50, minor: 5, major: 1},
package: }, capabilities: []}}
{execute:qmp_capabilities}
{return: {}}
{execute:screendump,arguments:{filename:test2.ppm}}
{return: {}}
cheers,
Gerd
Re: [Qemu-devel] [PATCH] make screendump an async command
On Fri, 2013-06-14 at 13:21 -0500, Anthony Liguori wrote:
Alon Levy al...@redhat.com writes:
This fixes the broken screendump behavior for qxl devices in native mode
since 81fb6f1504fb9ef71f2382f44af34756668296e8.
Note: due to QAPI not generating async commands yet I had to remove the
schema screendump definition.
Related RHBZ: 973374
This patch is not enough to fix said bz, with the linux qxl driver you get
garbage still, just not out of date garbage.
Signed-off-by: Alon Levy al...@redhat.com
Async commands are badly broken with respect to error handling.
Are there any ideas on how this should work? From the user perspective
nothing changes, so this is an internal qemu design issue afaict.
This needs to be done after we get the new QMP server.
Is there any ETA on this? I'm not pressuring, I'm just trying to figure
out if it is eminent or else I'll add a temporary patch to the
downstream (which is what I'm trying to avoid by sending this patch in
the first place).
Why is QXL unable to do a synchronous screendump?
The qxl device needs to flush all the commands that haven't been
rendered. The rendering is done off thread in the worker thread for that
device. The communication between the threads happens via a pipe that
the io thread is listening to. That is the same thread the qmp command
is received on, so there is no option to block unless I start a new
event loop in there, which is ugly. i.e. I need some kind of bottom
half, like the async command provides, which client_migrate_info uses.
Regards,
Anthony Liguori
---
hmp.c | 2 +-
hw/display/qxl-render.c | 1 +
hw/display/vga.c | 1 +
include/qapi/qmp/qerror.h | 6 +
include/ui/console.h | 10
qapi-schema.json | 13 ---
qmp-commands.hx | 3 ++-
ui/console.c | 58
++-
8 files changed, 78 insertions(+), 16 deletions(-)
diff --git a/hmp.c b/hmp.c
index 494a9aa..2a37975 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1346,7 +1346,7 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
const char *filename = qdict_get_str(qdict, filename);
Error *err = NULL;
-qmp_screendump(filename, err);
+hmp_screen_dump_helper(filename, err);
hmp_handle_error(mon, err);
}
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index f511a62..d719448 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -139,6 +139,7 @@ static void
qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
qxl-dirty[i].bottom - qxl-dirty[i].top);
}
qxl-num_dirty_rects = 0;
+console_screendump_complete(vga-con);
}
/*
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 21a108d..1fc52eb 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1922,6 +1922,7 @@ static void vga_update_display(void *opaque)
break;
}
}
+console_screendump_complete(s-con);
}
/* force a full display refresh */
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 6c0a18d..1bd7efa 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -237,6 +237,12 @@ void assert_no_error(Error *err);
#define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
ERROR_CLASS_GENERIC_ERROR, Migration is disabled when VirtFS export
path '%s' is mounted in the guest using mount_tag '%s'
+#define QERR_SCREENDUMP_NOT_AVAILABLE \
+ERROR_CLASS_GENERIC_ERROR, There is no QemuConsole I can screendump
from
+
+#define QERR_SCREENDUMP_IN_PROGRESS \
+ERROR_CLASS_GENERIC_ERROR, Existing screendump in progress
+
#define QERR_SOCKET_CONNECT_FAILED \
ERROR_CLASS_GENERIC_ERROR, Failed to connect to socket
diff --git a/include/ui/console.h b/include/ui/console.h
index 4307b5f..643da45 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -341,4 +341,14 @@ int index_from_keycode(int code);
void early_gtk_display_init(void);
void gtk_display_init(DisplayState *ds);
+/* hw/display/vga.c hw/display/qxl.c */
+void console_screendump_complete(QemuConsole *con);
+
+/* monitor.c */
+int qmp_screendump(Monitor *mon, const QDict *qdict,
+ MonitorCompletion cb, void *opaque);
+
+/* hmp.c */
+void hmp_screen_dump_helper(const char *filename, Error **errp);
+
#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 5ad6894..f5fdc2f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3112,19 +3112,6 @@
'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
##
-# @screendump:
-#
-# Write a PPM of the VGA screen to a file.
-#
-# @filename: the path of a new PPM file to store the image
-#
-# Returns: Nothing on success
-#
-# Since: 0.14.0
-##
-{ 'command
[Qemu-devel] [PATCH] make screendump an async command
This fixes the broken screendump behavior for qxl devices in native mode
since 81fb6f1504fb9ef71f2382f44af34756668296e8.
Note: due to QAPI not generating async commands yet I had to remove the
schema screendump definition.
Related RHBZ: 973374
This patch is not enough to fix said bz, with the linux qxl driver you get
garbage still, just not out of date garbage.
Signed-off-by: Alon Levy al...@redhat.com
---
hmp.c | 2 +-
hw/display/qxl-render.c | 1 +
hw/display/vga.c | 1 +
include/qapi/qmp/qerror.h | 6 +
include/ui/console.h | 10
qapi-schema.json | 13 ---
qmp-commands.hx | 3 ++-
ui/console.c | 58 ++-
8 files changed, 78 insertions(+), 16 deletions(-)
diff --git a/hmp.c b/hmp.c
index 494a9aa..2a37975 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1346,7 +1346,7 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
const char *filename = qdict_get_str(qdict, filename);
Error *err = NULL;
-qmp_screendump(filename, err);
+hmp_screen_dump_helper(filename, err);
hmp_handle_error(mon, err);
}
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index f511a62..d719448 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -139,6 +139,7 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice
*qxl)
qxl-dirty[i].bottom - qxl-dirty[i].top);
}
qxl-num_dirty_rects = 0;
+console_screendump_complete(vga-con);
}
/*
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 21a108d..1fc52eb 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1922,6 +1922,7 @@ static void vga_update_display(void *opaque)
break;
}
}
+console_screendump_complete(s-con);
}
/* force a full display refresh */
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 6c0a18d..1bd7efa 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -237,6 +237,12 @@ void assert_no_error(Error *err);
#define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
ERROR_CLASS_GENERIC_ERROR, Migration is disabled when VirtFS export path
'%s' is mounted in the guest using mount_tag '%s'
+#define QERR_SCREENDUMP_NOT_AVAILABLE \
+ERROR_CLASS_GENERIC_ERROR, There is no QemuConsole I can screendump from
+
+#define QERR_SCREENDUMP_IN_PROGRESS \
+ERROR_CLASS_GENERIC_ERROR, Existing screendump in progress
+
#define QERR_SOCKET_CONNECT_FAILED \
ERROR_CLASS_GENERIC_ERROR, Failed to connect to socket
diff --git a/include/ui/console.h b/include/ui/console.h
index 4307b5f..643da45 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -341,4 +341,14 @@ int index_from_keycode(int code);
void early_gtk_display_init(void);
void gtk_display_init(DisplayState *ds);
+/* hw/display/vga.c hw/display/qxl.c */
+void console_screendump_complete(QemuConsole *con);
+
+/* monitor.c */
+int qmp_screendump(Monitor *mon, const QDict *qdict,
+ MonitorCompletion cb, void *opaque);
+
+/* hmp.c */
+void hmp_screen_dump_helper(const char *filename, Error **errp);
+
#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 5ad6894..f5fdc2f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3112,19 +3112,6 @@
'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
##
-# @screendump:
-#
-# Write a PPM of the VGA screen to a file.
-#
-# @filename: the path of a new PPM file to store the image
-#
-# Returns: Nothing on success
-#
-# Since: 0.14.0
-##
-{ 'command': 'screendump', 'data': {'filename': 'str'} }
-
-##
# @nbd-server-start:
#
# Start an NBD server listening on the given host and port. Block
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 8cea5e5..bde8f43 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -146,7 +146,8 @@ EQMP
{
.name = screendump,
.args_type = filename:F,
-.mhandler.cmd_new = qmp_marshal_input_screendump,
+.mhandler.cmd_async = qmp_screendump,
+.flags = MONITOR_CMD_ASYNC,
},
SQMP
diff --git a/ui/console.c b/ui/console.c
index 28bba6d..0efd588 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -116,6 +116,12 @@ typedef enum {
struct QemuConsole {
Object parent;
+struct {
+char *filename;
+MonitorCompletion *completion_cb;
+void *completion_opaque;
+} screendump;
+
int index;
console_type_t console_type;
DisplayState *ds;
@@ -313,11 +319,61 @@ write_err:
goto out;
}
-void qmp_screendump(const char *filename, Error **errp)
+int qmp_screendump(Monitor *mon, const QDict *qdict,
+ MonitorCompletion cb, void *opaque)
{
QemuConsole *con = qemu_console_lookup_by_index(0);
+const char *filename = qdict_get_str(qdict, filename);
+
+if (con == NULL) {
+qerror_report(QERR_SCREENDUMP_NOT_AVAILABLE);
+return -1;
+}
+if (filename
Re: [Qemu-devel] [PATCH] make screendump an async command
Il 13/06/2013 15:27, Alon Levy ha scritto:
This fixes the broken screendump behavior for qxl devices in native mode
since 81fb6f1504fb9ef71f2382f44af34756668296e8.
Note: due to QAPI not generating async commands yet I had to remove the
schema screendump definition.
Related RHBZ: 973374
This patch is not enough to fix said bz, with the linux qxl driver you get
garbage still, just not out of date garbage.
Signed-off-by: Alon Levy al...@redhat.com
---
hmp.c | 2 +-
hw/display/qxl-render.c | 1 +
hw/display/vga.c | 1 +
include/qapi/qmp/qerror.h | 6 +
include/ui/console.h | 10
qapi-schema.json | 13 ---
qmp-commands.hx | 3 ++-
ui/console.c | 58
++-
8 files changed, 78 insertions(+), 16 deletions(-)
diff --git a/hmp.c b/hmp.c
index 494a9aa..2a37975 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1346,7 +1346,7 @@ void hmp_screen_dump(Monitor *mon, const QDict
*qdict)
const char *filename = qdict_get_str(qdict, filename);
Error *err = NULL;
-qmp_screendump(filename, err);
+hmp_screen_dump_helper(filename, err);
hmp_handle_error(mon, err);
}
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index f511a62..d719448 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -139,6 +139,7 @@ static void
qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
qxl-dirty[i].bottom - qxl-dirty[i].top);
}
qxl-num_dirty_rects = 0;
+console_screendump_complete(vga-con);
}
/*
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 21a108d..1fc52eb 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1922,6 +1922,7 @@ static void vga_update_display(void *opaque)
break;
}
}
+console_screendump_complete(s-con);
}
/* force a full display refresh */
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 6c0a18d..1bd7efa 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -237,6 +237,12 @@ void assert_no_error(Error *err);
#define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
ERROR_CLASS_GENERIC_ERROR, Migration is disabled when VirtFS export
path '%s' is mounted in the guest using mount_tag '%s'
+#define QERR_SCREENDUMP_NOT_AVAILABLE \
+ERROR_CLASS_GENERIC_ERROR, There is no QemuConsole I can screendump
from
s/QemuConsole/graphical console/
+#define QERR_SCREENDUMP_IN_PROGRESS \
+ERROR_CLASS_GENERIC_ERROR, Existing screendump in progress
+
Please use error_setg instead.
I'm not sure what you are proposing. Something like this?
diff --git a/ui/console.c b/ui/console.c
index 0efd588..3a1e6ed 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -334,7 +334,9 @@ int qmp_screendump(Monitor *mon, const QDict *qdict,
return -1;
}
if (con-screendump.filename != NULL) {
-qerror_report(QERR_SCREENDUMP_IN_PROGRESS);
+QError *err = NULL;
+error_setg(err, Existing screendump in progress);
+monitor_set_error(mon, err);
return -1;
}
con-screendump.filename = g_strdup(filename);
(minus defining err in the inner scope)
#define QERR_SOCKET_CONNECT_FAILED \
ERROR_CLASS_GENERIC_ERROR, Failed to connect to socket
diff --git a/include/ui/console.h b/include/ui/console.h
index 4307b5f..643da45 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -341,4 +341,14 @@ int index_from_keycode(int code);
void early_gtk_display_init(void);
void gtk_display_init(DisplayState *ds);
+/* hw/display/vga.c hw/display/qxl.c */
+void console_screendump_complete(QemuConsole *con);
+
+/* monitor.c */
+int qmp_screendump(Monitor *mon, const QDict *qdict,
+ MonitorCompletion cb, void *opaque);
+
+/* hmp.c */
+void hmp_screen_dump_helper(const char *filename, Error **errp);
+
#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 5ad6894..f5fdc2f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3112,19 +3112,6 @@
'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
##
-# @screendump:
-#
-# Write a PPM of the VGA screen to a file.
-#
-# @filename: the path of a new PPM file to store the image
-#
-# Returns: Nothing on success
-#
-# Since: 0.14.0
-##
-{ 'command': 'screendump', 'data': {'filename': 'str'} }
Please just comment out the declaration with an explanation.
Paolo
-
-##
# @nbd-server-start:
#
# Start an NBD server listening on the given host and port. Block
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 8cea5e5..bde8f43 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -146,7 +146,8 @@ EQMP
{
.name = screendump
Re: [Qemu-devel] [Qemu-trivial] [PATCH 1/5] oslib-posix: add qemu_pipe_non_block
05.06.2013 00:23, Alon Levy wrote: [PATCH 1/5] oslib-posix: add qemu_pipe_non_block [PATCH 2/5] use qemu_pipe_non_block [PATCH 3/5] libcacard/vscclient: fix leakage of socket on error paths [PATCH 4/5] libcacard/vreader.c: fix possible NULL dereference [PATCH 5/5] libcacard/vscclient.c: fix use of uninitialized variable So, what happened with this series? I still plan to do it, but didn't get to it yet. From the above 5 patches, only 3/5 (leakage of socket on error paths) is ready to be applied. Should I apply it now, or wait for the respin of whole series? (Or both?) I'll be happy if you do. (Both don't actually make sense :) Thanks, /mjt
[Qemu-devel] [PATCH 1/5] oslib-posix: add qemu_pipe_non_block
Used by the followin patch.
Signed-off-by: Alon Levy al...@redhat.com
---
include/qemu-common.h | 1 +
util/oslib-posix.c| 19 +++
2 files changed, 20 insertions(+)
diff --git a/include/qemu-common.h b/include/qemu-common.h
index cb82ef3..c24d75c 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -232,6 +232,7 @@ ssize_t qemu_recv_full(int fd, void *buf, size_t count, int
flags)
#ifndef _WIN32
int qemu_pipe(int pipefd[2]);
+int qemu_pipe_non_block(int pipefd[2]);
#endif
#ifdef _WIN32
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 3dc8b1b..bc2ce2e 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -188,6 +188,25 @@ int qemu_pipe(int pipefd[2])
return ret;
}
+int qemu_pipe_non_block(int pipefd[2])
+{
+int ret;
+
+ret = qemu_pipe(pipefd);
+if (ret) {
+return ret;
+}
+if (fcntl(card-pipe[0], F_SETFL, O_NONBLOCK) == -1) {
+return -errno;
+}
+if (fcntl(card-pipe[1], F_SETFL, O_NONBLOCK) == -1) {
+return -errno;
+}
+if (fcntl(card-pipe[0], F_SETOWN, getpid()) == -1) {
+return -errno;
+}
+}
+
int qemu_utimens(const char *path, const struct timespec *times)
{
struct timeval tv[2], tv_now;
--
1.8.2.1
[Qemu-devel] [PATCH 2/5] use qemu_pipe_non_block
This fixes six instances of unchecked fcntl return status, spotted by
Coverity.
Signed-off-by: Alon Levy al...@redhat.com
---
hw/display/qxl.c| 10 +++---
hw/usb/ccid-card-emulated.c | 8 +++-
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 9e5b7ad..25c8c5a 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1797,15 +1797,11 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t
events)
static void init_pipe_signaling(PCIQXLDevice *d)
{
-if (pipe(d-pipe) 0) {
-fprintf(stderr, %s:%s: qxl pipe creation failed\n,
-__FILE__, __func__);
+if (qxl_pipe_non_block(d-pipe)) {
+fprintf(stderr, %s:%s: qxl pipe creation failed: %s\n,
+__FILE__, __func__, stderror(errno));
exit(1);
}
-fcntl(d-pipe[0], F_SETFL, O_NONBLOCK);
-fcntl(d-pipe[1], F_SETFL, O_NONBLOCK);
-fcntl(d-pipe[0], F_SETOWN, getpid());
-
qemu_thread_get_self(d-main);
qemu_set_fd_handler(d-pipe[0], pipe_read, NULL, d);
}
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index deb6d47..2e6942e 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -406,13 +406,11 @@ static void pipe_read(void *opaque)
static int init_pipe_signaling(EmulatedState *card)
{
-if (pipe(card-pipe) 0) {
-DPRINTF(card, 2, pipe creation failed\n);
+if (qemu_pipe_non_block(card-pipe) 0) {
+DPRINTF(card, 2, pipe creation failed: %s\n,
+strerror(errno));
return -1;
}
-fcntl(card-pipe[0], F_SETFL, O_NONBLOCK);
-fcntl(card-pipe[1], F_SETFL, O_NONBLOCK);
-fcntl(card-pipe[0], F_SETOWN, getpid());
qemu_set_fd_handler(card-pipe[0], pipe_read, NULL, card);
return 0;
}
--
1.8.2.1
[Qemu-devel] [PATCH 4/5] libcacard/vreader.c: fix possible NULL dereference
Reported by Coverity:
Error: FORWARD_NULL (CWE-476):
qemu-1.5.0/libcacard/vreader.c:267: cond_false: Condition card == NULL,
taking false branch
qemu-1.5.0/libcacard/vreader.c:269: if_end: End of if statement
qemu-1.5.0/libcacard/vreader.c:272: cond_false: Condition apdu == NULL,
taking false branch
qemu-1.5.0/libcacard/vreader.c:275: else_branch: Reached else branch
qemu-1.5.0/libcacard/vreader.c:280: cond_false: Condition response, taking
false branch
qemu-1.5.0/libcacard/vreader.c:284: if_end: End of if statement
qemu-1.5.0/libcacard/vreader.c:280: var_compare_op: Comparing response to
null implies that response might be null.
qemu-1.5.0/libcacard/vreader.c:286: cond_true: Condition card_status ==
VCARD_DONE, taking true branch
qemu-1.5.0/libcacard/vreader.c:287: cond_true: Condition card_status ==
VCARD_DONE, taking true branch
qemu-1.5.0/libcacard/vreader.c:288: var_deref_op: Dereferencing null pointer
response.
Signed-off-by: Alon Levy al...@redhat.com
---
libcacard/vreader.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcacard/vreader.c b/libcacard/vreader.c
index 5793d73..60eb43b 100644
--- a/libcacard/vreader.c
+++ b/libcacard/vreader.c
@@ -260,7 +260,7 @@ vreader_xfr_bytes(VReader *reader,
{
VCardAPDU *apdu;
VCardResponse *response = NULL;
-VCardStatus card_status;
+VCardStatus card_status = VCARD_FAIL;
unsigned short status;
VCard *card = vreader_get_card(reader);
--
1.8.2.1
[Qemu-devel] [PATCH 3/5] libcacard/vscclient: fix leakage of socket on error paths
Spotted by Coverity.
Signed-off-by: Alon Levy al...@redhat.com
---
libcacard/vscclient.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index ac23647..5180d29 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -618,18 +618,22 @@ connect_to_qemu(
if (ret != 0) {
/* Error */
fprintf(stderr, getaddrinfo failed\n);
-return -1;
+goto cleanup_socket;
}
if (connect(sock, server-ai_addr, server-ai_addrlen) 0) {
/* Error */
fprintf(stderr, Could not connect\n);
-return -1;
+goto cleanup_socket;
}
if (verbose) {
printf(Connected (sizeof Header=%zd)!\n, sizeof(VSCMsgHeader));
}
return sock;
+
+cleanup_socket:
+closesocket(sock);
+return -1;
}
int
@@ -759,5 +763,6 @@ main(
g_io_channel_unref(channel_socket);
g_byte_array_unref(socket_to_send);
+closesocket(sock);
return 0;
}
--
1.8.2.1
[Qemu-devel] [PATCH 5/5] libcacard/vscclient.c: fix use of uninitialized variable
Found by Coverity.
Signed-off-by: Alon Levy al...@redhat.com
---
libcacard/vscclient.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index 5180d29..4275c23 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -645,7 +645,7 @@ main(
GIOChannel *channel_stdin;
char *qemu_host;
char *qemu_port;
-VSCMsgHeader mhHeader;
+VSCMsgHeader mhHeader = {0,};
VCardEmulOptions *command_line_options = NULL;
--
1.8.2.1
Re: [Qemu-devel] [PATCH 2/5] use qemu_pipe_non_block
On 4 June 2013 21:23, Alon Levy al...@redhat.com wrote:
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1797,15 +1797,11 @@ static void qxl_send_events(PCIQXLDevice *d,
uint32_t events)
static void init_pipe_signaling(PCIQXLDevice *d)
{
-if (pipe(d-pipe) 0) {
-fprintf(stderr, %s:%s: qxl pipe creation failed\n,
-__FILE__, __func__);
+if (qxl_pipe_non_block(d-pipe)) {
Surely this can't compile? -- this function doesn't exist.
I am abusing my right to post to this list, sorry. I didn't actually try to
compile.
thanks
-- PMM
Re: [Qemu-devel] [PATCH 2/5] use qemu_pipe_non_block
Il 04/06/2013 22:23, Alon Levy ha scritto: This fixes six instances of unchecked fcntl return status, spotted by Coverity. I think we're just assuming that they cannot fail... I don't think we need the previous patch and this one, unless they help porting stuff to Windows. This was purely to satisfy coverity, but I thought we would want to check fcntl return status? also, shouldn't we be looping if EINTR? Paolo
Re: [Qemu-devel] [PATCH 1/5] oslib-posix: add qemu_pipe_non_block
On 06/04/2013 02:23 PM, Alon Levy wrote:
Used by the followin patch.
s/followin/following/
Thanks.
+int qemu_pipe_non_block(int pipefd[2])
+{
+int ret;
+
+ret = qemu_pipe(pipefd);
qemu_pipe() already uses pipe2() when available; it seems like it would
be nicer to use pipe2's O_NONBLOCK option directly in one syscall (where
supported) instead of having to make additional syscalls after the fact.
Would it just be smarter to change the signature of qemu_pipe() to add
a bool block parameter, and then change the 5 existing callers to pass
false with your later patch in the series passing true, and do it
without creating a new wrapper?
Answered below.
+if (ret) {
+return ret;
+}
+if (fcntl(card-pipe[0], F_SETFL, O_NONBLOCK) == -1) {
+return -errno;
+}
+if (fcntl(card-pipe[1], F_SETFL, O_NONBLOCK) == -1) {
+return -errno;
Leaks fds. If you're going to report error, then you must close the fds
already created.
As Peter pointed out, I should not go here, so I'll drop these checks, instead
doing naked fcntl calls, so no fd leak possible (no returns).
+}
+if (fcntl(card-pipe[0], F_SETOWN, getpid()) == -1) {
+return -errno;
Same comment about fd leaks.
This part seems like a useful change, IF you plan on using SIGIO and
SIGURG signals; and it is something which pipe2() cannot optimize, so I
can see why you are adding a new function instead of changing
qemu_pipe() and adjust all its callers to pass an additional parameter.
But are you really planning on using SIGIO/SIGURG?
I don't plan on using those signals, so I'll add a parameter instead.
Furthermore, this is undefined behavior. According to POSIX, use of
F_SETOWN is only portable on sockets, not pipes. It may work on Linux,
but you'll need to be aware of what it does on other platforms.
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
[Qemu-devel] [PATCH] libcacard/vscclient: fix leakage of socket on error paths
Spotted by Coverity.
Signed-off-by: Alon Levy al...@redhat.com
---
libcacard/vscclient.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index ac23647..9fcc548 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -618,18 +618,22 @@ connect_to_qemu(
if (ret != 0) {
/* Error */
fprintf(stderr, getaddrinfo failed\n);
-return -1;
+goto cleanup_socket;
}
if (connect(sock, server-ai_addr, server-ai_addrlen) 0) {
/* Error */
fprintf(stderr, Could not connect\n);
-return -1;
+goto cleanup_socket;
}
if (verbose) {
printf(Connected (sizeof Header=%zd)!\n, sizeof(VSCMsgHeader));
}
return sock;
+
+cleanup_socket:
+close(sock);
+return -1;
}
int
--
1.8.2.1
Re: [Qemu-devel] [PATCH] libcacard/vscclient: fix leakage of socket on error paths
Spotted by Coverity.
Self NACK.
I'll send a more complete patch, and use closesocket.
Signed-off-by: Alon Levy al...@redhat.com
---
libcacard/vscclient.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index ac23647..9fcc548 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -618,18 +618,22 @@ connect_to_qemu(
if (ret != 0) {
/* Error */
fprintf(stderr, getaddrinfo failed\n);
-return -1;
+goto cleanup_socket;
}
if (connect(sock, server-ai_addr, server-ai_addrlen) 0) {
/* Error */
fprintf(stderr, Could not connect\n);
-return -1;
+goto cleanup_socket;
}
if (verbose) {
printf(Connected (sizeof Header=%zd)!\n, sizeof(VSCMsgHeader));
}
return sock;
+
+cleanup_socket:
+close(sock);
+return -1;
}
int
--
1.8.2.1
Re: [Qemu-devel] [RFC] Add a stp file for usage from build directory
On Mon, 2013-05-27 at 08:32 +0200, Paolo Bonzini wrote: Il 27/05/2013 04:19, Alon Levy ha scritto: For systemtap the location of the process being tapped is crucial, so the existing stp file requires installation to use. A new file providing qemu.local prefixed probes lets scripts run without an install step. Signed-off-by: Alon Levy al...@redhat.com --- Makefile.target | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Makefile.target b/Makefile.target index ce4391f..5d176e2 100644 --- a/Makefile.target +++ b/Makefile.target @@ -35,7 +35,7 @@ config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak ifdef CONFIG_TRACE_SYSTEMTAP -stap: $(QEMU_PROG).stp +stap: $(QEMU_PROG).stp $(QEMU_PROG).local.stp ifdef CONFIG_USER_ONLY TARGET_TYPE=user @@ -51,6 +51,17 @@ $(QEMU_PROG).stp: $(SRC_PATH)/trace-events --target-arch=$(TARGET_ARCH) \ --target-type=$(TARGET_TYPE) \ $ $@, GEN $(TARGET_DIR)$(QEMU_PROG).stp) + +$(QEMU_PROG).local.stp: $(SRC_PATH)/trace-events + $(call quiet-command,$(TRACETOOL) \ + --format=stap \ + --backend=$(TRACE_BACKEND) \ + --binary=$(SRC_PATH)/$(TARGET_DIR)$(QEMU_PROG) \ This is not the build directory for out-of-tree builds. I'll fix that. + --probe-prefix=qemu.local \ Why change the prefix? It was one way to verify I was using the correct file. I'll change it back. But in general does it make sense for you to have this in addition to the existing stp file? Paolo + --target-arch=$(TARGET_ARCH) \ + --target-type=$(TARGET_TYPE) \ +$ $@, GEN $(TARGET_DIR)$(QEMU_PROG).local.stp) + else stap: endif
Re: [Qemu-devel] [RFC] Add a stp file for usage from build directory
On Tue, 2013-05-28 at 14:18 +0200, Paolo Bonzini wrote:
Il 28/05/2013 14:09, Alon Levy ha scritto:
+ --probe-prefix=qemu.local \
Why change the prefix?
It was one way to verify I was using the correct file. I'll change it
back.
But in general does it make sense for you to have this in addition to
the existing stp file?
I think it does (with the same prefix so that you can share the
scripts). But I'm not sure how you'd use it. :) Can you show an
example and also put it in the commit message?
I though it would be useful to have a bunch of scripts for developers. I
have the following (didn't fix yet to use the same prefix):
diff --git a/scripts/stap-qxl-generic b/scripts/stap-qxl-generic
new file mode 100755
index 000..1c21911
--- /dev/null
+++ b/scripts/stap-qxl-generic
@@ -0,0 +1,4 @@
+#!/bin/bash -x
+STP=$0.stp
+ROOT=`dirname $0`/../
+sudo stap -v -I $ROOT/x86_64-softmmu $STP
diff --git a/scripts/stap-qxl-generic.stp b/scripts/stap-qxl-generic.stp
new file mode 100644
index 000..e201e69
--- /dev/null
+++ b/scripts/stap-qxl-generic.stp
@@ -0,0 +1,24 @@
+probe begin { printf(starting qxl generic probe\n) }
+
+global call, threads
+
+probe qemu.local.qxl* {
+//printf(%d: %s: %s\n, tid(), pp(), $$vars)
+//print_ubacktrace()
+call[tid(), probefunc()] 1
+threads[tid()] 1
+}
+
+probe timer.s(%( $# 0 %? $1 %: 5 %)) {
+ ansi_clear_screen()
+printf(%10s %45s %s\n, TID, , HITS);
+foreach([t] in threads-) {
+printf(%10d %45s %d\n, t, , @count(threads[t]));
+}
+ printf(%10s %45s %s\n,
+ TID, CALL, HITS)
+ foreach([tid, name] in call-) {
+ printf(%10d %45s %d\n, tid, name,
+ @count(call[tid, name]))
+ }
+}
Paolo
Re: [Qemu-devel] [RFC] Add a stp file for usage from build directory
Il 28/05/2013 14:33, Alon Levy ha scritto:
On Tue, 2013-05-28 at 14:18 +0200, Paolo Bonzini wrote:
Il 28/05/2013 14:09, Alon Levy ha scritto:
+ --probe-prefix=qemu.local \
Why change the prefix?
It was one way to verify I was using the correct file. I'll change it
back.
But in general does it make sense for you to have this in addition to
the existing stp file?
I think it does (with the same prefix so that you can share the
scripts). But I'm not sure how you'd use it. :) Can you show an
example and also put it in the commit message?
I though it would be useful to have a bunch of scripts for developers. I
have the following (didn't fix yet to use the same prefix):
Ok, so it's just a matter of using the -I option to stap. That's the
bit that has to be in the commit message.
Yes. Note that you add a directory, not a file. The only sure way to notice
which hooks were used I found is looking at the generated c file.
Thanks!
Paolo
diff --git a/scripts/stap-qxl-generic b/scripts/stap-qxl-generic
new file mode 100755
index 000..1c21911
--- /dev/null
+++ b/scripts/stap-qxl-generic
@@ -0,0 +1,4 @@
+#!/bin/bash -x
+STP=$0.stp
+ROOT=`dirname $0`/../
+sudo stap -v -I $ROOT/x86_64-softmmu $STP
diff --git a/scripts/stap-qxl-generic.stp b/scripts/stap-qxl-generic.stp
new file mode 100644
index 000..e201e69
--- /dev/null
+++ b/scripts/stap-qxl-generic.stp
@@ -0,0 +1,24 @@
+probe begin { printf(starting qxl generic probe\n) }
+
+global call, threads
+
+probe qemu.local.qxl* {
+//printf(%d: %s: %s\n, tid(), pp(), $$vars)
+//print_ubacktrace()
+call[tid(), probefunc()] 1
+threads[tid()] 1
+}
+
+probe timer.s(%( $# 0 %? $1 %: 5 %)) {
+ ansi_clear_screen()
+printf(%10s %45s %s\n, TID, , HITS);
+foreach([t] in threads-) {
+printf(%10d %45s %d\n, t, , @count(threads[t]));
+}
+ printf(%10s %45s %s\n,
+ TID, CALL, HITS)
+ foreach([tid, name] in call-) {
+ printf(%10d %45s %d\n, tid, name,
+ @count(call[tid, name]))
+ }
+}
Paolo
Re: [Qemu-devel] [RFC] Add a stp file for usage from build directory
On Tue, 2013-05-28 at 09:25 -0400, Alon Levy wrote:
Il 28/05/2013 14:33, Alon Levy ha scritto:
On Tue, 2013-05-28 at 14:18 +0200, Paolo Bonzini wrote:
Il 28/05/2013 14:09, Alon Levy ha scritto:
+ --probe-prefix=qemu.local \
Why change the prefix?
Actually the problem is having two stp files (qemu-system-x86_64.stp
qemu-system-x86_64.local.stp) in the same directory providing the same
probes is not workable with stap. In pass 2 it doesn't have any specific
order of going through the directory. Solutions
1. have a different prefix
2. introduce another subdirectory
x86_64-softmmu/local.stp/qemu-system-x86_64.stp
It was one way to verify I was using the correct file. I'll change it
back.
But in general does it make sense for you to have this in addition to
the existing stp file?
I think it does (with the same prefix so that you can share the
scripts). But I'm not sure how you'd use it. :) Can you show an
example and also put it in the commit message?
I though it would be useful to have a bunch of scripts for developers. I
have the following (didn't fix yet to use the same prefix):
Ok, so it's just a matter of using the -I option to stap. That's the
bit that has to be in the commit message.
Yes. Note that you add a directory, not a file. The only sure way to notice
which hooks were used I found is looking at the generated c file.
Thanks!
Paolo
diff --git a/scripts/stap-qxl-generic b/scripts/stap-qxl-generic
new file mode 100755
index 000..1c21911
--- /dev/null
+++ b/scripts/stap-qxl-generic
@@ -0,0 +1,4 @@
+#!/bin/bash -x
+STP=$0.stp
+ROOT=`dirname $0`/../
+sudo stap -v -I $ROOT/x86_64-softmmu $STP
diff --git a/scripts/stap-qxl-generic.stp b/scripts/stap-qxl-generic.stp
new file mode 100644
index 000..e201e69
--- /dev/null
+++ b/scripts/stap-qxl-generic.stp
@@ -0,0 +1,24 @@
+probe begin { printf(starting qxl generic probe\n) }
+
+global call, threads
+
+probe qemu.local.qxl* {
+//printf(%d: %s: %s\n, tid(), pp(), $$vars)
+//print_ubacktrace()
+call[tid(), probefunc()] 1
+threads[tid()] 1
+}
+
+probe timer.s(%( $# 0 %? $1 %: 5 %)) {
+ ansi_clear_screen()
+printf(%10s %45s %s\n, TID, , HITS);
+foreach([t] in threads-) {
+printf(%10d %45s %d\n, t, , @count(threads[t]));
+}
+ printf(%10s %45s %s\n,
+ TID, CALL, HITS)
+ foreach([tid, name] in call-) {
+ printf(%10d %45s %d\n, tid, name,
+ @count(call[tid, name]))
+ }
+}
Paolo
[Qemu-devel] [PATCH] Add a stp file for usage from build directory
For systemtap the location of the process being tapped is crucial, as a result the existing stp file requires installation for use. There are now two files: $(TARGET_DIR)/$(QEMU_PROG).stp-installed: copied to $(tapdir)/$(QEMU_PROG).stp $(TARGET_DIR)/$(QEMU_PROG).stp: usable locally To use: stap -I $(TARGET_DIR) .. Signed-off-by: Alon Levy al...@redhat.com --- Makefile.target | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/Makefile.target b/Makefile.target index ce4391f..54687e2 100644 --- a/Makefile.target +++ b/Makefile.target @@ -35,7 +35,7 @@ config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak ifdef CONFIG_TRACE_SYSTEMTAP -stap: $(QEMU_PROG).stp +stap: $(QEMU_PROG).stp-installed $(QEMU_PROG).local.stp ifdef CONFIG_USER_ONLY TARGET_TYPE=user @@ -43,14 +43,24 @@ else TARGET_TYPE=system endif -$(QEMU_PROG).stp: $(SRC_PATH)/trace-events +$(QEMU_PROG).stp-installed: $(SRC_PATH)/trace-events $(call quiet-command,$(TRACETOOL) \ --format=stap \ --backend=$(TRACE_BACKEND) \ --binary=$(bindir)/$(QEMU_PROG) \ --target-arch=$(TARGET_ARCH) \ --target-type=$(TARGET_TYPE) \ -$ $@, GEN $(TARGET_DIR)$(QEMU_PROG).stp) +$ $@, GEN $(TARGET_DIR)$(QEMU_PROG).stp-installed) + +$(QEMU_PROG).local.stp: $(SRC_PATH)/trace-events + $(call quiet-command,$(TRACETOOL) \ + --format=stap \ + --backend=$(TRACE_BACKEND) \ + --binary=$(realpath .)/$(QEMU_PROG) \ + --target-arch=$(TARGET_ARCH) \ + --target-type=$(TARGET_TYPE) \ +$ $@, GEN $(TARGET_DIR)$(QEMU_PROG).local.stp) + else stap: endif @@ -186,7 +196,7 @@ endif endif ifdef CONFIG_TRACE_SYSTEMTAP $(INSTALL_DIR) $(DESTDIR)$(qemu_datadir)/../systemtap/tapset - $(INSTALL_DATA) $(QEMU_PROG).stp $(DESTDIR)$(qemu_datadir)/../systemtap/tapset + $(INSTALL_DATA) $(QEMU_PROG).stp-installed $(DESTDIR)$(qemu_datadir)/../systemtap/tapset/$(QEMU_PROG).stp endif GENERATED_HEADERS += config-target.h -- 1.8.2.1
[Qemu-devel] [PATCH v2] Add a stp file for usage from build directory
For systemtap the location of the process being tapped is crucial, as a result the existing stp file requires installation for use. There are now two files: $(TARGET_DIR)/$(QEMU_PROG).stp-installed: copied to $(tapdir)/$(QEMU_PROG).stp $(TARGET_DIR)/$(QEMU_PROG).stp: pointing to the built binary, usable without installation To use: stap -I $(TARGET_DIR) ... Signed-off-by: Alon Levy al...@redhat.com --- Makefile.target | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/Makefile.target b/Makefile.target index ce4391f..a44c8ed 100644 --- a/Makefile.target +++ b/Makefile.target @@ -35,7 +35,7 @@ config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak ifdef CONFIG_TRACE_SYSTEMTAP -stap: $(QEMU_PROG).stp +stap: $(QEMU_PROG).stp-installed $(QEMU_PROG).stp ifdef CONFIG_USER_ONLY TARGET_TYPE=user @@ -43,14 +43,24 @@ else TARGET_TYPE=system endif -$(QEMU_PROG).stp: $(SRC_PATH)/trace-events +$(QEMU_PROG).stp-installed: $(SRC_PATH)/trace-events $(call quiet-command,$(TRACETOOL) \ --format=stap \ --backend=$(TRACE_BACKEND) \ --binary=$(bindir)/$(QEMU_PROG) \ --target-arch=$(TARGET_ARCH) \ --target-type=$(TARGET_TYPE) \ +$ $@, GEN $(TARGET_DIR)$(QEMU_PROG).stp-installed) + +$(QEMU_PROG).stp: $(SRC_PATH)/trace-events + $(call quiet-command,$(TRACETOOL) \ + --format=stap \ + --backend=$(TRACE_BACKEND) \ + --binary=$(realpath .)/$(QEMU_PROG) \ + --target-arch=$(TARGET_ARCH) \ + --target-type=$(TARGET_TYPE) \ $ $@, GEN $(TARGET_DIR)$(QEMU_PROG).stp) + else stap: endif @@ -186,7 +196,7 @@ endif endif ifdef CONFIG_TRACE_SYSTEMTAP $(INSTALL_DIR) $(DESTDIR)$(qemu_datadir)/../systemtap/tapset - $(INSTALL_DATA) $(QEMU_PROG).stp $(DESTDIR)$(qemu_datadir)/../systemtap/tapset + $(INSTALL_DATA) $(QEMU_PROG).stp-installed $(DESTDIR)$(qemu_datadir)/../systemtap/tapset/$(QEMU_PROG).stp endif GENERATED_HEADERS += config-target.h -- 1.8.2.1
[Qemu-devel] [RFC] Add a stp file for usage from build directory
For systemtap the location of the process being tapped is crucial, so the existing stp file requires installation to use. A new file providing qemu.local prefixed probes lets scripts run without an install step. Signed-off-by: Alon Levy al...@redhat.com --- Makefile.target | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Makefile.target b/Makefile.target index ce4391f..5d176e2 100644 --- a/Makefile.target +++ b/Makefile.target @@ -35,7 +35,7 @@ config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak ifdef CONFIG_TRACE_SYSTEMTAP -stap: $(QEMU_PROG).stp +stap: $(QEMU_PROG).stp $(QEMU_PROG).local.stp ifdef CONFIG_USER_ONLY TARGET_TYPE=user @@ -51,6 +51,17 @@ $(QEMU_PROG).stp: $(SRC_PATH)/trace-events --target-arch=$(TARGET_ARCH) \ --target-type=$(TARGET_TYPE) \ $ $@, GEN $(TARGET_DIR)$(QEMU_PROG).stp) + +$(QEMU_PROG).local.stp: $(SRC_PATH)/trace-events + $(call quiet-command,$(TRACETOOL) \ + --format=stap \ + --backend=$(TRACE_BACKEND) \ + --binary=$(SRC_PATH)/$(TARGET_DIR)$(QEMU_PROG) \ + --probe-prefix=qemu.local \ + --target-arch=$(TARGET_ARCH) \ + --target-type=$(TARGET_TYPE) \ +$ $@, GEN $(TARGET_DIR)$(QEMU_PROG).local.stp) + else stap: endif -- 1.8.2.1
[Qemu-devel] [PATCH v3] arch_init/ram_load: add error message for block length mismatch
Makes it easier to debug situations where the source and target have
different ram blocks in a device and migration fails due to that, for
instance a BAR size change on a PCI device.
Signed-off-by: Alon Levy al...@redhat.com
---
v3: use RAM_ADDR_FMT
arch_init.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch_init.c b/arch_init.c
index 49c5dc2..cd27fcf 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -808,6 +808,9 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
QTAILQ_FOREACH(block, ram_list.blocks, next) {
if (!strncmp(id, block-idstr, sizeof(id))) {
if (block-length != length) {
+fprintf(stderr, Length mismatch: %s: %ld
+in != RAM_ADDR_FMT \n, id, length,
+block-length);
ret = -EINVAL;
goto done;
}
--
1.8.2.1
[Qemu-devel] [PULL] libcacard fix from Cole Robinson
The following changes since commit 909eedb74f88d1d6d9e6bbdc34875772e7a8a5ab: target-ppc: slightly optimize lfiwax (2013-04-27 00:37:46 +0200) are available in the git repository at: git://people.freedesktop.org/~alon/qemu libcacard_ccid.2 for you to fetch changes up to ae12e3a643c66575c77211e1226ada041e56b889: ccid: Fix crash when backend isn't specified (2013-04-27 02:38:33 +0300) Cole Robinson (1): ccid: Fix crash when backend isn't specified hw/usb/ccid-card-emulated.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-)
[Qemu-devel] [PULL] libcacard and ccid fixes
The following changes since commit bb71623811686ce3c34ce724f073f5c5dd95f51b: Move TPM passthrough specific command line options to backend structure (2013-04-23 10:40:40 -0500) are available in the git repository at: git://people.freedesktop.org/~alon/qemu libcacard_ccid.1 for you to fetch changes up to 203a368e66f5211b832e17b85c6f5dacfc8d7bf9: libcacard/cac: change big switch functions to single return point (2013-04-24 10:57:48 +0300) Alon Levy (15): dev-smartcard-reader: white space fixes dev-smartcard-reader: nicer debug messages dev-smartcard-reader: remove aborts (never triggered, but just in case) dev-smartcard-reader: support windows guest dev-smartcard-reader: reuse usb.h definitions libcacard: change default ATR ccid-card-passthru: add atr check ccid-card-passthru, dev-smartcard-reader: add debug environment variables dev-smartcard-reader: define structs for CCID_Parameter internals dev-smartcard-reader: change default protocol to T=0 dev-smartcard-reader: copy atr protocol to ccid parameters libcacard/vreader: add debugging messages for apdu libcacard: move atr setting from macro to function dev-smartcard-reader: empty implementation for Mechanical (fail correctly) libcacard/cac: change big switch functions to single return point Jim Meyering (2): ccid: make backend_enum_table static const and adjust users ccid: declare DEFAULT_ATR table to be static const Marc-André Lureau (11): libcacard: correct T0 historical bytes size ccid-card-emul: do not crash if backend is not provided libcacard: use system config directory for nss db on win32 util: move socket_init() to osdep.c build-sys: must link with -fstack-protector libcacard: fix mingw64 cross-compilation libcacard: split vscclient main() from socket reading libcacard: vscclient to use QemuThread for portability libcacard: teach vscclient to use GMainLoop for portability libcacard: remove sql: prefix libcacard: remove default libcoolkey loading Makefile | 8 +- Makefile.objs | 1 + configure | 8 +- hw/usb/ccid-card-emulated.c | 9 +- hw/usb/ccid-card-passthru.c | 63 +- hw/usb/dev-smartcard-reader.c | 243 +++- include/qemu-common.h | 5 + libcacard/cac.c | 80 --- libcacard/cac.h | 8 + libcacard/vcard_emul_nss.c| 47 ++-- libcacard/vcardt.c| 40 libcacard/vcardt.h| 5 - libcacard/vcardt_internal.h | 6 + libcacard/vreader.c | 77 +++ libcacard/vscclient.c | 506 +- rules.mak | 4 +- util/cutils.c | 23 ++ util/osdep.c | 23 ++ util/qemu-sockets.c | 24 -- 19 files changed, 838 insertions(+), 342 deletions(-) create mode 100644 libcacard/vcardt.c create mode 100644 libcacard/vcardt_internal.h
[Qemu-devel] [PATCH v3 00/28] ccid and libcacard fixes for windows/mingw
This series: 1. fixes windows guests to show the ccid device 2. changes libcacard to use glib 3. makes libcacard build under mingw 4. does some cleanups It contains a few patches already posted to the list (the two Jim Meyering patches) which were already acked. I'll make a pull request once this had some time to be reviewed. Tested with a fedora and windows 7 guest. The main non cleanup patches are: hw/usb/dev-smartcard-reader: support windows guest libcacard: correct T0 historical bytes size v3: resend all the series per Marc Andre's request. v2: only resent patches that were not acked before. did all the changes per Marc Andre's comments. Please look at the g_debug one, I'm not sure about the ifdeffery there (not if it works, but if it looks ok). Alon Levy (15): dev-smartcard-reader: white space fixes dev-smartcard-reader: nicer debug messages dev-smartcard-reader: remove aborts (never triggered, but just in case) dev-smartcard-reader: support windows guest dev-smartcard-reader: reuse usb.h definitions libcacard: change default ATR ccid-card-passthru: add atr check ccid-card-passthru, dev-smartcard-reader: add debug environment variables dev-smartcard-reader: define structs for CCID_Parameter internals dev-smartcard-reader: change default protocol to T=0 dev-smartcard-reader: copy atr protocol to ccid parameters libcacard/vreader: add debugging messages for apdu libcacard: move atr setting from macro to function dev-smartcard-reader: empty implementation for Mechanical (fail correctly) libcacard/cac: change big switch functions to single return point Jim Meyering (2): ccid: make backend_enum_table static const and adjust users ccid: declare DEFAULT_ATR table to be static const Marc-André Lureau (11): libcacard: correct T0 historical bytes size ccid-card-emul: do not crash if backend is not provided libcacard: use system config directory for nss db on win32 util: move socket_init() to osdep.c build-sys: must link with -fstack-protector libcacard: fix mingw64 cross-compilation libcacard: split vscclient main() from socket reading libcacard: vscclient to use QemuThread for portability libcacard: teach vscclient to use GMainLoop for portability libcacard: remove sql: prefix libcacard: remove default libcoolkey loading Makefile | 8 +- Makefile.objs | 1 + configure | 8 +- hw/usb/ccid-card-emulated.c | 9 +- hw/usb/ccid-card-passthru.c | 63 +- hw/usb/dev-smartcard-reader.c | 243 +++- include/qemu-common.h | 5 + libcacard/cac.c | 80 --- libcacard/cac.h | 8 + libcacard/vcard_emul_nss.c| 47 ++-- libcacard/vcardt.c| 40 libcacard/vcardt.h| 5 - libcacard/vcardt_internal.h | 6 + libcacard/vreader.c | 77 +++ libcacard/vscclient.c | 506 +- rules.mak | 4 +- util/cutils.c | 23 ++ util/osdep.c | 23 ++ util/qemu-sockets.c | 24 -- 19 files changed, 838 insertions(+), 342 deletions(-) create mode 100644 libcacard/vcardt.c create mode 100644 libcacard/vcardt_internal.h -- 1.8.2
[Qemu-devel] [PATCH v3 04/28] ccid: declare DEFAULT_ATR table to be static const
From: Jim Meyering meyer...@redhat.com
Signed-off-by: Jim Meyering meyer...@redhat.com
---
hw/usb/ccid-card-passthru.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
index 71a45f6..275b887 100644
--- a/hw/usb/ccid-card-passthru.c
+++ b/hw/usb/ccid-card-passthru.c
@@ -27,7 +27,7 @@ do {\
#define D_VERBOSE 4
/* TODO: do we still need this? */
-uint8_t DEFAULT_ATR[] = {
+static const uint8_t DEFAULT_ATR[] = {
/*
* From some example somewhere
* 0x3B, 0xB0, 0x18, 0x00, 0xD1, 0x81, 0x05, 0xB1, 0x40, 0x38, 0x1F, 0x03, 0x28
--
1.8.2
[Qemu-devel] [PATCH v3 07/28] build-sys: must link with -fstack-protector
From: Marc-André Lureau marcandre.lur...@gmail.com It is needed to give that flag to the linker as well, but latest libtool 2.4.2 still swallows that argument, so let's pass it with libtool -Wc argument. qemu-1.4.0/stubs/arch-query-cpu-def.c:6: undefined reference to `__stack_chk_guard' Signed-off-by: Marc-André Lureau marcandre.lur...@redhat.com --- configure | 8 +++- rules.mak | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/configure b/configure index ed49f91..dcc4767 100755 --- a/configure +++ b/configure @@ -1238,7 +1238,7 @@ fi gcc_flags=-Wold-style-declaration -Wold-style-definition -Wtype-limits gcc_flags=-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags gcc_flags=-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags -gcc_flags=-fstack-protector-all -Wendif-labels $gcc_flags +gcc_flags=-Wendif-labels $gcc_flags gcc_flags=-Wno-initializer-overrides $gcc_flags # Note that we do not add -Werror to gcc_flags here, because that would # enable it for all configure tests. If a configure test failed due @@ -1257,6 +1257,11 @@ for flag in $gcc_flags; do fi done +if compile_prog -Werror -fstack-protector-all ; then +QEMU_CFLAGS=$QEMU_CFLAGS -fstack-protector-all +LIBTOOLFLAGS=$LIBTOOLFLAGS -Wc,-fstack-protector-all +fi + # Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC and # large functions that use global variables. The bug is in all releases of # GCC, but it became particularly acute in 4.6.x and 4.7.x. It is fixed in @@ -4068,6 +4073,7 @@ else echo AUTOCONF_HOST := $config_host_mak fi echo LDFLAGS=$LDFLAGS $config_host_mak +echo LIBTOOLFLAGS=$LIBTOOLFLAGS $config_host_mak echo LIBS+=$LIBS $config_host_mak echo LIBS_TOOLS+=$libs_tools $config_host_mak echo EXESUF=$EXESUF $config_host_mak diff --git a/rules.mak b/rules.mak index edc2552..36aba2d 100644 --- a/rules.mak +++ b/rules.mak @@ -36,6 +36,7 @@ LINK = $(call quiet-command,\ $(if $(filter %.lo %.la,$^),$(LIBTOOL) --mode=link --tag=CC \ )$(CC) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \ $(sort $(filter %.o, $1)) $(filter-out %.o, $1) $(version-obj-y) \ + $(if $(filter %.lo %.la,$^),$(LIBTOOLFLAGS)) \ $(LIBS),$(if $(filter %.lo %.la,$^),lt LINK , LINK )$(TARGET_DIR)$@) endif -- 1.8.2
[Qemu-devel] [PATCH v3 06/28] util: move socket_init() to osdep.c
From: Marc-André Lureau marcandre.lur...@redhat.com
vscclient needs to call socket_init() for portability.
Moving to osdep.c since it has no internal dependency.
Signed-off-by: Marc-André Lureau marcandre.lur...@redhat.com
---
util/osdep.c| 23 +++
util/qemu-sockets.c | 24
2 files changed, 23 insertions(+), 24 deletions(-)
diff --git a/util/osdep.c b/util/osdep.c
index bd59ac9..6ae5aaf 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -406,3 +406,26 @@ bool fips_get_state(void)
return fips_enabled;
}
+#ifdef _WIN32
+static void socket_cleanup(void)
+{
+WSACleanup();
+}
+#endif
+
+int socket_init(void)
+{
+#ifdef _WIN32
+WSADATA Data;
+int ret, err;
+
+ret = WSAStartup(MAKEWORD(2, 2), Data);
+if (ret != 0) {
+err = WSAGetLastError();
+fprintf(stderr, WSAStartup: %d\n, err);
+return -1;
+}
+atexit(socket_cleanup);
+#endif
+return 0;
+}
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 94581aa..fdd8dc4 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -974,27 +974,3 @@ int socket_dgram(SocketAddress *remote, SocketAddress
*local, Error **errp)
qemu_opts_del(opts);
return fd;
}
-
-#ifdef _WIN32
-static void socket_cleanup(void)
-{
-WSACleanup();
-}
-#endif
-
-int socket_init(void)
-{
-#ifdef _WIN32
-WSADATA Data;
-int ret, err;
-
-ret = WSAStartup(MAKEWORD(2,2), Data);
-if (ret != 0) {
-err = WSAGetLastError();
-fprintf(stderr, WSAStartup: %d\n, err);
-return -1;
-}
-atexit(socket_cleanup);
-#endif
-return 0;
-}
--
1.8.2
[Qemu-devel] [PATCH v3 02/28] ccid-card-emul: do not crash if backend is not provided
From: Marc-André Lureau marcandre.lur...@gmail.com
Program received signal SIGSEGV, Segmentation fault.
__strcmp_sse42 () at ../sysdeps/x86_64/multiarch/strcmp-sse42.S:164
164 movdqu(%rsi), %xmm2
(gdb) bt
at /home/elmarco/320g/src/qemu/hw/ccid-card-emulated.c:477
at /home/elmarco/320g/src/qemu/hw/ccid-card-emulated.c:503
---
hw/usb/ccid-card-emulated.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index d534c94..6cbb176 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -473,6 +473,9 @@ static uint32_t parse_enumeration(char *str,
{
uint32_t ret = not_found_value;
+if (str == NULL)
+return 0;
+
while (table-name != NULL) {
if (strcmp(table-name, str) == 0) {
ret = table-value;
--
1.8.2
[Qemu-devel] [PATCH v3 03/28] ccid: make backend_enum_table static const and adjust users
From: Jim Meyering meyer...@redhat.com
Signed-off-by: Jim Meyering meyer...@redhat.com
---
hw/usb/ccid-card-emulated.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index 6cbb176..094284d 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -462,14 +462,14 @@ typedef struct EnumTable {
uint32_t value;
} EnumTable;
-EnumTable backend_enum_table[] = {
+static const EnumTable backend_enum_table[] = {
{BACKEND_NSS_EMULATED_NAME, BACKEND_NSS_EMULATED},
{BACKEND_CERTIFICATES_NAME, BACKEND_CERTIFICATES},
{NULL, 0},
};
static uint32_t parse_enumeration(char *str,
-EnumTable *table, uint32_t not_found_value)
+const EnumTable *table, uint32_t not_found_value)
{
uint32_t ret = not_found_value;
@@ -490,7 +490,7 @@ static int emulated_initfn(CCIDCardState *base)
{
EmulatedState *card = DO_UPCAST(EmulatedState, base, base);
VCardEmulError ret;
-EnumTable *ptable;
+const EnumTable *ptable;
QSIMPLEQ_INIT(card-event_list);
QSIMPLEQ_INIT(card-guest_apdu_list);
--
1.8.2
[Qemu-devel] [PATCH v3 08/28] libcacard: fix mingw64 cross-compilation
From: Marc-André Lureau marcandre.lur...@gmail.com Compile and link with version.lo Signed-off-by: Marc-André Lureau marcandre.lur...@redhat.com --- Makefile | 8 ++-- rules.mak | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 88375dc..b6ad03a 100644 --- a/Makefile +++ b/Makefile @@ -166,11 +166,15 @@ recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES) bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS) -version.o: $(SRC_PATH)/version.rc config-host.h +version.o: $(SRC_PATH)/version.rc config-host.h | version.lo $(call quiet-command,$(WINDRES) -I. -o $@ $, RC$(TARGET_DIR)$@) +version.lo: $(SRC_PATH)/version.rc config-host.h + $(call quiet-command,$(LIBTOOL) --mode=compile --tag=RC $(WINDRES) -I. -o $@ $,lt RC $(TARGET_DIR)$@) version-obj-$(CONFIG_WIN32) += version.o -Makefile: $(version-obj-y) +version-lobj-$(CONFIG_WIN32) += $(if $(LIBTOOL),version.lo) +Makefile: $(version-obj-y) $(version-lobj-y) + ## # Build libraries diff --git a/rules.mak b/rules.mak index 36aba2d..292a422 100644 --- a/rules.mak +++ b/rules.mak @@ -35,7 +35,8 @@ LIBTOOL += $(if $(V),,--quiet) LINK = $(call quiet-command,\ $(if $(filter %.lo %.la,$^),$(LIBTOOL) --mode=link --tag=CC \ )$(CC) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \ - $(sort $(filter %.o, $1)) $(filter-out %.o, $1) $(version-obj-y) \ + $(sort $(filter %.o, $1)) $(filter-out %.o, $1) \ + $(if $(filter %.lo %.la,$^),$(version-lobj-y),$(version-obj-y)) \ $(if $(filter %.lo %.la,$^),$(LIBTOOLFLAGS)) \ $(LIBS),$(if $(filter %.lo %.la,$^),lt LINK , LINK )$(TARGET_DIR)$@) endif -- 1.8.2
[Qemu-devel] [PATCH v3 12/28] libcacard: remove sql: prefix
From: Marc-André Lureau marcandre.lur...@gmail.com
For some reason, with sql:/ prefix, the PKCS11 modules are not loaded.
This patch goes on top of Alon smartcard series.
---
libcacard/vcard_emul_nss.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index 21d4689..6b1ca8a 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -893,7 +893,7 @@ vcard_emul_init(const VCardEmulOptions *options)
if (options-nss_db) {
rv = NSS_Init(options-nss_db);
} else {
-gchar *path, *db;
+gchar *path;
#ifndef _WIN32
path = g_strdup(/etc/pki/nssdb);
#else
@@ -905,10 +905,8 @@ vcard_emul_init(const VCardEmulOptions *options)
path = g_build_filename(
g_get_system_config_dirs()[0], pki, nssdb, NULL);
#endif
-db = g_strdup_printf(sql:%s, path);
-rv = NSS_Init(db);
-g_free(db);
+rv = NSS_Init(path);
g_free(path);
}
if (rv != SECSuccess) {
--
1.8.2
[Qemu-devel] [PATCH v3 13/28] libcacard: remove default libcoolkey loading
From: Marc-André Lureau marcandre.lur...@gmail.com
Use only the modules defined in the NSS database.
---
libcacard/vcard_emul_nss.c | 17 +
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index 6b1ca8a..9ba80fb 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -870,7 +870,7 @@ VCardEmulError
vcard_emul_init(const VCardEmulOptions *options)
{
SECStatus rv;
-PRBool ret, has_readers = PR_FALSE, need_coolkey_module;
+PRBool ret, has_readers = PR_FALSE;
VReader *vreader;
VReaderEmul *vreader_emul;
SECMODListLock *module_lock;
@@ -983,30 +983,15 @@ vcard_emul_init(const VCardEmulOptions *options)
/* make sure we have some PKCS #11 module loaded */
module_lock = SECMOD_GetDefaultModuleListLock();
module_list = SECMOD_GetDefaultModuleList();
-need_coolkey_module = !has_readers;
SECMOD_GetReadLock(module_lock);
for (mlp = module_list; mlp; mlp = mlp-next) {
SECMODModule *module = mlp-module;
if (module_has_removable_hw_slots(module)) {
-need_coolkey_module = PR_FALSE;
break;
}
}
SECMOD_ReleaseReadLock(module_lock);
-if (need_coolkey_module) {
-SECMODModule *module;
-module = SECMOD_LoadUserModule(
-(char *)library=libcoolkeypk11.so name=Coolkey,
-NULL, PR_FALSE);
-if (module == NULL) {
-return VCARD_EMUL_FAIL;
-}
-SECMOD_DestroyModule(module); /* free our reference, Module will still
- * be on the list.
- * until we destroy it */
-}
-
/* now examine all the slots, finding which should be readers */
/* We should control this with options. For now we mirror out any
* removable hardware slot */
--
1.8.2
[Qemu-devel] [PATCH v3 05/28] libcacard: use system config directory for nss db on win32
From: Marc-André Lureau marcandre.lur...@gmail.com
It's a bit nicer to look for default database under
CSIDL_COMMON_APPDATA\pki\nss rather that /etc/pki/nss.
Signed-off-by: Marc-André Lureau marcandre.lur...@redhat.com
---
libcacard/vcard_emul_nss.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index df79476..21d4689 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -893,7 +893,23 @@ vcard_emul_init(const VCardEmulOptions *options)
if (options-nss_db) {
rv = NSS_Init(options-nss_db);
} else {
-rv = NSS_Init(sql:/etc/pki/nssdb);
+gchar *path, *db;
+#ifndef _WIN32
+path = g_strdup(/etc/pki/nssdb);
+#else
+if (g_get_system_config_dirs() == NULL ||
+g_get_system_config_dirs()[0] == NULL) {
+return VCARD_EMUL_FAIL;
+}
+
+path = g_build_filename(
+g_get_system_config_dirs()[0], pki, nssdb, NULL);
+#endif
+db = g_strdup_printf(sql:%s, path);
+
+rv = NSS_Init(db);
+g_free(db);
+g_free(path);
}
if (rv != SECSuccess) {
return VCARD_EMUL_FAIL;
--
1.8.2
[Qemu-devel] [PATCH v3 14/28] dev-smartcard-reader: white space fixes
Signed-off-by: Alon Levy al...@redhat.com
---
hw/usb/dev-smartcard-reader.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index db8ce02..57eb803 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -471,6 +471,7 @@ static const USBDesc desc_ccid = {
static const uint8_t *ccid_card_get_atr(CCIDCardState *card, uint32_t *len)
{
CCIDCardClass *cc = CCID_CARD_GET_CLASS(card);
+
if (cc-get_atr) {
return cc-get_atr(card, len);
}
@@ -482,6 +483,7 @@ static void ccid_card_apdu_from_guest(CCIDCardState *card,
uint32_t len)
{
CCIDCardClass *cc = CCID_CARD_GET_CLASS(card);
+
if (cc-apdu_from_guest) {
cc-apdu_from_guest(card, apdu, len);
}
@@ -490,6 +492,7 @@ static void ccid_card_apdu_from_guest(CCIDCardState *card,
static int ccid_card_exitfn(CCIDCardState *card)
{
CCIDCardClass *cc = CCID_CARD_GET_CLASS(card);
+
if (cc-exitfn) {
return cc-exitfn(card);
}
@@ -499,6 +502,7 @@ static int ccid_card_exitfn(CCIDCardState *card)
static int ccid_card_initfn(CCIDCardState *card)
{
CCIDCardClass *cc = CCID_CARD_GET_CLASS(card);
+
if (cc-initfn) {
return cc-initfn(card);
}
--
1.8.2
[Qemu-devel] [PATCH v3 17/28] dev-smartcard-reader: support windows guest
By not advertising USB wakeup support (which we don't).
Signed-off-by: Alon Levy al...@redhat.com
---
hw/usb/dev-smartcard-reader.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 6133edf..38c3c0e 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -359,11 +359,11 @@ static const uint8_t qemu_ccid_descriptor[] = {
* 2 Short APDU level exchange with CCID
* 4 Short and Extended APDU level exchange with CCID
*
- * + 10 USB Wake up signaling supported on card
+ * 10 USB Wake up signaling supported on card
* insertion and removal. Must set bit 5 in bmAttributes
* in Configuration descriptor if 10 is set.
*/
-0xfe, 0x04, 0x11, 0x00,
+0xfe, 0x04, 0x01, 0x00,
/*
* u32 dwMaxCCIDMessageLength; For extended APDU in
* [261 + 10 , 65544 + 10]. Otherwise the minimum is
--
1.8.2
[Qemu-devel] [PATCH v3 11/28] libcacard: teach vscclient to use GMainLoop for portability
From: Marc-André Lureau marcandre.lur...@gmail.com
This version handles non-blocking sending and receiving from the
socket.
Signed-off-by: Marc-André Lureau marcandre.lur...@redhat.com
---
libcacard/vscclient.c | 391 +++---
1 file changed, 246 insertions(+), 145 deletions(-)
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index 5f47634..ac23647 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -10,7 +10,10 @@
* See the COPYING.LIB file in the top-level directory.
*/
+#ifndef _WIN32
#include netdb.h
+#endif
+#include glib.h
#include qemu-common.h
#include qemu/thread.h
@@ -22,9 +25,7 @@
#include vcard_emul.h
#include vevent.h
-int verbose;
-
-int sock;
+static int verbose;
static void
print_byte_array(
@@ -51,7 +52,47 @@ print_usage(void) {
vcard_emul_usage();
}
-static QemuMutex write_lock;
+static GIOChannel *channel_socket;
+static GByteArray *socket_to_send;
+static QemuMutex socket_to_send_lock;
+static guint socket_tag;
+
+static void
+update_socket_watch(gboolean out);
+
+static gboolean
+do_socket_send(GIOChannel *source,
+ GIOCondition condition,
+ gpointer data)
+{
+gsize bw;
+GError *err = NULL;
+
+g_return_val_if_fail(socket_to_send-len != 0, FALSE);
+g_return_val_if_fail(condition G_IO_OUT, FALSE);
+
+g_io_channel_write_chars(channel_socket,
+(gchar *)socket_to_send-data, socket_to_send-len, bw, err);
+if (err != NULL) {
+g_error(Error while sending socket %s, err-message);
+return FALSE;
+}
+g_byte_array_remove_range(socket_to_send, 0, bw);
+
+if (socket_to_send-len == 0) {
+update_socket_watch(FALSE);
+return FALSE;
+}
+return TRUE;
+}
+
+static gboolean
+socket_prepare_sending(gpointer user_data)
+{
+update_socket_watch(TRUE);
+
+return FALSE;
+}
static int
send_msg(
@@ -60,10 +101,9 @@ send_msg(
const void *msg,
unsigned int length
) {
-int rv;
VSCMsgHeader mhHeader;
-qemu_mutex_lock(write_lock);
+qemu_mutex_lock(socket_to_send_lock);
if (verbose 10) {
printf(sending type=%d id=%u, len =%u (0x%x)\n,
@@ -73,23 +113,11 @@ send_msg(
mhHeader.type = htonl(type);
mhHeader.reader_id = 0;
mhHeader.length = htonl(length);
-rv = write(sock, mhHeader, sizeof(mhHeader));
-if (rv 0) {
-/* Error */
-fprintf(stderr, write header error\n);
-close(sock);
-qemu_mutex_unlock(write_lock);
-return 16;
-}
-rv = write(sock, msg, length);
-if (rv 0) {
-/* Error */
-fprintf(stderr, write error\n);
-close(sock);
-qemu_mutex_unlock(write_lock);
-return 16;
-}
-qemu_mutex_unlock(write_lock);
+g_byte_array_append(socket_to_send, (guint8 *)mhHeader, sizeof(mhHeader));
+g_byte_array_append(socket_to_send, (guint8 *)msg, length);
+g_idle_add(socket_prepare_sending, NULL);
+
+qemu_mutex_unlock(socket_to_send_lock);
return 0;
}
@@ -245,139 +273,203 @@ on_host_init(VSCMsgHeader *mhHeader, VSCMsgInit
*incoming)
return 0;
}
+
+enum {
+STATE_HEADER,
+STATE_MESSAGE,
+};
+
#define APDUBufSize 270
-static int
-do_socket_read(void)
+static gboolean
+do_socket_read(GIOChannel *source,
+ GIOCondition condition,
+ gpointer data)
{
int rv;
int dwSendLength;
int dwRecvLength;
uint8_t pbRecvBuffer[APDUBufSize];
-uint8_t pbSendBuffer[APDUBufSize];
+static uint8_t pbSendBuffer[APDUBufSize];
VReaderStatus reader_status;
VReader *reader = NULL;
-VSCMsgHeader mhHeader;
+static VSCMsgHeader mhHeader;
VSCMsgError *error_msg;
+GError *err = NULL;
-rv = read(sock, mhHeader, sizeof(mhHeader));
-if (rv sizeof(mhHeader)) {
-/* Error */
-if (rv 0) {
-perror(header read error\n);
-} else {
-fprintf(stderr, header short read %d\n, rv);
-}
-return -1;
-}
-mhHeader.type = ntohl(mhHeader.type);
-mhHeader.reader_id = ntohl(mhHeader.reader_id);
-mhHeader.length = ntohl(mhHeader.length);
-if (verbose) {
-printf(Header: type=%d, reader_id=%u length=%d (0x%x)\n,
- mhHeader.type, mhHeader.reader_id, mhHeader.length,
- mhHeader.length);
-}
-switch (mhHeader.type) {
-case VSC_APDU:
-case VSC_Flush:
-case VSC_Error:
-case VSC_Init:
-rv = read(sock, pbSendBuffer, mhHeader.length);
-break;
-default:
-fprintf(stderr, Unexpected message of type 0x%X\n, mhHeader.type);
-return -1;
+static gchar *buf;
+static gsize br, to_read;
+static int state = STATE_HEADER;
+
+if (state == STATE_HEADER to_read == 0) {
+buf = (gchar *)mhHeader;
+to_read = sizeof(mhHeader);
}
-switch (mhHeader.type) {
-case VSC_APDU:
-if
[Qemu-devel] [PATCH v3 19/28] libcacard: change default ATR
Signed-off-by: Alon Levy al...@redhat.com
---
libcacard/vcardt.h | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/libcacard/vcardt.h b/libcacard/vcardt.h
index 538bdde..3b9a619 100644
--- a/libcacard/vcardt.h
+++ b/libcacard/vcardt.h
@@ -26,9 +26,17 @@ typedef struct VCardEmulStruct VCardEmul;
#define MAX_CHANNEL 4
/* create an ATR with appropriate historical bytes */
-#define VCARD_ATR_PREFIX(size) 0x3b, 0x66+(size), 0x00, 0xff, \
- 'V', 'C', 'A', 'R', 'D', '_'
+#define TS_DIRECT_CONVENTION 0x3b
+#define TA_PRESENT 0x10
+#define TB_PRESENT 0x20
+#define TC_PRESENT 0x40
+#define TD_PRESENT 0x80
+#define VCARD_ATR_PREFIX(size) \
+TS_DIRECT_CONVENTION, \
+TD_PRESENT + (6 + size), \
+0x00, \
+'V', 'C', 'A', 'R', 'D', '_'
typedef enum {
VCARD_DONE,
--
1.8.2
[Qemu-devel] [PATCH v3 16/28] dev-smartcard-reader: remove aborts (never triggered, but just in case)
Signed-off-by: Alon Levy al...@redhat.com
---
hw/usb/dev-smartcard-reader.c | 20
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 6653619..6133edf 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -796,6 +796,12 @@ static void ccid_write_data_block(USBCCIDState *s, uint8_t
slot, uint8_t seq,
ccid_reset_error_status(s);
}
+static void ccid_report_error_failed(USBCCIDState *s, uint8_t error)
+{
+s-bmCommandStatus = COMMAND_STATUS_FAILED;
+s-bError = error;
+}
+
static void ccid_write_data_block_answer(USBCCIDState *s,
const uint8_t *data, uint32_t len)
{
@@ -803,7 +809,9 @@ static void ccid_write_data_block_answer(USBCCIDState *s,
uint8_t slot;
if (!ccid_has_pending_answers(s)) {
-abort();
+DPRINTF(s, D_WARN, error: no pending answer to return to guest\n);
+ccid_report_error_failed(s, ERROR_ICC_MUTE);
+return;
}
ccid_remove_pending_answer(s, slot, seq);
ccid_write_data_block(s, slot, seq, data, len);
@@ -857,12 +865,6 @@ static void ccid_reset_parameters(USBCCIDState *s)
memcpy(s-abProtocolDataStructure, abDefaultProtocolDataStructure, len);
}
-static void ccid_report_error_failed(USBCCIDState *s, uint8_t error)
-{
-s-bmCommandStatus = COMMAND_STATUS_FAILED;
-s-bError = error;
-}
-
/* NOTE: only a single slot is supported (SLOT_0) */
static void ccid_on_slot_change(USBCCIDState *s, bool full)
{
@@ -1129,7 +1131,9 @@ void ccid_card_send_apdu_to_guest(CCIDCardState *card,
s-bmCommandStatus = COMMAND_STATUS_NO_ERROR;
answer = ccid_peek_next_answer(s);
if (answer == NULL) {
-abort();
+DPRINTF(s, D_WARN, %s: error: unexpected lack of answer\n, __func__);
+ccid_report_error_failed(s, ERROR_HW_ERROR);
+return;
}
DPRINTF(s, 1, APDU returned to guest %d (answer seq %d, slot %d)\n,
len, answer-seq, answer-slot);
--
1.8.2
[Qemu-devel] [PATCH v3 20/28] ccid-card-passthru: add atr check
Signed-off-by: Alon Levy al...@redhat.com
---
hw/usb/ccid-card-passthru.c | 59 +
1 file changed, 59 insertions(+)
diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
index 275b887..16d51c9 100644
--- a/hw/usb/ccid-card-passthru.c
+++ b/hw/usb/ccid-card-passthru.c
@@ -138,6 +138,59 @@ static void ccid_card_vscard_handle_init(
ccid_card_vscard_send_init(card);
}
+static int check_atr(PassthruState *card, uint8_t *data, int len)
+{
+int historical_length, opt_bytes;
+int td_count = 0;
+int td;
+
+if (len 2) {
+return 0;
+}
+historical_length = data[1] 0xf;
+opt_bytes = 0;
+if (data[0] != 0x3b data[0] != 0x3f) {
+DPRINTF(card, D_WARN, atr's T0 is 0x%X, not in {0x3b, 0x3f}\n,
+data[0]);
+return 0;
+}
+td_count = 0;
+td = data[1] 4;
+while (td td_count 2 opt_bytes + historical_length + 2 len) {
+td_count++;
+if (td 0x1) {
+opt_bytes++;
+}
+if (td 0x2) {
+opt_bytes++;
+}
+if (td 0x4) {
+opt_bytes++;
+}
+if (td 0x8) {
+opt_bytes++;
+td = data[opt_bytes + 2] 4;
+}
+}
+if (len 2 + historical_length + opt_bytes) {
+DPRINTF(card, D_WARN,
+atr too short: len %d, but historical_len %d, T1 0x%X\n,
+len, historical_length, data[1]);
+return 0;
+}
+if (len 2 + historical_length + opt_bytes) {
+DPRINTF(card, D_WARN,
+atr too long: len %d, but hist/opt %d/%d, T1 0x%X\n,
+len, historical_length, opt_bytes, data[1]);
+/* let it through */
+}
+DPRINTF(card, D_VERBOSE,
+atr passes check: %d total length, %d historical, %d optional\n,
+len, historical_length, opt_bytes);
+
+return 1;
+}
+
static void ccid_card_vscard_handle_message(PassthruState *card,
VSCMsgHeader *scr_msg_header)
{
@@ -152,6 +205,12 @@ static void ccid_card_vscard_handle_message(PassthruState
*card,
VSC_GENERAL_ERROR);
break;
}
+if (!check_atr(card, data, scr_msg_header-length)) {
+error_report(ATR is inconsistent, ignoring);
+ccid_card_vscard_send_error(card, scr_msg_header-reader_id,
+VSC_GENERAL_ERROR);
+break;
+}
memcpy(card-atr, data, scr_msg_header-length);
card-atr_length = scr_msg_header-length;
ccid_card_card_inserted(card-base);
--
1.8.2
[Qemu-devel] [PATCH v3 22/28] dev-smartcard-reader: define structs for CCID_Parameter internals
Signed-off-by: Alon Levy al...@redhat.com
---
hw/usb/dev-smartcard-reader.c | 74 +++
1 file changed, 47 insertions(+), 27 deletions(-)
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 35f234e..0d482a9 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -189,10 +189,34 @@ typedef struct QEMU_PACKED CCID_SlotStatus {
uint8_t bClockStatus;
} CCID_SlotStatus;
+typedef struct QEMU_PACKED CCID_T0ProtocolDataStructure {
+uint8_t bmFindexDindex;
+uint8_t bmTCCKST0;
+uint8_t bGuardTimeT0;
+uint8_t bWaitingIntegerT0;
+uint8_t bClockStop;
+} CCID_T0ProtocolDataStructure;
+
+typedef struct QEMU_PACKED CCID_T1ProtocolDataStructure {
+uint8_t bmFindexDindex;
+uint8_t bmTCCKST1;
+uint8_t bGuardTimeT1;
+uint8_t bWaitingIntegerT1;
+uint8_t bClockStop;
+uint8_t bIFSC;
+uint8_t bNadValue;
+} CCID_T1ProtocolDataStructure;
+
+typedef union CCID_ProtocolDataStructure {
+CCID_T0ProtocolDataStructure t0;
+CCID_T1ProtocolDataStructure t1;
+uint8_t data[7]; /* must be = max(sizeof(t0), sizeof(t1)) */
+} CCID_ProtocolDataStructure;
+
typedef struct QEMU_PACKED CCID_Parameter {
CCID_BULK_IN b;
uint8_t bProtocolNum;
-uint8_t abProtocolDataStructure[0];
+CCID_ProtocolDataStructure abProtocolDataStructure;
} CCID_Parameter;
typedef struct QEMU_PACKED CCID_DataBlock {
@@ -224,7 +248,7 @@ typedef struct QEMU_PACKED CCID_SetParameters {
CCID_Header hdr;
uint8_t bProtocolNum;
uint16_t abRFU;
-uint8_tabProtocolDataStructure[0];
+CCID_ProtocolDataStructure abProtocolDataStructure;
} CCID_SetParameters;
typedef struct CCID_Notify_Slot_Change {
@@ -254,8 +278,6 @@ typedef struct CCIDBus {
BusState qbus;
} CCIDBus;
-#define MAX_PROTOCOL_SIZE 7
-
/*
* powered - defaults to true, changed by PowerOn/PowerOff messages
*/
@@ -279,7 +301,7 @@ typedef struct USBCCIDState {
uint8_t bError;
uint8_t bmCommandStatus;
uint8_t bProtocolNum;
-uint8_t abProtocolDataStructure[MAX_PROTOCOL_SIZE];
+CCID_ProtocolDataStructure abProtocolDataStructure;
uint32_t ulProtocolDataStructureSize;
uint32_t state_vmstate;
uint32_t migration_target_ip;
@@ -765,7 +787,7 @@ static void ccid_write_parameters(USBCCIDState *s,
CCID_Header *recv)
h-b.bStatus = ccid_calc_status(s);
h-b.bError = s-bError;
h-bProtocolNum = s-bProtocolNum;
-memcpy(h-abProtocolDataStructure, s-abProtocolDataStructure, len);
+h-abProtocolDataStructure = s-abProtocolDataStructure;
ccid_reset_error_status(s);
}
@@ -825,38 +847,36 @@ static void ccid_write_data_block_atr(USBCCIDState *s,
CCID_Header *recv)
static void ccid_set_parameters(USBCCIDState *s, CCID_Header *recv)
{
CCID_SetParameters *ph = (CCID_SetParameters *) recv;
-uint32_t len = 0;
-if ((ph-bProtocolNum 3) == 0) {
-len = 5;
-}
-if ((ph-bProtocolNum 3) == 1) {
-len = 7;
-}
-if (len == 0) {
-s-bmCommandStatus = COMMAND_STATUS_FAILED;
-s-bError = 7; /* Protocol invalid or not supported */
+uint32_t protocol_num = ph-bProtocolNum 3;
+
+if (protocol_num != 0 protocol_num != 1) {
+ccid_report_error_failed(s, ERROR_CMD_NOT_SUPPORTED);
return;
}
-s-bProtocolNum = ph-bProtocolNum;
-memcpy(s-abProtocolDataStructure, ph-abProtocolDataStructure, len);
-s-ulProtocolDataStructureSize = len;
-DPRINTF(s, 1, %s: using len %d\n, __func__, len);
+s-bProtocolNum = protocol_num;
+s-abProtocolDataStructure = ph-abProtocolDataStructure;
}
/*
* must be 5 bytes for T=0, 7 bytes for T=1
* See page 52
*/
-static const uint8_t abDefaultProtocolDataStructure[7] = {
-0x77, 0x00, 0x00, 0x00, 0x00, 0xfe /*IFSC*/, 0x00 /*NAD*/ };
+static const CCID_ProtocolDataStructure defaultProtocolDataStructure = {
+.t1 = {
+.bmFindexDindex = 0x77,
+.bmTCCKST1 = 0x00,
+.bGuardTimeT1 = 0x00,
+.bWaitingIntegerT1 = 0x00,
+.bClockStop = 0x00,
+.bIFSC = 0xfe,
+.bNadValue = 0x00,
+}
+};
static void ccid_reset_parameters(USBCCIDState *s)
{
- uint32_t len = sizeof(abDefaultProtocolDataStructure);
-
s-bProtocolNum = 1; /* T=1 */
- s-ulProtocolDataStructureSize = len;
- memcpy(s-abProtocolDataStructure, abDefaultProtocolDataStructure, len);
+ s-abProtocolDataStructure = defaultProtocolDataStructure;
}
/* NOTE: only a single slot is supported (SLOT_0) */
@@ -1345,7 +1365,7 @@ static VMStateDescription ccid_vmstate = {
VMSTATE_UINT8(bError, USBCCIDState),
VMSTATE_UINT8(bmCommandStatus, USBCCIDState),
VMSTATE_UINT8(bProtocolNum, USBCCIDState),
-VMSTATE_BUFFER(abProtocolDataStructure, USBCCIDState),
+VMSTATE_BUFFER(abProtocolDataStructure.data, USBCCIDState
[Qemu-devel] [PATCH v3 25/28] libcacard/vreader: add debugging messages for apdu
Using g_debug with log domain libcacard
Signed-off-by: Alon Levy al...@redhat.com
---
libcacard/cac.c | 7 -
libcacard/cac.h | 8 ++
libcacard/vreader.c | 77 +
3 files changed, 85 insertions(+), 7 deletions(-)
diff --git a/libcacard/cac.c b/libcacard/cac.c
index 927a4ca..5864539 100644
--- a/libcacard/cac.c
+++ b/libcacard/cac.c
@@ -12,13 +12,6 @@
#include vcard_emul.h
#include card_7816.h
-#define CAC_GET_PROPERTIES 0x56
-#define CAC_GET_ACR 0x4c
-#define CAC_READ_BUFFER 0x52
-#define CAC_UPDATE_BUFFER 0x58
-#define CAC_SIGN_DECRYPT0x42
-#define CAC_GET_CERTIFICATE 0x36
-
/* private data for PKI applets */
typedef struct CACPKIAppletDataStruct {
unsigned char *cert;
diff --git a/libcacard/cac.h b/libcacard/cac.h
index 15a61be..d24a2a8 100644
--- a/libcacard/cac.h
+++ b/libcacard/cac.h
@@ -9,6 +9,14 @@
#define CAC_H 1
#include vcard.h
#include vreader.h
+
+#define CAC_GET_PROPERTIES 0x56
+#define CAC_GET_ACR 0x4c
+#define CAC_READ_BUFFER 0x52
+#define CAC_UPDATE_BUFFER 0x58
+#define CAC_SIGN_DECRYPT0x42
+#define CAC_GET_CERTIFICATE 0x36
+
/*
* Initialize the cac card. This is the only public function in this file. All
* the rest are connected through function pointers.
diff --git a/libcacard/vreader.c b/libcacard/vreader.c
index f3efc27..5793d73 100644
--- a/libcacard/vreader.c
+++ b/libcacard/vreader.c
@@ -5,6 +5,12 @@
* See the COPYING.LIB file in the top-level directory.
*/
+#ifdef G_LOG_DOMAIN
+#undef G_LOG_DOMAIN
+#endif
+#define G_LOG_DOMAIN libcacard
+#include glib.h
+
#include qemu-common.h
#include qemu/thread.h
@@ -13,6 +19,9 @@
#include card_7816.h
#include vreader.h
#include vevent.h
+#include cac.h /* just for debugging defines */
+
+#define LIBCACARD_LOG_DOMAIN libcacard
struct VReaderStruct {
intreference_count;
@@ -24,6 +33,66 @@ struct VReaderStruct {
VReaderEmulFree reader_private_free;
};
+/*
+ * Debug helpers
+ */
+
+static const char *
+apdu_ins_to_string(int ins)
+{
+switch (ins) {
+case VCARD7816_INS_MANAGE_CHANNEL:
+return manage channel;
+case VCARD7816_INS_EXTERNAL_AUTHENTICATE:
+return external authenticate;
+case VCARD7816_INS_GET_CHALLENGE:
+return get challenge;
+case VCARD7816_INS_INTERNAL_AUTHENTICATE:
+return internal authenticate;
+case VCARD7816_INS_ERASE_BINARY:
+return erase binary;
+case VCARD7816_INS_READ_BINARY:
+return read binary;
+case VCARD7816_INS_WRITE_BINARY:
+return write binary;
+case VCARD7816_INS_UPDATE_BINARY:
+return update binary;
+case VCARD7816_INS_READ_RECORD:
+return read record;
+case VCARD7816_INS_WRITE_RECORD:
+return write record;
+case VCARD7816_INS_UPDATE_RECORD:
+return update record;
+case VCARD7816_INS_APPEND_RECORD:
+return append record;
+case VCARD7816_INS_ENVELOPE:
+return envelope;
+case VCARD7816_INS_PUT_DATA:
+return put data;
+case VCARD7816_INS_GET_DATA:
+return get data;
+case VCARD7816_INS_SELECT_FILE:
+return select file;
+case VCARD7816_INS_VERIFY:
+return verify;
+case VCARD7816_INS_GET_RESPONSE:
+return get response;
+case CAC_GET_PROPERTIES:
+return get properties;
+case CAC_GET_ACR:
+return get acr;
+case CAC_READ_BUFFER:
+return read buffer;
+case CAC_UPDATE_BUFFER:
+return update buffer;
+case CAC_SIGN_DECRYPT:
+return sign decrypt;
+case CAC_GET_CERTIFICATE:
+return get certificate;
+}
+return unknown;
+}
+
/* manage locking */
static inline void
vreader_lock(VReader *reader)
@@ -204,7 +273,15 @@ vreader_xfr_bytes(VReader *reader,
response = vcard_make_response(status);
card_status = VCARD_DONE;
} else {
+g_debug(%s: CLS=0x%x,INS=0x%x,P1=0x%x,P2=0x%x,Lc=%d,Le=%d %s\n,
+ __func__, apdu-a_cla, apdu-a_ins, apdu-a_p1, apdu-a_p2,
+ apdu-a_Lc, apdu-a_Le, apdu_ins_to_string(apdu-a_ins));
card_status = vcard_process_apdu(card, apdu, response);
+if (response) {
+g_debug(%s: status=%d sw1=0x%x sw2=0x%x len=%d (total=%d)\n,
+ __func__, response-b_status, response-b_sw1,
+ response-b_sw2, response-b_len, response-b_total_len);
+}
}
assert(card_status == VCARD_DONE);
if (card_status == VCARD_DONE) {
--
1.8.2
[Qemu-devel] [PATCH v3 21/28] ccid-card-passthru, dev-smartcard-reader: add debug environment variables
Introduces a new utility function: parse_debug_env to avoid code
duplication.
This overrides whatever debug value is set on the corresponding devices
from the command line, and is meant to ease the usage with any
management stack. For libvirt you can set environment variables by
extending the dom namespace, i.e:
domain type='kvm' id='3'
xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'
qemu:commandline
qemu:env name='QEMU_CCID_PASSTHRU_DEBUG' value='4'/
qemu:env name='QEMU_CCID_DEBUG' value='4'/
/qemu:commandline
/domain
Signed-off-by: Alon Levy al...@redhat.com
---
hw/usb/ccid-card-passthru.c | 2 ++
hw/usb/dev-smartcard-reader.c | 1 +
include/qemu-common.h | 5 +
util/cutils.c | 23 +++
4 files changed, 31 insertions(+)
diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
index 16d51c9..01c7e6f 100644
--- a/hw/usb/ccid-card-passthru.c
+++ b/hw/usb/ccid-card-passthru.c
@@ -350,6 +350,8 @@ static int passthru_initfn(CCIDCardState *base)
error_report(missing chardev);
return -1;
}
+card-debug = parse_debug_env(QEMU_CCID_PASSTHRU_DEBUG, D_VERBOSE,
+ card-debug);
assert(sizeof(DEFAULT_ATR) = MAX_ATR_SIZE);
memcpy(card-atr, DEFAULT_ATR, sizeof(DEFAULT_ATR));
card-atr_length = sizeof(DEFAULT_ATR);
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 98f3be1..35f234e 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1260,6 +1260,7 @@ static int ccid_initfn(USBDevice *dev)
s-bulk_out_pos = 0;
ccid_reset_parameters(s);
ccid_reset(s);
+s-debug = parse_debug_env(QEMU_CCID_DEBUG, D_VERBOSE, s-debug);
return 0;
}
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 3b1873e..a39cdba 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -482,4 +482,9 @@ can_use_buffer_find_nonzero_offset(const void *buf, size_t
len)
}
size_t buffer_find_nonzero_offset(const void *buf, size_t len);
+/*
+ * helper to parse debug environment variables
+ */
+int parse_debug_env(const char *name, int max, int initial);
+
#endif
diff --git a/util/cutils.c b/util/cutils.c
index 5024253..a165819 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -482,3 +482,26 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n)
return 2;
}
}
+
+/*
+ * helper to parse debug environment variables
+ */
+int parse_debug_env(const char *name, int max, int initial)
+{
+char *debug_env = getenv(name);
+char *inv = NULL;
+int debug;
+
+if (!debug_env) {
+return initial;
+}
+debug = strtol(debug_env, inv, 10);
+if (inv == debug_env) {
+return initial;
+}
+if (debug 0 || debug max) {
+fprintf(stderr, warning: %s not in [0, %d], name, max);
+return initial;
+}
+return debug;
+}
--
1.8.2
[Qemu-devel] [PATCH v3 10/28] libcacard: vscclient to use QemuThread for portability
From: Marc-André Lureau marcandre.lur...@redhat.com
---
libcacard/vscclient.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index 5e00db3..5f47634 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -218,8 +218,7 @@ on_host_init(VSCMsgHeader *mhHeader, VSCMsgInit *incoming)
int num_capabilities =
1 + ((mhHeader-length - sizeof(VSCMsgInit)) / sizeof(uint32_t));
int i;
-int rv;
-pthread_t thread_id;
+QemuThread thread_id;
incoming-version = ntohl(incoming-version);
if (incoming-version != VSCARD_VERSION) {
@@ -242,11 +241,7 @@ on_host_init(VSCMsgHeader *mhHeader, VSCMsgInit *incoming)
send_msg(VSC_ReaderRemove, VSCARD_MINIMAL_READER_ID, NULL, 0);
/* launch the event_thread. This will trigger reader adds for all the
* existing readers */
-rv = pthread_create(thread_id, NULL, event_thread, NULL);
-if (rv 0) {
-perror(pthread_create);
-return rv;
-}
+qemu_thread_create(thread_id, event_thread, NULL, 0);
return 0;
}
--
1.8.2
[Qemu-devel] [PATCH v3 01/28] libcacard: correct T0 historical bytes size
From: Marc-André Lureau marcandre.lur...@gmail.com The VCARD_ATR_PREFIX macro adds a prefix of 6 characters only. pcsc_scan was complaining before the patch: + Historical bytes: 56 43 41 52 44 5F 4E 53 53 ERROR! ATR is truncated: 2 byte(s) is/are missing --- libcacard/vcardt.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcacard/vcardt.h b/libcacard/vcardt.h index d3e9522..538bdde 100644 --- a/libcacard/vcardt.h +++ b/libcacard/vcardt.h @@ -26,7 +26,7 @@ typedef struct VCardEmulStruct VCardEmul; #define MAX_CHANNEL 4 /* create an ATR with appropriate historical bytes */ -#define VCARD_ATR_PREFIX(size) 0x3b, 0x68+(size), 0x00, 0xff, \ +#define VCARD_ATR_PREFIX(size) 0x3b, 0x66+(size), 0x00, 0xff, \ 'V', 'C', 'A', 'R', 'D', '_' -- 1.8.2
[Qemu-devel] [PATCH v3 15/28] dev-smartcard-reader: nicer debug messages
Signed-off-by: Alon Levy al...@redhat.com
---
hw/usb/dev-smartcard-reader.c | 69 +++
1 file changed, 63 insertions(+), 6 deletions(-)
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 57eb803..6653619 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -639,13 +639,47 @@ static void ccid_handle_reset(USBDevice *dev)
ccid_reset(s);
}
+static const char *ccid_control_to_str(USBCCIDState *s, int request)
+{
+switch (request) {
+/* generic - should be factored out if there are other debugees */
+case DeviceOutRequest | USB_REQ_SET_ADDRESS:
+return (generic) set address;
+case DeviceRequest | USB_REQ_GET_DESCRIPTOR:
+return (generic) get descriptor;
+case DeviceRequest | USB_REQ_GET_CONFIGURATION:
+return (generic) get configuration;
+case DeviceOutRequest | USB_REQ_SET_CONFIGURATION:
+return (generic) set configuration;
+case DeviceRequest | USB_REQ_GET_STATUS:
+return (generic) get status;
+case DeviceOutRequest | USB_REQ_CLEAR_FEATURE:
+return (generic) clear feature;
+case DeviceOutRequest | USB_REQ_SET_FEATURE:
+return (generic) set_feature;
+case InterfaceRequest | USB_REQ_GET_INTERFACE:
+return (generic) get interface;
+case InterfaceOutRequest | USB_REQ_SET_INTERFACE:
+return (generic) set interface;
+/* class requests */
+case ClassInterfaceOutRequest | CCID_CONTROL_ABORT:
+return ABORT;
+case ClassInterfaceRequest | CCID_CONTROL_GET_CLOCK_FREQUENCIES:
+return GET_CLOCK_FREQUENCIES;
+case ClassInterfaceRequest | CCID_CONTROL_GET_DATA_RATES:
+return GET_DATA_RATES;
+}
+return unknown;
+}
+
static void ccid_handle_control(USBDevice *dev, USBPacket *p, int request,
int value, int index, int length, uint8_t *data)
{
USBCCIDState *s = DO_UPCAST(USBCCIDState, dev, dev);
int ret;
-DPRINTF(s, 1, got control %x, value %x\n, request, value);
+DPRINTF(s, 1, %s: got control %s (%x), value %x\n, __func__,
+ccid_control_to_str(s, request), request, value);
ret = usb_desc_handle_control(dev, p, request, value, index, length, data);
if (ret = 0) {
return;
@@ -695,7 +729,7 @@ static uint8_t ccid_calc_status(USBCCIDState *s)
* bmCommandStatus
*/
uint8_t ret = ccid_card_status(s) | (s-bmCommandStatus 6);
-DPRINTF(s, D_VERBOSE, status = %d\n, ret);
+DPRINTF(s, D_VERBOSE, %s: status = %d\n, __func__, ret);
return ret;
}
@@ -756,7 +790,7 @@ static void ccid_write_data_block(USBCCIDState *s, uint8_t
slot, uint8_t seq,
p-b.bStatus = ccid_calc_status(s);
p-b.bError = s-bError;
if (p-b.bError) {
-DPRINTF(s, D_VERBOSE, error %d, p-b.bError);
+DPRINTF(s, D_VERBOSE, error %d\n, p-b.bError);
}
memcpy(p-abData, data, len);
ccid_reset_error_status(s);
@@ -873,6 +907,28 @@ static void ccid_on_apdu_from_guest(USBCCIDState *s,
CCID_XferBlock *recv)
}
}
+static const char *ccid_message_type_to_str(uint8_t type)
+{
+switch (type) {
+case CCID_MESSAGE_TYPE_PC_to_RDR_IccPowerOn: return IccPowerOn;
+case CCID_MESSAGE_TYPE_PC_to_RDR_IccPowerOff: return IccPowerOff;
+case CCID_MESSAGE_TYPE_PC_to_RDR_GetSlotStatus: return GetSlotStatus;
+case CCID_MESSAGE_TYPE_PC_to_RDR_XfrBlock: return XfrBlock;
+case CCID_MESSAGE_TYPE_PC_to_RDR_GetParameters: return GetParameters;
+case CCID_MESSAGE_TYPE_PC_to_RDR_ResetParameters: return ResetParameters;
+case CCID_MESSAGE_TYPE_PC_to_RDR_SetParameters: return SetParameters;
+case CCID_MESSAGE_TYPE_PC_to_RDR_Escape: return Escape;
+case CCID_MESSAGE_TYPE_PC_to_RDR_IccClock: return IccClock;
+case CCID_MESSAGE_TYPE_PC_to_RDR_T0APDU: return T0APDU;
+case CCID_MESSAGE_TYPE_PC_to_RDR_Secure: return Secure;
+case CCID_MESSAGE_TYPE_PC_to_RDR_Mechanical: return Mechanical;
+case CCID_MESSAGE_TYPE_PC_to_RDR_Abort: return Abort;
+case CCID_MESSAGE_TYPE_PC_to_RDR_SetDataRateAndClockFrequency:
+return SetDataRateAndClockFrequency;
+}
+return unknown;
+}
+
static void ccid_handle_bulk_out(USBCCIDState *s, USBPacket *p)
{
CCID_Header *ccid_header;
@@ -895,13 +951,15 @@ static void ccid_handle_bulk_out(USBCCIDState *s,
USBPacket *p)
%s: bad USB_TOKEN_OUT length, should be at least 10 bytes\n,
__func__);
} else {
-DPRINTF(s, D_MORE_INFO, %s %x\n, __func__,
ccid_header-bMessageType);
+DPRINTF(s, D_MORE_INFO, %s %x %s\n, __func__,
+ccid_header-bMessageType,
+ccid_message_type_to_str(ccid_header-bMessageType));
switch (ccid_header-bMessageType) {
case CCID_MESSAGE_TYPE_PC_to_RDR_GetSlotStatus:
ccid_write_slot_status(s, ccid_header);
break;
case
[Qemu-devel] [PATCH v3 26/28] libcacard: move atr setting from macro to function
Only because qemu's checkpatch complains about it.
Signed-off-by: Alon Levy al...@redhat.com
---
Makefile.objs | 1 +
libcacard/vcard_emul_nss.c | 14 +++---
libcacard/vcardt.c | 40
libcacard/vcardt.h | 13 -
libcacard/vcardt_internal.h | 6 ++
5 files changed, 58 insertions(+), 16 deletions(-)
create mode 100644 libcacard/vcardt.c
create mode 100644 libcacard/vcardt_internal.h
diff --git a/Makefile.objs b/Makefile.objs
index a473348..fcb303a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -32,6 +32,7 @@ libcacard-y += libcacard/vcard.o libcacard/vreader.o
libcacard-y += libcacard/vcard_emul_nss.o
libcacard-y += libcacard/vcard_emul_type.o
libcacard-y += libcacard/card_7816.o
+libcacard-y += libcacard/vcardt.o
##
# Target independent part of system emulation. The long term path is to
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index 9ba80fb..1a3e568 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -33,6 +33,9 @@
#include vreader.h
#include vevent.h
+#include libcacard/vcardt_internal.h
+
+
typedef enum {
VCardEmulUnknown = -1,
VCardEmulFalse = 0,
@@ -519,18 +522,23 @@ vcard_emul_reader_get_slot(VReader *vreader)
}
/*
- * Card ATR's map to physical cards. VCARD_ATR_PREFIX will set appropriate
+ * Card ATR's map to physical cards. vcard_alloc_atr will set appropriate
* historical bytes for any software emulated card. The remaining bytes can be
* used to indicate the actual emulator
*/
-static const unsigned char nss_atr[] = { VCARD_ATR_PREFIX(3), 'N', 'S', 'S' };
+static unsigned char *nss_atr;
+static int nss_atr_len;
void
vcard_emul_get_atr(VCard *card, unsigned char *atr, int *atr_len)
{
-int len = MIN(sizeof(nss_atr), *atr_len);
+int len;
assert(atr != NULL);
+if (nss_atr == NULL) {
+nss_atr = vcard_alloc_atr(NSS, nss_atr_len);
+}
+len = MIN(nss_atr_len, *atr_len);
memcpy(atr, nss_atr, len);
*atr_len = len;
}
diff --git a/libcacard/vcardt.c b/libcacard/vcardt.c
new file mode 100644
index 000..9ce4648
--- /dev/null
+++ b/libcacard/vcardt.c
@@ -0,0 +1,40 @@
+#include stdlib.h
+#include string.h
+#include glib.h
+
+#include libcacard/vcardt.h
+
+#include libcacard/vcardt_internal.h
+
+/* create an ATR with appropriate historical bytes */
+#define ATR_TS_DIRECT_CONVENTION 0x3b
+#define ATR_TA_PRESENT 0x10
+#define ATR_TB_PRESENT 0x20
+#define ATR_TC_PRESENT 0x40
+#define ATR_TD_PRESENT 0x80
+
+unsigned char *vcard_alloc_atr(const char *postfix, int *atr_len)
+{
+int postfix_len;
+const char prefix[] = VCARD_;
+const char default_postfix[] = DEFAULT;
+const int prefix_len = sizeof(prefix) - 1;
+int total_len;
+unsigned char *atr;
+
+if (postfix == NULL) {
+postfix = default_postfix;
+}
+postfix_len = strlen(postfix);
+total_len = 3 + prefix_len + postfix_len;
+atr = g_malloc(total_len);
+atr[0] = ATR_TS_DIRECT_CONVENTION;
+atr[1] = ATR_TD_PRESENT + prefix_len + postfix_len;
+atr[2] = 0x00;
+memcpy(atr[3], prefix, prefix_len);
+memcpy(atr[3 + prefix_len], postfix, postfix_len);
+if (atr_len) {
+*atr_len = total_len;
+}
+return atr;
+}
diff --git a/libcacard/vcardt.h b/libcacard/vcardt.h
index 3b9a619..795e265 100644
--- a/libcacard/vcardt.h
+++ b/libcacard/vcardt.h
@@ -25,19 +25,6 @@ typedef struct VCardEmulStruct VCardEmul;
#define MAX_CHANNEL 4
-/* create an ATR with appropriate historical bytes */
-#define TS_DIRECT_CONVENTION 0x3b
-#define TA_PRESENT 0x10
-#define TB_PRESENT 0x20
-#define TC_PRESENT 0x40
-#define TD_PRESENT 0x80
-
-#define VCARD_ATR_PREFIX(size) \
-TS_DIRECT_CONVENTION, \
-TD_PRESENT + (6 + size), \
-0x00, \
-'V', 'C', 'A', 'R', 'D', '_'
-
typedef enum {
VCARD_DONE,
VCARD_NEXT,
diff --git a/libcacard/vcardt_internal.h b/libcacard/vcardt_internal.h
new file mode 100644
index 000..e5c8d2d
--- /dev/null
+++ b/libcacard/vcardt_internal.h
@@ -0,0 +1,6 @@
+#ifndef VCARDT_INTERNAL_H
+#define VCARDT_INTERNAL_H
+
+unsigned char *vcard_alloc_atr(const char *postfix, int *atr_len);
+
+#endif
--
1.8.2
[Qemu-devel] [PATCH v3 27/28] dev-smartcard-reader: empty implementation for Mechanical (fail correctly)
Signed-off-by: Alon Levy al...@redhat.com --- hw/usb/dev-smartcard-reader.c | 4 1 file changed, 4 insertions(+) diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c index 78ef504..125cc2c 100644 --- a/hw/usb/dev-smartcard-reader.c +++ b/hw/usb/dev-smartcard-reader.c @@ -1051,6 +1051,10 @@ static void ccid_handle_bulk_out(USBCCIDState *s, USBPacket *p) ccid_reset_error_status(s); ccid_write_parameters(s, ccid_header); break; +case CCID_MESSAGE_TYPE_PC_to_RDR_Mechanical: +ccid_report_error_failed(s, 0); +ccid_write_slot_status(s, ccid_header); +break; default: DPRINTF(s, 1, handle_data: ERROR: unhandled message type %Xh\n, -- 1.8.2
[Qemu-devel] [PATCH v3 18/28] dev-smartcard-reader: reuse usb.h definitions
Signed-off-by: Alon Levy al...@redhat.com
---
hw/usb/dev-smartcard-reader.c | 16 +---
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 38c3c0e..98f3be1 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -68,12 +68,6 @@ do { \
#define BULK_IN_BUF_SIZE 384
#define BULK_IN_PENDING_NUM 8
-#define InterfaceOutClass \
-((USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE)8)
-
-#define InterfaceInClass \
-((USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE)8)
-
#define CCID_MAX_PACKET_SIZE64
#define CCID_CONTROL_ABORT 0x1
@@ -410,8 +404,8 @@ static const USBDescStrings desc_strings = {
static const USBDescIface desc_iface0 = {
.bInterfaceNumber = 0,
.bNumEndpoints = 3,
-.bInterfaceClass = 0x0b,
-.bInterfaceSubClass= 0x00,
+.bInterfaceClass = USB_CLASS_CSCID,
+.bInterfaceSubClass= USB_SUBCLASS_UNDEFINED,
.bInterfaceProtocol= 0x00,
.iInterface= STR_INTERFACE,
.ndesc = 1,
@@ -687,15 +681,15 @@ static void ccid_handle_control(USBDevice *dev, USBPacket
*p, int request,
switch (request) {
/* Class specific requests. */
-case InterfaceOutClass | CCID_CONTROL_ABORT:
+case ClassInterfaceOutRequest | CCID_CONTROL_ABORT:
DPRINTF(s, 1, ccid_control abort UNIMPLEMENTED\n);
p-status = USB_RET_STALL;
break;
-case InterfaceInClass | CCID_CONTROL_GET_CLOCK_FREQUENCIES:
+case ClassInterfaceRequest | CCID_CONTROL_GET_CLOCK_FREQUENCIES:
DPRINTF(s, 1, ccid_control get clock frequencies UNIMPLEMENTED\n);
p-status = USB_RET_STALL;
break;
-case InterfaceInClass | CCID_CONTROL_GET_DATA_RATES:
+case ClassInterfaceRequest | CCID_CONTROL_GET_DATA_RATES:
DPRINTF(s, 1, ccid_control get data rates UNIMPLEMENTED\n);
p-status = USB_RET_STALL;
break;
--
1.8.2
[Qemu-devel] [PATCH v3 23/28] dev-smartcard-reader: change default protocol to T=0
We don't support T=1 so we shouldn't advertise it by default.
Two independent changes:
* Default ATR
sets T=0. This gets overwritten by the client provided ATR later.
* Class descriptor
changes dwAdvertise dwProtocols. to 0x1 and dwProtocols.=0 per spec.
Signed-off-by: Alon Levy al...@redhat.com
---
hw/usb/dev-smartcard-reader.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 0d482a9..8022f9f 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -335,8 +335,8 @@ static const uint8_t qemu_ccid_descriptor[] = {
*/
0x07, /* u8 bVoltageSupport; 01h - 5.0v, 02h - 3.0, 03 - 1.8 */
-0x03, 0x00, /* u32 dwProtocols; . = h.*/
-0x00, 0x00, /* : 0001h = Protocol T=0, 0002h = Protocol T=1 */
+0x00, 0x00, /* u32 dwProtocols; . = h.*/
+0x01, 0x00, /* : 0001h = Protocol T=0, 0002h = Protocol T=1 */
/* u32 dwDefaultClock; in kHZ (0x0fa0 is 4 MHz) */
0xa0, 0x0f, 0x00, 0x00,
/* u32 dwMaximumClock; */
@@ -875,7 +875,7 @@ static const CCID_ProtocolDataStructure
defaultProtocolDataStructure = {
static void ccid_reset_parameters(USBCCIDState *s)
{
- s-bProtocolNum = 1; /* T=1 */
+ s-bProtocolNum = 0; /* T=0 */
s-abProtocolDataStructure = defaultProtocolDataStructure;
}
--
1.8.2
[Qemu-devel] [PATCH v3 28/28] libcacard/cac: change big switch functions to single return point
Signed-off-by: Alon Levy al...@redhat.com
---
libcacard/cac.c | 73 -
1 file changed, 52 insertions(+), 21 deletions(-)
diff --git a/libcacard/cac.c b/libcacard/cac.c
index 5864539..7a06b5a 100644
--- a/libcacard/cac.c
+++ b/libcacard/cac.c
@@ -40,41 +40,51 @@ static VCardStatus
cac_common_process_apdu(VCard *card, VCardAPDU *apdu, VCardResponse **response)
{
int ef;
+VCardStatus ret = VCARD_FAIL;
switch (apdu-a_ins) {
case VCARD7816_INS_SELECT_FILE:
if (apdu-a_p1 != 0x02) {
/* let the 7816 code handle applet switches */
-return VCARD_NEXT;
+ret = VCARD_NEXT;
+break;
}
/* handle file id setting */
if (apdu-a_Lc != 2) {
*response = vcard_make_response(
VCARD7816_STATUS_ERROR_DATA_INVALID);
-return VCARD_DONE;
+ret = VCARD_DONE;
+break;
}
/* CAC 1.0 only supports ef = 0 */
ef = apdu-a_body[0] | (apdu-a_body[1] 8);
if (ef != 0) {
*response = vcard_make_response(
VCARD7816_STATUS_ERROR_FILE_NOT_FOUND);
-return VCARD_DONE;
+ret = VCARD_DONE;
+break;
}
*response = vcard_make_response(VCARD7816_STATUS_SUCCESS);
-return VCARD_DONE;
+ret = VCARD_DONE;
+break;
case VCARD7816_INS_GET_RESPONSE:
case VCARD7816_INS_VERIFY:
/* let the 7816 code handle these */
-return VCARD_NEXT;
+ret = VCARD_NEXT;
+break;
case CAC_GET_PROPERTIES:
case CAC_GET_ACR:
/* skip these for now, this will probably be needed */
*response =
vcard_make_response(VCARD7816_STATUS_ERROR_P1_P2_INCORRECT);
-return VCARD_DONE;
+ret = VCARD_DONE;
+break;
+default:
+*response = vcard_make_response(
+VCARD7816_STATUS_ERROR_COMMAND_NOT_SUPPORTED);
+ret = VCARD_DONE;
+break;
}
-*response = vcard_make_response(
-VCARD7816_STATUS_ERROR_COMMAND_NOT_SUPPORTED);
-return VCARD_DONE;
+return ret;
}
/*
@@ -108,6 +118,7 @@ cac_applet_pki_process_apdu(VCard *card, VCardAPDU *apdu,
int size, next;
unsigned char *sign_buffer;
vcard_7816_status_t status;
+VCardStatus ret = VCARD_FAIL;
applet_private = vcard_get_current_applet_private(card, apdu-a_channel);
assert(applet_private);
@@ -117,7 +128,8 @@ cac_applet_pki_process_apdu(VCard *card, VCardAPDU *apdu,
case CAC_UPDATE_BUFFER:
*response = vcard_make_response(
VCARD7816_STATUS_ERROR_CONDITION_NOT_SATISFIED);
-return VCARD_DONE;
+ret = VCARD_DONE;
+break;
case CAC_GET_CERTIFICATE:
if ((apdu-a_p2 != 0) || (apdu-a_p1 != 0)) {
*response = vcard_make_response(
@@ -147,7 +159,8 @@ cac_applet_pki_process_apdu(VCard *card, VCardAPDU *apdu,
*response = vcard_make_response(
VCARD7816_STATUS_EXC_ERROR_MEMORY_FAILURE);
}
-return VCARD_DONE;
+ret = VCARD_DONE;
+break;
case CAC_SIGN_DECRYPT:
if (apdu-a_p2 != 0) {
*response = vcard_make_response(
@@ -164,7 +177,8 @@ cac_applet_pki_process_apdu(VCard *card, VCardAPDU *apdu,
pki_applet-sign_buffer_len = 0;
*response = vcard_make_response(
VCARD7816_STATUS_EXC_ERROR_MEMORY_FAILURE);
-return VCARD_DONE;
+ret = VCARD_DONE;
+break;
}
memcpy(sign_buffer+pki_applet-sign_buffer_len, apdu-a_body, size);
size += pki_applet-sign_buffer_len;
@@ -175,7 +189,8 @@ cac_applet_pki_process_apdu(VCard *card, VCardAPDU *apdu,
pki_applet-sign_buffer = sign_buffer;
pki_applet-sign_buffer_len = size;
*response = vcard_make_response(VCARD7816_STATUS_SUCCESS);
-return VCARD_DONE;
+ret = VCARD_DONE;
+break;
case 0x00:
/* we now have the whole buffer, do the operation, result will be
* in the sign_buffer */
@@ -200,15 +215,20 @@ cac_applet_pki_process_apdu(VCard *card, VCardAPDU *apdu,
g_free(sign_buffer);
pki_applet-sign_buffer = NULL;
pki_applet-sign_buffer_len = 0;
-return VCARD_DONE;
+ret = VCARD_DONE;
+break;
case CAC_READ_BUFFER:
/* new CAC call, go ahead and use the old version for now */
/* TODO: implement */
*response = vcard_make_response(
VCARD7816_STATUS_ERROR_COMMAND_NOT_SUPPORTED);
-return VCARD_DONE;
+ret = VCARD_DONE;
+break;
+default:
+ret = cac_common_process_apdu(card, apdu, response);
+break;
}
-return cac_common_process_apdu(card, apdu, response);
+return
[Qemu-devel] [PATCH v3 09/28] libcacard: split vscclient main() from socket reading
From: Marc-André Lureau marcandre.lur...@redhat.com
---
libcacard/vscclient.c | 314 ++
1 file changed, 162 insertions(+), 152 deletions(-)
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index 9b744f2..5e00db3 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -211,6 +211,166 @@ get_id_from_string(char *string, unsigned int default_id)
return id;
}
+static int
+on_host_init(VSCMsgHeader *mhHeader, VSCMsgInit *incoming)
+{
+uint32_t *capabilities = (incoming-capabilities);
+int num_capabilities =
+1 + ((mhHeader-length - sizeof(VSCMsgInit)) / sizeof(uint32_t));
+int i;
+int rv;
+pthread_t thread_id;
+
+incoming-version = ntohl(incoming-version);
+if (incoming-version != VSCARD_VERSION) {
+if (verbose 0) {
+printf(warning: host has version %d, we have %d\n,
+verbose, VSCARD_VERSION);
+}
+}
+if (incoming-magic != VSCARD_MAGIC) {
+printf(unexpected magic: got %d, expected %d\n,
+incoming-magic, VSCARD_MAGIC);
+return -1;
+}
+for (i = 0 ; i num_capabilities; ++i) {
+capabilities[i] = ntohl(capabilities[i]);
+}
+/* Future: check capabilities */
+/* remove whatever reader might be left in qemu,
+ * in case of an unclean previous exit. */
+send_msg(VSC_ReaderRemove, VSCARD_MINIMAL_READER_ID, NULL, 0);
+/* launch the event_thread. This will trigger reader adds for all the
+ * existing readers */
+rv = pthread_create(thread_id, NULL, event_thread, NULL);
+if (rv 0) {
+perror(pthread_create);
+return rv;
+}
+return 0;
+}
+
+#define APDUBufSize 270
+
+static int
+do_socket_read(void)
+{
+int rv;
+int dwSendLength;
+int dwRecvLength;
+uint8_t pbRecvBuffer[APDUBufSize];
+uint8_t pbSendBuffer[APDUBufSize];
+VReaderStatus reader_status;
+VReader *reader = NULL;
+VSCMsgHeader mhHeader;
+VSCMsgError *error_msg;
+
+rv = read(sock, mhHeader, sizeof(mhHeader));
+if (rv sizeof(mhHeader)) {
+/* Error */
+if (rv 0) {
+perror(header read error\n);
+} else {
+fprintf(stderr, header short read %d\n, rv);
+}
+return -1;
+}
+mhHeader.type = ntohl(mhHeader.type);
+mhHeader.reader_id = ntohl(mhHeader.reader_id);
+mhHeader.length = ntohl(mhHeader.length);
+if (verbose) {
+printf(Header: type=%d, reader_id=%u length=%d (0x%x)\n,
+ mhHeader.type, mhHeader.reader_id, mhHeader.length,
+ mhHeader.length);
+}
+switch (mhHeader.type) {
+case VSC_APDU:
+case VSC_Flush:
+case VSC_Error:
+case VSC_Init:
+rv = read(sock, pbSendBuffer, mhHeader.length);
+break;
+default:
+fprintf(stderr, Unexpected message of type 0x%X\n, mhHeader.type);
+return -1;
+}
+switch (mhHeader.type) {
+case VSC_APDU:
+if (rv 0) {
+/* Error */
+fprintf(stderr, read error\n);
+close(sock);
+return -1;
+}
+if (verbose) {
+printf( recv APDU: );
+print_byte_array(pbSendBuffer, mhHeader.length);
+}
+/* Transmit received APDU */
+dwSendLength = mhHeader.length;
+dwRecvLength = sizeof(pbRecvBuffer);
+reader = vreader_get_reader_by_id(mhHeader.reader_id);
+reader_status = vreader_xfr_bytes(reader,
+ pbSendBuffer, dwSendLength,
+ pbRecvBuffer, dwRecvLength);
+if (reader_status == VREADER_OK) {
+mhHeader.length = dwRecvLength;
+if (verbose) {
+printf( send response: );
+print_byte_array(pbRecvBuffer, mhHeader.length);
+}
+send_msg(VSC_APDU, mhHeader.reader_id,
+ pbRecvBuffer, dwRecvLength);
+} else {
+rv = reader_status; /* warning: not meaningful */
+send_msg(VSC_Error, mhHeader.reader_id, rv, sizeof(uint32_t));
+}
+vreader_free(reader);
+reader = NULL; /* we've freed it, don't use it by accident
+ again */
+break;
+case VSC_Flush:
+/* TODO: actually flush */
+send_msg(VSC_FlushComplete, mhHeader.reader_id, NULL, 0);
+break;
+case VSC_Error:
+error_msg = (VSCMsgError *) pbSendBuffer;
+if (error_msg-code == VSC_SUCCESS) {
+qemu_mutex_lock(pending_reader_lock);
+if (pending_reader) {
+vreader_set_id(pending_reader, mhHeader.reader_id);
+vreader_free(pending_reader);
+pending_reader = NULL;
+qemu_cond_signal(pending_reader_condition);
+}
+qemu_mutex_unlock(pending_reader_lock);
+break;
+
[Qemu-devel] [PATCH v3 24/28] dev-smartcard-reader: copy atr protocol to ccid parameters
Adds todos.
Signed-off-by: Alon Levy al...@redhat.com
---
hw/usb/dev-smartcard-reader.c | 45 +++
1 file changed, 45 insertions(+)
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 8022f9f..78ef504 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -833,14 +833,59 @@ static void ccid_write_data_block_answer(USBCCIDState *s,
ccid_write_data_block(s, slot, seq, data, len);
}
+static uint8_t atr_get_protocol_num(const uint8_t *atr, uint32_t len)
+{
+int i;
+
+if (len 2 || !(atr[1] 0x80)) {
+/* too short or TD1 not included */
+return 0; /* T=0, default */
+}
+i = 1 + !!(atr[1] 0x10) + !!(atr[1] 0x20) + !!(atr[1] 0x40);
+i += !!(atr[1] 0x80);
+return atr[i] 0x0f;
+}
+
static void ccid_write_data_block_atr(USBCCIDState *s, CCID_Header *recv)
{
const uint8_t *atr = NULL;
uint32_t len = 0;
+uint8_t atr_protocol_num;
+CCID_T0ProtocolDataStructure *t0 = s-abProtocolDataStructure.t0;
+CCID_T1ProtocolDataStructure *t1 = s-abProtocolDataStructure.t1;
if (s-card) {
atr = ccid_card_get_atr(s-card, len);
}
+atr_protocol_num = atr_get_protocol_num(atr, len);
+DPRINTF(s, D_VERBOSE, %s: atr contains protocol=%d\n, __func__,
+atr_protocol_num);
+/* set parameters from ATR - see spec page 109 */
+s-bProtocolNum = (atr_protocol_num = 1 ? atr_protocol_num
+ : s-bProtocolNum);
+switch (atr_protocol_num) {
+case 0:
+/* TODO: unimplemented ATR T0 parameters */
+t0-bmFindexDindex = 0;
+t0-bmTCCKST0 = 0;
+t0-bGuardTimeT0 = 0;
+t0-bWaitingIntegerT0 = 0;
+t0-bClockStop = 0;
+break;
+case 1:
+/* TODO: unimplemented ATR T1 parameters */
+t1-bmFindexDindex = 0;
+t1-bmTCCKST1 = 0;
+t1-bGuardTimeT1 = 0;
+t1-bWaitingIntegerT1 = 0;
+t1-bClockStop = 0;
+t1-bIFSC = 0;
+t1-bNadValue = 0;
+break;
+default:
+DPRINTF(s, D_WARN, %s: error: unsupported ATR protocol %d\n,
+__func__, atr_protocol_num);
+}
ccid_write_data_block(s, recv-bSlot, recv-bSeq, atr, len);
}
--
1.8.2
Re: [Qemu-devel] [PATCH v2 00/10] ccid and libcacard fixes for windows/mingw
On Mon, 2013-04-15 at 15:31 +0200, Marc-André Lureau wrote: Hi On Wed, Mar 27, 2013 at 9:36 PM, Alon Levy al...@redhat.com wrote: This series: 1. fixes windows guests to show the ccid device 2. changes libcacard to use glib 3. makes libcacard build under mingw 4. does some cleanups It contains a few patches already posted to the list (the two Jim Meyering patches) which were already acked. I'll make a pull request once this had some time to be reviewed. Tested with a fedora and windows 7 guest. The main non cleanup patches are: hw/usb/dev-smartcard-reader: support windows guest libcacard: correct T0 historical bytes size The patch series doesn't do what you describe. So you want me to update this cover letter that doesn't get committed, no problem, I will in a follow up message, but meanwhile could you take a look at the updated patches per your requests from v1? Many patches are missing from the v1: http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg02925.html Many patches are missing on purpose, because they are either yours and so I acked them implicitly by sending the v1 patch series, or they are mine and have already been acked by me. -- Marc-André Lureau
Re: [Qemu-devel] [PATCH] ccid: Fix crash when backend isn't specified
On Sun, 2013-04-14 at 16:06 -0400, Cole Robinson wrote:
Reproducer:
./x86_64-softmmu/qemu-system-x86_64 -device usb-ccid,id=ccid0 -usb -device
ccid-card-emulated -monitor stdio
ACK, thanks.
Marc-Andre, could you review the fixed patches I sent previously, and
then I can put this on top and send a pull request?
http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg04927.html
Signed-off-by: Cole Robinson crobi...@redhat.com
---
hw/usb/ccid-card-emulated.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index 29dcd7a..65e1432 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -500,9 +500,15 @@ static int emulated_initfn(CCIDCardState *base)
if (init_pipe_signaling(card) 0) {
return -1;
}
-card-backend = parse_enumeration(card-backend_str, backend_enum_table,
0);
+
+card-backend = 0;
+if (card-backend_str) {
+card-backend = parse_enumeration(card-backend_str,
+ backend_enum_table, 0);
+}
+
if (card-backend == 0) {
-printf(unknown backend, must be one of:\n);
+printf(backend must be one of:\n);
for (ptable = backend_enum_table; ptable-name != NULL; ++ptable) {
printf(%s\n, ptable-name);
}
--
1.8.1.4
Re: [Qemu-devel] [PATCH 18/26] hw/usb/dev-smartcard-reader.c: copy atr's protocol to ccid's parameters (adds todo's)
On Fri, Mar 22, 2013 at 03:23:15PM +0100, Marc-André Lureau wrote:
Hi,
On Mon, Mar 18, 2013 at 2:11 PM, Alon Levy al...@redhat.com wrote:
+if (atr_protocol_num == 0) {
+DPRINTF(s, D_WARN, %s: error: unimplemented ATR T0 parameters
+ setting\n, __func__);
+t0-bmFindexDindex = 0;
+t0-bmTCCKST0 = 0;
+t0-bGuardTimeT0 = 0;
+t0-bWaitingIntegerT0 = 0;
+t0-bClockStop = 0;
+} else {
+if (atr_protocol_num != 1) {
+DPRINTF(s, D_WARN, %s: error: unsupported ATR protocol %d\n,
+__func__, atr_protocol_num);
+} else {
+DPRINTF(s, D_WARN, %s: error: unimplemented ATR T1 parameters
+ setting\n, __func__);
+t1-bmFindexDindex = 0;
+t1-bmTCCKST1 = 0;
+t1-bGuardTimeT1 = 0;
+t1-bWaitingIntegerT1 = 0;
+t1-bClockStop = 0;
+t1-bIFSC = 0;
+t1-bNadValue = 0;
+}
+}
Those blocks could be at the same indentation level, or perhaps use a switch?
A switch is a good idea.
Is it sensible to warn in all cases?
Turning them into TODO comments instead (except for the now default
case).
--
Marc-André Lureau
[Qemu-devel] [PATCH v2 00/10] ccid and libcacard fixes for windows/mingw
This series: 1. fixes windows guests to show the ccid device 2. changes libcacard to use glib 3. makes libcacard build under mingw 4. does some cleanups It contains a few patches already posted to the list (the two Jim Meyering patches) which were already acked. I'll make a pull request once this had some time to be reviewed. Tested with a fedora and windows 7 guest. The main non cleanup patches are: hw/usb/dev-smartcard-reader: support windows guest libcacard: correct T0 historical bytes size v2: only resent patches that were not acked before. did all the changes per Marc Andre's comments. Please look at the g_debug one, I'm not sure about the ifdeffery there (not if it works, but if it looks ok). Alon Levy (10): libcacard: change default ATR ccid-card-passthru: add atr check ccid-card-passthru, dev-smartcard-reader: add debug environment variables dev-smartcard-reader: define structs for CCID_Parameter internals dev-smartcard-reader: change default protocol to T=0 dev-smartcard-reader: copy atr protocol to ccid parameters libcacard/vreader: add debugging messages for apdu libcacard: move atr setting from macro to function dev-smartcard-reader: empty implementation for Mechanical (fail correctly) libcacard/cac: change big switch functions to single return point Makefile.objs | 1 + hw/ccid-card-passthru.c | 61 hw/usb/dev-smartcard-reader.c | 130 -- include/qemu-common.h | 5 ++ libcacard/cac.c | 80 +- libcacard/cac.h | 8 +++ libcacard/vcard_emul_nss.c| 14 - libcacard/vcardt.c| 40 + libcacard/vcardt.h| 5 -- libcacard/vcardt_internal.h | 6 ++ libcacard/vreader.c | 77 + util/cutils.c | 23 12 files changed, 384 insertions(+), 66 deletions(-) create mode 100644 libcacard/vcardt.c create mode 100644 libcacard/vcardt_internal.h -- 1.8.1.4
[Qemu-devel] [PATCH v2 01/10] libcacard: change default ATR
Signed-off-by: Alon Levy al...@redhat.com
---
libcacard/vcardt.h | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/libcacard/vcardt.h b/libcacard/vcardt.h
index 538bdde..3b9a619 100644
--- a/libcacard/vcardt.h
+++ b/libcacard/vcardt.h
@@ -26,9 +26,17 @@ typedef struct VCardEmulStruct VCardEmul;
#define MAX_CHANNEL 4
/* create an ATR with appropriate historical bytes */
-#define VCARD_ATR_PREFIX(size) 0x3b, 0x66+(size), 0x00, 0xff, \
- 'V', 'C', 'A', 'R', 'D', '_'
+#define TS_DIRECT_CONVENTION 0x3b
+#define TA_PRESENT 0x10
+#define TB_PRESENT 0x20
+#define TC_PRESENT 0x40
+#define TD_PRESENT 0x80
+#define VCARD_ATR_PREFIX(size) \
+TS_DIRECT_CONVENTION, \
+TD_PRESENT + (6 + size), \
+0x00, \
+'V', 'C', 'A', 'R', 'D', '_'
typedef enum {
VCARD_DONE,
--
1.8.1.4
[Qemu-devel] [PATCH v2 02/10] ccid-card-passthru: add atr check
Signed-off-by: Alon Levy al...@redhat.com
---
hw/ccid-card-passthru.c | 59 +
1 file changed, 59 insertions(+)
diff --git a/hw/ccid-card-passthru.c b/hw/ccid-card-passthru.c
index 3566e55..111894f 100644
--- a/hw/ccid-card-passthru.c
+++ b/hw/ccid-card-passthru.c
@@ -138,6 +138,59 @@ static void ccid_card_vscard_handle_init(
ccid_card_vscard_send_init(card);
}
+static int check_atr(PassthruState *card, uint8_t *data, int len)
+{
+int historical_length, opt_bytes;
+int td_count = 0;
+int td;
+
+if (len 2) {
+return 0;
+}
+historical_length = data[1] 0xf;
+opt_bytes = 0;
+if (data[0] != 0x3b data[0] != 0x3f) {
+DPRINTF(card, D_WARN, atr's T0 is 0x%X, not in {0x3b, 0x3f}\n,
+data[0]);
+return 0;
+}
+td_count = 0;
+td = data[1] 4;
+while (td td_count 2 opt_bytes + historical_length + 2 len) {
+td_count++;
+if (td 0x1) {
+opt_bytes++;
+}
+if (td 0x2) {
+opt_bytes++;
+}
+if (td 0x4) {
+opt_bytes++;
+}
+if (td 0x8) {
+opt_bytes++;
+td = data[opt_bytes + 2] 4;
+}
+}
+if (len 2 + historical_length + opt_bytes) {
+DPRINTF(card, D_WARN,
+atr too short: len %d, but historical_len %d, T1 0x%X\n,
+len, historical_length, data[1]);
+return 0;
+}
+if (len 2 + historical_length + opt_bytes) {
+DPRINTF(card, D_WARN,
+atr too long: len %d, but hist/opt %d/%d, T1 0x%X\n,
+len, historical_length, opt_bytes, data[1]);
+/* let it through */
+}
+DPRINTF(card, D_VERBOSE,
+atr passes check: %d total length, %d historical, %d optional\n,
+len, historical_length, opt_bytes);
+
+return 1;
+}
+
static void ccid_card_vscard_handle_message(PassthruState *card,
VSCMsgHeader *scr_msg_header)
{
@@ -152,6 +205,12 @@ static void ccid_card_vscard_handle_message(PassthruState
*card,
VSC_GENERAL_ERROR);
break;
}
+if (!check_atr(card, data, scr_msg_header-length)) {
+error_report(ATR is inconsistent, ignoring);
+ccid_card_vscard_send_error(card, scr_msg_header-reader_id,
+VSC_GENERAL_ERROR);
+break;
+}
memcpy(card-atr, data, scr_msg_header-length);
card-atr_length = scr_msg_header-length;
ccid_card_card_inserted(card-base);
--
1.8.1.4
[Qemu-devel] [PATCH v2 05/10] dev-smartcard-reader: change default protocol to T=0
We don't support T=1 so we shouldn't advertise it by default.
Two independent changes:
* Default ATR
sets T=0. This gets overwritten by the client provided ATR later.
* Class descriptor
changes dwAdvertise dwProtocols. to 0x1 and dwProtocols.=0 per spec.
Signed-off-by: Alon Levy al...@redhat.com
---
hw/usb/dev-smartcard-reader.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 014c0b1..bb96f02 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -335,8 +335,8 @@ static const uint8_t qemu_ccid_descriptor[] = {
*/
0x07, /* u8 bVoltageSupport; 01h - 5.0v, 02h - 3.0, 03 - 1.8 */
-0x03, 0x00, /* u32 dwProtocols; . = h.*/
-0x00, 0x00, /* : 0001h = Protocol T=0, 0002h = Protocol T=1 */
+0x00, 0x00, /* u32 dwProtocols; . = h.*/
+0x01, 0x00, /* : 0001h = Protocol T=0, 0002h = Protocol T=1 */
/* u32 dwDefaultClock; in kHZ (0x0fa0 is 4 MHz) */
0xa0, 0x0f, 0x00, 0x00,
/* u32 dwMaximumClock; */
@@ -875,7 +875,7 @@ static const CCID_ProtocolDataStructure
defaultProtocolDataStructure = {
static void ccid_reset_parameters(USBCCIDState *s)
{
- s-bProtocolNum = 1; /* T=1 */
+ s-bProtocolNum = 0; /* T=0 */
s-abProtocolDataStructure = defaultProtocolDataStructure;
}
--
1.8.1.4
[Qemu-devel] [PATCH v2 08/10] libcacard: move atr setting from macro to function
Only because qemu's checkpatch complains about it.
Signed-off-by: Alon Levy al...@redhat.com
---
Makefile.objs | 1 +
libcacard/vcard_emul_nss.c | 14 +++---
libcacard/vcardt.c | 40
libcacard/vcardt.h | 13 -
libcacard/vcardt_internal.h | 6 ++
5 files changed, 58 insertions(+), 16 deletions(-)
create mode 100644 libcacard/vcardt.c
create mode 100644 libcacard/vcardt_internal.h
diff --git a/Makefile.objs b/Makefile.objs
index f99841c..6d47567 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -41,6 +41,7 @@ libcacard-y += libcacard/vcard.o libcacard/vreader.o
libcacard-y += libcacard/vcard_emul_nss.o
libcacard-y += libcacard/vcard_emul_type.o
libcacard-y += libcacard/card_7816.o
+libcacard-y += libcacard/vcardt.o
##
# Target independent part of system emulation. The long term path is to
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index 9ba80fb..1a3e568 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -33,6 +33,9 @@
#include vreader.h
#include vevent.h
+#include libcacard/vcardt_internal.h
+
+
typedef enum {
VCardEmulUnknown = -1,
VCardEmulFalse = 0,
@@ -519,18 +522,23 @@ vcard_emul_reader_get_slot(VReader *vreader)
}
/*
- * Card ATR's map to physical cards. VCARD_ATR_PREFIX will set appropriate
+ * Card ATR's map to physical cards. vcard_alloc_atr will set appropriate
* historical bytes for any software emulated card. The remaining bytes can be
* used to indicate the actual emulator
*/
-static const unsigned char nss_atr[] = { VCARD_ATR_PREFIX(3), 'N', 'S', 'S' };
+static unsigned char *nss_atr;
+static int nss_atr_len;
void
vcard_emul_get_atr(VCard *card, unsigned char *atr, int *atr_len)
{
-int len = MIN(sizeof(nss_atr), *atr_len);
+int len;
assert(atr != NULL);
+if (nss_atr == NULL) {
+nss_atr = vcard_alloc_atr(NSS, nss_atr_len);
+}
+len = MIN(nss_atr_len, *atr_len);
memcpy(atr, nss_atr, len);
*atr_len = len;
}
diff --git a/libcacard/vcardt.c b/libcacard/vcardt.c
new file mode 100644
index 000..9ce4648
--- /dev/null
+++ b/libcacard/vcardt.c
@@ -0,0 +1,40 @@
+#include stdlib.h
+#include string.h
+#include glib.h
+
+#include libcacard/vcardt.h
+
+#include libcacard/vcardt_internal.h
+
+/* create an ATR with appropriate historical bytes */
+#define ATR_TS_DIRECT_CONVENTION 0x3b
+#define ATR_TA_PRESENT 0x10
+#define ATR_TB_PRESENT 0x20
+#define ATR_TC_PRESENT 0x40
+#define ATR_TD_PRESENT 0x80
+
+unsigned char *vcard_alloc_atr(const char *postfix, int *atr_len)
+{
+int postfix_len;
+const char prefix[] = VCARD_;
+const char default_postfix[] = DEFAULT;
+const int prefix_len = sizeof(prefix) - 1;
+int total_len;
+unsigned char *atr;
+
+if (postfix == NULL) {
+postfix = default_postfix;
+}
+postfix_len = strlen(postfix);
+total_len = 3 + prefix_len + postfix_len;
+atr = g_malloc(total_len);
+atr[0] = ATR_TS_DIRECT_CONVENTION;
+atr[1] = ATR_TD_PRESENT + prefix_len + postfix_len;
+atr[2] = 0x00;
+memcpy(atr[3], prefix, prefix_len);
+memcpy(atr[3 + prefix_len], postfix, postfix_len);
+if (atr_len) {
+*atr_len = total_len;
+}
+return atr;
+}
diff --git a/libcacard/vcardt.h b/libcacard/vcardt.h
index 3b9a619..795e265 100644
--- a/libcacard/vcardt.h
+++ b/libcacard/vcardt.h
@@ -25,19 +25,6 @@ typedef struct VCardEmulStruct VCardEmul;
#define MAX_CHANNEL 4
-/* create an ATR with appropriate historical bytes */
-#define TS_DIRECT_CONVENTION 0x3b
-#define TA_PRESENT 0x10
-#define TB_PRESENT 0x20
-#define TC_PRESENT 0x40
-#define TD_PRESENT 0x80
-
-#define VCARD_ATR_PREFIX(size) \
-TS_DIRECT_CONVENTION, \
-TD_PRESENT + (6 + size), \
-0x00, \
-'V', 'C', 'A', 'R', 'D', '_'
-
typedef enum {
VCARD_DONE,
VCARD_NEXT,
diff --git a/libcacard/vcardt_internal.h b/libcacard/vcardt_internal.h
new file mode 100644
index 000..e5c8d2d
--- /dev/null
+++ b/libcacard/vcardt_internal.h
@@ -0,0 +1,6 @@
+#ifndef VCARDT_INTERNAL_H
+#define VCARDT_INTERNAL_H
+
+unsigned char *vcard_alloc_atr(const char *postfix, int *atr_len);
+
+#endif
--
1.8.1.4
[Qemu-devel] [PATCH v2 04/10] dev-smartcard-reader: define structs for CCID_Parameter internals
Signed-off-by: Alon Levy al...@redhat.com
---
hw/usb/dev-smartcard-reader.c | 74 +++
1 file changed, 47 insertions(+), 27 deletions(-)
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 751bd04..014c0b1 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -189,10 +189,34 @@ typedef struct QEMU_PACKED CCID_SlotStatus {
uint8_t bClockStatus;
} CCID_SlotStatus;
+typedef struct QEMU_PACKED CCID_T0ProtocolDataStructure {
+uint8_t bmFindexDindex;
+uint8_t bmTCCKST0;
+uint8_t bGuardTimeT0;
+uint8_t bWaitingIntegerT0;
+uint8_t bClockStop;
+} CCID_T0ProtocolDataStructure;
+
+typedef struct QEMU_PACKED CCID_T1ProtocolDataStructure {
+uint8_t bmFindexDindex;
+uint8_t bmTCCKST1;
+uint8_t bGuardTimeT1;
+uint8_t bWaitingIntegerT1;
+uint8_t bClockStop;
+uint8_t bIFSC;
+uint8_t bNadValue;
+} CCID_T1ProtocolDataStructure;
+
+typedef union CCID_ProtocolDataStructure {
+CCID_T0ProtocolDataStructure t0;
+CCID_T1ProtocolDataStructure t1;
+uint8_t data[7]; /* must be = max(sizeof(t0), sizeof(t1)) */
+} CCID_ProtocolDataStructure;
+
typedef struct QEMU_PACKED CCID_Parameter {
CCID_BULK_IN b;
uint8_t bProtocolNum;
-uint8_t abProtocolDataStructure[0];
+CCID_ProtocolDataStructure abProtocolDataStructure;
} CCID_Parameter;
typedef struct QEMU_PACKED CCID_DataBlock {
@@ -224,7 +248,7 @@ typedef struct QEMU_PACKED CCID_SetParameters {
CCID_Header hdr;
uint8_t bProtocolNum;
uint16_t abRFU;
-uint8_tabProtocolDataStructure[0];
+CCID_ProtocolDataStructure abProtocolDataStructure;
} CCID_SetParameters;
typedef struct CCID_Notify_Slot_Change {
@@ -254,8 +278,6 @@ typedef struct CCIDBus {
BusState qbus;
} CCIDBus;
-#define MAX_PROTOCOL_SIZE 7
-
/*
* powered - defaults to true, changed by PowerOn/PowerOff messages
*/
@@ -279,7 +301,7 @@ typedef struct USBCCIDState {
uint8_t bError;
uint8_t bmCommandStatus;
uint8_t bProtocolNum;
-uint8_t abProtocolDataStructure[MAX_PROTOCOL_SIZE];
+CCID_ProtocolDataStructure abProtocolDataStructure;
uint32_t ulProtocolDataStructureSize;
uint32_t state_vmstate;
uint32_t migration_target_ip;
@@ -765,7 +787,7 @@ static void ccid_write_parameters(USBCCIDState *s,
CCID_Header *recv)
h-b.bStatus = ccid_calc_status(s);
h-b.bError = s-bError;
h-bProtocolNum = s-bProtocolNum;
-memcpy(h-abProtocolDataStructure, s-abProtocolDataStructure, len);
+h-abProtocolDataStructure = s-abProtocolDataStructure;
ccid_reset_error_status(s);
}
@@ -825,38 +847,36 @@ static void ccid_write_data_block_atr(USBCCIDState *s,
CCID_Header *recv)
static void ccid_set_parameters(USBCCIDState *s, CCID_Header *recv)
{
CCID_SetParameters *ph = (CCID_SetParameters *) recv;
-uint32_t len = 0;
-if ((ph-bProtocolNum 3) == 0) {
-len = 5;
-}
-if ((ph-bProtocolNum 3) == 1) {
-len = 7;
-}
-if (len == 0) {
-s-bmCommandStatus = COMMAND_STATUS_FAILED;
-s-bError = 7; /* Protocol invalid or not supported */
+uint32_t protocol_num = ph-bProtocolNum 3;
+
+if (protocol_num != 0 protocol_num != 1) {
+ccid_report_error_failed(s, ERROR_CMD_NOT_SUPPORTED);
return;
}
-s-bProtocolNum = ph-bProtocolNum;
-memcpy(s-abProtocolDataStructure, ph-abProtocolDataStructure, len);
-s-ulProtocolDataStructureSize = len;
-DPRINTF(s, 1, %s: using len %d\n, __func__, len);
+s-bProtocolNum = protocol_num;
+s-abProtocolDataStructure = ph-abProtocolDataStructure;
}
/*
* must be 5 bytes for T=0, 7 bytes for T=1
* See page 52
*/
-static const uint8_t abDefaultProtocolDataStructure[7] = {
-0x77, 0x00, 0x00, 0x00, 0x00, 0xfe /*IFSC*/, 0x00 /*NAD*/ };
+static const CCID_ProtocolDataStructure defaultProtocolDataStructure = {
+.t1 = {
+.bmFindexDindex = 0x77,
+.bmTCCKST1 = 0x00,
+.bGuardTimeT1 = 0x00,
+.bWaitingIntegerT1 = 0x00,
+.bClockStop = 0x00,
+.bIFSC = 0xfe,
+.bNadValue = 0x00,
+}
+};
static void ccid_reset_parameters(USBCCIDState *s)
{
- uint32_t len = sizeof(abDefaultProtocolDataStructure);
-
s-bProtocolNum = 1; /* T=1 */
- s-ulProtocolDataStructureSize = len;
- memcpy(s-abProtocolDataStructure, abDefaultProtocolDataStructure, len);
+ s-abProtocolDataStructure = defaultProtocolDataStructure;
}
/* NOTE: only a single slot is supported (SLOT_0) */
@@ -1345,7 +1365,7 @@ static VMStateDescription ccid_vmstate = {
VMSTATE_UINT8(bError, USBCCIDState),
VMSTATE_UINT8(bmCommandStatus, USBCCIDState),
VMSTATE_UINT8(bProtocolNum, USBCCIDState),
-VMSTATE_BUFFER(abProtocolDataStructure, USBCCIDState),
+VMSTATE_BUFFER(abProtocolDataStructure.data, USBCCIDState
[Qemu-devel] [PATCH v2 06/10] dev-smartcard-reader: copy atr protocol to ccid parameters
Adds todos.
Signed-off-by: Alon Levy al...@redhat.com
---
hw/usb/dev-smartcard-reader.c | 45 +++
1 file changed, 45 insertions(+)
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index bb96f02..7040e8d 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -833,14 +833,59 @@ static void ccid_write_data_block_answer(USBCCIDState *s,
ccid_write_data_block(s, slot, seq, data, len);
}
+static uint8_t atr_get_protocol_num(const uint8_t *atr, uint32_t len)
+{
+int i;
+
+if (len 2 || !(atr[1] 0x80)) {
+/* too short or TD1 not included */
+return 0; /* T=0, default */
+}
+i = 1 + !!(atr[1] 0x10) + !!(atr[1] 0x20) + !!(atr[1] 0x40);
+i += !!(atr[1] 0x80);
+return atr[i] 0x0f;
+}
+
static void ccid_write_data_block_atr(USBCCIDState *s, CCID_Header *recv)
{
const uint8_t *atr = NULL;
uint32_t len = 0;
+uint8_t atr_protocol_num;
+CCID_T0ProtocolDataStructure *t0 = s-abProtocolDataStructure.t0;
+CCID_T1ProtocolDataStructure *t1 = s-abProtocolDataStructure.t1;
if (s-card) {
atr = ccid_card_get_atr(s-card, len);
}
+atr_protocol_num = atr_get_protocol_num(atr, len);
+DPRINTF(s, D_VERBOSE, %s: atr contains protocol=%d\n, __func__,
+atr_protocol_num);
+/* set parameters from ATR - see spec page 109 */
+s-bProtocolNum = (atr_protocol_num = 1 ? atr_protocol_num
+ : s-bProtocolNum);
+switch (atr_protocol_num) {
+case 0:
+/* TODO: unimplemented ATR T0 parameters */
+t0-bmFindexDindex = 0;
+t0-bmTCCKST0 = 0;
+t0-bGuardTimeT0 = 0;
+t0-bWaitingIntegerT0 = 0;
+t0-bClockStop = 0;
+break;
+case 1:
+/* TODO: unimplemented ATR T1 parameters */
+t1-bmFindexDindex = 0;
+t1-bmTCCKST1 = 0;
+t1-bGuardTimeT1 = 0;
+t1-bWaitingIntegerT1 = 0;
+t1-bClockStop = 0;
+t1-bIFSC = 0;
+t1-bNadValue = 0;
+break;
+default:
+DPRINTF(s, D_WARN, %s: error: unsupported ATR protocol %d\n,
+__func__, atr_protocol_num);
+}
ccid_write_data_block(s, recv-bSlot, recv-bSeq, atr, len);
}
--
1.8.1.4
[Qemu-devel] [PATCH v2 07/10] libcacard/vreader: add debugging messages for apdu
Using g_debug with log domain libcacard
Signed-off-by: Alon Levy al...@redhat.com
---
libcacard/cac.c | 7 -
libcacard/cac.h | 8 ++
libcacard/vreader.c | 77 +
3 files changed, 85 insertions(+), 7 deletions(-)
diff --git a/libcacard/cac.c b/libcacard/cac.c
index 927a4ca..5864539 100644
--- a/libcacard/cac.c
+++ b/libcacard/cac.c
@@ -12,13 +12,6 @@
#include vcard_emul.h
#include card_7816.h
-#define CAC_GET_PROPERTIES 0x56
-#define CAC_GET_ACR 0x4c
-#define CAC_READ_BUFFER 0x52
-#define CAC_UPDATE_BUFFER 0x58
-#define CAC_SIGN_DECRYPT0x42
-#define CAC_GET_CERTIFICATE 0x36
-
/* private data for PKI applets */
typedef struct CACPKIAppletDataStruct {
unsigned char *cert;
diff --git a/libcacard/cac.h b/libcacard/cac.h
index 15a61be..d24a2a8 100644
--- a/libcacard/cac.h
+++ b/libcacard/cac.h
@@ -9,6 +9,14 @@
#define CAC_H 1
#include vcard.h
#include vreader.h
+
+#define CAC_GET_PROPERTIES 0x56
+#define CAC_GET_ACR 0x4c
+#define CAC_READ_BUFFER 0x52
+#define CAC_UPDATE_BUFFER 0x58
+#define CAC_SIGN_DECRYPT0x42
+#define CAC_GET_CERTIFICATE 0x36
+
/*
* Initialize the cac card. This is the only public function in this file. All
* the rest are connected through function pointers.
diff --git a/libcacard/vreader.c b/libcacard/vreader.c
index f3efc27..5793d73 100644
--- a/libcacard/vreader.c
+++ b/libcacard/vreader.c
@@ -5,6 +5,12 @@
* See the COPYING.LIB file in the top-level directory.
*/
+#ifdef G_LOG_DOMAIN
+#undef G_LOG_DOMAIN
+#endif
+#define G_LOG_DOMAIN libcacard
+#include glib.h
+
#include qemu-common.h
#include qemu/thread.h
@@ -13,6 +19,9 @@
#include card_7816.h
#include vreader.h
#include vevent.h
+#include cac.h /* just for debugging defines */
+
+#define LIBCACARD_LOG_DOMAIN libcacard
struct VReaderStruct {
intreference_count;
@@ -24,6 +33,66 @@ struct VReaderStruct {
VReaderEmulFree reader_private_free;
};
+/*
+ * Debug helpers
+ */
+
+static const char *
+apdu_ins_to_string(int ins)
+{
+switch (ins) {
+case VCARD7816_INS_MANAGE_CHANNEL:
+return manage channel;
+case VCARD7816_INS_EXTERNAL_AUTHENTICATE:
+return external authenticate;
+case VCARD7816_INS_GET_CHALLENGE:
+return get challenge;
+case VCARD7816_INS_INTERNAL_AUTHENTICATE:
+return internal authenticate;
+case VCARD7816_INS_ERASE_BINARY:
+return erase binary;
+case VCARD7816_INS_READ_BINARY:
+return read binary;
+case VCARD7816_INS_WRITE_BINARY:
+return write binary;
+case VCARD7816_INS_UPDATE_BINARY:
+return update binary;
+case VCARD7816_INS_READ_RECORD:
+return read record;
+case VCARD7816_INS_WRITE_RECORD:
+return write record;
+case VCARD7816_INS_UPDATE_RECORD:
+return update record;
+case VCARD7816_INS_APPEND_RECORD:
+return append record;
+case VCARD7816_INS_ENVELOPE:
+return envelope;
+case VCARD7816_INS_PUT_DATA:
+return put data;
+case VCARD7816_INS_GET_DATA:
+return get data;
+case VCARD7816_INS_SELECT_FILE:
+return select file;
+case VCARD7816_INS_VERIFY:
+return verify;
+case VCARD7816_INS_GET_RESPONSE:
+return get response;
+case CAC_GET_PROPERTIES:
+return get properties;
+case CAC_GET_ACR:
+return get acr;
+case CAC_READ_BUFFER:
+return read buffer;
+case CAC_UPDATE_BUFFER:
+return update buffer;
+case CAC_SIGN_DECRYPT:
+return sign decrypt;
+case CAC_GET_CERTIFICATE:
+return get certificate;
+}
+return unknown;
+}
+
/* manage locking */
static inline void
vreader_lock(VReader *reader)
@@ -204,7 +273,15 @@ vreader_xfr_bytes(VReader *reader,
response = vcard_make_response(status);
card_status = VCARD_DONE;
} else {
+g_debug(%s: CLS=0x%x,INS=0x%x,P1=0x%x,P2=0x%x,Lc=%d,Le=%d %s\n,
+ __func__, apdu-a_cla, apdu-a_ins, apdu-a_p1, apdu-a_p2,
+ apdu-a_Lc, apdu-a_Le, apdu_ins_to_string(apdu-a_ins));
card_status = vcard_process_apdu(card, apdu, response);
+if (response) {
+g_debug(%s: status=%d sw1=0x%x sw2=0x%x len=%d (total=%d)\n,
+ __func__, response-b_status, response-b_sw1,
+ response-b_sw2, response-b_len, response-b_total_len);
+}
}
assert(card_status == VCARD_DONE);
if (card_status == VCARD_DONE) {
--
1.8.1.4
