[Qemu-devel] [PATCH 1/4] usb-desc: fix user trigerrable segfaults (!config)

[Qemu-devel] [PATCH 1/4] usb-desc: fix user trigerrable segfaults (!config)

[Alon Levy]

Check for dev->config being NULL in two places:
 USB_REQ_GET_CONFIGURATION and USB_REQ_GET_STATUS.

The behavior of USB_REQ_GET_STATUS is unspecified in the Default state,
that corresponds to dev->config being NULL (it defaults to NULL and is
reset whenever a SET_CONFIGURATION with value 0, or attachment). I
implemented it to correspond with the state before
ed5a83ddd8c1d8ec7b1015315530cf29949e7c48, the commit moving SET_STATUS
to usb-desc; if dev->config is not set we return whatever is in the
first configuration.

The behavior of USB_REQ_GET_CONFIGURATION is also undefined before any
SET_CONFIGURATION, but here we just return 0 (same as specified for the
Address state).

A win7 guest failed to initialize the device before this patch,
segfaulting when GET_STATUS was called with dev->config == NULL. With
this patch the passthrough device still doesn't work but the failure is
unrelated.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/usb-desc.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/usb-desc.c b/hw/usb-desc.c
index 3c3ed6a..ccf85ad 100644
--- a/hw/usb-desc.c
+++ b/hw/usb-desc.c
@@ -536,7 +536,11 @@ int usb_desc_handle_control(USBDevice *dev, USBPacket *p,
         break;
 
     case DeviceRequest | USB_REQ_GET_CONFIGURATION:
-        data[0] = dev->config->bConfigurationValue;
+        /*
+         * 9.4.2: 0 should be returned if the device is unconfigured, otherwise
+         * the non zero value of bConfigurationValue.
+         */
+        data[0] = dev->config ? dev->config->bConfigurationValue : 0;
         ret = 1;
         break;
     case DeviceOutRequest | USB_REQ_SET_CONFIGURATION:
@@ -544,9 +548,18 @@ int usb_desc_handle_control(USBDevice *dev, USBPacket *p,
         trace_usb_set_config(dev->addr, value, ret);
         break;
 
-    case DeviceRequest | USB_REQ_GET_STATUS:
+    case DeviceRequest | USB_REQ_GET_STATUS: {
+        const USBDescConfig *config = dev->config ?
+            dev->config : &dev->device->confs[0];
+
         data[0] = 0;
-        if (dev->config->bmAttributes & 0x40) {
+        /*
+         * Default state: Device behavior when this request is received while
+         *                the device is in the Default state is not specified.
+         * We return the same value that a configured device would return if
+         * it used the first configuration.
+         */
+        if (config->bmAttributes & 0x40) {
             data[0] |= 1 << USB_DEVICE_SELF_POWERED;
         }
         if (dev->remote_wakeup) {
@@ -555,6 +568,7 @@ int usb_desc_handle_control(USBDevice *dev, USBPacket *p,
         data[1] = 0x00;
         ret = 2;
         break;
+    }
     case DeviceOutRequest | USB_REQ_CLEAR_FEATURE:
         if (value == USB_DEVICE_REMOTE_WAKEUP) {
             dev->remote_wakeup = 0;
-- 
1.7.9.1

[Qemu-devel] [PATCH 2/4] libcacard: link with glib for g_strndup

[Alon Levy]

Without it the produced library for make libcacard.la has an unresolved
symbol.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 configure |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 9535f66..88c5fd9 100755
--- a/configure
+++ b/configure
@@ -2571,8 +2571,8 @@ if test "$smartcard" != "no" ; then
 int main(void) { PK11_FreeSlot(0); return 0; }
 EOF
         smartcard_cflags="-I\$(SRC_PATH)/libcacard"
-        libcacard_libs=$($pkg_config --libs nss 2>/dev/null)
-        libcacard_cflags=$($pkg_config --cflags nss 2>/dev/null)
+        libcacard_libs="$($pkg_config --libs nss 2>/dev/null) $glib_libs"
+        libcacard_cflags="$($pkg_config --cflags nss 2>/dev/null) $glib_cflags"
         if $pkg_config --atleast-version=3.12.8 nss >/dev/null 2>&1 && \
           compile_prog "$smartcard_cflags $libcacard_cflags" "$libcacard_libs"; then
             smartcard_nss="yes"
-- 
1.7.9.1

[Qemu-devel] [PATCH 3/4] usb-ccid: advertise SELF_POWERED

[Alon Levy]

Before commit ed5a83ddd8c1d8ec7b1015315530cf29949e7c48 each device
provided it's own response to USB_REQ_GET_STATUS, but after it that
response was based on bmAttributes, which was errounously set for
usb-ccid as 0xa0 and not 0xe0.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/usb-ccid.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/usb-ccid.c b/hw/usb-ccid.c
index 0b2ac80..ce01e34 100644
--- a/hw/usb-ccid.c
+++ b/hw/usb-ccid.c
@@ -447,7 +447,7 @@ static const USBDescDevice desc_device = {
         {
             .bNumInterfaces        = 1,
             .bConfigurationValue   = 1,
-            .bmAttributes          = 0xa0,
+            .bmAttributes          = 0xe0,
             .bMaxPower             = 50,
             .nif = 1,
             .ifs = &desc_iface0,
-- 
1.7.9.1

[Qemu-devel] [PATCH 4/4] libcacard: fix reported ATR length

[Alon Levy]

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 libcacard/vcardt.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcacard/vcardt.h b/libcacard/vcardt.h
index 538bdde..d4d8e2e 100644
--- a/libcacard/vcardt.h
+++ b/libcacard/vcardt.h
@@ -26,8 +26,8 @@ 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 VCARD_ATR_PREFIX(size) (0x3b, 0x68+(size), 0x00, 0xff, \
+                               'V', 'C', 'A', 'R', 'D', '_')
 
 
 typedef enum {
-- 
1.7.9.1

Re: [Qemu-devel] [PATCH 1/4] usb-desc: fix user trigerrable segfaults (!config)

[Alon Levy]

On Sun, Feb 26, 2012 at 05:09:21PM +0100, Alon Levy wrote:
> Check for dev->config being NULL in two places:
>  USB_REQ_GET_CONFIGURATION and USB_REQ_GET_STATUS.
> 
> The behavior of USB_REQ_GET_STATUS is unspecified in the Default state,
> that corresponds to dev->config being NULL (it defaults to NULL and is
> reset whenever a SET_CONFIGURATION with value 0, or attachment). I
> implemented it to correspond with the state before
> ed5a83ddd8c1d8ec7b1015315530cf29949e7c48, the commit moving SET_STATUS
> to usb-desc; if dev->config is not set we return whatever is in the
> first configuration.
> 
> The behavior of USB_REQ_GET_CONFIGURATION is also undefined before any
> SET_CONFIGURATION, but here we just return 0 (same as specified for the
> Address state).
> 
> A win7 guest failed to initialize the device before this patch,

s/the device/a usb-ccid device/

Using the default win7 smartcard driver.

> segfaulting when GET_STATUS was called with dev->config == NULL. With
> this patch the passthrough device still doesn't work but the failure is
> unrelated.
> 
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
>  hw/usb-desc.c |   20 +++++++++++++++++---
>  1 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/usb-desc.c b/hw/usb-desc.c
> index 3c3ed6a..ccf85ad 100644
> --- a/hw/usb-desc.c
> +++ b/hw/usb-desc.c
> @@ -536,7 +536,11 @@ int usb_desc_handle_control(USBDevice *dev, USBPacket *p,
>          break;
>  
>      case DeviceRequest | USB_REQ_GET_CONFIGURATION:
> -        data[0] = dev->config->bConfigurationValue;
> +        /*
> +         * 9.4.2: 0 should be returned if the device is unconfigured, otherwise
> +         * the non zero value of bConfigurationValue.
> +         */
> +        data[0] = dev->config ? dev->config->bConfigurationValue : 0;
>          ret = 1;
>          break;
>      case DeviceOutRequest | USB_REQ_SET_CONFIGURATION:
> @@ -544,9 +548,18 @@ int usb_desc_handle_control(USBDevice *dev, USBPacket *p,
>          trace_usb_set_config(dev->addr, value, ret);
>          break;
>  
> -    case DeviceRequest | USB_REQ_GET_STATUS:
> +    case DeviceRequest | USB_REQ_GET_STATUS: {
> +        const USBDescConfig *config = dev->config ?
> +            dev->config : &dev->device->confs[0];
> +
>          data[0] = 0;
> -        if (dev->config->bmAttributes & 0x40) {
> +        /*
> +         * Default state: Device behavior when this request is received while
> +         *                the device is in the Default state is not specified.
> +         * We return the same value that a configured device would return if
> +         * it used the first configuration.
> +         */
> +        if (config->bmAttributes & 0x40) {
>              data[0] |= 1 << USB_DEVICE_SELF_POWERED;
>          }
>          if (dev->remote_wakeup) {
> @@ -555,6 +568,7 @@ int usb_desc_handle_control(USBDevice *dev, USBPacket *p,
>          data[1] = 0x00;
>          ret = 2;
>          break;
> +    }
>      case DeviceOutRequest | USB_REQ_CLEAR_FEATURE:
>          if (value == USB_DEVICE_REMOTE_WAKEUP) {
>              dev->remote_wakeup = 0;
> -- 
> 1.7.9.1
> 
> 

Re: [Qemu-devel] [PATCH 1/4] usb-desc: fix user trigerrable segfaults (!config)

[Alon Levy]

On Sun, Feb 26, 2012 at 05:09:21PM +0100, Alon Levy wrote:
> Check for dev->config being NULL in two places:
>  USB_REQ_GET_CONFIGURATION and USB_REQ_GET_STATUS.
> 

Ping.

> The behavior of USB_REQ_GET_STATUS is unspecified in the Default state,
> that corresponds to dev->config being NULL (it defaults to NULL and is
> reset whenever a SET_CONFIGURATION with value 0, or attachment). I
> implemented it to correspond with the state before
> ed5a83ddd8c1d8ec7b1015315530cf29949e7c48, the commit moving SET_STATUS
> to usb-desc; if dev->config is not set we return whatever is in the
> first configuration.
> 
> The behavior of USB_REQ_GET_CONFIGURATION is also undefined before any
> SET_CONFIGURATION, but here we just return 0 (same as specified for the
> Address state).
> 
> A win7 guest failed to initialize the device before this patch,
> segfaulting when GET_STATUS was called with dev->config == NULL. With
> this patch the passthrough device still doesn't work but the failure is
> unrelated.
> 
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
>  hw/usb-desc.c |   20 +++++++++++++++++---
>  1 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/usb-desc.c b/hw/usb-desc.c
> index 3c3ed6a..ccf85ad 100644
> --- a/hw/usb-desc.c
> +++ b/hw/usb-desc.c
> @@ -536,7 +536,11 @@ int usb_desc_handle_control(USBDevice *dev, USBPacket *p,
>          break;
>  
>      case DeviceRequest | USB_REQ_GET_CONFIGURATION:
> -        data[0] = dev->config->bConfigurationValue;
> +        /*
> +         * 9.4.2: 0 should be returned if the device is unconfigured, otherwise
> +         * the non zero value of bConfigurationValue.
> +         */
> +        data[0] = dev->config ? dev->config->bConfigurationValue : 0;
>          ret = 1;
>          break;
>      case DeviceOutRequest | USB_REQ_SET_CONFIGURATION:
> @@ -544,9 +548,18 @@ int usb_desc_handle_control(USBDevice *dev, USBPacket *p,
>          trace_usb_set_config(dev->addr, value, ret);
>          break;
>  
> -    case DeviceRequest | USB_REQ_GET_STATUS:
> +    case DeviceRequest | USB_REQ_GET_STATUS: {
> +        const USBDescConfig *config = dev->config ?
> +            dev->config : &dev->device->confs[0];
> +
>          data[0] = 0;
> -        if (dev->config->bmAttributes & 0x40) {
> +        /*
> +         * Default state: Device behavior when this request is received while
> +         *                the device is in the Default state is not specified.
> +         * We return the same value that a configured device would return if
> +         * it used the first configuration.
> +         */
> +        if (config->bmAttributes & 0x40) {
>              data[0] |= 1 << USB_DEVICE_SELF_POWERED;
>          }
>          if (dev->remote_wakeup) {
> @@ -555,6 +568,7 @@ int usb_desc_handle_control(USBDevice *dev, USBPacket *p,
>          data[1] = 0x00;
>          ret = 2;
>          break;
> +    }
>      case DeviceOutRequest | USB_REQ_CLEAR_FEATURE:
>          if (value == USB_DEVICE_REMOTE_WAKEUP) {
>              dev->remote_wakeup = 0;
> -- 
> 1.7.9.1
> 
>