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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c81540cc-e485-0c45-9e4e-248d3279e1ea@quicinc.com>
Date:   Fri, 30 Sep 2022 18:29:02 +0530
From:   Shivnandan Kumar <quic_kshivnan@...cinc.com>
To:     Cristian Marussi <cristian.marussi@....com>
CC:     <sudeep.holla@....com>, <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <quic_rgottimu@...cinc.com>,
        <quic_avajid@...cinc.com>
Subject: Re: Query regarding "firmware: arm_scmi: Free mailbox channels if
 probe fails"

hi Cristian,

Thanks for your support in providing the patch to try.

I found one race condition in our downstream mbox controller driver 
while accessing con_priv, when I serialized access to this, issue is not 
seen on 3 days of testing.

As you rightly mentioned that your provided patch will impact all the 
other users.

Also if  we take your provided patch, same race still exists while 
accessing con_priv in our downstream mbox controller so this issue will 
still be there.

So, we are planning to merge the patch( serialized access to con_priv) 
in our downstream mbox controller now.


Thanks,

Shivnandan


On 9/22/2022 8:58 PM, Cristian Marussi wrote:
> On Thu, Sep 22, 2022 at 03:54:31PM +0100, Cristian Marussi wrote:
>> On Thu, Sep 22, 2022 at 10:31:47AM +0530, Shivnandan Kumar wrote:
>>> Hi Cristian,
>>>
> Hi Shivnandan,
>   
> [snip]
>
>> Looking at the transport layer that you use, mailbox, I see that while
>> setup/free helpers are synchronized on an internal chan->lock, the RX
>> path inside the mailbox core is not, so I tried this:
>>
>>
>> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
>> index 4229b9b5da98..bb6173c0ad54 100644
>> --- a/drivers/mailbox/mailbox.c
>> +++ b/drivers/mailbox/mailbox.c
>> @@ -157,9 +157,13 @@ static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer)
>>    */
>>   void mbox_chan_received_data(struct mbox_chan *chan, void *mssg)
>>   {
>> +       unsigned long flags;
>> +
>> +       spin_lock_irqsave(&chan->lock, flags);
>>          /* No buffering the received data */
>>          if (chan->cl->rx_callback)
>>                  chan->cl->rx_callback(chan->cl, mssg);
>> +       spin_unlock_irqrestore(&chan->lock, flags);
>>   }
>>   EXPORT_SYMBOL_GPL(mbox_chan_received_data);
>>   
> ...sorry... a small change on the tentative above fix...
>
> ----8<------
>
> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> index 4229b9b5da98..6fbe183acdae 100644
> --- a/drivers/mailbox/mailbox.c
> +++ b/drivers/mailbox/mailbox.c
> @@ -157,9 +157,13 @@ static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer)
>    */
>   void mbox_chan_received_data(struct mbox_chan *chan, void *mssg)
>   {
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&chan->lock, flags);
>          /* No buffering the received data */
> -       if (chan->cl->rx_callback)
> +       if (chan->cl && chan->cl->rx_callback)
>                  chan->cl->rx_callback(chan->cl, mssg);
> +       spin_unlock_irqrestore(&chan->lock, flags);
>   }
>   EXPORT_SYMBOL_GPL(mbox_chan_received_data);
>
> ------8<-----
>
> Thanks,
> Cristian
>

Powered by blists - more mailing lists