Re: [RFC PATCH 2/2] hw/usb/dev-uas: Report command additional adb length as unsupported

[Philippe Mathieu-Daudé]

On 1/18/21 6:03 PM, Philippe Mathieu-Daudé wrote:
> We are not ready to handle additional CDB data.
> 
> If a guest send a packet with such additional data,
> report the command parameter as not supported.
> 
> We can then explicit there is nothing in this additional
> buffer, by fixing its size to zero.
> 
> This fixes an error when building with Clang 11:
> 
>   usb/dev-uas.c:157:31: error: field 'status' with variable sized type 
> 'uas_iu' not at the end of a struct or class is a GNU extension 
> [-Werror,-Wgnu-variable-sized-type-not-at-end]
>       uas_iu                    status;
>                                 ^
> 
> Reported-by: Daniele Buono <dbuono@linux.vnet.ibm.com>

TBH this should be (chronological order):
Reported-by: Ed Maste <emaste@FreeBSD.org>
Reported-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
Reported-by: Han Han <hhan@redhat.com>

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Cc: Ed Maste <emaste@FreeBSD.org>
> Cc: Han Han <hhan@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  hw/usb/dev-uas.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
> index cec071d96c4..b6434ad4b9c 100644
> --- a/hw/usb/dev-uas.c
> +++ b/hw/usb/dev-uas.c
> @@ -16,6 +16,7 @@
>  #include "qemu/error-report.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/module.h"
> +#include "qemu/log.h"
>  
>  #include "hw/usb.h"
>  #include "migration/vmstate.h"
> @@ -70,7 +71,7 @@ typedef struct {
>      uint8_t    reserved_2;
>      uint64_t   lun;
>      uint8_t    cdb[16];
> -    uint8_t    add_cdb[];
> +    uint8_t    add_cdb[0];      /* not supported by QEMU */
>  } QEMU_PACKED  uas_iu_command;
>  
>  typedef struct {
> @@ -700,6 +701,11 @@ static void usb_uas_command(UASDevice *uas, uas_iu *iu)
>      uint32_t len;
>      uint16_t tag = be16_to_cpu(iu->hdr.tag);
>  
> +    if (iu->command.add_cdb_length > 0) {
> +        qemu_log_mask(LOG_UNIMP, "additional adb length not yet 
> supported\n");
> +        goto unsupported_len;
> +    }
> +
>      if (uas_using_streams(uas) && tag > UAS_MAX_STREAMS) {
>          goto invalid_tag;
>      }
> @@ -735,6 +741,10 @@ static void usb_uas_command(UASDevice *uas, uas_iu *iu)
>      }
>      return;
>  
> +unsupported_len:
> +    usb_uas_queue_fake_sense(uas, tag, sense_code_INVALID_PARAM_VALUE);
> +    return;
> +
>  invalid_tag:
>      usb_uas_queue_fake_sense(uas, tag, sense_code_INVALID_TAG);
>      return;
>