Hi Javi, Thank you for spotting and fixing this. On Tue, Jan 09, 2024 at 03:31:55PM +0000, Julien Grall wrote: > Hi Javi, > > Title: Any reason this is titled for-4.18? Shouldn't this patch also be > merged in staging? > > On 09/01/2024 14:19, Javi Merino wrote: > > In remove_nodes(), overlay_node is dereferenced when printing the > > error message even though it is known to be NULLL. Fix the error > > Typo: s/NULLL/NULL/ > > This can be fixed on commit if there is nothing else. > > > message to avoid dereferencing a NULL pointer. > > > > The semantic patch that spots this code is available in > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/null/deref_null.cocci?id=1f874787ed9a2d78ed59cb21d0d90ac0178eceb0 > > Good catch and glad to see that coccinelle can work on Xen. I am looking > forward for more work in that area :). > > > > > Fixes: 7e5c4a8b86f1 ("xen/arm: Implement device tree node removal > > functionalities") > > Signed-off-by: Javi Merino <[email protected]> > c> --- > > CC: Vikram Garhwal <[email protected]> > > > > Vikram, I didn't know what to put in the error message. Feel free to > > suggest something more appropriate than "Device not present in the > > tree". > > More questions for Vikram, looking at the code, it is not 100% clear in > which condition overlay_node could be NULL. Is this a programming error? if > so, maybe this should be an ASSERT_UNREACHABLE() (could be added separately) > and it would be fine to print nothing. > This can happen with failures in add_nodes() function. add_nodes() failure will try to call remove_nodes function. Depending on where add_nodes() is failed, nodes_address may or may not be NULL.
We also added a detailed comment on this: https://github.com/xen-project/xen/blob/5a3ace21f3d779b291a2d305824b2820d88de7f1/xen/common/dt-overlay.c#L816 For now, we can return from here without printing anything as error message will be printed by the caller of remove_nodes() anyway. > Cheers, > > -- > Julien Grall
