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
| ||
|
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