arm psci_probe() bug

arm psci_probe() bug

[Alexander Shiyan]

Hello.

I found a bug in psci processing.
Currently the kernel writes the following on boot:
psci: probing for conduit method from DT.
psci: invalid "method" property:
...

In the psci_probe() function we do the following:
ret = of_property_read_string(dev->of_node, "method", &method);
and use this (method) variable as an argument for of_psci_fixup().
I suspect we are free this variable at of_property_write_string ->
of_set_property > of_delete_property.

Can you confirm my thought and correct it in the right direction?

Thanks!

Re: arm psci_probe() bug

[Sascha Hauer]

Hi Alexander,

On Thu, Aug 03, 2023 at 11:04:36AM +0300, Alexander Shiyan wrote:
> Hello.
> 
> I found a bug in psci processing.
> Currently the kernel writes the following on boot:
> psci: probing for conduit method from DT.
> psci: invalid "method" property:
> ...
> 
> In the psci_probe() function we do the following:
> ret = of_property_read_string(dev->of_node, "method", &method);
> and use this (method) variable as an argument for of_psci_fixup().
> I suspect we are free this variable at of_property_write_string ->
> of_set_property > of_delete_property.
> 
> Can you confirm my thought and correct it in the right direction?

Are you using the internal device tree for booting? If yes, then I think
this can happen. As I just saw when booting with the internal device
tree then of_get_fixed_tree() is called with the barebox live tree as
argument. We shouldn't do this and use a copy instead.

Nevertheless I think we shouldn't assume the string returned from
of_property_read_string() is valid for all eternity and make a copy
there as well.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: arm psci_probe() bug

[Sascha Hauer]

On Thu, Aug 03, 2023 at 10:30:16AM +0200, Sascha Hauer wrote:
> Hi Alexander,
> 
> On Thu, Aug 03, 2023 at 11:04:36AM +0300, Alexander Shiyan wrote:
> > Hello.
> > 
> > I found a bug in psci processing.
> > Currently the kernel writes the following on boot:
> > psci: probing for conduit method from DT.
> > psci: invalid "method" property:
> > ...
> > 
> > In the psci_probe() function we do the following:
> > ret = of_property_read_string(dev->of_node, "method", &method);
> > and use this (method) variable as an argument for of_psci_fixup().
> > I suspect we are free this variable at of_property_write_string ->
> > of_set_property > of_delete_property.
> > 
> > Can you confirm my thought and correct it in the right direction?
> 
> Are you using the internal device tree for booting? If yes, then I think
> this can happen. As I just saw when booting with the internal device
> tree then of_get_fixed_tree() is called with the barebox live tree as
> argument. We shouldn't do this and use a copy instead.

Whether or not this is your actual problem I just sent a patch
addressing this.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |