I'll send a v6 in a bit; thanks for the review.
That looks correct to me, but it's maybe better to memmove outside the
loop,
such that memmove is called once every read and not for every
message processed.
Maybe it does not matter as this was not noticed so far, so messages
mostly come one at a time.
size_t ppos = 0
do {
rc = process_message(ccid, buf + ppos, pos - ppos);
ppos += rc
} while (rc> 0)
if (ppos > 0 && ppos != pos)
memmove(buf, buf+ppos, pos - ppos)
To be honest, I think that code will never trigger, so I think I'm going
to keep it in the format I put forth.
I did write a psychotic set of tests that deliberately under and overran
the buffer, and confirmed that the new code seems to behave (and the old
code was buggy, as expected).
I'm attaching a 'v5-v6.diff' which has just the set of changes from this
last round of comments. I'll also send in v6.
The only open item I'm aware of is Uri's request to spin on EINTR, and
I'm hoping to get a pass on that one.
Cheers,
Jeremy
diff --git a/configure.ac b/configure.ac
index d9da852..27b1cc7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -152,6 +152,10 @@ AC_SUBST(cciddir)
if test "x$enable_ccid" != "xno"; then
PKG_CHECK_MODULES(LIBPCSCLITE, [libpcsclite])
PKG_CHECK_MODULES(LIBCACARD, [libcacard])
+
+ if test "x$enable_xspice" = "xno"; then
+ AC_MSG_ERROR([Building with ccid requires xspice, but xspice is not enabled])
+ fi
fi
diff --git a/src/spiceccid/spiceccid.c b/src/spiceccid/spiceccid.c
index ef9c931..9f630d2 100644
--- a/src/spiceccid/spiceccid.c
+++ b/src/spiceccid/spiceccid.c
@@ -154,7 +154,13 @@ static int send_tx_buffer(smartcard_ccid_t *ccid, void *data, int len)
static void process_reader_add(smartcard_ccid_t *ccid, VSCMsgHeader *h, char *data)
{
+ if (ccid->state & STATE_READER_ADDED) {
+ send_reply(ccid, VSC_GENERAL_ERROR);
+ return;
+ }
+
ccid->state |= STATE_READER_ADDED;
+ ccid->state &= ~STATE_READER_REMOVED;
pthread_mutex_init(&ccid->apdu_lock, NULL);
ccid->apdu_list = NULL;
@@ -166,6 +172,11 @@ static void process_reader_remove(smartcard_ccid_t *ccid, VSCMsgHeader *h)
{
apdu_t *p;
+ if (ccid->state & STATE_READER_REMOVED) {
+ send_reply(ccid, VSC_GENERAL_ERROR);
+ return;
+ }
+
ccid->state |= STATE_READER_REMOVED;
ccid->state &= ~STATE_READER_ADDED;
@@ -179,7 +190,13 @@ static void process_reader_remove(smartcard_ccid_t *ccid, VSCMsgHeader *h)
static void process_atr(smartcard_ccid_t *ccid, VSCMsgHeader *h, char *data)
{
- ccid->atr_len = MIN(h->length, sizeof(ccid->atr));
+ ccid->atr_len = h->length;
+ if (h->length > 0 && h->length > sizeof(ccid->atr)) {
+ fprintf(stderr, "Supplied ATR of length %d exceeds %d maximum\n",
+ h->length, sizeof(ccid->atr));
+ send_reply(ccid, VSC_GENERAL_ERROR);
+ return;
+ }
memset(ccid->atr, 0, sizeof(ccid->atr));
memcpy(ccid->atr, data, ccid->atr_len);
@@ -189,7 +206,8 @@ static void process_atr(smartcard_ccid_t *ccid, VSCMsgHeader *h, char *data)
static void process_apdu(smartcard_ccid_t *ccid, VSCMsgHeader *h, char *data)
{
- push_apdu(ccid, data, h->length);
+ if (ccid->state & STATE_READER_ADDED)
+ push_apdu(ccid, data, h->length);
}
static void process_card_remove(smartcard_ccid_t *ccid, VSCMsgHeader *h)
@@ -237,7 +255,7 @@ static int process_message(smartcard_ccid_t *ccid, char *buf, int len)
}
- return(MIN(len, h.length + sizeof(h)));
+ return(h.length + sizeof(h));
}
static void * lun_thread(void *arg)
@@ -263,7 +281,10 @@ static void * lun_thread(void *arg)
do {
rc = process_message(ccid, buf, pos);
pos -= rc;
- } while (rc > 0);
+
+ if (rc > 0 && pos > 0)
+ memmove(buf, buf + rc, pos);
+ } while (rc > 0 && pos > 0);
}
return NULL;
@@ -348,6 +369,7 @@ RESPONSECODE IFDHCloseChannel(DWORD Lun)
luns[i].fd = -1;
luns[i].lun = 0;
luns[i].atr_len = 0;
+ luns[i].state &= ~STATE_OPEN;
break;
}
}
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel