| 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: Fri, 12 Aug 2016 16:04:02 +0900 From: Masahiro Yamada <yamada.masahiro@...ionext.com> To: Stephen Boyd <sboyd@...eaurora.org> Cc: Rob Herring <robh@...nel.org>, linux-clk <linux-clk@...r.kernel.org>, Michael Turquette <mturquette@...libre.com>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org> Subject: Re: of_clk_add_(hw_)providers multipule times for one node? 2016-08-11 8:08 GMT+09:00 Stephen Boyd <sboyd@...eaurora.org>: > On 08/10, Masahiro Yamada wrote: >> Hi Stephen, >> >> >> >> 2016-08-09 8:37 GMT+09:00 Stephen Boyd <sboyd@...eaurora.org>: >> > On 08/08, Masahiro Yamada wrote: >> >> Hi Stephen, >> >> >> >> >> >> 2016-08-05 6:25 GMT+09:00 Stephen Boyd <sboyd@...eaurora.org>: >> > >> >> >> >> of_clk_add_provider() calls of_clk_del_provider() >> >> in its failure path. >> >> >> >> Notice of_clk_del_provider() unregister >> >> all the providers associated with the device node. >> > >> > Where is that? I see a break statement in the while loop after >> > the first matching np is found. >> >> Ah, I missed the "break". >> >> So, this works *almost* well. >> >> I mean *almost* because the of_clk_mutex is released >> between of_clk_add_hw_provider() and of_clk_del_provider(). >> >> What if two providers are added concurrently. >> I know it never happens in use-cases we assume, though. > > Agreed, that would be bad. We can definitely do better in that > case and properly delete the provider that we have already > registered without calling of_clk_del_provider() though. We have > everything in the local scope at the time. > >> >> >> >> >> >> Some platform drivers call of_clk_del_provider() in a .remove callback, >> >> so the same problem could happen. >> >> >> >> Why does of_clk_del_provider() take (struct device_node *np) ? >> >> Shouldn't it take (struct of_clk_provider *cp)? >> >> >> > >> > Not sure. Probably someone thought they could hide the structure >> > from consumers and just return success or failure. >> >> consumers? or did you mean providers? >> I think consumers have no chance to call of_clk_del_provider(). > > Sorry, bad choice of words. I meant users of this > of_clk_add*_provider() API. > >> >> >> > The best we can do is have the framework only return probe defer >> > if there isn't a provider registered. Once a provider is >> > registered, it needs to do the right thing and return the >> > appropriate error (invalid or probe defer for example) at the >> > right time. >> >> Agreed. > > Ok. I think I will merge my patch then to restore previous > behavior. > >> >> Lastly, we have two solutions so far. Which do you think is better? >> >> One solution is, as others suggested, >> CLK_OF_DECLARE() can allocate a bigger array than it needs, >> so that blank entries can be filled by a platfrom_driver later. >> >> >> The other way is, >> CLK_OF_DECLARE() and a platfrom_driver >> allocate separate of_clk_provider for each of them. >> > > I believe we have precedence for the former case, so there's some > momentum around that approach. It doesn't make me feel great > though because we have published the provider before all clks are > registered, and then we go back and modify the array in place > while consumers could potentially be using it. I suppose we're > saved because cpus access the pointer in the array and only see > the whole pointer and not half of the old one and half of the new > one? I am not sure. But, maybe just filling the blank entries of the array seems safe. In this case, filling should be done at the end of the probe callback. Otherwise, devm_clk_hw_register() will free the clk_hw when the driver is detached. -- Best Regards Masahiro Yamada
Powered by blists - more mailing lists