Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> 於 2022年12月13日 週二 晚上10:29寫道： > > Hi, > > On 13.12.2022 15:19, ChiYuan Huang wrote: > > Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> 於 2022年12月13日 週二 晚上7:33寫道： > >> On 06.12.2022 08:22, cy_huang wrote: > >>> From: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > >>> > >>> Following by the below discussion, there's the potential UAF issue > >>> between regulator and mfd. > >>> https://lore.kernel.org/all/20221128143601.1698148-1-yangyingliang@xxxxxxxxxx/ > >>> > >>> >From the analysis of Yingliang > >>> > >>> CPU A |CPU B > >>> mt6370_probe() | > >>> devm_mfd_add_devices() | > >>> |mt6370_regulator_probe() > >>> | regulator_register() > >>> | //allocate init_data and add it to devres > >>> | regulator_of_get_init_data() > >>> i2c_unregister_device() | > >>> device_del() | > >>> devres_release_all() | > >>> // init_data is freed | > >>> release_nodes() | > >>> | // using init_data causes UAF > >>> | regulator_register() > >>> > >>> It's common to use mfd core to create child device for the regulator. > >>> In order to do the DT lookup for init data, the child that registered > >>> the regulator would pass its parent as the parameter. And this causes > >>> init data resource allocated to its parent, not itself. The issue happen > >>> when parent device is going to release and regulator core is still doing > >>> some operation of init data constraint for the regulator of child device. > >>> > >>> To fix it, this patch expand 'regulator_register' API to use the > >>> different devices for init data allocation and DT lookup. > >>> > >>> Reported-by: Yang Yingliang <yangyingliang@xxxxxxxxxx> > >>> Signed-off-by: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > >> > >> This patch landed in linux-next 202212 as commit 8f3cbcd6b440 > >> ("regulator: core: Use different devices for resource allocation and DT > >> lookup"). Unfortunately it causes serious regression on my test systems. > >> It looks that some supplies are not resolved correctly and then turned > >> off as 'unused', even if they provide power to other core regulators in > >> the system. I've observed this issue on Samsung Chromebook Peach-Pit and > >> Peach-Pi (ARM 32bit Exynos based). The symptoms are somehow similar to > >> the issue reported here some time ago: > >> > >> https://lore.kernel.org/all/58b92e75-f373-dae7-7031-8abd465bb874@xxxxxxxxxxx/ > >> > >> I've post more information once I analyze this issue further. > >> > > It seems the issue occurs in 'regulator register' resolve supply. > > Due to the parent device don't have the of_node, to resolve the > > supply, it may need to get the > > dt node by recursively finding child regulator. > > Like this > > parent { > > regulators { > > xxx-supply = <&vdd12-ldo>; > > vdd12-ldo: vdd12-ldo { > > regulator-name = "xxx"; > > regulator-min-microvolts = <xxxxx>; > > regulator-max-microvolts = <xxxxx>; > > } > > }; > > }; > > >From this case, 'resolve supply' need to parse at least the more top > > level like 'regulators'. > > But now, it only take 'vdd12-ldo' as the node or its child to parse its supply. > > > > Below's the fix I guess. > > It'll make the parent of the regulator the same as the dev parameter > > in 'regulator_config'. > > > > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c > > index ea4a720..7c5036e 100644 > > --- a/drivers/regulator/core.c > > +++ b/drivers/regulator/core.c > > @@ -5526,7 +5526,7 @@ regulator_register(struct device *dev, > > > > /* register with sysfs */ > > rdev->dev.class = ®ulator_class; > > - rdev->dev.parent = dev; > > + rdev->dev.parent = config->dev; > > dev_set_name(&rdev->dev, "regulator.%lu", > > (unsigned long) atomic_inc_return(®ulator_no)); > > dev_set_drvdata(&rdev->dev, rdev); > > > > I don't have the board. Could you help to test this change to see > > whether it's been fixed or not? > > The above change fixes the issue. Thanks! Feel free to add following > tags to the final patch: > > Reported-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> > > Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> > Thanks. I'll submit the change to fix it. > > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland >
