Linux Hardware Monitor development
 help / color / mirror / Atom feed
* [PATCH v01] mailbox/pcc.c:  add query channel function
@ 2026-06-04 20:37 Adam Young
  2026-06-04 20:49 ` sashiko-bot
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Adam Young @ 2026-06-04 20:37 UTC (permalink / raw)
  To: Sudeep Holla, Jassi Brar, Rafael J. Wysocki, Saket Dumbre,
	Len Brown
  Cc: linux-kernel, linux-hwmon, linux-acpi, Andi Shyti, Guenter Roeck,
	Huisong Li, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	linux-arm-kernel

Drivers need information about a channel prior to creating a channel
or they risk triggering message delivery on the remote side of a
connection.

One of those pieces of infomration is the type of channel.

Add PCC channel type to records and expose PCC channel type to client.

Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
 drivers/mailbox/pcc.c | 39 +++++++++++++++++++++++++++++++++++++++
 include/acpi/pcc.h    | 12 ++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 0deaf7907ed6..c27bea426967 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -348,6 +348,44 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
+/**
+ * pcc_mbox_query_channel - returns information about the channel
+ *              without activating the channel.
+ *
+ * @q_chan a pointer to an already allocated struct pcc_mbox_chan
+ *              that will be populated with the channel data.
+ *
+ * Return: 0 upon success or non-zero upon error.
+ */
+int
+pcc_mbox_query_channel(struct pcc_mbox_chan *q_chan, int subspace_id)
+{
+	struct pcc_mbox_chan *pcc_mchan;
+	struct pcc_chan_info *pchan;
+	struct mbox_chan *chan;
+
+	if (subspace_id < 0 || subspace_id >= pcc_chan_count)
+		return -ENOENT;
+	pchan = chan_info + subspace_id;
+	chan = pchan->chan.mchan;
+	if (IS_ERR(chan)) {
+		pr_err("Channel not found for idx: %d\n", subspace_id);
+		return -EBUSY;
+	}
+	pcc_mchan = &pchan->chan;
+
+	q_chan->shmem_base_addr = pcc_mchan->shmem_base_addr;
+	q_chan->shmem = NULL;
+	q_chan->shmem_size = pcc_mchan->shmem_size;
+	q_chan->latency = pcc_mchan->latency;
+	q_chan->max_access_rate = pcc_mchan->max_access_rate;
+	q_chan->min_turnaround_time = pcc_mchan->min_turnaround_time;
+	q_chan->type = pcc_mchan->type;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pcc_mbox_query_channel);
+
 /**
  * pcc_mbox_request_channel - PCC clients call this function to
  *		request a pointer to their PCC subspace, from which they
@@ -834,6 +872,7 @@ static int pcc_mbox_probe(struct platform_device *pdev)
 		pcc_parse_subspace_shmem(pchan, pcct_entry);
 
 		pchan->type = pcct_entry->type;
+		pchan->chan.type = pcct_entry->type;
 		pcct_entry = (struct acpi_subtable_header *)
 			((unsigned long) pcct_entry + pcct_entry->length);
 	}
diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
index 840bfc95bae3..8d0fada6e31f 100644
--- a/include/acpi/pcc.h
+++ b/include/acpi/pcc.h
@@ -8,6 +8,10 @@
 
 #include <linux/mailbox_controller.h>
 #include <linux/mailbox_client.h>
+#include <linux/acpi.h>
+//#include <acpi/actypes.h>
+//#include <acpi/actbl.h>
+//#include <acpi/actbl2.h>
 
 struct pcc_mbox_chan {
 	struct mbox_chan *mchan;
@@ -17,6 +21,7 @@ struct pcc_mbox_chan {
 	u32 latency;
 	u32 max_access_rate;
 	u16 min_turnaround_time;
+	enum acpi_pcct_type type;
 };
 
 /* Generic Communications Channel Shared Memory Region */
@@ -37,6 +42,8 @@ struct pcc_mbox_chan {
 extern struct pcc_mbox_chan *
 pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id);
 extern void pcc_mbox_free_channel(struct pcc_mbox_chan *chan);
+extern int
+pcc_mbox_query_channel(struct pcc_mbox_chan *q_chan, int subspace_id);
 #else
 static inline struct pcc_mbox_chan *
 pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
@@ -44,6 +51,11 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
 	return ERR_PTR(-ENODEV);
 }
 static inline void pcc_mbox_free_channel(struct pcc_mbox_chan *chan) { }
+static inline int
+pcc_mbox_query_channel(struct pcc_mbox_chan *q_chan, int subspace_id)
+{
+	return -ENODEV;
+}
 #endif
 
 #endif /* _PCC_H */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v01] mailbox/pcc.c:  add query channel function
  2026-06-04 20:37 [PATCH v01] mailbox/pcc.c: add query channel function Adam Young
@ 2026-06-04 20:49 ` sashiko-bot
  2026-06-05  6:54 ` kernel test robot
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-06-04 20:49 UTC (permalink / raw)
  To: Adam Young; +Cc: linux-hwmon

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Medium] Uninitialized `mchan` field in the user-provided output structure.
- [Low] Dead code in `IS_ERR(chan)` check provides a false sense of security.
- [Low] Commented-out `#include` directives left in a public header file.

Pre-existing issues:
- [High] Use-After-Free (UAF) vulnerabilities due to missing driver teardown on unbind.
--

commit a70542fb899e92a284eaf8ce05841d6088a0dece
Author: Adam Young <admiyo@os.amperecomputing.com>

mailbox/pcc.c: add query channel function

This commit adds a new function to allow drivers to query PCC channel
information before creating a channel. It also exposes the PCC channel type
to the client by adding it to the channel records.

> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 636879ae1db76..30577c6beefaa 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
[ ... ]
> +int
> +pcc_mbox_query_channel(struct pcc_mbox_chan *q_chan, int subspace_id)
> +{
> +	struct pcc_mbox_chan *pcc_mchan;
> +	struct pcc_chan_info *pchan;
> +	struct mbox_chan *chan;
> +
> +	if (subspace_id < 0 || subspace_id >= pcc_chan_count)
> +		return -ENOENT;
> +	pchan = chan_info + subspace_id;
> +	chan = pchan->chan.mchan;
> +	if (IS_ERR(chan)) {

[Severity: Low]
Is this check effective? Since chan is initialized in pcc_mbox_probe() as the
address of an element within a devm_kcalloc allocated array
(&pcc_mbox_channels[i]), it seems it can never be an ERR_PTR. Also, IS_ERR()
would evaluate to false for a NULL pointer.

> +		pr_err("Channel not found for idx: %d\n", subspace_id);
> +		return -EBUSY;
> +	}
> +	pcc_mchan = &pchan->chan;
> +
> +	q_chan->shmem_base_addr = pcc_mchan->shmem_base_addr;
> +	q_chan->shmem = NULL;
> +	q_chan->shmem_size = pcc_mchan->shmem_size;
> +	q_chan->latency = pcc_mchan->latency;
> +	q_chan->max_access_rate = pcc_mchan->max_access_rate;
> +	q_chan->min_turnaround_time = pcc_mchan->min_turnaround_time;
> +	q_chan->type = pcc_mchan->type;

[Severity: Medium]
Does this code leave the mchan field uninitialized? If a caller allocates
q_chan on the stack without zero-initializing it, the mchan field will retain
uninitialized stack memory.

If the caller subsequently checks mchan or mistakenly passes the structure to
pcc_mbox_free_channel(), could this bypass null-checks and cause arbitrary
pointer dereferences?

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(pcc_mbox_query_channel);

[Severity: High]
This is a pre-existing issue, but could this lead to a use-after-free on
driver unbind?

The platform driver lacks a .remove callback and does not set
.suppress_bind_attrs = true:

static struct platform_driver pcc_mbox_driver = {
	.probe = pcc_mbox_probe,
	.driver = {
		.name = "PCCT",
	},
};

If the driver is unbound via sysfs, the driver core automatically frees all
devm_-managed memory, including pcc_mbox_ctrl, pcc_mbox_channels, and
chan_info.

However, pcc_mbox_probe() uses mbox_controller_register() instead of
devm_mbox_controller_register():

	rc = mbox_controller_register(pcc_mbox_ctrl);

This leaves the controller registered in the mailbox framework. Subsequent
attempts to request or query a channel would then access the freed
chan_info or pcc_mbox_ctrl.

[ ... ]
> diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
> index 840bfc95bae33..8d0fada6e31fb 100644
> --- a/include/acpi/pcc.h
> +++ b/include/acpi/pcc.h
> @@ -8,6 +8,10 @@
>  
>  #include <linux/mailbox_controller.h>
>  #include <linux/mailbox_client.h>
> +#include <linux/acpi.h>
> +//#include <acpi/actypes.h>
> +//#include <acpi/actbl.h>
> +//#include <acpi/actbl2.h>

[Severity: Low]
Are these commented-out include directives intended to be kept? Leaving dead
code in a public header file can clutter the API.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604203749.168752-1-admiyo@os.amperecomputing.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v01] mailbox/pcc.c:  add query channel function
  2026-06-04 20:37 [PATCH v01] mailbox/pcc.c: add query channel function Adam Young
  2026-06-04 20:49 ` sashiko-bot
@ 2026-06-05  6:54 ` kernel test robot
  2026-06-05  7:55 ` Sudeep Holla
  2026-06-05 14:39 ` kernel test robot
  3 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2026-06-05  6:54 UTC (permalink / raw)
  To: Adam Young, Sudeep Holla, Jassi Brar, Rafael J. Wysocki,
	Saket Dumbre, Len Brown
  Cc: llvm, oe-kbuild-all, linux-kernel, linux-hwmon, linux-acpi,
	Andi Shyti, Guenter Roeck, Huisong Li, MyungJoo Ham,
	Kyungmin Park, Chanwoo Choi, linux-arm-kernel

Hi Adam,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jassibrar-mailbox/for-next]
[also build test WARNING on rafael-pm/linux-next rafael-pm/bleeding-edge soc/for-next linus/master v6.16-rc1 next-20260604]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Adam-Young/mailbox-pcc-c-add-query-channel-function/20260605-044323
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jassibrar/mailbox.git for-next
patch link:    https://lore.kernel.org/r/20260604203749.168752-1-admiyo%40os.amperecomputing.com
patch subject: [PATCH v01] mailbox/pcc.c:  add query channel function
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260605/202606050825.2uv1FZrY-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project f43d6834093b19baf79beda8c0337ab020ac5f17)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260605/202606050825.2uv1FZrY-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202606050825.2uv1FZrY-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/mailbox/pcc.c:358 function parameter 'q_chan' not described in 'pcc_mbox_query_channel'
>> Warning: drivers/mailbox/pcc.c:358 function parameter 'subspace_id' not described in 'pcc_mbox_query_channel'
>> Warning: drivers/mailbox/pcc.c:358 function parameter 'q_chan' not described in 'pcc_mbox_query_channel'
>> Warning: drivers/mailbox/pcc.c:358 function parameter 'subspace_id' not described in 'pcc_mbox_query_channel'

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v01] mailbox/pcc.c:  add query channel function
  2026-06-04 20:37 [PATCH v01] mailbox/pcc.c: add query channel function Adam Young
  2026-06-04 20:49 ` sashiko-bot
  2026-06-05  6:54 ` kernel test robot
@ 2026-06-05  7:55 ` Sudeep Holla
  2026-06-05 14:19   ` Adam Young
  2026-06-05 14:39 ` kernel test robot
  3 siblings, 1 reply; 6+ messages in thread
From: Sudeep Holla @ 2026-06-05  7:55 UTC (permalink / raw)
  To: Adam Young
  Cc: Jassi Brar, Rafael J. Wysocki, Saket Dumbre, Len Brown,
	Sudeep Holla, linux-kernel, linux-hwmon, linux-acpi, Andi Shyti,
	Guenter Roeck, Huisong Li, MyungJoo Ham, Kyungmin Park,
	Chanwoo Choi, linux-arm-kernel

On Thu, Jun 04, 2026 at 04:37:48PM -0400, Adam Young wrote:
> Drivers need information about a channel prior to creating a channel
> or they risk triggering message delivery on the remote side of a
> connection.
> 
> One of those pieces of infomration is the type of channel.
> 
> Add PCC channel type to records and expose PCC channel type to client.
> 

Please point me to the user of this interface.

-- 
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v01] mailbox/pcc.c: add query channel function
  2026-06-05  7:55 ` Sudeep Holla
@ 2026-06-05 14:19   ` Adam Young
  0 siblings, 0 replies; 6+ messages in thread
From: Adam Young @ 2026-06-05 14:19 UTC (permalink / raw)
  To: Sudeep Holla, Adam Young
  Cc: Jassi Brar, Rafael J. Wysocki, Saket Dumbre, Len Brown,
	linux-kernel, linux-hwmon, linux-acpi, Andi Shyti, Guenter Roeck,
	Huisong Li, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	linux-arm-kernel

MCTP-PCC.  I was not sure if I should ad the Patch to this one or not, 
as the MCTP-PCC is still going through review.

Essentially, it need two pieces of information before start up. 1, it 
needs to be able to check the sizes of the two shared memory buffers in 
order to set the MTU.  2, it confirms that the Channels are indeed Type3 
and Type4.

The base patch for that is


https://lore.kernel.org/lkml/20260522193610.234166-1-admiyo@os.amperecomputing.com/

This is the diff that would use them.


commit c02159450cf38cb778105f10a11708d358a3b633 
(admiyo/mctp-pcc-v45-net-next, mctp-pcc-v45-net-next)
Author: Adam Young <admiyo@os.amperecomputing.com>
Date:   Thu Apr 30 10:48:41 2026 -0700

     mctp-pcc query channel information without opening channel.

     Opening the channel can trigger the sending of
     a message from the remote side before the driver
     is ready to read it.  Take advantage of the API that allows
     querying of the channel data without opening the channel.

diff --git a/drivers/net/mctp/mctp-pcc.c b/drivers/net/mctp/mctp-pcc.c
index bb5d53ee3d7c..01a9966eddd2 100644
--- a/drivers/net/mctp/mctp-pcc.c
+++ b/drivers/net/mctp/mctp-pcc.c
@@ -318,25 +318,50 @@ static void mctp_cleanup_netdev(void *data)
         mctp_unregister_netdev(ndev);
  }

+static int check_channel_types(struct mctp_pcc_ndev *mctp_pcc_ndev)
+{
+       struct mctp_pcc_mailbox *outbox;
+       struct mctp_pcc_mailbox *inbox;
+       struct pcc_mbox_chan chan;
+       int actual_type;
+
+       outbox = &mctp_pcc_ndev->outbox;
+        if (pcc_mbox_query_channel(&chan, outbox->index))
+               return -EINVAL;
+       actual_type = chan.type;
+       if (actual_type != ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE){
+               pr_err("MCTP-PCC outbox channel wrong type: 
%d",actual_type);
+               return -EINVAL;
+       }
+
+       inbox = &mctp_pcc_ndev->inbox;
+        if (pcc_mbox_query_channel(&chan, inbox->index))
+               return -EINVAL;
+       actual_type = chan.type;
+       if (actual_type != ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE){
+               pr_err("MCTP-PCC inbox channel wrong type: %d",actual_type);
+               return -EINVAL;
+       }
+
+       return 0;
+}
+
  static int initialize_mtu(struct net_device *ndev)
  {
         struct mctp_pcc_ndev *mctp_pcc_ndev;
         struct mctp_pcc_mailbox *outbox;
         struct pcc_mbox_chan *pchan;
+       struct pcc_mbox_chan chan;
         int mctp_pcc_max_mtu;

         mctp_pcc_ndev = netdev_priv(ndev);
         outbox = &mctp_pcc_ndev->outbox;
-       pchan = pcc_mbox_request_channel(&outbox->client, outbox->index);
-       if (IS_ERR(pchan))
-               return PTR_ERR(pchan);
-       if (pchan->shmem_size < MCTP_MIN_MTU + sizeof(struct 
acpi_pcct_ext_pcc_shared_memory)) {
-               pcc_mbox_free_channel(pchan);
+       if (pcc_mbox_query_channel(&chan, outbox->index))
+               return -EINVAL;
+       if (chan.shmem_size < MCTP_MIN_MTU + sizeof(struct 
acpi_pcct_ext_pcc_shared_memory)) {
                 return -EINVAL;
         }
-       mctp_pcc_max_mtu = pchan->shmem_size - sizeof(struct 
acpi_pcct_ext_pcc_shared_memory);
-       pcc_mbox_free_channel(pchan);
-
+       mctp_pcc_max_mtu = chan.shmem_size - sizeof(struct 
acpi_pcct_ext_pcc_shared_memory);
         ndev->mtu = MCTP_MIN_MTU;
         ndev->max_mtu = mctp_pcc_max_mtu;
         ndev->min_mtu = MCTP_MIN_MTU;
@@ -378,7 +403,6 @@ static int mctp_pcc_driver_add(struct acpi_device 
*acpi_dev)
                 return -ENOMEM;

         mctp_pcc_ndev = netdev_priv(ndev);
-
         mctp_pcc_ndev->inbox.index = context.inbox_index;
         mctp_pcc_ndev->inbox.client.dev = dev;
         mctp_pcc_ndev->outbox.index = context.outbox_index;



On 6/5/26 03:55, Sudeep Holla wrote:
> On Thu, Jun 04, 2026 at 04:37:48PM -0400, Adam Young wrote:
>> Drivers need information about a channel prior to creating a channel
>> or they risk triggering message delivery on the remote side of a
>> connection.
>>
>> One of those pieces of infomration is the type of channel.
>>
>> Add PCC channel type to records and expose PCC channel type to client.
>>
> Please point me to the user of this interface.
>

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v01] mailbox/pcc.c:  add query channel function
  2026-06-04 20:37 [PATCH v01] mailbox/pcc.c: add query channel function Adam Young
                   ` (2 preceding siblings ...)
  2026-06-05  7:55 ` Sudeep Holla
@ 2026-06-05 14:39 ` kernel test robot
  3 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2026-06-05 14:39 UTC (permalink / raw)
  To: Adam Young, Sudeep Holla, Jassi Brar, Rafael J. Wysocki,
	Saket Dumbre, Len Brown
  Cc: llvm, oe-kbuild-all, linux-kernel, linux-hwmon, linux-acpi,
	Andi Shyti, Guenter Roeck, Huisong Li, MyungJoo Ham,
	Kyungmin Park, Chanwoo Choi, linux-arm-kernel

