[Spice-devel] [PATCH spice-common v2] Fix generation of Smartcard channel

Frediano Ziglio fziglio at redhat.com
Wed May 16 17:00:35 UTC 2018


The Smartcard channel definition has been always broken.
Multiple client messages with the same ID are defined in the channel.
This cause on server demarshaller to only have last message defined,
while on the client marshaller code all message marshallers are
defined but client uses only header message.

Following the difference of the generated code.

  diff -rup old/generated_client_marshallers.c common/generated_client_marshallers.c
  --- old/generated_client_marshallers.c	2018-05-14 22:49:07.641778414 +0100
  +++ common/generated_client_marshallers.c	2018-05-14 22:49:22.266329296 +0100
  @@ -389,27 +389,6 @@ static void spice_marshall_msgc_tunnel_s
   }

   #ifdef USE_SMARTCARD
  -static void spice_marshall_msgc_smartcard_data(SPICE_GNUC_UNUSED SpiceMarshaller *m, SPICE_GNUC_UNUSED SpiceMsgcSmartcard *msg, SpiceMarshaller **reader_name_out)
  -{
  -    SPICE_GNUC_UNUSED SpiceMarshaller *m2;
  -    SpiceMsgcSmartcard *src;
  -    *reader_name_out = NULL;
  -    src = (SpiceMsgcSmartcard *)msg;
  -
  -    /* header */ {
  -        spice_marshaller_add_uint32(m, src->header.type);
  -        spice_marshaller_add_uint32(m, src->header.reader_id);
  -        spice_marshaller_add_uint32(m, src->header.length);
  -    }
  -    if (src->header.type == SPICE_VSC_MESSAGE_TYPE_ReaderAdd) {
  -        /* Don't marshall @nomarshal reader_name */
  -    } else if (src->header.type == SPICE_VSC_MESSAGE_TYPE_ATR || src->header.type == SPICE_VSC_MESSAGE_TYPE_APDU) {
  -        /* Remaining data must be appended manually */
  -    } else if (src->header.type == SPICE_VSC_MESSAGE_TYPE_Error) {
  -        spice_marshaller_add_uint32(m, src->error.code);
  -    }
  -}
  -
   static void spice_marshall_msgc_smartcard_header(SPICE_GNUC_UNUSED SpiceMarshaller *m, SPICE_GNUC_UNUSED VSCMsgHeader *msg)
   {
       SPICE_GNUC_UNUSED SpiceMarshaller *m2;
  @@ -421,25 +400,6 @@ static void spice_marshall_msgc_smartcar
       spice_marshaller_add_uint32(m, src->length);
   }

  -static void spice_marshall_msgc_smartcard_error(SPICE_GNUC_UNUSED SpiceMarshaller *m, SPICE_GNUC_UNUSED VSCMsgError *msg)
  -{
  -    SPICE_GNUC_UNUSED SpiceMarshaller *m2;
  -    VSCMsgError *src;
  -    src = (VSCMsgError *)msg;
  -
  -    spice_marshaller_add_uint32(m, src->code);
  -}
  -
  -static void spice_marshall_msgc_smartcard_atr(SPICE_GNUC_UNUSED SpiceMarshaller *m, SPICE_GNUC_UNUSED VSCMsgATR *msg)
  -{
  -    SPICE_GNUC_UNUSED SpiceMarshaller *m2;
  -}
  -
  -static void spice_marshall_msgc_smartcard_reader_add(SPICE_GNUC_UNUSED SpiceMarshaller *m, SPICE_GNUC_UNUSED VSCMsgReaderAdd *msg)
  -{
  -    SPICE_GNUC_UNUSED SpiceMarshaller *m2;
  -}
  -
   #endif /* USE_SMARTCARD */
   static void spice_marshall_SpiceMsgCompressedData(SPICE_GNUC_UNUSED SpiceMarshaller *m, SPICE_GNUC_UNUSED SpiceMsgCompressedData *msg)
   {
  @@ -496,20 +456,8 @@ SpiceMessageMarshallers * spice_message_
       marshallers.msgc_record_mode = spice_marshall_msgc_record_mode;
       marshallers.msgc_record_start_mark = spice_marshall_msgc_record_start_mark;
   #ifdef USE_SMARTCARD
  -    marshallers.msgc_smartcard_atr = spice_marshall_msgc_smartcard_atr;
  -#endif /* USE_SMARTCARD */
  -#ifdef USE_SMARTCARD
  -    marshallers.msgc_smartcard_data = spice_marshall_msgc_smartcard_data;
  -#endif /* USE_SMARTCARD */
  -#ifdef USE_SMARTCARD
  -    marshallers.msgc_smartcard_error = spice_marshall_msgc_smartcard_error;
  -#endif /* USE_SMARTCARD */
  -#ifdef USE_SMARTCARD
       marshallers.msgc_smartcard_header = spice_marshall_msgc_smartcard_header;
   #endif /* USE_SMARTCARD */
  -#ifdef USE_SMARTCARD
  -    marshallers.msgc_smartcard_reader_add = spice_marshall_msgc_smartcard_reader_add;
  -#endif /* USE_SMARTCARD */
       marshallers.msgc_tunnel_service_add = spice_marshall_msgc_tunnel_service_add;
       marshallers.msgc_tunnel_service_remove = spice_marshall_msgc_tunnel_service_remove;
       marshallers.msgc_tunnel_socket_closed = spice_marshall_msgc_tunnel_socket_closed;
  diff -rup old/generated_client_marshallers.h common/generated_client_marshallers.h
  --- old/generated_client_marshallers.h	2018-05-14 22:49:07.641778414 +0100
  +++ common/generated_client_marshallers.h	2018-05-14 22:49:22.739358627 +0100
  @@ -61,11 +61,7 @@ typedef struct {
       void (*msgc_tunnel_socket_data)(SpiceMarshaller *m, SpiceMsgcTunnelSocketData *msg);
       void (*msgc_tunnel_socket_token)(SpiceMarshaller *m, SpiceMsgcTunnelSocketTokens *msg);
   #ifdef USE_SMARTCARD
  -    void (*msgc_smartcard_data)(SpiceMarshaller *m, SpiceMsgcSmartcard *msg, SpiceMarshaller **reader_name_out);
       void (*msgc_smartcard_header)(SpiceMarshaller *m, VSCMsgHeader *msg);
  -    void (*msgc_smartcard_error)(SpiceMarshaller *m, VSCMsgError *msg);
  -    void (*msgc_smartcard_atr)(SpiceMarshaller *m, VSCMsgATR *msg);
  -    void (*msgc_smartcard_reader_add)(SpiceMarshaller *m, VSCMsgReaderAdd *msg);
   #endif /* USE_SMARTCARD */
       void (*msg_SpiceMsgCompressedData)(SpiceMarshaller *m, SpiceMsgCompressedData *msg);
       void (*msgc_port_event)(SpiceMarshaller *m, SpiceMsgcPortEvent *msg);
  Only in common/: generated_client_marshallers.lo
  diff -rup old/generated_server_demarshallers.c common/generated_server_demarshallers.c
  --- old/generated_server_demarshallers.c	2018-05-14 22:49:07.641778414 +0100
  +++ common/generated_server_demarshallers.c	2018-05-14 22:49:23.498405695 +0100
  @@ -1957,24 +1957,18 @@ static uint8_t * parse_TunnelChannel_msg

   #ifdef USE_SMARTCARD

  -static uint8_t * parse_msgc_smartcard_reader_add(uint8_t *message_start, uint8_t *message_end, SPICE_GNUC_UNUSED int minor, size_t *size, message_destructor_t *free_message)
  +static uint8_t * parse_msgc_smartcard_header(uint8_t *message_start, uint8_t *message_end, SPICE_GNUC_UNUSED int minor, size_t *size, message_destructor_t *free_message)
   {
       SPICE_GNUC_UNUSED uint8_t *pos;
       uint8_t *start = message_start;
       uint8_t *data = NULL;
       uint64_t nw_size;
  +    uint64_t mem_size;
       uint8_t *in, *end;
  -    uint64_t reader_name__nw_size;
  -    uint64_t reader_name__nelements;
  -    VSCMsgReaderAdd *out;
  +    VSCMsgHeader *out;

  -    { /* reader_name */
  -        reader_name__nelements = message_end - (start + 0);
  -
  -        reader_name__nw_size = reader_name__nelements;
  -    }
  -
  -    nw_size = 0 + reader_name__nw_size;
  +    nw_size = 12;
  +    mem_size = sizeof(VSCMsgHeader);

       /* Check if message fits in reported side */
       if (nw_size > (uintptr_t) (message_end - start)) {
  @@ -1986,13 +1980,14 @@ static uint8_t * parse_msgc_smartcard_re
       if (SPICE_UNLIKELY(data == NULL)) {
           goto error;
       }
  -    end = data + sizeof(VSCMsgReaderAdd);
  +    end = data + sizeof(VSCMsgHeader);
       in = start;

  -    out = (VSCMsgReaderAdd *)data;
  +    out = (VSCMsgHeader *)data;

  -    memcpy(out->reader_name, in, reader_name__nelements);
  -    in += reader_name__nelements;
  +    out->type = consume_uint32(&in);
  +    out->reader_id = consume_uint32(&in);
  +    out->length = consume_uint32(&in);

       assert(in <= message_end);
       assert(end <= data + mem_size);
  @@ -2017,7 +2012,7 @@ static uint8_t * parse_SmartcardChannel_
           parse_msgc_disconnecting
       };
       static parse_msg_func_t funcs2[1] =  {
  -        parse_msgc_smartcard_reader_add
  +        parse_msgc_smartcard_header
       };
       if (message_type >= 1 && message_type < 7) {
           return funcs1[message_type-1](message_start, message_end, minor, size_out, free_message);

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 spice.proto | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Changes since v1:
- remove spurious changes.

diff --git a/spice.proto b/spice.proto
index 4d916bb..69f169e 100644
--- a/spice.proto
+++ b/spice.proto
@@ -1400,6 +1400,12 @@ channel SmartcardChannel : BaseChannel {
     } @ctype(SpiceMsgSmartcard) data = 101;
 
  client:
+/* Some of the following messages are duplicated, the protocol
+ * definition was broken. Messages, as you can see have same ID.
+ * Also code was not used and didn't compile correctly.
+ * Keeping in the hope could be useful in the future.
+ */
+/*
     message {
 	VscMessageHeader header;
 	switch (header.type) {
@@ -1412,13 +1418,14 @@ channel SmartcardChannel : BaseChannel {
 	    VscMessageError error;
 	} u @anon;
     } @ctype(SpiceMsgcSmartcard) data = 101;
-
+*/
     message {
 	vsc_message_type type;
 	uint32 reader_id;
 	uint32 length;
     } @ctype(VSCMsgHeader) header = 101;
-
+/* See comment on client data message above */
+/*
     message {
 	uint32 code;
     } @ctype(VSCMsgError) error = 101;
@@ -1430,6 +1437,7 @@ channel SmartcardChannel : BaseChannel {
     message {
 	int8 reader_name[] @zero_terminated @nonnull;
     } @ctype(VSCMsgReaderAdd) reader_add = 101;
+*/
 } @ifdef(USE_SMARTCARD);
 
 channel SpicevmcChannel : BaseChannel {
-- 
2.17.0



More information about the Spice-devel mailing list