[PATCH v2 0/7] platform/x86/amd/hsmp: ACPI/platform HSMP concurrency and lifecycle hardening

[PATCH v2 0/7] platform/x86/amd/hsmp: ACPI/platform HSMP concurrency and lifecycle hardening

[Muralidhara M K]

This series hardens the AMD HSMP ACPI and platform drivers against
probe/remove concurrency, use-after-free of the per-socket state, and
resource leaks across unbind/rebind cycles.

On multi-socket systems several ACPI platform devices are probed and
removed independently while a single /dev/hsmp (and the hwmon sysfs
attributes) drive a lock-free data plane.  The existing code raced the
global is_probed handshake and the one-time socket allocation, published
the per-socket readiness gate before the mailbox was mapped, freed the
socket array from under in-flight hsmp_send_message() callers, and leaked
metric-table mappings on rebind.

The patches, in order:

  1. Serialize ACPI probe/remove/init_acpi() with a dedicated mutex.
  2. Validate the ACPI UID and _DSD mailbox package against malformed
     firmware before dereferencing them.
  3. Map the metric-table DRAM explicitly and add per-socket mutexes.
  4. Publish sock->dev last with smp_store_release() and read it with
     smp_load_acquire() so a message can never reach a socket still in
     bring-up.
  5. Add hsmp_sock_rwsem so the lock-free data plane is drained and kept
     out while a socket is torn down; wire the platform path into it.
  6. Refcount the ACPI socket platform devices with struct kref and run a
     coordinated release on the final put (deregister /dev/hsmp, unmap
     metric DRAM, free the socket array); clear per-socket dev under the
     teardown write lock on remove and probe failure.
  7. Drop any stale metric-table mapping before remapping on rebind.

Testing:
  - Each patch builds individually (W=1) with no new warnings; the series
    is bisectable.
  - checkpatch.pl --strict is clean on every patch.

Changes since v1:
  - Split the work into smaller, single-purpose patches for review.
  - Added ACPI UID / _DSD mailbox package validation (new patch).
  - Replaced the synchronous devm free of the socket array with a kref
    based, coordinated release and serialized it against the data plane
    via a new rwsem; clear the per-socket dev under the write lock on
    both remove and probe-failure paths.
  - Added smp_store_release()/smp_load_acquire() ordering on sock->dev so
    the readiness gate cannot be observed before the mailbox is mapped.
  - Wired the non-ACPI platform path into the same teardown drain and
    fixed the metric-table mapping leak on unbind/rebind.

v1: https://lore.kernel.org/platform-driver-x86/eb609912-1c42-6354-22e6-5ffd1f097e9f@linux.intel.com/T/#t

Muralidhara M K (7):
  platform/x86/amd/hsmp: Serialize ACPI HSMP is_probed with a probe
    mutex
  platform/x86/amd/hsmp: Validate ACPI UID and _DSD mailbox package
  platform/x86/amd/hsmp: Add explicit metric DRAM mapping and per-socket
    mutexes
  platform/x86/amd/hsmp: Gate the data plane on a fully initialized
    socket
  platform/x86/amd/hsmp: Serialize the data plane against socket
    teardown
  platform/x86/amd/hsmp: ACPI HSMP refcounted sockets and coordinated
    release
  platform/x86/amd/hsmp: Drop stale metric table mapping on rebind

 drivers/platform/x86/amd/hsmp/acpi.c | 188 +++++++++++++++++++++++----
 drivers/platform/x86/amd/hsmp/hsmp.c | 116 ++++++++++++++++-
 drivers/platform/x86/amd/hsmp/hsmp.h |  11 +-
 drivers/platform/x86/amd/hsmp/plat.c |  39 +++++-
 4 files changed, 318 insertions(+), 36 deletions(-)

-- 
2.43.0

[PATCH v2 1/7] platform/x86/amd/hsmp: Serialize ACPI HSMP is_probed with a probe mutex

[Muralidhara M K]

Add hsmp_acpi_probe_mutex and hold it across ACPI probe, remove, and
init_acpi() so concurrent platform probes cannot race the global is_probed
handshake or the one-time socket allocation.  Use lockdep_assert_held() in
init_acpi() to catch incorrect locking under lockdep.

Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
 drivers/platform/x86/amd/hsmp/acpi.c | 15 +++++++++++++++
 drivers/platform/x86/amd/hsmp/hsmp.c |  9 +++++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index 97ed71593bdf..696884a91c22 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -18,8 +18,11 @@
 #include <linux/device.h>
 #include <linux/dev_printk.h>
 #include <linux/ioport.h>
+#include <linux/cleanup.h>
+#include <linux/lockdep.h>
 #include <linux/kstrtox.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/sysfs.h>
 #include <linux/topology.h>
@@ -38,6 +41,8 @@
 
 static struct hsmp_plat_device *hsmp_pdev;
 
+static DEFINE_MUTEX(hsmp_acpi_probe_mutex);
+
 struct hsmp_sys_attr {
 	struct device_attribute dattr;
 	u32 msg_id;
@@ -459,11 +464,17 @@ static ssize_t hsmp_freq_limit_source_show(struct device *dev, struct device_att
 	return len;
 }
 
+/**
+ * init_acpi() - Parse ACPI mailbox resources for one socket and validate HSMP.
+ * @dev: ACPI companion device for this socket.
+ */
 static int init_acpi(struct device *dev)
 {
 	u16 sock_ind;
 	int ret;
 
+	lockdep_assert_held(&hsmp_acpi_probe_mutex);
+
 	ret = hsmp_get_uid(dev, &sock_ind);
 	if (ret)
 		return ret;
@@ -584,6 +595,8 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
 	if (!hsmp_pdev)
 		return -ENOMEM;
 
+	guard(mutex)(&hsmp_acpi_probe_mutex);
+
 	if (!hsmp_pdev->is_probed) {
 		hsmp_pdev->num_sockets = topology_max_packages();
 		if (!hsmp_pdev->num_sockets) {
@@ -619,6 +632,7 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
 
 static void hsmp_acpi_remove(struct platform_device *pdev)
 {
+	mutex_lock(&hsmp_acpi_probe_mutex);
 	/*
 	 * We register only one misc_device even on multi-socket system.
 	 * So, deregister should happen only once.
@@ -627,6 +641,7 @@ static void hsmp_acpi_remove(struct platform_device *pdev)
 		hsmp_misc_deregister();
 		hsmp_pdev->is_probed = false;
 	}
+	mutex_unlock(&hsmp_acpi_probe_mutex);
 }
 
 static struct platform_driver amd_hsmp_driver = {
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 6a26937fc2b5..91be0cdb6af1 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -223,6 +223,15 @@ int hsmp_send_message(struct hsmp_message *msg)
 	sock_ind = array_index_nospec(msg->sock_ind, hsmp_pdev.num_sockets);
 	sock = &hsmp_pdev.sock[sock_ind];
 
+	/*
+	 * A slot exists for every possible socket, but its state (including
+	 * hsmp_sem) is only initialized once that socket has actually been
+	 * probed.  Reject messages aimed at a socket that was never brought
+	 * up so we never operate on a zero-initialized semaphore.
+	 */
+	if (!sock->dev)
+		return -ENODEV;
+
 	ret = down_interruptible(&sock->hsmp_sem);
 	if (ret < 0)
 		return ret;
-- 
2.43.0

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

[Muralidhara M K]

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.

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;
-- 
2.43.0

[PATCH v2 3/7] platform/x86/amd/hsmp: Add explicit metric DRAM mapping and per-socket mutexes

[Muralidhara M K]

Add hsmp_destroy_metric_read_locks(), hsmp_init_metric_read_locks(), and
switch metric table DRAM from devm_ioremap() to ioremap() so ACPI can
coordinate teardown.  Embed struct mutex metric_read_lock in each
hsmp_socket for later serialization of metric table reads.

Call the new helpers from the platform driver probe and remove paths
so non-ACPI builds initialize and tear down metric_read_lock consistently
with the ACPI driver.

Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
 drivers/platform/x86/amd/hsmp/hsmp.c | 38 ++++++++++++++++++++++++++--
 drivers/platform/x86/amd/hsmp/hsmp.h |  5 ++++
 drivers/platform/x86/amd/hsmp/plat.c | 16 ++++++++++--
 3 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 91be0cdb6af1..a96c59fcba0a 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -12,7 +12,11 @@
 #include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/io.h>
+#include <linux/mutex.h>
+#include <linux/nospec.h>
 #include <linux/semaphore.h>
+#include <linux/slab.h>
 #include <linux/sysfs.h>
 
 #include "hsmp.h"
@@ -410,6 +414,37 @@ ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
 }
 EXPORT_SYMBOL_NS_GPL(hsmp_metric_tbl_read, "AMD_HSMP");
 
+void hsmp_init_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets)
+{
+	u16 i;
+
+	if (!pdev->sock || !num_sockets)
+		return;
+
+	for (i = 0; i < num_sockets; i++)
+		mutex_init(&pdev->sock[i].metric_read_lock);
+}
+EXPORT_SYMBOL_NS_GPL(hsmp_init_metric_read_locks, "AMD_HSMP");
+
+void hsmp_destroy_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets)
+{
+	u16 i;
+
+	if (!pdev->sock || !num_sockets)
+		return;
+
+	for (i = 0; i < num_sockets; i++) {
+		struct hsmp_socket *s = &pdev->sock[i];
+
+		if (s->metric_tbl_addr) {
+			iounmap(s->metric_tbl_addr);
+			s->metric_tbl_addr = NULL;
+		}
+		mutex_destroy(&s->metric_read_lock);
+	}
+}
+EXPORT_SYMBOL_NS_GPL(hsmp_destroy_metric_read_locks, "AMD_HSMP");
+
 int hsmp_get_tbl_dram_base(u16 sock_ind)
 {
 	struct hsmp_socket *sock = &hsmp_pdev.sock[sock_ind];
@@ -434,8 +469,7 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
 		dev_err(sock->dev, "Invalid DRAM address for metric table\n");
 		return -ENOMEM;
 	}
-	sock->metric_tbl_addr = devm_ioremap(sock->dev, dram_addr,
-					     sizeof(struct hsmp_metric_table));
+	sock->metric_tbl_addr = ioremap(dram_addr, sizeof(struct hsmp_metric_table));
 	if (!sock->metric_tbl_addr) {
 		dev_err(sock->dev, "Failed to ioremap metric table addr\n");
 		return -ENOMEM;
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index 0509a442eaae..91bc21232646 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -15,6 +15,7 @@
 #include <linux/hwmon.h>
 #include <linux/kconfig.h>
 #include <linux/miscdevice.h>
+#include <linux/mutex.h>
 #include <linux/pci.h>
 #include <linux/semaphore.h>
 #include <linux/sysfs.h>
@@ -41,6 +42,8 @@ struct hsmp_socket {
 	struct bin_attribute hsmp_attr;
 	struct hsmp_mbaddr_info mbinfo;
 	void __iomem *metric_tbl_addr;
+	/* Protects metric table snapshot reads for this socket */
+	struct mutex metric_read_lock;
 	void __iomem *virt_base_addr;
 	struct semaphore hsmp_sem;
 	char name[HSMP_ATTR_GRP_NAME_SIZE];
@@ -63,7 +66,9 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg);
 void hsmp_misc_deregister(void);
 int hsmp_misc_register(struct device *dev);
 int hsmp_get_tbl_dram_base(u16 sock_ind);
+void hsmp_init_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets);
 ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size);
+void hsmp_destroy_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets);
 struct hsmp_plat_device *get_hsmp_pdev(void);
 #if IS_ENABLED(CONFIG_HWMON)
 int hsmp_create_sensor(struct device *dev, u16 sock_ind);
diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
index e07f68575055..685f2d2c574b 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -211,25 +211,37 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
 	if (!hsmp_pdev->sock)
 		return -ENOMEM;
 
+	hsmp_init_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
+
 	ret = init_platform_device(&pdev->dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
-		return ret;
+		goto err_destroy_locks;
 	}
 
 	ret = hsmp_misc_register(&pdev->dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register misc device\n");
-		return ret;
+		goto err_destroy_locks;
 	}
 
 	dev_dbg(&pdev->dev, "AMD HSMP is probed successfully\n");
 	return 0;
+
+err_destroy_locks:
+	/*
+	 * init_platform_device() may have ioremap()ed metric tables before
+	 * failing.  hsmp_destroy_metric_read_locks() unmaps them and tears
+	 * down the per-socket mutexes; the socket array itself is devm-managed.
+	 */
+	hsmp_destroy_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
+	return ret;
 }
 
 static void hsmp_pltdrv_remove(struct platform_device *pdev)
 {
 	hsmp_misc_deregister();
+	hsmp_destroy_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
 }
 
 static struct platform_driver amd_hsmp_driver = {
-- 
2.43.0

[PATCH v2 4/7] platform/x86/amd/hsmp: Gate the data plane on a fully initialized socket

[Muralidhara M K]

hsmp_parse_acpi_table() published sock->dev before hsmp_read_acpi_crs()
had mapped virt_base_addr.  sock->dev is the readiness gate for the
lock-free data plane, so on a multi-socket system - where socket 0
exposes /dev/hsmp before later sockets finish probing - an ioctl aimed
at a socket still in bring-up could pass the gate and dereference a NULL
virt_base_addr.

Pass the struct device explicitly to hsmp_read_acpi_crs() and
hsmp_read_acpi_dsd() instead of reading it back from sock->dev, publish
sock->dev last with smp_store_release() once virt_base_addr, the mailbox
offsets and the semaphore are initialized, and read it with
smp_load_acquire() in hsmp_send_message() so a non-NULL dev guarantees
the rest of the socket state is visible.

Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
 drivers/platform/x86/amd/hsmp/acpi.c | 38 +++++++++++++++++++---------
 drivers/platform/x86/amd/hsmp/hsmp.c | 13 ++++++----
 2 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index 24296747df47..bf5601229c6c 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -112,7 +112,7 @@ static acpi_status hsmp_resource(struct acpi_resource *res, void *data)
 	return AE_OK;
 }
 