Hi Adam,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jassibrar-mailbox/for-next]
[also build test WARNING on rafael-pm/linux-next rafael-pm/bleeding-edge soc/for-next linus/master v7.1-rc6 next-20260603]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Adam-Young/mailbox-pcc-c-add-query-channel-function/20260605-044323
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jassibrar/mailbox.git for-next
patch link:    https://lore.kernel.org/r/20260604203749.168752-1-admiyo%40os.amperecomputing.com
patch subject: [PATCH v01] mailbox/pcc.c:  add query channel function
config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20260605/202606052232.LAlOTWJr-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 7917772d7d61384696c61102c08c2ea158e610fa)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260605/202606052232.LAlOTWJr-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202606052232.LAlOTWJr-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/mailbox/pcc.c:358 function parameter 'q_chan' not described in 'pcc_mbox_query_channel'
>> Warning: drivers/mailbox/pcc.c:358 function parameter 'subspace_id' not described in 'pcc_mbox_query_channel'
>> Warning: drivers/mailbox/pcc.c:358 function parameter 'q_chan' not described in 'pcc_mbox_query_channel'
>> Warning: drivers/mailbox/pcc.c:358 function parameter 'subspace_id' not described in 'pcc_mbox_query_channel'

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-05 14:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-04 20:37 [PATCH v01] mailbox/pcc.c: add query channel function Adam Young
2026-06-04 20:49 ` sashiko-bot
2026-06-05  6:54 ` kernel test robot
2026-06-05  7:55 ` Sudeep Holla
2026-06-05 14:19   ` Adam Young
2026-06-05 14:39 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox