[PATCH 0/2] ASoC: SOF: sof-client: Enforce client access scope

[PATCH 0/2] ASoC: SOF: sof-client: Enforce client access scope

[Peter Ujfalusi]

Hi,

Up to this point we had a spoken rule what sof-client can access and what
they must not (they cannot use sdev directly), recently there were attempts
to break this rule as clients could access sdev via the sof client dev.

The series will re-arrange the client device struct to 'hide' the sdev (and
the list) from client drivers to prevent abuse.

Regards,
Peter
---
Peter Ujfalusi (2):
  ASoC: SOF: sof-client: Convert sof_client_dev_to_sof_dev into function
  ASoC: SOF: sof-client: Introduce sof_client_dev_entry structure

 sound/soc/sof/sof-client.c | 105 ++++++++++++++++++++++++++-----------
 sound/soc/sof/sof-client.h |   6 ---
 sound/soc/sof/sof-priv.h   |   9 ++++
 3 files changed, 83 insertions(+), 37 deletions(-)

-- 
2.50.1

[PATCH 1/2] ASoC: SOF: sof-client: Convert sof_client_dev_to_sof_dev into function

[Peter Ujfalusi]

Remove the sof_client_dev_to_sof_dev() from the header file and add it as
a function in sof-client.c to avoid it's use by client drivers.

At the same time mark the sdev and list of sof_client_dev as restricted
fro core use only.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/sof-client.c | 6 ++++++
 sound/soc/sof/sof-client.h | 6 ++----
 sound/soc/sof/sof-priv.h   | 9 +++++++++
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sof/sof-client.c b/sound/soc/sof/sof-client.c
index 4c7951338c66..2db12753f79e 100644
--- a/sound/soc/sof/sof-client.c
+++ b/sound/soc/sof/sof-client.c
@@ -611,3 +611,9 @@ enum sof_fw_state sof_client_get_fw_state(struct sof_client_dev *cdev)
 	return sdev->fw_state;
 }
 EXPORT_SYMBOL_NS_GPL(sof_client_get_fw_state, "SND_SOC_SOF_CLIENT");
+
+struct snd_sof_dev *sof_client_dev_to_sof_dev(struct sof_client_dev *cdev)
+{
+	return cdev->sdev;
+}
+EXPORT_SYMBOL_NS_GPL(sof_client_dev_to_sof_dev, "SND_SOC_SOF_CLIENT");
diff --git a/sound/soc/sof/sof-client.h b/sound/soc/sof/sof-client.h
index b6ccc2cd69e5..8b267b0b9ff1 100644
--- a/sound/soc/sof/sof-client.h
+++ b/sound/soc/sof/sof-client.h
@@ -18,8 +18,8 @@ struct sof_ipc4_fw_module;
 /**
  * struct sof_client_dev - SOF client device
  * @auxdev:	auxiliary device
- * @sdev:	pointer to SOF core device struct
- * @list:	item in SOF core client dev list
+ * @sdev:	pointer to SOF core device struct, resticted for core use only
+ * @list:	item in SOF core client dev list, resticted for core use only
  * @data:	device specific data
  */
 struct sof_client_dev {
@@ -29,8 +29,6 @@ struct sof_client_dev {
 	void *data;
 };
 
-#define sof_client_dev_to_sof_dev(cdev)		((cdev)->sdev)
-
 #define auxiliary_dev_to_sof_client_dev(auxiliary_dev) \
 	container_of(auxiliary_dev, struct sof_client_dev, auxdev)
 
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index abbb5ee7e08c..0f624d8cde20 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -838,7 +838,11 @@ int sof_stream_pcm_close(struct snd_sof_dev *sdev,
 			 struct snd_pcm_substream *substream);
 
 /* SOF client support */
+struct sof_client_dev;
+
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_CLIENT)
+struct snd_sof_dev *sof_client_dev_to_sof_dev(struct sof_client_dev *cdev);
+
 int sof_client_dev_register(struct snd_sof_dev *sdev, const char *name, u32 id,
 			    const void *data, size_t size);
 void sof_client_dev_unregister(struct snd_sof_dev *sdev, const char *name, u32 id);
@@ -849,6 +853,11 @@ void sof_client_fw_state_dispatcher(struct snd_sof_dev *sdev);
 int sof_suspend_clients(struct snd_sof_dev *sdev, pm_message_t state);
 int sof_resume_clients(struct snd_sof_dev *sdev);
 #else /* CONFIG_SND_SOC_SOF_CLIENT */
+static inline struct snd_sof_dev *
+sof_client_dev_to_sof_dev(struct sof_client_dev *cdev) {
+	return NULL;
+}
+
 static inline int sof_client_dev_register(struct snd_sof_dev *sdev, const char *name,
 					  u32 id, const void *data, size_t size)
 {
-- 
2.50.1

[PATCH 2/2] ASoC: SOF: sof-client: Introduce sof_client_dev_entry structure

[Peter Ujfalusi]

Introduce a new internal structure for wrapping the sof_client_dev and
move members away from the client visible struct that they must not
access, let alone see (sdev and the list).

The changes are mechanical in nature and contained within sof-client core
code, no functional change or change in behavior is introduced.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/sof-client.c | 101 +++++++++++++++++++++++++------------
 sound/soc/sof/sof-client.h |   4 --
 2 files changed, 69 insertions(+), 36 deletions(-)

diff --git a/sound/soc/sof/sof-client.c b/sound/soc/sof/sof-client.c
index 2db12753f79e..12be97540e4c 100644
--- a/sound/soc/sof/sof-client.c
+++ b/sound/soc/sof/sof-client.c
@@ -45,13 +45,30 @@ struct sof_state_event_entry {
 	struct list_head list;
 };
 
+/**
+ * struct sof_client_dev_entry - client device entry for internal management use
+ * @sdev:	pointer to SOF core device struct
+ * @list:	item in SOF core client dev list
+ * @client_dev: SOF client device
+ */
+struct sof_client_dev_entry {
+	struct snd_sof_dev *sdev;
+	struct list_head list;
+
+	struct sof_client_dev client_dev;
+};
+
+#define cdev_to_centry(cdev) \
+	container_of(cdev, struct sof_client_dev_entry, client_dev)
+
 static void sof_client_auxdev_release(struct device *dev)
 {
 	struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
 	struct sof_client_dev *cdev = auxiliary_dev_to_sof_client_dev(auxdev);
+	struct sof_client_dev_entry *centry = cdev_to_centry(cdev);
 
 	kfree(cdev->auxdev.dev.platform_data);
-	kfree(cdev);
+	kfree(centry);
 }
 
 static int sof_client_dev_add_data(struct sof_client_dev *cdev, const void *data,
@@ -208,15 +225,18 @@ void sof_unregister_clients(struct snd_sof_dev *sdev)
 int sof_client_dev_register(struct snd_sof_dev *sdev, const char *name, u32 id,
 			    const void *data, size_t size)
 {
+	struct sof_client_dev_entry *centry;
 	struct auxiliary_device *auxdev;
 	struct sof_client_dev *cdev;
 	int ret;
 
-	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
-	if (!cdev)
+	centry = kzalloc(sizeof(*centry), GFP_KERNEL);
+	if (!centry)
 		return -ENOMEM;
 
-	cdev->sdev = sdev;
+	cdev = &centry->client_dev;
+
+	centry->sdev = sdev;
 	auxdev = &cdev->auxdev;
 	auxdev->name = name;
 	auxdev->dev.parent = sdev->dev;
@@ -246,7 +266,7 @@ int sof_client_dev_register(struct snd_sof_dev *sdev, const char *name, u32 id,
 
 	/* add to list of SOF client devices */
 	mutex_lock(&sdev->ipc_client_mutex);
-	list_add(&cdev->list, &sdev->ipc_client_list);
+	list_add(&centry->list, &sdev->ipc_client_list);
 	mutex_unlock(&sdev->ipc_client_mutex);
 
 	return 0;
@@ -255,7 +275,7 @@ int sof_client_dev_register(struct snd_sof_dev *sdev, const char *name, u32 id,
 	kfree(cdev->auxdev.dev.platform_data);
 
 err_dev_add_data:
-	kfree(cdev);
+	kfree(centry);
 
 	return ret;
 }
@@ -263,7 +283,7 @@ EXPORT_SYMBOL_NS_GPL(sof_client_dev_register, "SND_SOC_SOF_CLIENT");
 
 void sof_client_dev_unregister(struct snd_sof_dev *sdev, const char *name, u32 id)
 {
-	struct sof_client_dev *cdev;
+	struct sof_client_dev_entry *centry;
 
 	mutex_lock(&sdev->ipc_client_mutex);
 
@@ -271,9 +291,11 @@ void sof_client_dev_unregister(struct snd_sof_dev *sdev, const char *name, u32 i
 	 * sof_client_auxdev_release() will be invoked to free up memory
 	 * allocations through put_device()
 	 */
-	list_for_each_entry(cdev, &sdev->ipc_client_list, list) {
+	list_for_each_entry(centry, &sdev->ipc_client_list, list) {
+		struct sof_client_dev *cdev = &centry->client_dev;
+
 		if (!strcmp(cdev->auxdev.name, name) && cdev->auxdev.id == id) {
-			list_del(&cdev->list);
+			list_del(&centry->list);
 			auxiliary_device_delete(&cdev->auxdev);
 			auxiliary_device_uninit(&cdev->auxdev);
 			break;
@@ -287,15 +309,17 @@ EXPORT_SYMBOL_NS_GPL(sof_client_dev_unregister, "SND_SOC_SOF_CLIENT");
 int sof_client_ipc_tx_message(struct sof_client_dev *cdev, void *ipc_msg,
 			      void *reply_data, size_t reply_bytes)
 {
-	if (cdev->sdev->pdata->ipc_type == SOF_IPC_TYPE_3) {
+	struct snd_sof_dev *sdev = sof_client_dev_to_sof_dev(cdev);
+
+	if (sdev->pdata->ipc_type == SOF_IPC_TYPE_3) {
 		struct sof_ipc_cmd_hdr *hdr = ipc_msg;
 
-		return sof_ipc_tx_message(cdev->sdev->ipc, ipc_msg, hdr->size,
+		return sof_ipc_tx_message(sdev->ipc, ipc_msg, hdr->size,
 					  reply_data, reply_bytes);
-	} else if (cdev->sdev->pdata->ipc_type == SOF_IPC_TYPE_4) {
+	} else if (sdev->pdata->ipc_type == SOF_IPC_TYPE_4) {
 		struct sof_ipc4_msg *msg = ipc_msg;
 
-		return sof_ipc_tx_message(cdev->sdev->ipc, ipc_msg, msg->data_size,
+		return sof_ipc_tx_message(sdev->ipc, ipc_msg, msg->data_size,
 					  reply_data, reply_bytes);
 	}
 
@@ -305,16 +329,18 @@ EXPORT_SYMBOL_NS_GPL(sof_client_ipc_tx_message, "SND_SOC_SOF_CLIENT");
 
 int sof_client_ipc_rx_message(struct sof_client_dev *cdev, void *ipc_msg, void *msg_buf)
 {
+	struct snd_sof_dev *sdev = sof_client_dev_to_sof_dev(cdev);
+
 	if (IS_ENABLED(CONFIG_SND_SOC_SOF_IPC3) &&
-	    cdev->sdev->pdata->ipc_type == SOF_IPC_TYPE_3) {
+	    sdev->pdata->ipc_type == SOF_IPC_TYPE_3) {
 		struct sof_ipc_cmd_hdr *hdr = ipc_msg;
 
 		if (hdr->size < sizeof(hdr)) {
-			dev_err(cdev->sdev->dev, "The received message size is invalid\n");
+			dev_err(sdev->dev, "The received message size is invalid\n");
 			return -EINVAL;
 		}
 
-		sof_ipc3_do_rx_work(cdev->sdev, ipc_msg, msg_buf);
+		sof_ipc3_do_rx_work(sdev, ipc_msg, msg_buf);
 		return 0;
 	}
 
@@ -325,16 +351,17 @@ EXPORT_SYMBOL_NS_GPL(sof_client_ipc_rx_message, "SND_SOC_SOF_CLIENT");
 int sof_client_ipc_set_get_data(struct sof_client_dev *cdev, void *ipc_msg,
 				bool set)
 {
-	if (cdev->sdev->pdata->ipc_type == SOF_IPC_TYPE_3) {
+	struct snd_sof_dev *sdev = sof_client_dev_to_sof_dev(cdev);
+
+	if (sdev->pdata->ipc_type == SOF_IPC_TYPE_3) {
 		struct sof_ipc_cmd_hdr *hdr = ipc_msg;
 
-		return sof_ipc_set_get_data(cdev->sdev->ipc, ipc_msg, hdr->size,
-					    set);
-	} else if (cdev->sdev->pdata->ipc_type == SOF_IPC_TYPE_4) {
+		return sof_ipc_set_get_data(sdev->ipc, ipc_msg, hdr->size, set);
+	} else if (sdev->pdata->ipc_type == SOF_IPC_TYPE_4) {
 		struct sof_ipc4_msg *msg = ipc_msg;
 
-		return sof_ipc_set_get_data(cdev->sdev->ipc, ipc_msg,
-					    msg->data_size, set);
+		return sof_ipc_set_get_data(sdev->ipc, ipc_msg, msg->data_size,
+					    set);
 	}
 
 	return -EINVAL;
@@ -344,7 +371,7 @@ EXPORT_SYMBOL_NS_GPL(sof_client_ipc_set_get_data, "SND_SOC_SOF_CLIENT");
 #ifdef CONFIG_SND_SOC_SOF_IPC4
 struct sof_ipc4_fw_module *sof_client_ipc4_find_module(struct sof_client_dev *c, const guid_t *uuid)
 {
-	struct snd_sof_dev *sdev = c->sdev;
+	struct snd_sof_dev *sdev = sof_client_dev_to_sof_dev(c);
 
 	if (sdev->pdata->ipc_type == SOF_IPC_TYPE_4)
 		return sof_ipc4_find_module_by_uuid(sdev, uuid);
@@ -358,11 +385,13 @@ EXPORT_SYMBOL_NS_GPL(sof_client_ipc4_find_module, "SND_SOC_SOF_CLIENT");
 int sof_suspend_clients(struct snd_sof_dev *sdev, pm_message_t state)
 {
 	const struct auxiliary_driver *adrv;
-	struct sof_client_dev *cdev;
+	struct sof_client_dev_entry *centry;
 
 	mutex_lock(&sdev->ipc_client_mutex);
 
-	list_for_each_entry(cdev, &sdev->ipc_client_list, list) {
+	list_for_each_entry(centry, &sdev->ipc_client_list, list) {
+		struct sof_client_dev *cdev = &centry->client_dev;
+
 		/* Skip devices without loaded driver */
 		if (!cdev->auxdev.dev.driver)
 			continue;
@@ -381,11 +410,13 @@ EXPORT_SYMBOL_NS_GPL(sof_suspend_clients, "SND_SOC_SOF_CLIENT");
 int sof_resume_clients(struct snd_sof_dev *sdev)
 {
 	const struct auxiliary_driver *adrv;
-	struct sof_client_dev *cdev;
+	struct sof_client_dev_entry *centry;
 
 	mutex_lock(&sdev->ipc_client_mutex);
 
-	list_for_each_entry(cdev, &sdev->ipc_client_list, list) {
+	list_for_each_entry(centry, &sdev->ipc_client_list, list) {
+		struct sof_client_dev *cdev = &centry->client_dev;
+
 		/* Skip devices without loaded driver */
 		if (!cdev->auxdev.dev.driver)
 			continue;
@@ -403,14 +434,18 @@ EXPORT_SYMBOL_NS_GPL(sof_resume_clients, "SND_SOC_SOF_CLIENT");
 
 struct dentry *sof_client_get_debugfs_root(struct sof_client_dev *cdev)
 {
-	return cdev->sdev->debugfs_root;
+	struct snd_sof_dev *sdev = sof_client_dev_to_sof_dev(cdev);
+
+	return sdev->debugfs_root;
 }
 EXPORT_SYMBOL_NS_GPL(sof_client_get_debugfs_root, "SND_SOC_SOF_CLIENT");
 
 /* DMA buffer allocation in client drivers must use the core SOF device */
 struct device *sof_client_get_dma_dev(struct sof_client_dev *cdev)
 {
-	return cdev->sdev->dev;
+	struct snd_sof_dev *sdev = sof_client_dev_to_sof_dev(cdev);
+
+	return sdev->dev;
 }
 EXPORT_SYMBOL_NS_GPL(sof_client_get_dma_dev, "SND_SOC_SOF_CLIENT");
 
@@ -498,10 +533,10 @@ int sof_client_register_ipc_rx_handler(struct sof_client_dev *cdev,
 	if (!callback)
 		return -EINVAL;
 
-	if (cdev->sdev->pdata->ipc_type == SOF_IPC_TYPE_3) {
+	if (sdev->pdata->ipc_type == SOF_IPC_TYPE_3) {
 		if (!(ipc_msg_type & SOF_GLB_TYPE_MASK))
 			return -EINVAL;
-	} else if (cdev->sdev->pdata->ipc_type == SOF_IPC_TYPE_4) {
+	} else if (sdev->pdata->ipc_type == SOF_IPC_TYPE_4) {
 		if (!(ipc_msg_type & SOF_IPC4_NOTIFICATION_TYPE_MASK))
 			return -EINVAL;
 	} else {
@@ -614,6 +649,8 @@ EXPORT_SYMBOL_NS_GPL(sof_client_get_fw_state, "SND_SOC_SOF_CLIENT");
 
 struct snd_sof_dev *sof_client_dev_to_sof_dev(struct sof_client_dev *cdev)
 {
-	return cdev->sdev;
+	struct sof_client_dev_entry *centry = cdev_to_centry(cdev);
+
+	return centry->sdev;
 }
 EXPORT_SYMBOL_NS_GPL(sof_client_dev_to_sof_dev, "SND_SOC_SOF_CLIENT");
diff --git a/sound/soc/sof/sof-client.h b/sound/soc/sof/sof-client.h
index 8b267b0b9ff1..6e9dd77f7c9b 100644
--- a/sound/soc/sof/sof-client.h
+++ b/sound/soc/sof/sof-client.h
@@ -18,14 +18,10 @@ struct sof_ipc4_fw_module;
 /**
  * struct sof_client_dev - SOF client device
  * @auxdev:	auxiliary device
- * @sdev:	pointer to SOF core device struct, resticted for core use only
- * @list:	item in SOF core client dev list, resticted for core use only
  * @data:	device specific data
  */
 struct sof_client_dev {
 	struct auxiliary_device auxdev;
-	struct snd_sof_dev *sdev;
-	struct list_head list;
 	void *data;
 };
 
-- 
2.50.1

Re: [PATCH 0/2] ASoC: SOF: sof-client: Enforce client access scope

[Mark Brown]

On Fri, 29 Aug 2025 13:25:08 +0300, Peter Ujfalusi wrote:
> Up to this point we had a spoken rule what sof-client can access and what
> they must not (they cannot use sdev directly), recently there were attempts
> to break this rule as clients could access sdev via the sof client dev.
> 
> The series will re-arrange the client device struct to 'hide' the sdev (and
> the list) from client drivers to prevent abuse.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: SOF: sof-client: Convert sof_client_dev_to_sof_dev into function
      commit: 850c5dfb17b381f30c4052a68c43da5abc678e74
[2/2] ASoC: SOF: sof-client: Introduce sof_client_dev_entry structure
      commit: 07752abfa5dbf7cb4d9ce69fa94dc3b12bc597d9

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark