Re: [PATCH v11 17/18] HID: asus: report the XG Mobile GPU vendor to asus-wmi

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] HID: asus: u8 gpu_id[4] truncates 16-bit PCI vendor ID in asus_xgm_init()
--

commit 739505f96f6292806a14e712690bf33782dd803a
Author: Denis Benato <denis.benato@linux.dev>

HID: asus: report the XG Mobile GPU vendor to asus-wmi

This commit parses the 4-part GPU PCI ID from the XG Mobile station version
query response and reports the GPU vendor ID to asus-wmi on initialization,
clearing it when the station is unbound.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index e444cad85b32c..244d103fbd4c8 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -186,6 +186,7 @@ struct asus_xgm_info {
>  	char part_number[ASUS_XGM_INFO_LEN];
>  	char firmware_version[ASUS_XGM_INFO_LEN];
>  	char generation[ASUS_XGM_INFO_LEN];
> +	u8 gpu_id[4];
>  };
>  
>  struct asus_touchpad_info {
> @@ -6313,6 +6314,12 @@ static int asus_xgm_get_version(struct hid_device *hdev,
>  	asus_xgm_tlv_str(info->firmware_version, sizeof(info->firmware_version),
>  			 buf, len, "VER");
>  
> +	/* the 4-part PCI ID of the GPU installed in the station: buf[3..10] */
> +	if (len < 11)
> +		return -ENODATA;
> +
> +	memcpy(info->gpu_id, buf + 3, sizeof(info->gpu_id));
> +
>  	return 0;
>  }
>  
> @@ -6685,6 +6692,8 @@ static int asus_xgm_init(struct hid_device *hdev, struct asus_drvdata *drvdata)
>  		hid_info(hdev, "XG Mobile: firmware %s, generation %s\n",
>  			 info->firmware_version, info->generation);
>  
> +	asus_wmi_egpu_set_vendor(info->gpu_id[0]);

[Severity: High]
Does declaring gpu_id as u8 gpu_id[4] in struct asus_xgm_info truncate the
16-bit PCI vendor ID to a single byte?

In asus_xgm_get_version(), the comment and length check expect the 4-part
PCI ID to occupy 8 bytes in buf[3..10] (four 16-bit fields), but
sizeof(info->gpu_id) is 4, so memcpy() only copies buf[3..6].

When asus_probe() -> asus_xgm_init() calls
asus_wmi_egpu_set_vendor(info->gpu_id[0]), it passes a single u8 value
(0x00..0xff) to asus_wmi_egpu_set_vendor(u16 vendor). Downstream in
drivers/platform/x86/asus-armoury.c:egpu_default_enable_value():

static u32 egpu_default_enable_value(void)
{
	switch (asus_wmi_egpu_vendor()) {
	case PCI_VENDOR_ID_ATI:
		return 0x0101;
	case PCI_VENDOR_ID_INTEL:
		return 0x0201;
	default:
		return 0x0001;
	}
}

the 8-bit value can never match the 16-bit constants PCI_VENDOR_ID_ATI
(0x1002) or PCI_VENDOR_ID_INTEL (0x8086), causing egpu_enable to always fall
back to 0x0001.

Should gpu_id be a 4-element 16-bit array (such as u16 gpu_id[4] or __le16
gpu_id[4] with the appropriate endianness conversion) so the full 8-byte
PCI ID is stored and the 16-bit vendor ID is passed to
asus_wmi_egpu_set_vendor()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025235232.129945-1-denis.benato@linux.dev?part=17