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

login
register
mail settings
Submitter Ivan Hu
Date Nov. 27, 2013, 6:09 a.m.
Message ID <1385532572-30954-1-git-send-email-ivan.hu@canonical.com>
Download mbox | patch
Permalink /patch/294494/
State Rejected
Headers show

Comments

Ivan Hu - Nov. 27, 2013, 6:09 a.m.
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@canonical.com>
---
 src/lib/include/fwts_uefi.h  |    8 ++++++++
 src/uefi/uefidump/uefidump.c |   19 +++++++++++++++++++
 2 files changed, 27 insertions(+)
Colin King - Nov. 27, 2013, 12:46 p.m.
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@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.

> +} 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];
> +				uint8_t *adr_sn = (uint8_t *)u + sizeof(fwts_uefi_usb_wwid_dev_path);
> +				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);
> +
> +				path = uefidump_vprintf(path, ",%s)", tmp);
> +			}
> +			break;
>  		default:
>  			path = uefidump_vprintf(path, "\\Unknown-MESSAGING-DEV-PATH(0x%" PRIx8 ")", dev_path->subtype);
>  			break;
>
Ivan Hu - Nov. 28, 2013, 1:49 a.m.
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@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.

>> +} 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];
>> +				uint8_t *adr_sn = (uint8_t *)u + sizeof(fwts_uefi_usb_wwid_dev_path);
>> +				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);
>> +
>> +				path = uefidump_vprintf(path, ",%s)", tmp);
>> +			}
>> +			break;
>>   		default:
>>   			path = uefidump_vprintf(path, "\\Unknown-MESSAGING-DEV-PATH(0x%" PRIx8 ")", dev_path->subtype);
>>   			break;
>>
>
>
Colin King - Nov. 28, 2013, 9:16 a.m.
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@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;
>>>
>>
>>
> 
>

Patch

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;
+} 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];
+				uint8_t *adr_sn = (uint8_t *)u + sizeof(fwts_uefi_usb_wwid_dev_path);
+				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);
+
+				path = uefidump_vprintf(path, ",%s)", tmp);
+			}
+			break;
 		default:
 			path = uefidump_vprintf(path, "\\Unknown-MESSAGING-DEV-PATH(0x%" PRIx8 ")", dev_path->subtype);
 			break;