-static int hsmp_read_acpi_dsd(struct hsmp_socket *sock)
+static int hsmp_read_acpi_dsd(struct hsmp_socket *sock, struct device *dev)
 {
 	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *guid, *mailbox_package;
@@ -121,10 +121,10 @@ static int hsmp_read_acpi_dsd(struct hsmp_socket *sock)
 	int ret = 0;
 	int j;
 
-	status = acpi_evaluate_object_typed(ACPI_HANDLE(sock->dev), "_DSD", NULL,
+	status = acpi_evaluate_object_typed(ACPI_HANDLE(dev), "_DSD", NULL,
 					    &buf, ACPI_TYPE_PACKAGE);
 	if (ACPI_FAILURE(status)) {
-		dev_err(sock->dev, "Failed to read mailbox reg offsets from DSD table, err: %s\n",
+		dev_err(dev, "Failed to read mailbox reg offsets from DSD table, err: %s\n",
 			acpi_format_exception(status));
 		return -ENODEV;
 	}
@@ -147,7 +147,7 @@ static int hsmp_read_acpi_dsd(struct hsmp_socket *sock)
 	guid = &dsd->package.elements[0];
 	mailbox_package = &dsd->package.elements[1];
 	if (!is_acpi_hsmp_uuid(guid) || mailbox_package->type != ACPI_TYPE_PACKAGE) {
-		dev_err(sock->dev, "Invalid hsmp _DSD table data\n");
+		dev_err(dev, "Invalid hsmp _DSD table data\n");
 		ret = -EINVAL;
 		goto free_buf;
 	}
@@ -197,14 +197,14 @@ static int hsmp_read_acpi_dsd(struct hsmp_socket *sock)
 	return ret;
 }
 
-static int hsmp_read_acpi_crs(struct hsmp_socket *sock)
+static int hsmp_read_acpi_crs(struct hsmp_socket *sock, struct device *dev)
 {
 	acpi_status status;
 
-	status = acpi_walk_resources(ACPI_HANDLE(sock->dev), METHOD_NAME__CRS,
+	status = acpi_walk_resources(ACPI_HANDLE(dev), METHOD_NAME__CRS,
 				     hsmp_resource, sock);
 	if (ACPI_FAILURE(status)) {
-		dev_err(sock->dev, "Failed to look up MP1 base address from CRS method, err: %s\n",
+		dev_err(dev, "Failed to look up MP1 base address from CRS method, err: %s\n",
 			acpi_format_exception(status));
 		return -EINVAL;
 	}
@@ -212,10 +212,10 @@ static int hsmp_read_acpi_crs(struct hsmp_socket *sock)
 		return -EINVAL;
 
 	/* The mapped region should be un-cached */
-	sock->virt_base_addr = devm_ioremap_uc(sock->dev, sock->mbinfo.base_addr,
+	sock->virt_base_addr = devm_ioremap_uc(dev, sock->mbinfo.base_addr,
 					       sock->mbinfo.size);
 	if (!sock->virt_base_addr) {
-		dev_err(sock->dev, "Failed to ioremap MP1 base address\n");
+		dev_err(dev, "Failed to ioremap MP1 base address\n");
 		return -ENOMEM;
 	}
 
@@ -229,7 +229,6 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
 	int ret;
 
 	sock->sock_ind		= sock_ind;
-	sock->dev		= dev;
 	sock->amd_hsmp_rdwr	= amd_hsmp_acpi_rdwr;
 
 	sema_init(&sock->hsmp_sem, 1);
@@ -237,12 +236,27 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
 	dev_set_drvdata(dev, sock);
 
 	/* Read MP1 base address from CRS method */
-	ret = hsmp_read_acpi_crs(sock);
+	ret = hsmp_read_acpi_crs(sock, dev);
 	if (ret)
 		return ret;
 
 	/* Read mailbox offsets from DSD table */
-	return hsmp_read_acpi_dsd(sock);
+	ret = hsmp_read_acpi_dsd(sock, dev);
+	if (ret)
+		return ret;
+
+	/*
+	 * Publish sock->dev last.  hsmp_send_message() uses it (via
+	 * smp_load_acquire()) as the readiness gate for the lock-free data
+	 * plane, so it must become visible only after virt_base_addr, the
+	 * mailbox offsets and the semaphore are fully initialized.  On a
+	 * multi-socket system socket 0 exposes /dev/hsmp before later sockets
+	 * finish probing, so without this an ioctl aimed at a socket still in
+	 * bring-up could pass the gate and dereference a NULL virt_base_addr.
+	 */
+	smp_store_release(&sock->dev, dev);
+
+	return 0;
 }
 
 static ssize_t hsmp_metric_tbl_acpi_read(struct file *filp, struct kobject *kobj,
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index a96c59fcba0a..c3939908d95f 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -228,12 +228,15 @@ int hsmp_send_message(struct hsmp_message *msg)
 	sock = &hsmp_pdev.sock[sock_ind];
 
 	/*
-	 * A slot exists for every possible socket, but its state (including
-	 * hsmp_sem) is only initialized once that socket has actually been
-	 * probed.  Reject messages aimed at a socket that was never brought
-	 * up so we never operate on a zero-initialized semaphore.
+	 * A slot exists for every possible socket, but it is only usable
+	 * between a successful probe and the matching remove.  Reject messages
+	 * aimed at a socket that was never brought up, is still in bring-up, or
+	 * has already been removed, so we never operate on a zero-initialized
+	 * semaphore or an unmapped mailbox.  A non-NULL dev also guarantees
+	 * virt_base_addr, the mailbox offsets and the semaphore are visible.
 	 */
-	if (!sock->dev)
+	/* Pairs with smp_store_release(&sock->dev) in hsmp_parse_acpi_table(). */
+	if (!smp_load_acquire(&sock->dev))
 		return -ENODEV;
 
 	ret = down_interruptible(&sock->hsmp_sem);
-- 
2.43.0

[PATCH v2 5/7] platform/x86/amd/hsmp: Serialize the data plane against socket teardown

[Muralidhara M K]

The HSMP data plane is lock-free: open /dev/hsmp fds and hwmon sysfs
reads call hsmp_send_message() without any coordination with driver
teardown.  misc_deregister() does not drain already-open fds, so an
in-flight message can race a concurrent unbind and touch a freed socket
array or an unmapped mailbox.

Add hsmp_sock_rwsem.  hsmp_send_message() now holds it for read across
the whole bounds-check + MMIO access, and hsmp_sock_teardown_lock()/
hsmp_sock_teardown_unlock() let a teardown path hold it for write to
drain any in-flight message and keep new ones out while it tears the
socket down.

Wire the non-ACPI platform path into the drain: hsmp_pltdrv_remove()
and the probe-failure cleanup take the write lock and drop the global
socket pointer before devres frees the devm_kcalloc() array.  num_sockets
is left intact so a later rebind can re-create the array.

Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
 drivers/platform/x86/amd/hsmp/hsmp.c | 55 +++++++++++++++++++++++++---
 drivers/platform/x86/amd/hsmp/hsmp.h |  2 +
 drivers/platform/x86/amd/hsmp/plat.c | 23 ++++++++++++
 3 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index c3939908d95f..c15acba241c4 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -15,6 +15,7 @@
 #include <linux/io.h>
 #include <linux/mutex.h>
 #include <linux/nospec.h>
+#include <linux/rwsem.h>
 #include <linux/semaphore.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
@@ -44,6 +45,16 @@
 
 static struct hsmp_plat_device hsmp_pdev;
 
+/*
+ * Serializes the lock-free data plane (hsmp_send_message() and the per-socket
+ * MMIO access it performs) against socket teardown.  Callers of the data plane
+ * hold it for read so multiple sockets can be driven concurrently; ACPI
+ * removal holds it for write while it clears sock->dev, frees the socket array
+ * and unmaps the mailbox, so a reader can never observe a half-torn-down or
+ * freed socket.
+ */
+static DECLARE_RWSEM(hsmp_sock_rwsem);
+
 /*
  * Send a message to the HSMP port via PCI-e config space registers
  * or by writing to MMIO space.
@@ -215,8 +226,19 @@ int hsmp_send_message(struct hsmp_message *msg)
 	if (ret)
 		return ret;
 
-	if (!hsmp_pdev.sock || msg->sock_ind >= hsmp_pdev.num_sockets)
-		return -ENODEV;
+	/*
+	 * Hold the teardown rwsem for read across the whole MMIO access.  ACPI
+	 * removal takes it for write before clearing sock->dev, freeing the
+	 * socket array and unmapping the mailbox, so the lock-free data plane
+	 * (open /dev/hsmp fds and hwmon sysfs reads) can never dereference a
+	 * freed socket or touch an unmapped mailbox.
+	 */
+	down_read(&hsmp_sock_rwsem);
+
+	if (!hsmp_pdev.sock || msg->sock_ind >= hsmp_pdev.num_sockets) {
+		ret = -ENODEV;
+		goto out_unlock;
+	}
 
 	/*
 	 * Sanitize sock_ind after the bounds check.  A mispredicted branch can
@@ -235,18 +257,22 @@ int hsmp_send_message(struct hsmp_message *msg)
 	 * semaphore or an unmapped mailbox.  A non-NULL dev also guarantees
 	 * virt_base_addr, the mailbox offsets and the semaphore are visible.
 	 */
-	/* Pairs with smp_store_release(&sock->dev) in hsmp_parse_acpi_table(). */
-	if (!smp_load_acquire(&sock->dev))
-		return -ENODEV;
+	/* Held under hsmp_sock_rwsem; pairs with smp_store_release(&sock->dev). */
+	if (!smp_load_acquire(&sock->dev)) {
+		ret = -ENODEV;
+		goto out_unlock;
+	}
 
 	ret = down_interruptible(&sock->hsmp_sem);
 	if (ret < 0)
-		return ret;
+		goto out_unlock;
 
 	ret = __hsmp_send_message(sock, msg);
 
 	up(&sock->hsmp_sem);
 
+out_unlock:
+	up_read(&hsmp_sock_rwsem);
 	return ret;
 }
 EXPORT_SYMBOL_NS_GPL(hsmp_send_message, "AMD_HSMP");
@@ -529,6 +555,23 @@ struct hsmp_plat_device *get_hsmp_pdev(void)
 }
 EXPORT_SYMBOL_NS_GPL(get_hsmp_pdev, "AMD_HSMP");
 
+/*
+ * Take the write side of the data-plane rwsem.  A caller tearing a socket down
+ * uses this to drain any in-flight hsmp_send_message() and to keep new ones out
+ * while it clears sock->dev, frees the socket array or unmaps the mailbox.
+ */
+void hsmp_sock_teardown_lock(void)
+{
+	down_write(&hsmp_sock_rwsem);
+}
+EXPORT_SYMBOL_NS_GPL(hsmp_sock_teardown_lock, "AMD_HSMP");
+
+void hsmp_sock_teardown_unlock(void)
+{
+	up_write(&hsmp_sock_rwsem);
+}
+EXPORT_SYMBOL_NS_GPL(hsmp_sock_teardown_unlock, "AMD_HSMP");
+
 MODULE_DESCRIPTION("AMD HSMP Common driver");
 MODULE_VERSION(DRIVER_VERSION);
 MODULE_LICENSE("GPL");
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index 91bc21232646..5d0a6d819865 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -70,6 +70,8 @@ void hsmp_init_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets)
 ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size);
 void hsmp_destroy_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets);
 struct hsmp_plat_device *get_hsmp_pdev(void);
+void hsmp_sock_teardown_lock(void);
+void hsmp_sock_teardown_unlock(void);
 #if IS_ENABLED(CONFIG_HWMON)
 int hsmp_create_sensor(struct device *dev, u16 sock_ind);
 #else
diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
index 685f2d2c574b..26c1363e79a1 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -233,15 +233,38 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
 	 * init_platform_device() may have ioremap()ed metric tables before
 	 * failing.  hsmp_destroy_metric_read_locks() unmaps them and tears
 	 * down the per-socket mutexes; the socket array itself is devm-managed.
+	 *
+	 * init_platform_device() also runs the data plane (hsmp_test()), so
+	 * drain it via the teardown rwsem and drop the global socket pointer
+	 * before devres frees the array.  num_sockets is left intact: it is
+	 * only computed once in __init (amd_num_nodes()) and is needed to
+	 * re-create the array on a later rebind.
 	 */
+	hsmp_sock_teardown_lock();
 	hsmp_destroy_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
+	hsmp_pdev->sock = NULL;
+	hsmp_sock_teardown_unlock();
 	return ret;
 }
 
 static void hsmp_pltdrv_remove(struct platform_device *pdev)
 {
+	/*
+	 * Drain the lock-free data plane and keep it out while the sockets are
+	 * torn down.  misc_deregister() does not drain already-open /dev/hsmp
+	 * fds and the driver permits sysfs unbind, so without this an in-flight
+	 * hsmp_send_message() could touch the devres-freed socket array or an
+	 * iounmap()ed metric table.  Dropping the global socket pointer makes
+	 * later messages bail out at the first check.  num_sockets is left
+	 * intact so an unbind/rebind cycle can re-create the array; it is only
+	 * computed once in __init (amd_num_nodes()) and is never recomputed on
+	 * probe.
+	 */
+	hsmp_sock_teardown_lock();
 	hsmp_misc_deregister();
 	hsmp_destroy_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
+	hsmp_pdev->sock = NULL;
+	hsmp_sock_teardown_unlock();
 }
 
 static struct platform_driver amd_hsmp_driver = {
-- 
2.43.0

[PATCH v2 6/7] platform/x86/amd/hsmp: ACPI HSMP refcounted sockets and coordinated release

[Muralidhara M K]

Replace the global is_probed flag with miscdevice.this_device for misc
registration state, count ACPI socket platform devices with struct kref,
and run hsmp_acpi_sock_release() on the final put to deregister /dev/hsmp
when needed, unmap metric DRAM, and free the socket array.  Extend struct
hsmp_plat_device with acpi_sock_kref and acpi_sock_kref_started so ACPI
teardown can coordinate with the refcount, and clear mdev.this_device in
hsmp_misc_deregister() so a later re-probe does not skip registration on
a stale pointer.

Switch ACPI socket storage to kcalloc(), initialize the per-socket metric
mutexes once the array exists, and free the allocation on early probe
failures before any kref reference is handed out.

Both teardown paths run under the data-plane rwsem via
hsmp_sock_teardown_lock(), so they are serialized against the lock-free
data plane.  hsmp_acpi_remove() clears this socket's dev under the write
lock before devres unmaps the mailbox, so a message issued after a
non-final unbind cannot reach an unmapped mailbox on multi-socket systems.

The probe-failure path clears the socket's dev under the same write lock
too.  On a multi-socket system a non-first socket can fail inside
init_acpi() after hsmp_parse_acpi_table() has published sock->dev and
mapped the mailbox; remove() is not called for a failed probe and the
array stays alive (owned by an already-probed socket), so without this
clear devres would unmap the mailbox while a /dev/hsmp ioctl to that
index still reaches it.

Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
 drivers/platform/x86/amd/hsmp/acpi.c | 124 +++++++++++++++++++++++----
 drivers/platform/x86/amd/hsmp/hsmp.c |   6 ++
 drivers/platform/x86/amd/hsmp/hsmp.h |   4 +-
 3 files changed, 118 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index bf5601229c6c..475f0076d262 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -610,6 +610,72 @@ static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
 };
 MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
 
