linux-kernel - Re: [PATCH] Revert "mailbox/pcc: support mailbox management of the shared buffer"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251001-masterful-benevolent-dolphin-a3fbea@sudeepholla>
Date: Wed, 1 Oct 2025 12:57:21 +0100
From: Sudeep Holla <sudeep.holla@....com>
To: Adam Young <admiyo@...eremail.onmicrosoft.com>
Cc: Jassi Brar <jassisinghbrar@...il.com>,
	Sudeep Holla <sudeep.holla@....com>,
	Adam Young <admiyo@...amperecomputing.com>,
	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Revert "mailbox/pcc: support mailbox management of the
 shared buffer"

On Wed, Oct 01, 2025 at 01:25:42AM -0400, Adam Young wrote:
> 
> On 9/29/25 20:19, Jassi Brar wrote:
> > On Mon, Sep 29, 2025 at 12:11 PM Adam Young
> > <admiyo@...eremail.onmicrosoft.com> wrote:
> > > I posted a patch that addresses a few of these issues.  Here is a top
> > > level description of the isse
> > > 
> > > 
> > > The correct way to use the mailbox API would be to allocate a buffer for
> > > the message,write the message to that buffer, and pass it in to
> > > mbox_send_message.  The abstraction is designed to then provide
> > > sequential access to the shared resource in order to send the messages
> > > in order.  The existing PCC Mailbox implementation violated this
> > > abstraction.  It requires each individual driver re-implement all of the
> > > sequential ordering to access the shared buffer.
> > > 
> > > Why? Because they are all type 2 drivers, and the shared buffer is
> > > 64bits in length:  32bits for signature, 16 bits for command, 16 bits
> > > for status.  It would be execessive to kmalloc a buffer of this size.
> > > 
> > > This shows the shortcoming of the mailbox API.  The mailbox API assumes
> > > that there is a large enough buffer passed in to only provide a void *
> > > pointer to the message.  Since the value is small enough to fit into a
> > > single register, it the mailbox abstraction could provide an
> > > implementation that stored a union of a void * and word.
> > > 
> > Mailbox api does not make assumptions about the format of message
> > hence it simply asks for void*.
> > Probably I don't understand your requirement, but why can't you pass the pointer
> > to the 'word' you want to use otherwise?
> > 
> > -jassi
> The mbox_send_message call will then take the pointer value that you give it
> and put it in a ring buffer.  The function then returns, and the value may
> be popped off the stack before the message is actually sent.  In practice we
> don't see this because much of the code that calls it is blocking code, so
> the value stays on the stack until it is read.  Or, in the case of the PCC
> mailbox, the value is never read or used.  But, as the API is designed, the
> memory passed into to the function should expect to live longer than the
> function call, and should not be allocated on the stack.

I’m still not clear on what exactly you are looking for. Let’s look at
mbox_send_message(). It adds the provided data pointer to the queue, and then
passes the same pointer to tx_prepare() just before calling send_data(). This
is what I’ve been pointing out that you can obtain the buffer pointer there and
use it to update the shared memory in the client driver.

-- 
Regards,
Sudeep