On 1/12/25 04:42, Sam Protsenko wrote:
Specifying efi_mgr in 'bootmeths' environment variable leads to NULL pointer dereference when 'bootflow scan' is executed, with call trace like this:
priv->fake_dev // NULL pointer dereference .read_bootflow = efi_mgr_read_bootflow() bootmeth_get_bootflow() bootflow_check() bootflow_scan_first() do_bootflow_scan() 'bootflow scan -l'That happens because in case when 'bootmeths' env var is defined the bootmeth_efi_mgr driver is not probed, and the memory for its private data isn't allocated by .priv_auto. In case when 'bootmeths' env var is not defined, the std->bootmeth_count is 0, and the execution flow in bootmeth_setup_iter_order() takes "no ordering" path, which in turn runs uclass_get_device_by_seq() -> ... -> device_probe(), so issue isn't present there. But when 'bootmeths' is defined and contains efi_mgr, the std->bootmeth_count > 0, so bootmeth_setup_iter_order() follows the "we have an ordering" path, where devices are not probed. In other words:
'bootmeths' defined 'bootmeths' not defined -------------------------------------------------------- priv == NULL priv != NULL ^ ^ | device_alloc_priv() no probe device_of_to_plat() ^ device_probe() | uclass_get_device_tail() dev = order[i] uclass_get_device_by_seq() ^ ^ | have an ordering | no ordering +----------------+---------------+ | bootmeth_setup_iter_order() bootflow_scan_first() do_bootflow_scan()Add an explicit device_probe() call in "we have an ordering" case to fix the issue.
Fixes: c627cfc14c08 ("bootstd: Allow scanning for global bootmeths separately") Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
boot/bootmeth-uclass.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c index ff36da78d5a1..049389403191 100644 --- a/boot/bootmeth-uclass.c +++ b/boot/bootmeth-uclass.c @@ -11,6 +11,7 @@ #include <bootmeth.h> #include <bootstd.h> #include <dm.h> +#include <dm/device-internal.h> #include <env_internal.h> #include <fs.h> #include <malloc.h> @@ -146,6 +147,12 @@ int bootmeth_setup_iter_order(struct bootflow_iter *iter, bool include_global) struct bootmeth_uc_plat *ucp; bool is_global;
ret = device_probe(dev);if (ret) {ret = log_msg_ret("probe", ret);goto err_order;}
This would mean that we fail boot if probing one of the boot devices fails. Shouldn't we remove the device from the order instead?
Best regards
Heinrich
ucp = dev_get_uclass_plat(dev); is_global = ucp->flags & BOOTMETHF_GLOBAL;