linux-kernel - Re: [PATCH v2 1/2] mailbox: check mailbox queue is full or not

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] [day] [month] [year] [list]

Message-ID: <CABb+yY0qZktWThE82RppmCN1cC=UvKkKp-F3T1ydwiUfyOZGkw@mail.gmail.com>
Date: Wed, 3 Dec 2025 20:13:37 -0600
From: Jassi Brar <jassisinghbrar@...il.com>
To: tanmay.shah@....com
Cc: andersson@...nel.org, mathieu.poirier@...aro.org, 
	linux-kernel@...r.kernel.org, linux-remoteproc@...r.kernel.org
Subject: Re: [PATCH v2 1/2] mailbox: check mailbox queue is full or not

On Tue, Nov 18, 2025 at 12:51 PM Tanmay Shah <tanmay.shah@....com> wrote:
> On 11/15/25 11:38 AM, Jassi Brar wrote:
> > On Wed, Oct 15, 2025 at 10:27 AM Tanmay Shah <tanmay.shah@....com> wrote:
> >>>>> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> >>>>> index 5cd8ae222073..c2e187aa5d22 100644
> >>>>> --- a/drivers/mailbox/mailbox.c
> >>>>> +++ b/drivers/mailbox/mailbox.c
> >>>>> @@ -35,6 +35,7 @@ static int add_to_rbuf(struct mbox_chan *chan, void
> >>>>> *mssg)
> >>>>>           idx = chan->msg_free;
> >>>>>           chan->msg_data[idx] = mssg;
> >>>>>           chan->msg_count++;
> >>>>> +       chan->msg_slot_ro = (MBOX_TX_QUEUE_LEN - chan->msg_count);
> >>>>>
> >>>>>           if (idx == MBOX_TX_QUEUE_LEN - 1)
> >>>>>                   chan->msg_free = 0;
> >>>>> @@ -70,6 +71,7 @@ static void msg_submit(struct mbox_chan *chan)
> >>>>>                   if (!err) {
> >>>>>                           chan->active_req = data;
> >>>>>                           chan->msg_count--;
> >>>>> +                       chan->msg_slot_ro = (MBOX_TX_QUEUE_LEN -
> >>>>> chan->msg_count);
> >>>>>
> >>>> No, I had suggested adding this info in client structure.
> >>>> There is no point in carrying msg_count and msg_slot_ro in mbox_chan.
> >>>> The client needs this info but can/should not access the chan internals.
> >>>>
> >>>
> >>> Hi Jassi,
> >>>
> >>> If I move msg_slot_ro to mbox_client that means, each channel needs its
> >>> own client structure. But it is possible to map single client to
> >>> multiple channels.
> >>>
> >>> How about if I rename msg_slot_ro to msg_slot_tx_ro -> that says this
> >>> field should be used only for "tx" channel.
> >>>
> >>> Or is it must to map unique client data structure to each channel? If
> >>> so, can I update mbox_client structure documentation ?
> >>>
> >>
> >> Hi Jassi,
> >>
> >> I looked into this further. Looks like it's not possible to introduce
> >> msg_slot_ro in the client data structure as of today. Because multiple
> >> drivers are sharing same client for "tx" and "rx" both channels.
> >> [references: 1, 2, 3]
> >>
> >> so, msg_slot_ro won't be calculated correctly I believe.
> >>
> > I don't see it. Can you please explain how the calculated value could be wrong?
> >
>
> Hi Jassi,
>
> so on my platform I introduced some extra logs:
>
> [   80.827479] mbox chan Rx send message
> [   80.827485] add_to_rbuf: &chan->cl->msg_slot_ro = 00000000ab5fa771,
> msg slot ro = 19
> [   80.827494] msg_submit: &chan->cl->msg_slot_ro = 00000000ab5fa771,
> msg slot ro = 20
> [   80.833064] mbox chan Tx send message
> [   80.833070] add_to_rbuf: &chan->cl->msg_slot_ro = 00000000ab5fa771,
> msg slot ro = 19
> [   80.833079] msg_submit: &chan->cl->msg_slot_ro = 00000000ab5fa771,
> msg slot ro = 20
> [   80.837486] mbox chan Rx send message
> [   80.837492] add_to_rbuf: &chan->cl->msg_slot_ro = 00000000ab5fa771,
> msg slot ro = 19
> [   80.837501] msg_submit: &chan->cl->msg_slot_ro = 00000000ab5fa771,
> msg slot ro = 20
>
> Tx and Rx both channels are updating same address of msg_slot_ro. If
> user wants to check msg_slot_ro for Tx channel, then it is possible that
> it was updated by Rx channel instead. Ideally there should be two copies
> of msg_slot_ro, one for Tx and one for Rx channel.
>
> This happens because Tx and Rx both channels shares same client data
> structure.
>
> Same can happen on other platforms as well.
>
The queue is only for TX.
The received data is directly handed to the client. So RX path should
not be modifying that queue availability variable.

thanks