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
| ||
|
Date: Wed, 25 Nov 2020 10:09:11 -0800 From: rishabhb@...eaurora.org To: Bjorn Andersson <bjorn.andersson@...aro.org> Cc: Andy Gross <agross@...nel.org>, Ohad Ben-Cohen <ohad@...ery.com>, Siddharth Gupta <sidgup@...eaurora.org>, Mathieu Poirier <mathieu.poirier@...aro.org>, linux-arm-msm@...r.kernel.org, linux-remoteproc@...r.kernel.org, linux-kernel@...r.kernel.org, stable@...r.kernel.org Subject: Re: [PATCH v3 1/4] remoteproc: sysmon: Ensure remote notification ordering On 2020-11-21 21:41, Bjorn Andersson wrote: > The reliance on the remoteproc's state for determining when to send > sysmon notifications to a remote processor is racy with regard to > concurrent remoteproc operations. > > Further more the advertisement of the state of other remote processor > to > a newly started remote processor might not only send the wrong state, > but might result in a stream of state changes that are out of order. > > Address this by introducing state tracking within the sysmon instances > themselves and extend the locking to ensure that the notifications are > consistent with this state. > > Fixes: 1f36ab3f6e3b ("remoteproc: sysmon: Inform current rproc about > all active rprocs") > Fixes: 1877f54f75ad ("remoteproc: sysmon: Add notifications for > events") > Fixes: 1fb82ee806d1 ("remoteproc: qcom: Introduce sysmon") > Cc: stable@...r.kernel.org > Signed-off-by: Bjorn Andersson <bjorn.andersson@...aro.org> > --- > > Changes since v2: > - Hold sysmon_lock during traversal of sysmons in sysmon_start() > > drivers/remoteproc/qcom_sysmon.c | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > diff --git a/drivers/remoteproc/qcom_sysmon.c > b/drivers/remoteproc/qcom_sysmon.c > index 9eb2f6bccea6..b37b111b15b3 100644 > --- a/drivers/remoteproc/qcom_sysmon.c > +++ b/drivers/remoteproc/qcom_sysmon.c > @@ -22,6 +22,9 @@ struct qcom_sysmon { > struct rproc_subdev subdev; > struct rproc *rproc; > > + int state; > + struct mutex state_lock; > + > struct list_head node; > > const char *name; > @@ -448,7 +451,10 @@ static int sysmon_prepare(struct rproc_subdev > *subdev) > .ssr_event = SSCTL_SSR_EVENT_BEFORE_POWERUP > }; > > + mutex_lock(&sysmon->state_lock); > + sysmon->state = SSCTL_SSR_EVENT_BEFORE_POWERUP; > blocking_notifier_call_chain(&sysmon_notifiers, 0, (void *)&event); > + mutex_unlock(&sysmon->state_lock); > > return 0; > } > @@ -472,20 +478,25 @@ static int sysmon_start(struct rproc_subdev > *subdev) > .ssr_event = SSCTL_SSR_EVENT_AFTER_POWERUP > }; > > + mutex_lock(&sysmon->state_lock); > + sysmon->state = SSCTL_SSR_EVENT_AFTER_POWERUP; > blocking_notifier_call_chain(&sysmon_notifiers, 0, (void *)&event); > + mutex_unlock(&sysmon->state_lock); > > mutex_lock(&sysmon_lock); > list_for_each_entry(target, &sysmon_list, node) { > - if (target == sysmon || > - target->rproc->state != RPROC_RUNNING) > + if (target == sysmon) > continue; > > + mutex_lock(&target->state_lock); > event.subsys_name = target->name; > + event.ssr_event = target->state; > > if (sysmon->ssctl_version == 2) > ssctl_send_event(sysmon, &event); > else if (sysmon->ept) > sysmon_send_event(sysmon, &event); > + mutex_unlock(&target->state_lock); > } > mutex_unlock(&sysmon_lock); > > @@ -500,7 +511,10 @@ static void sysmon_stop(struct rproc_subdev > *subdev, bool crashed) > .ssr_event = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN > }; > > + mutex_lock(&sysmon->state_lock); > + sysmon->state = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN; > blocking_notifier_call_chain(&sysmon_notifiers, 0, (void *)&event); > + mutex_unlock(&sysmon->state_lock); > > /* Don't request graceful shutdown if we've crashed */ > if (crashed) > @@ -521,7 +535,10 @@ static void sysmon_unprepare(struct rproc_subdev > *subdev) > .ssr_event = SSCTL_SSR_EVENT_AFTER_SHUTDOWN > }; > > + mutex_lock(&sysmon->state_lock); > + sysmon->state = SSCTL_SSR_EVENT_AFTER_SHUTDOWN; > blocking_notifier_call_chain(&sysmon_notifiers, 0, (void *)&event); > + mutex_unlock(&sysmon->state_lock); > } > > /** > @@ -534,11 +551,10 @@ static int sysmon_notify(struct notifier_block > *nb, unsigned long event, > void *data) > { > struct qcom_sysmon *sysmon = container_of(nb, struct qcom_sysmon, > nb); > - struct rproc *rproc = sysmon->rproc; > struct sysmon_event *sysmon_event = data; > > /* Skip non-running rprocs and the originating instance */ > - if (rproc->state != RPROC_RUNNING || > + if (sysmon->state != SSCTL_SSR_EVENT_AFTER_POWERUP || > !strcmp(sysmon_event->subsys_name, sysmon->name)) { > dev_dbg(sysmon->dev, "not notifying %s\n", sysmon->name); > return NOTIFY_DONE; > @@ -591,6 +607,7 @@ struct qcom_sysmon *qcom_add_sysmon_subdev(struct > rproc *rproc, > init_completion(&sysmon->ind_comp); > init_completion(&sysmon->shutdown_comp); > mutex_init(&sysmon->lock); > + mutex_init(&sysmon->state_lock); > > sysmon->shutdown_irq = of_irq_get_byname(sysmon->dev->of_node, > "shutdown-ack"); Reviewed-by: Rishabh Bhatnagar <rishabhb@...eaurora.org>
Powered by blists - more mailing lists