[PATCH 3/4] uefi: uefidump: add the USB WWID subtype-16 support on messaging device path type for uefidump(LP#:1255387)

[Colin Ian King]

On 28/11/13 01:49, IvanHu wrote:
> On 11/27/2013 08:46 PM, Colin Ian King wrote:
>> On 27/11/13 06:09, Ivan Hu wrote:
>>> Add the USB WWID subtype 16 support on messaging device path follow
>>> the section 9.3.5.7 on UEFI spec2.4.
>>>
>>> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
>>> ---
>>>   src/lib/include/fwts_uefi.h  |    8 ++++++++
>>>   src/uefi/uefidump/uefidump.c |   19 +++++++++++++++++++
>>>   2 files changed, 27 insertions(+)
>>>
>>> diff --git a/src/lib/include/fwts_uefi.h b/src/lib/include/fwts_uefi.h
>>> index 4d45f25..4add24e 100644
>>> --- a/src/lib/include/fwts_uefi.h
>>> +++ b/src/lib/include/fwts_uefi.h
>>> @@ -204,6 +204,7 @@ typedef enum {
>>>       FWTS_UEFI_IPV6_DEVICE_PATH_SUBTYPE =        (0x0d),
>>>       FWTS_UEFI_UART_DEVICE_PATH_SUBTYPE =        (0x0e),
>>>       FWTS_UEFI_USB_CLASS_DEVICE_PATH_SUBTYPE =    (0x0f),
>>> +    FWTS_UEFI_USB_WWID_DEVICE_PATH_SUBTYPE =    (0x10),
>>>       FWTS_UEFI_SATA_DEVICE_PATH_SUBTYPE =         (0x12),
>>>       FWTS_UEFI_FIBRE_CHANNEL_EX_DEVICE_PATH_SUBTYPE = (0x15)
>>>
>>> @@ -389,6 +390,13 @@ typedef struct {
>>>
>>>   typedef struct {
>>>       fwts_uefi_dev_path dev_path;
>>> +    uint16_t interface_num;
>>> +    uint16_t vendor_id;
>>> +    uint16_t product_id;
>>
>> I think there is a missing UTF-16 serial number that may need to be
>> considered too.
>>
> 
> Thanks,
> I directly parser the serial number on the uefidump.c instead of
> defining here, since it's variable length.

I guess I was expecting something like the following:

typedef struct {
	fwts_uefi_dev_path dev_path;
	uint16_t interface_num;
	uint16_t vendor_id;
	uint16_t product_id;
	uint16_t serial_number[0];
} fwts_uefi_usb_wwid_dev_path;


> 
>>> +} fwts_uefi_usb_wwid_dev_path;
>>> +
>>> +typedef struct {
>>> +    fwts_uefi_dev_path dev_path;
>>>       uint32_t partition_number;
>>>       uint64_t partition_start;
>>>       uint64_t partition_size;
>>> diff --git a/src/uefi/uefidump/uefidump.c b/src/uefi/uefidump/uefidump.c
>>> index 9dd209e..542df11 100644
>>> --- a/src/uefi/uefidump/uefidump.c
>>> +++ b/src/uefi/uefidump/uefidump.c
>>> @@ -349,6 +349,25 @@ static char *uefidump_build_dev_path(char *path,
>>> fwts_uefi_dev_path *dev_path, c
>>>                       s->hbapn, s->pmpn, s->lun);
>>>               }
>>>               break;
>>> +        case FWTS_UEFI_USB_WWID_DEVICE_PATH_SUBTYPE:
>>> +            if (dev_path_len >= sizeof(fwts_uefi_usb_wwid_dev_path)) {
>>> +                fwts_uefi_usb_wwid_dev_path *u =
>>> (fwts_uefi_usb_wwid_dev_path *)dev_path;
>>> +                path = uefidump_vprintf(path, "\\USBWWID(0x%" PRIx16
>>> ",0x%" PRIx16 ",0x%" PRIx16,
>>> +                    u->interface_num, u->vendor_id, u->product_id);
>>> +
>>> +                /* Adding Serial Number */
>>> +                char tmp[1024];

Is 1024 the max size of the serial number? Could it be larger? I just
don't want to get caught out with a buffer overflow.

>>> +                uint8_t *adr_sn = (uint8_t *)u +
>>> sizeof(fwts_uefi_usb_wwid_dev_path);

	remove adr_sn

>>> +                uint16_t len = u->dev_path.length[0] |
>>> (((uint16_t)u->dev_path.length[1]) << 8);
>>> +                if (len <= sizeof(fwts_uefi_usb_wwid_dev_path)) {
>>> +                    path = uefidump_vprintf(path, ")");
>>> +                    break;
>>> +                }
>>> +                fwts_uefi_str16_to_str(tmp, (len -
>>> sizeof(fwts_uefi_usb_wwid_dev_path))/sizeof(uint16_t), (uint16_t
>>> *)adr_sn);

		fwts_uefi_str16_to_str(tmp, (len -
sizeof(fwts_uefi_usb_wwid_dev_path))/sizeof(uint16_t), u->serial_number);

>>> +
>>> +                path = uefidump_vprintf(path, ",%s)", tmp);
>>> +            }
>>> +            break;
>>>           default:
>>>               path = uefidump_vprintf(path,
>>> "\\Unknown-MESSAGING-DEV-PATH(0x%" PRIx8 ")", dev_path->subtype);
>>>               break;
>>>
>>
>>
> 
>