| Message ID | 20230201101559.15529-3-johan+linaro@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e0e7089bf9a87bc5e3997422e4e24563424f9018 |
| Headers | show |
| Series | interconnect: fix racy provider registration | expand |
On 1.02.2023 11:15, Johan Hovold wrote: > The interconnect framework currently expects that providers are only > removed when there are no users and after all nodes have been removed. > > There is currently nothing that guarantees this to be the case and the > framework does not do any reference counting, but refusing to remove the > provider is never correct as that would leave a dangling pointer to a > resource that is about to be released in the global provider list (e.g. > accessible through debugfs). > > Replace the current sanity checks with WARN_ON() so that the provider is > always removed. I spent a considerable amount of time scratching my head what WARN_ON has to do with removing list items.. I suppose "don't return early and replace pr_warn with WARN_ON" would have been clearer, but maybe that's just me in the morning.. > > Fixes: 11f1ceca7031 ("interconnect: Add generic on-chip interconnect API") > Cc: stable@vger.kernel.org # 5.1: 680f8666baf6: interconnect: Make icc_provider_del() return void > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > drivers/interconnect/core.c | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c > index dc61620a0191..43c5c8503ee8 100644 > --- a/drivers/interconnect/core.c > +++ b/drivers/interconnect/core.c > @@ -1062,18 +1062,8 @@ EXPORT_SYMBOL_GPL(icc_provider_add); > void icc_provider_del(struct icc_provider *provider) > { > mutex_lock(&icc_lock); > - if (provider->users) { > - pr_warn("interconnect provider still has %d users\n", > - provider->users); > - mutex_unlock(&icc_lock); > - return; > - } > - > - if (!list_empty(&provider->nodes)) { > - pr_warn("interconnect provider still has nodes\n"); > - mutex_unlock(&icc_lock); > - return; > - } > + WARN_ON(provider->users); > + WARN_ON(!list_empty(&provider->nodes)); > > list_del(&provider->provider_list); > mutex_unlock(&icc_lock);
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index dc61620a0191..43c5c8503ee8 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -1062,18 +1062,8 @@ EXPORT_SYMBOL_GPL(icc_provider_add); void icc_provider_del(struct icc_provider *provider) { mutex_lock(&icc_lock); - if (provider->users) { - pr_warn("interconnect provider still has %d users\n", - provider->users); - mutex_unlock(&icc_lock); - return; - } - - if (!list_empty(&provider->nodes)) { - pr_warn("interconnect provider still has nodes\n"); - mutex_unlock(&icc_lock); - return; - } + WARN_ON(provider->users); + WARN_ON(!list_empty(&provider->nodes)); list_del(&provider->provider_list); mutex_unlock(&icc_lock);
The interconnect framework currently expects that providers are only removed when there are no users and after all nodes have been removed. There is currently nothing that guarantees this to be the case and the framework does not do any reference counting, but refusing to remove the provider is never correct as that would leave a dangling pointer to a resource that is about to be released in the global provider list (e.g. accessible through debugfs). Replace the current sanity checks with WARN_ON() so that the provider is always removed. Fixes: 11f1ceca7031 ("interconnect: Add generic on-chip interconnect API") Cc: stable@vger.kernel.org # 5.1: 680f8666baf6: interconnect: Make icc_provider_del() return void Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/interconnect/core.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)