The kzalloc() in armada_375_380_coherency_init() will return
null if the physical memory has run out. As a result, if we
dereference the property pointer, the null pointer dereference
bug will happen.
This patch adds a check to avoid null pointer dereference.
Fixes: 497a92308af8 ("ARM: mvebu: implement L2/PCIe deadlock workaround")
Signed-off-by: Duoming Zhou <[email protected]>
---
arch/arm/mach-mvebu/coherency.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
index a6b621ff0b8..a81a3c8c19a 100644
--- a/arch/arm/mach-mvebu/coherency.c
+++ b/arch/arm/mach-mvebu/coherency.c
@@ -191,6 +191,8 @@ static void __init armada_375_380_coherency_init(struct device_node *np)
struct property *p;
p = kzalloc(sizeof(*p), GFP_KERNEL);
+ if (!p)
+ continue;
p->name = kstrdup("arm,io-coherent", GFP_KERNEL);
of_add_property(cache_dn, p);
}
--
2.17.1
On Sun, Feb 25, 2024 at 02:04:50PM +0800, Duoming Zhou wrote:
> The kzalloc() in armada_375_380_coherency_init() will return
> null if the physical memory has run out. As a result, if we
> dereference the property pointer, the null pointer dereference
> bug will happen.
>
> This patch adds a check to avoid null pointer dereference.
Again, what if kstrdup() fails?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Sun, 25 Feb 2024 11:30:20 +0000 Russell King (Oracle) wrote:
> > The kzalloc() in armada_375_380_coherency_init() will return
> > null if the physical memory has run out. As a result, if we
> > dereference the property pointer, the null pointer dereference
> > bug will happen.
> >
> > This patch adds a check to avoid null pointer dereference.
>
> Again, what if kstrdup() fails?
Thank you for your suggestions, I will also add a check to judge
whether kstrdup() fails.
Best regards,
Duoming Zhou
On Sun, Feb 25, 2024 at 02:04:50PM +0800, Duoming Zhou wrote:
> The kzalloc() in armada_375_380_coherency_init() will return
> null if the physical memory has run out. As a result, if we
> dereference the property pointer, the null pointer dereference
> bug will happen.
>
> This patch adds a check to avoid null pointer dereference.
>
> Fixes: 497a92308af8 ("ARM: mvebu: implement L2/PCIe deadlock workaround")
> Signed-off-by: Duoming Zhou <[email protected]>
I have to wounder how we can run out of memory here. This code is
being called from:
postcore_initcall(coherency_late_init);
If you look at:
https://elixir.bootlin.com/linux/latest/source/include/linux/init.h#L299
You can see that only true kernel core stuff has been called before
that. If that has consumed all the available memory, something is very
seriously wrong, and the machine is not going to last another couple
of milliseconds before it crashes no matter what checking you do.
So i do wounder if your time could be better spent in other places?
Andrew
On Mon, Feb 26, 2024 at 02:39:37PM +0100, Andrew Lunn wrote:
> On Sun, Feb 25, 2024 at 02:04:50PM +0800, Duoming Zhou wrote:
> > The kzalloc() in armada_375_380_coherency_init() will return
> > null if the physical memory has run out. As a result, if we
> > dereference the property pointer, the null pointer dereference
> > bug will happen.
> >
> > This patch adds a check to avoid null pointer dereference.
> >
> > Fixes: 497a92308af8 ("ARM: mvebu: implement L2/PCIe deadlock workaround")
> > Signed-off-by: Duoming Zhou <[email protected]>
>
> I have to wounder how we can run out of memory here. This code is
> being called from:
>
> postcore_initcall(coherency_late_init);
>
> If you look at:
>
> https://elixir.bootlin.com/linux/latest/source/include/linux/init.h#L299
>
> You can see that only true kernel core stuff has been called before
> that. If that has consumed all the available memory, something is very
> seriously wrong, and the machine is not going to last another couple
> of milliseconds before it crashes no matter what checking you do.
>
> So i do wounder if your time could be better spent in other places?
Sadly, it's an easy patch generation target for newbies getting involved
with kernel development. "Find all kzalloc()s and look to see whether
they check for a NULL pointer, if not generate a patch".
This results in people doing exactly that, not looking at the bigger
picture, and not considering whether a NULL pointer could occur there.
The other issue is that if a NULL pointer is returned at this point,
the resulting oops at least allows a developer to debug it (maybe not
a user if the console isn't up.) Adding this patch which basically
just continues the loop silently means that there's no diagnostic that
something went wrong, and it's up to someone to figure out "why does
XYZ no longer work" to figure it out...
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!