+static void hsmp_acpi_sock_release(struct kref *kref)
+{
+	struct hsmp_plat_device *pdev = container_of(kref, struct hsmp_plat_device,
+						     acpi_sock_kref);
+
+	/*
+	 * The caller (hsmp_acpi_remove()) drops the last reference while
+	 * holding hsmp_acpi_probe_mutex, so the get/put and the teardown done
+	 * here are fully serialized against a concurrent probe.  It also holds
+	 * the write side of the data-plane rwsem (hsmp_sock_teardown_lock()),
+	 * which has drained any in-flight hsmp_send_message() and keeps new
+	 * ones out, so unmapping the mailbox and freeing the socket array here
+	 * cannot race the lock-free data plane.
+	 */
+	lockdep_assert_held(&hsmp_acpi_probe_mutex);
+
+	if (!IS_ERR_OR_NULL(pdev->mdev.this_device))
+		hsmp_misc_deregister();
+	hsmp_destroy_metric_read_locks(pdev, pdev->num_sockets);
+	kfree(pdev->sock);
+	pdev->sock = NULL;
+	pdev->num_sockets = 0;
+	pdev->proto_ver = 0;
+	pdev->acpi_sock_kref_started = false;
+}
+
+/**
+ * hsmp_acpi_probe_failure_cleanup() - Undo a failed ACPI socket probe.
+ * @dev: ACPI companion device whose probe failed.
+ *
+ * Runs the whole cleanup under the teardown rwsem so it is serialized against
+ * the lock-free data plane (init_acpi() runs hsmp_test() and a previously
+ * probed socket may already have exposed /dev/hsmp).
+ *
+ * Always clears this socket's dev: on a probe failure for a socket other than
+ * the first, the socket array stays alive (owned by an already-probed socket)
+ * and remove() is never called for this device, yet devres unmaps its mailbox
+ * once probe() returns.  Without clearing dev, a later message to this index
+ * would pass every gate in hsmp_send_message() and reach the unmapped mailbox.
+ *
+ * When no ACPI socket reference has been handed out via kref yet (the first
+ * socket's failure), it also frees the array and destroys the per-socket
+ * mutexes; hsmp_destroy_metric_read_locks() additionally unmaps any metric
+ * table DRAM that init_acpi() may have ioremap()ed, so there is no leak.
+ */
+static void hsmp_acpi_probe_failure_cleanup(struct device *dev)
+{
+	struct hsmp_socket *sock = dev_get_drvdata(dev);
+
+	lockdep_assert_held(&hsmp_acpi_probe_mutex);
+
+	hsmp_sock_teardown_lock();
+
+	if (sock)
+		sock->dev = NULL;
+
+	if (!hsmp_pdev->acpi_sock_kref_started && hsmp_pdev->sock) {
+		hsmp_destroy_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
+		kfree(hsmp_pdev->sock);
+		hsmp_pdev->sock = NULL;
+		hsmp_pdev->num_sockets = 0;
+	}
+
+	hsmp_sock_teardown_unlock();
+}
+
 static int hsmp_acpi_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -620,34 +686,44 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
 
 	guard(mutex)(&hsmp_acpi_probe_mutex);
 
-	if (!hsmp_pdev->is_probed) {
+	if (!hsmp_pdev->sock) {
 		hsmp_pdev->num_sockets = topology_max_packages();
 		if (!hsmp_pdev->num_sockets) {
 			dev_err(&pdev->dev, "No CPU sockets detected\n");
 			return -ENODEV;
 		}
 
-		hsmp_pdev->sock = devm_kcalloc(&pdev->dev, hsmp_pdev->num_sockets,
-					       sizeof(*hsmp_pdev->sock),
-					       GFP_KERNEL);
+		hsmp_pdev->sock = kcalloc(hsmp_pdev->num_sockets,
+					  sizeof(*hsmp_pdev->sock),
+					  GFP_KERNEL);
 		if (!hsmp_pdev->sock)
 			return -ENOMEM;
+
+		hsmp_init_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
 	}
 
 	ret = init_acpi(&pdev->dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n");
+		hsmp_acpi_probe_failure_cleanup(&pdev->dev);
 		return ret;
 	}
 
-	if (!hsmp_pdev->is_probed) {
+	if (IS_ERR_OR_NULL(hsmp_pdev->mdev.this_device)) {
 		ret = hsmp_misc_register(&pdev->dev);
 		if (ret) {
 			dev_err(&pdev->dev, "Failed to register misc device\n");
+			hsmp_acpi_probe_failure_cleanup(&pdev->dev);
 			return ret;
 		}
-		hsmp_pdev->is_probed = true;
-		dev_dbg(&pdev->dev, "AMD HSMP ACPI is probed successfully\n");
+		dev_dbg(&pdev->dev, "AMD HSMP ACPI misc device registered\n");
+	}
+
+	if (!hsmp_pdev->acpi_sock_kref_started) {
+		kref_init(&hsmp_pdev->acpi_sock_kref);
+		hsmp_pdev->acpi_sock_kref_started = true;
+	} else {
+		kref_get(&hsmp_pdev->acpi_sock_kref);
 	}
 
 	return 0;
@@ -655,16 +731,34 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
 
 static void hsmp_acpi_remove(struct platform_device *pdev)
 {
-	mutex_lock(&hsmp_acpi_probe_mutex);
+	struct hsmp_socket *sock = dev_get_drvdata(&pdev->dev);
+
 	/*
-	 * We register only one misc_device even on multi-socket system.
-	 * So, deregister should happen only once.
+	 * Serialize the final put (and the teardown it triggers) against a
+	 * concurrent probe so the refcount cannot be revived from zero.
 	 */
-	if (hsmp_pdev->is_probed) {
-		hsmp_misc_deregister();
-		hsmp_pdev->is_probed = false;
-	}
-	mutex_unlock(&hsmp_acpi_probe_mutex);
+	guard(mutex)(&hsmp_acpi_probe_mutex);
+
+	/*
+	 * Drain the lock-free data plane and keep it out for the duration of
+	 * the teardown.  This covers both the per-socket unbind (this socket's
+	 * mailbox is unmapped by devres once we return) and the final put that
+	 * frees the socket array in hsmp_acpi_sock_release().
+	 */
+	hsmp_sock_teardown_lock();
+
+	/*
+	 * Clear this socket's dev so hsmp_send_message() rejects it before
+	 * touching the mailbox that devres is about to unmap.  On a non-final
+	 * unbind the socket array stays alive, so without this a later message
+	 * to this index would reach an unmapped iomem region.
+	 */
+	if (sock)
+		sock->dev = NULL;
+
+	kref_put(&hsmp_pdev->acpi_sock_kref, hsmp_acpi_sock_release);
+
+	hsmp_sock_teardown_unlock();
 }
 
 static struct platform_driver amd_hsmp_driver = {
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index c15acba241c4..2e836124f486 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -546,6 +546,12 @@ EXPORT_SYMBOL_NS_GPL(hsmp_misc_register, "AMD_HSMP");
 void hsmp_misc_deregister(void)
 {
 	misc_deregister(&hsmp_pdev.mdev);
+	/*
+	 * misc_deregister() leaves mdev.this_device pointing at the now
+	 * destroyed device.  Clear it so a subsequent re-probe does not skip
+	 * registration on a stale pointer.
+	 */
+	hsmp_pdev.mdev.this_device = NULL;
 }
 EXPORT_SYMBOL_NS_GPL(hsmp_misc_deregister, "AMD_HSMP");
 
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index 5d0a6d819865..118922785d18 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -14,6 +14,7 @@
 #include <linux/device.h>
 #include <linux/hwmon.h>
 #include <linux/kconfig.h>
+#include <linux/kref.h>
 #include <linux/miscdevice.h>
 #include <linux/mutex.h>
 #include <linux/pci.h>
@@ -57,7 +58,8 @@ struct hsmp_plat_device {
 	struct hsmp_socket *sock;
 	u32 proto_ver;
 	u16 num_sockets;
-	bool is_probed;
+	struct kref acpi_sock_kref;
+	bool acpi_sock_kref_started;
 };
 
 int hsmp_cache_proto_ver(u16 sock_ind);
-- 
2.43.0

[PATCH v2 7/7] platform/x86/amd/hsmp: Drop stale metric table mapping on rebind

[Muralidhara M K]

Now that the ACPI socket array persists across a non-final unbind, a
stale metric_tbl_addr from a previous bind can still be recorded in the
socket when hsmp_get_tbl_dram_base() runs on a later (re)probe.
Re-ioremap()ing over it leaks one metric-table mapping per unbind/rebind
cycle.

iounmap() any existing mapping before remapping.  This runs during
(re)probe before the metric sysfs attribute is exposed, so no reader can
be using the old mapping.

Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
 drivers/platform/x86/amd/hsmp/hsmp.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 2e836124f486..a75190ea41c4 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -498,6 +498,17 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
 		dev_err(sock->dev, "Invalid DRAM address for metric table\n");
 		return -ENOMEM;
 	}
+	/*
+	 * The ACPI socket array now persists across a non-final unbind, so a
+	 * stale mapping from a previous bind can still be recorded here.  Drop
+	 * it before remapping to avoid leaking one metric-table mapping per
+	 * unbind/rebind cycle.  This runs during (re)probe before the metric
+	 * sysfs attribute is exposed, so no reader can be using it.
+	 */
+	if (sock->metric_tbl_addr) {
+		iounmap(sock->metric_tbl_addr);
+		sock->metric_tbl_addr = NULL;
+	}
 	sock->metric_tbl_addr = ioremap(dram_addr, sizeof(struct hsmp_metric_table));
 	if (!sock->metric_tbl_addr) {
 		dev_err(sock->dev, "Failed to ioremap metric table addr\n");
-- 
2.43.0

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

[Ilpo Järvinen]

[-- 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;
> 

Re: [PATCH v2 5/7] platform/x86/amd/hsmp: Serialize the data plane against socket teardown

[Ilpo Järvinen]

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

> The HSMP data plane is lock-free: open /dev/hsmp fds and hwmon sysfs
> reads call hsmp_send_message() without any coordination with driver
> teardown.  misc_deregister() does not drain already-open fds, so an
> in-flight message can race a concurrent unbind and touch a freed socket
> array or an unmapped mailbox.
> 
> Add hsmp_sock_rwsem.  hsmp_send_message() now holds it for read across
> the whole bounds-check + MMIO access, and hsmp_sock_teardown_lock()/
> hsmp_sock_teardown_unlock() let a teardown path hold it for write to
> drain any in-flight message and keep new ones out while it tears the
> socket down.
> 
> Wire the non-ACPI platform path into the drain: hsmp_pltdrv_remove()
> and the probe-failure cleanup take the write lock and drop the global
> socket pointer before devres frees the devm_kcalloc() array.  num_sockets
> is left intact so a later rebind can re-create the array.
> 
> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
> ---
>  drivers/platform/x86/amd/hsmp/hsmp.c | 55 +++++++++++++++++++++++++---
>  drivers/platform/x86/amd/hsmp/hsmp.h |  2 +
>  drivers/platform/x86/amd/hsmp/plat.c | 23 ++++++++++++
>  3 files changed, 74 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index c3939908d95f..c15acba241c4 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -15,6 +15,7 @@
>  #include <linux/io.h>
>  #include <linux/mutex.h>
>  #include <linux/nospec.h>
> +#include <linux/rwsem.h>
>  #include <linux/semaphore.h>
>  #include <linux/slab.h>
>  #include <linux/sysfs.h>
> @@ -44,6 +45,16 @@
>  
>  static struct hsmp_plat_device hsmp_pdev;
>  
> +/*
> + * Serializes the lock-free data plane (hsmp_send_message() and the per-socket
> + * MMIO access it performs) against socket teardown.  Callers of the data plane
> + * hold it for read so multiple sockets can be driven concurrently; ACPI
> + * removal holds it for write while it clears sock->dev,

I don't see acpi change in this patch (just hsmp_send_message and plat 
changes)???

> frees the socket array
> + * and unmaps the mailbox, so a reader can never observe a half-torn-down or
> + * freed socket.
> + */
> +static DECLARE_RWSEM(hsmp_sock_rwsem);
> +
>  /*
>   * Send a message to the HSMP port via PCI-e config space registers
>   * or by writing to MMIO space.
> @@ -215,8 +226,19 @@ int hsmp_send_message(struct hsmp_message *msg)
>  	if (ret)
>  		return ret;
>  
> -	if (!hsmp_pdev.sock || msg->sock_ind >= hsmp_pdev.num_sockets)
> -		return -ENODEV;
> +	/*
> +	 * Hold the teardown rwsem for read across the whole MMIO access.  ACPI
> +	 * removal takes it for write before clearing sock->dev, freeing the
> +	 * socket array and unmapping the mailbox, so the lock-free data plane
> +	 * (open /dev/hsmp fds and hwmon sysfs reads) can never dereference a
> +	 * freed socket or touch an unmapped mailbox.
> +	 */
> +	down_read(&hsmp_sock_rwsem);
> +
> +	if (!hsmp_pdev.sock || msg->sock_ind >= hsmp_pdev.num_sockets) {
> +		ret = -ENODEV;
> +		goto out_unlock;
> +	}
>  
>  	/*
>  	 * Sanitize sock_ind after the bounds check.  A mispredicted branch can
> @@ -235,18 +257,22 @@ int hsmp_send_message(struct hsmp_message *msg)
>  	 * semaphore or an unmapped mailbox.  A non-NULL dev also guarantees
>  	 * virt_base_addr, the mailbox offsets and the semaphore are visible.
>  	 */
> -	/* Pairs with smp_store_release(&sock->dev) in hsmp_parse_acpi_table(). */
> -	if (!smp_load_acquire(&sock->dev))
> -		return -ENODEV;
> +	/* Held under hsmp_sock_rwsem; pairs with smp_store_release(&sock->dev). */
> +	if (!smp_load_acquire(&sock->dev)) {
> +		ret = -ENODEV;
> +		goto out_unlock;
> +	}
>  
>  	ret = down_interruptible(&sock->hsmp_sem);
>  	if (ret < 0)
> -		return ret;
> +		goto out_unlock;
>  
>  	ret = __hsmp_send_message(sock, msg);
>  
>  	up(&sock->hsmp_sem);
>  
> +out_unlock:
> +	up_read(&hsmp_sock_rwsem);

Don't add unlock labels at the end but please use cleanup.h. It will 
simplify the patch too as you don't need to alter those direct returns.

>  	return ret;
>  }
>  EXPORT_SYMBOL_NS_GPL(hsmp_send_message, "AMD_HSMP");
> @@ -529,6 +555,23 @@ struct hsmp_plat_device *get_hsmp_pdev(void)
>  }
>  EXPORT_SYMBOL_NS_GPL(get_hsmp_pdev, "AMD_HSMP");
>  
> +/*
> + * Take the write side of the data-plane rwsem.  A caller tearing a socket down
> + * uses this to drain any in-flight hsmp_send_message() and to keep new ones out
> + * while it clears sock->dev, frees the socket array or unmaps the mailbox.
> + */
> +void hsmp_sock_teardown_lock(void)
> +{
> +	down_write(&hsmp_sock_rwsem);
> +}
> +EXPORT_SYMBOL_NS_GPL(hsmp_sock_teardown_lock, "AMD_HSMP");
> +
> +void hsmp_sock_teardown_unlock(void)
> +{
> +	up_write(&hsmp_sock_rwsem);
> +}
> +EXPORT_SYMBOL_NS_GPL(hsmp_sock_teardown_unlock, "AMD_HSMP");

Just export the lock to be taken directly in plat/acpi. It's much easier 
to follow who locks what when you don't add wrappers like this.

I don't understand why acpi doesn't use write side of this lock though 
but has it's own lock (hsmp_acpi_probe_mutex)?

>  MODULE_DESCRIPTION("AMD HSMP Common driver");
>  MODULE_VERSION(DRIVER_VERSION);
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
> index 91bc21232646..5d0a6d819865 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
> @@ -70,6 +70,8 @@ void hsmp_init_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets)
>  ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size);
>  void hsmp_destroy_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets);
>  struct hsmp_plat_device *get_hsmp_pdev(void);
> +void hsmp_sock_teardown_lock(void);
> +void hsmp_sock_teardown_unlock(void);
>  #if IS_ENABLED(CONFIG_HWMON)
>  int hsmp_create_sensor(struct device *dev, u16 sock_ind);
>  #else
> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
> index 685f2d2c574b..26c1363e79a1 100644
> --- a/drivers/platform/x86/amd/hsmp/plat.c
> +++ b/drivers/platform/x86/amd/hsmp/plat.c
> @@ -233,15 +233,38 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
>  	 * init_platform_device() may have ioremap()ed metric tables before
>  	 * failing.  hsmp_destroy_metric_read_locks() unmaps them and tears
>  	 * down the per-socket mutexes; the socket array itself is devm-managed.
> +	 *
> +	 * init_platform_device() also runs the data plane (hsmp_test()), so
> +	 * drain it via the teardown rwsem and drop the global socket pointer
> +	 * before devres frees the array.  num_sockets is left intact: it is
> +	 * only computed once in __init (amd_num_nodes()) and is needed to
> +	 * re-create the array on a later rebind.
>  	 */
> +	hsmp_sock_teardown_lock();
>  	hsmp_destroy_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
> +	hsmp_pdev->sock = NULL;
> +	hsmp_sock_teardown_unlock();
>  	return ret;
>  }
>  
>  static void hsmp_pltdrv_remove(struct platform_device *pdev)
>  {
> +	/*
> +	 * Drain the lock-free data plane and keep it out while the sockets are
> +	 * torn down.  misc_deregister() does not drain already-open /dev/hsmp
> +	 * fds and the driver permits sysfs unbind, so without this an in-flight
> +	 * hsmp_send_message() could touch the devres-freed socket array or an
> +	 * iounmap()ed metric table.  Dropping the global socket pointer makes
> +	 * later messages bail out at the first check.  num_sockets is left
> +	 * intact so an unbind/rebind cycle can re-create the array; it is only
> +	 * computed once in __init (amd_num_nodes()) and is never recomputed on
> +	 * probe.
> +	 */
> +	hsmp_sock_teardown_lock();
>  	hsmp_misc_deregister();
>  	hsmp_destroy_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
> +	hsmp_pdev->sock = NULL;
> +	hsmp_sock_teardown_unlock();
>  }
>  
>  static struct platform_driver amd_hsmp_driver = {
> 

-- 
 i.

Re: [PATCH v2 3/7] platform/x86/amd/hsmp: Add explicit metric DRAM mapping and per-socket mutexes

[Ilpo Järvinen]

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

> Add hsmp_destroy_metric_read_locks(), hsmp_init_metric_read_locks(), and
> switch metric table DRAM from devm_ioremap() to ioremap() so ACPI can
> coordinate teardown.  Embed struct mutex metric_read_lock in each
> hsmp_socket for later serialization of metric table reads.
> 
> Call the new helpers from the platform driver probe and remove paths
> so non-ACPI builds initialize and tear down metric_read_lock consistently
> with the ACPI driver.
> 
> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
> ---
>  drivers/platform/x86/amd/hsmp/hsmp.c | 38 ++++++++++++++++++++++++++--
>  drivers/platform/x86/amd/hsmp/hsmp.h |  5 ++++
>  drivers/platform/x86/amd/hsmp/plat.c | 16 ++++++++++--
>  3 files changed, 55 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 91be0cdb6af1..a96c59fcba0a 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -12,7 +12,11 @@
>  #include <linux/acpi.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/mutex.h>
> +#include <linux/nospec.h>
>  #include <linux/semaphore.h>
> +#include <linux/slab.h>
>  #include <linux/sysfs.h>
>  
>  #include "hsmp.h"
> @@ -410,6 +414,37 @@ ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
>  }
>  EXPORT_SYMBOL_NS_GPL(hsmp_metric_tbl_read, "AMD_HSMP");
>  
> +void hsmp_init_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets)
> +{
> +	u16 i;
> +
> +	if (!pdev->sock || !num_sockets)
> +		return;
> +
> +	for (i = 0; i < num_sockets; i++)
> +		mutex_init(&pdev->sock[i].metric_read_lock);
> +}
> +EXPORT_SYMBOL_NS_GPL(hsmp_init_metric_read_locks, "AMD_HSMP");
> +
> +void hsmp_destroy_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets)
> +{
> +	u16 i;
> +
> +	if (!pdev->sock || !num_sockets)
> +		return;
> +
> +	for (i = 0; i < num_sockets; i++) {
> +		struct hsmp_socket *s = &pdev->sock[i];
> +
> +		if (s->metric_tbl_addr) {
> +			iounmap(s->metric_tbl_addr);
> +			s->metric_tbl_addr = NULL;
> +		}
> +		mutex_destroy(&s->metric_read_lock);
> +	}
> +}
> +EXPORT_SYMBOL_NS_GPL(hsmp_destroy_metric_read_locks, "AMD_HSMP");
> +
>  int hsmp_get_tbl_dram_base(u16 sock_ind)
>  {
>  	struct hsmp_socket *sock = &hsmp_pdev.sock[sock_ind];
> @@ -434,8 +469,7 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
>  		dev_err(sock->dev, "Invalid DRAM address for metric table\n");
>  		return -ENOMEM;
>  	}
> -	sock->metric_tbl_addr = devm_ioremap(sock->dev, dram_addr,
> -					     sizeof(struct hsmp_metric_table));
> +	sock->metric_tbl_addr = ioremap(dram_addr, sizeof(struct hsmp_metric_table));
>  	if (!sock->metric_tbl_addr) {
>  		dev_err(sock->dev, "Failed to ioremap metric table addr\n");
>  		return -ENOMEM;
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
> index 0509a442eaae..91bc21232646 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
> @@ -15,6 +15,7 @@
>  #include <linux/hwmon.h>
>  #include <linux/kconfig.h>
>  #include <linux/miscdevice.h>
> +#include <linux/mutex.h>
>  #include <linux/pci.h>
>  #include <linux/semaphore.h>
>  #include <linux/sysfs.h>
> @@ -41,6 +42,8 @@ struct hsmp_socket {
>  	struct bin_attribute hsmp_attr;
>  	struct hsmp_mbaddr_info mbinfo;
>  	void __iomem *metric_tbl_addr;
> +	/* Protects metric table snapshot reads for this socket */
> +	struct mutex metric_read_lock;

After applying all the patches in your series (in this series only), I 
only see this:

~/linux/platform/drivers/platform/x86/amd/hsmp$ git grep -w metric_read_lock
hsmp.c:         mutex_init(&pdev->sock[i].metric_read_lock);
hsmp.c:         mutex_destroy(&s->metric_read_lock);
hsmp.h: struct mutex metric_read_lock;

Is this lock ever even taken???

--
 i.

>  	void __iomem *virt_base_addr;
>  	struct semaphore hsmp_sem;
>  	char name[HSMP_ATTR_GRP_NAME_SIZE];
> @@ -63,7 +66,9 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg);
>  void hsmp_misc_deregister(void);
>  int hsmp_misc_register(struct device *dev);
>  int hsmp_get_tbl_dram_base(u16 sock_ind);
> +void hsmp_init_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets);
>  ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size);
> +void hsmp_destroy_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets);
>  struct hsmp_plat_device *get_hsmp_pdev(void);
>  #if IS_ENABLED(CONFIG_HWMON)
>  int hsmp_create_sensor(struct device *dev, u16 sock_ind);
> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
> index e07f68575055..685f2d2c574b 100644
> --- a/drivers/platform/x86/amd/hsmp/plat.c
> +++ b/drivers/platform/x86/amd/hsmp/plat.c
> @@ -211,25 +211,37 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
>  	if (!hsmp_pdev->sock)
>  		return -ENOMEM;
>  
> +	hsmp_init_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
> +
>  	ret = init_platform_device(&pdev->dev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
> -		return ret;
> +		goto err_destroy_locks;
>  	}
>  
>  	ret = hsmp_misc_register(&pdev->dev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to register misc device\n");
> -		return ret;
> +		goto err_destroy_locks;
>  	}
>  
>  	dev_dbg(&pdev->dev, "AMD HSMP is probed successfully\n");
>  	return 0;
> +
> +err_destroy_locks:
> +	/*
> +	 * init_platform_device() may have ioremap()ed metric tables before
> +	 * failing.  hsmp_destroy_metric_read_locks() unmaps them and tears
> +	 * down the per-socket mutexes; the socket array itself is devm-managed.
> +	 */
> +	hsmp_destroy_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
> +	return ret;
>  }
>  
>  static void hsmp_pltdrv_remove(struct platform_device *pdev)
>  {
>  	hsmp_misc_deregister();
> +	hsmp_destroy_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
>  }
>  
>  static struct platform_driver amd_hsmp_driver = {
> 

Re: [PATCH v2 5/7] platform/x86/amd/hsmp: Serialize the data plane against socket teardown

[M K, Muralidhara]

On 7/6/2026 5:11 PM, Ilpo Järvinen wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Thu, 25 Jun 2026, Muralidhara M K wrote:
> 
>> The HSMP data plane is lock-free: open /dev/hsmp fds and hwmon sysfs
>> reads call hsmp_send_message() without any coordination with driver
>> teardown.  misc_deregister() does not drain already-open fds, so an
>> in-flight message can race a concurrent unbind and touch a freed socket
>> array or an unmapped mailbox.
>>
>> Add hsmp_sock_rwsem.  hsmp_send_message() now holds it for read across
>> the whole bounds-check + MMIO access, and hsmp_sock_teardown_lock()/
>> hsmp_sock_teardown_unlock() let a teardown path hold it for write to
>> drain any in-flight message and keep new ones out while it tears the
>> socket down.
>>
>> Wire the non-ACPI platform path into the drain: hsmp_pltdrv_remove()
>> and the probe-failure cleanup take the write lock and drop the global
>> socket pointer before devres frees the devm_kcalloc() array.  num_sockets
>> is left intact so a later rebind can re-create the array.
>>
>> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
>> ---
>>   drivers/platform/x86/amd/hsmp/hsmp.c | 55 +++++++++++++++++++++++++---
>>   drivers/platform/x86/amd/hsmp/hsmp.h |  2 +
>>   drivers/platform/x86/amd/hsmp/plat.c | 23 ++++++++++++
>>   3 files changed, 74 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
>> index c3939908d95f..c15acba241c4 100644
>> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
>> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
>> @@ -15,6 +15,7 @@
>>   #include <linux/io.h>
>>   #include <linux/mutex.h>
>>   #include <linux/nospec.h>
>> +#include <linux/rwsem.h>
>>   #include <linux/semaphore.h>
>>   #include <linux/slab.h>
>>   #include <linux/sysfs.h>
>> @@ -44,6 +45,16 @@
>>
>>   static struct hsmp_plat_device hsmp_pdev;
>>
>> +/*
>> + * Serializes the lock-free data plane (hsmp_send_message() and the per-socket
>> + * MMIO access it performs) against socket teardown.  Callers of the data plane
>> + * hold it for read so multiple sockets can be driven concurrently; ACPI
>> + * removal holds it for write while it clears sock->dev,
> 
> I don't see acpi change in this patch (just hsmp_send_message and plat
> changes)???
> 
I have to correct the comments.

>> frees the socket array
>> + * and unmaps the mailbox, so a reader can never observe a half-torn-down or
>> + * freed socket.
>> + */
>> +static DECLARE_RWSEM(hsmp_sock_rwsem);
>> +
>>   /*
>>    * Send a message to the HSMP port via PCI-e config space registers
>>    * or by writing to MMIO space.
>> @@ -215,8 +226,19 @@ int hsmp_send_message(struct hsmp_message *msg)
>>        if (ret)
>>                return ret;
>>
>> -     if (!hsmp_pdev.sock || msg->sock_ind >= hsmp_pdev.num_sockets)
>> -             return -ENODEV;
>> +     /*
>> +      * Hold the teardown rwsem for read across the whole MMIO access.  ACPI
>> +      * removal takes it for write before clearing sock->dev, freeing the
>> +      * socket array and unmapping the mailbox, so the lock-free data plane
>> +      * (open /dev/hsmp fds and hwmon sysfs reads) can never dereference a
>> +      * freed socket or touch an unmapped mailbox.
>> +      */
>> +     down_read(&hsmp_sock_rwsem);
>> +
>> +     if (!hsmp_pdev.sock || msg->sock_ind >= hsmp_pdev.num_sockets) {
>> +             ret = -ENODEV;
>> +             goto out_unlock;
>> +     }
>>
>>        /*
>>         * Sanitize sock_ind after the bounds check.  A mispredicted branch can
>> @@ -235,18 +257,22 @@ int hsmp_send_message(struct hsmp_message *msg)
>>         * semaphore or an unmapped mailbox.  A non-NULL dev also guarantees
>>         * virt_base_addr, the mailbox offsets and the semaphore are visible.
>>         */
>> -     /* Pairs with smp_store_release(&sock->dev) in hsmp_parse_acpi_table(). */
>> -     if (!smp_load_acquire(&sock->dev))
>> -             return -ENODEV;
>> +     /* Held under hsmp_sock_rwsem; pairs with smp_store_release(&sock->dev). */
>> +     if (!smp_load_acquire(&sock->dev)) {
>> +             ret = -ENODEV;
>> +             goto out_unlock;
>> +     }
>>
>>        ret = down_interruptible(&sock->hsmp_sem);
>>        if (ret < 0)
>> -             return ret;
>> +             goto out_unlock;
>>
>>        ret = __hsmp_send_message(sock, msg);
>>
>>        up(&sock->hsmp_sem);
>>
>> +out_unlock:
>> +     up_read(&hsmp_sock_rwsem);
> 
> Don't add unlock labels at the end but please use cleanup.h. It will
> simplify the patch too as you don't need to alter those direct returns.
> 
Undersrood. Will update anbd send in next version.

>>        return ret;
>>   }
>>   EXPORT_SYMBOL_NS_GPL(hsmp_send_message, "AMD_HSMP");
>> @@ -529,6 +555,23 @@ struct hsmp_plat_device *get_hsmp_pdev(void)
>>   }
>>   EXPORT_SYMBOL_NS_GPL(get_hsmp_pdev, "AMD_HSMP");
>>
>> +/*
>> + * Take the write side of the data-plane rwsem.  A caller tearing a socket down
>> + * uses this to drain any in-flight hsmp_send_message() and to keep new ones out
>> + * while it clears sock->dev, frees the socket array or unmaps the mailbox.
>> + */
>> +void hsmp_sock_teardown_lock(void)
>> +{
>> +     down_write(&hsmp_sock_rwsem);
>> +}
>> +EXPORT_SYMBOL_NS_GPL(hsmp_sock_teardown_lock, "AMD_HSMP");
>> +
>> +void hsmp_sock_teardown_unlock(void)
>> +{
>> +     up_write(&hsmp_sock_rwsem);
>> +}
>> +EXPORT_SYMBOL_NS_GPL(hsmp_sock_teardown_unlock, "AMD_HSMP");
> 
> Just export the lock to be taken directly in plat/acpi. It's much easier
> to follow who locks what when you don't add wrappers like this.
> 
Thanks for the input. I will address them.

> I don't understand why acpi doesn't use write side of this lock though
> but has it's own lock (hsmp_acpi_probe_mutex)?
> 
>>   MODULE_DESCRIPTION("AMD HSMP Common driver");
>>   MODULE_VERSION(DRIVER_VERSION);
>>   MODULE_LICENSE("GPL");
>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
>> index 91bc21232646..5d0a6d819865 100644
>> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
>> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
>> @@ -70,6 +70,8 @@ void hsmp_init_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets)
>>   ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size);
>>   void hsmp_destroy_metric_read_locks(struct hsmp_plat_device *pdev, u16 num_sockets);
>>   struct hsmp_plat_device *get_hsmp_pdev(void);
>> +void hsmp_sock_teardown_lock(void);
>> +void hsmp_sock_teardown_unlock(void);
>>   #if IS_ENABLED(CONFIG_HWMON)
>>   int hsmp_create_sensor(struct device *dev, u16 sock_ind);
>>   #else
>> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
>> index 685f2d2c574b..26c1363e79a1 100644
>> --- a/drivers/platform/x86/amd/hsmp/plat.c
>> +++ b/drivers/platform/x86/amd/hsmp/plat.c
>> @@ -233,15 +233,38 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
>>         * init_platform_device() may have ioremap()ed metric tables before
>>         * failing.  hsmp_destroy_metric_read_locks() unmaps them and tears
>>         * down the per-socket mutexes; the socket array itself is devm-managed.
>> +      *
>> +      * init_platform_device() also runs the data plane (hsmp_test()), so
>> +      * drain it via the teardown rwsem and drop the global socket pointer
>> +      * before devres frees the array.  num_sockets is left intact: it is
>> +      * only computed once in __init (amd_num_nodes()) and is needed to
>> +      * re-create the array on a later rebind.
>>         */
>> +     hsmp_sock_teardown_lock();
>>        hsmp_destroy_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
>> +     hsmp_pdev->sock = NULL;
>> +     hsmp_sock_teardown_unlock();
>>        return ret;
>>   }
>>
>>   static void hsmp_pltdrv_remove(struct platform_device *pdev)
>>   {
>> +     /*
>> +      * Drain the lock-free data plane and keep it out while the sockets are
>> +      * torn down.  misc_deregister() does not drain already-open /dev/hsmp
>> +      * fds and the driver permits sysfs unbind, so without this an in-flight
>> +      * hsmp_send_message() could touch the devres-freed socket array or an
>> +      * iounmap()ed metric table.  Dropping the global socket pointer makes
>> +      * later messages bail out at the first check.  num_sockets is left
>> +      * intact so an unbind/rebind cycle can re-create the array; it is only
>> +      * computed once in __init (amd_num_nodes()) and is never recomputed on
>> +      * probe.
>> +      */
>> +     hsmp_sock_teardown_lock();
>>        hsmp_misc_deregister();
>>        hsmp_destroy_metric_read_locks(hsmp_pdev, hsmp_pdev->num_sockets);
>> +     hsmp_pdev->sock = NULL;
>> +     hsmp_sock_teardown_unlock();
>>   }
>>
>>   static struct platform_driver amd_hsmp_driver = {
>>
> 
> --
>   i.
>