Re: [PATCH v2 2/7] platform/x86/amd/hsmp: Validate ACPI UID and _DSD mailbox package

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3061 bytes --]

On Thu, 25 Jun 2026, Muralidhara M K wrote:

> Harden the ACPI table parsing against malformed firmware:
> 
> hsmp_get_uid() passed the device UID directly to kstrtou16(uid + 2)
> without checking it.  A NULL UID or one shorter than three characters
> would dereference a NULL pointer or read past the end of the string.
> Reject such UIDs with -EINVAL before stripping the "ID" prefix.
> 
> hsmp_read_acpi_dsd() dereferenced elements[0] and elements[1] of each
> mailbox sub-package before confirming the package actually held two
> elements, allowing an out-of-bounds read on a malformed _DSD.  Verify
> package.count >= 2 first, then fetch the string and integer objects.

These look two paragraphs describe what looks totally independent changes 
so they should be split to two patches, also their diffs don't overlap. 
Once the split is done,

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

I see you went beyond just concurrency fixes, thanks. :-)

I'll try to look at the locking changes later (that is, there's no point 
in resubmitting the next version yet simply because of the patch split 
if you don't have any other changes to make; I haven't checked sashiko's 
analysis yet).

The locking changes are considerable harder to review so it will take some 
time for me to go through them.

--
 i.

> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
> ---
>  drivers/platform/x86/amd/hsmp/acpi.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
> index 696884a91c22..24296747df47 100644
> --- a/drivers/platform/x86/amd/hsmp/acpi.c
> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
> @@ -24,6 +24,7 @@
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/platform_device.h>
> +#include <linux/string.h>
>  #include <linux/sysfs.h>
>  #include <linux/topology.h>
>  #include <linux/uuid.h>
> @@ -82,6 +83,8 @@ static inline int hsmp_get_uid(struct device *dev, u16 *sock_ind)
>  	 * bytes to integer.
>  	 */
>  	uid = acpi_device_uid(ACPI_COMPANION(dev));
> +	if (!uid || strlen(uid) < 3)
> +		return -EINVAL;
>  
>  	return kstrtou16(uid + 2, 10, sock_ind);
>  }
> @@ -153,12 +156,18 @@ static int hsmp_read_acpi_dsd(struct hsmp_socket *sock)
>  		union acpi_object *msgobj, *msgstr, *msgint;
>  
>  		msgobj	= &mailbox_package->package.elements[j];
> -		msgstr	= &msgobj->package.elements[0];
> -		msgint	= &msgobj->package.elements[1];
>  
>  		/* package should have 1 string and 1 integer object */
>  		if (msgobj->type != ACPI_TYPE_PACKAGE ||
> -		    msgstr->type != ACPI_TYPE_STRING ||
> +		    msgobj->package.count < 2) {
> +			ret = -EINVAL;
> +			goto free_buf;
> +		}
> +
> +		msgstr	= &msgobj->package.elements[0];
> +		msgint	= &msgobj->package.elements[1];
> +
> +		if (msgstr->type != ACPI_TYPE_STRING ||
>  		    msgint->type != ACPI_TYPE_INTEGER) {
>  			ret = -EINVAL;
>  			goto free_buf;
>