2024-02-25 06:04:49

by Duoming Zhou

[permalink] [raw]
Subject: [PATCH] ARM: mvebu: Prevent null pointer dereference caused by kzalloc failure

The kzalloc() in i2c_quirk() will return null if the physical
memory has run out. As a result, if we dereference the new_compat
pointer, the null pointer dereference bug will happen.

This patch adds a check to avoid null pointer dereference.

Fixes: 5fd62066d290 ("ARM: mvebu: Add thermal quirk for the Armada 375 DB board")
Signed-off-by: Duoming Zhou <[email protected]>
---
arch/arm/mach-mvebu/board-v7.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
index fd5d0c8ff69..7d2cb12e349 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -125,6 +125,8 @@ static void __init i2c_quirk(void)
struct property *new_compat;

new_compat = kzalloc(sizeof(*new_compat), GFP_KERNEL);
+ if (!new_compat)
+ continue;

new_compat->name = kstrdup("compatible", GFP_KERNEL);
new_compat->length = sizeof("marvell,mv78230-a0-i2c");
--
2.17.1



2024-02-25 13:13:04

by Duoming Zhou

[permalink] [raw]
Subject: Re: [PATCH] ARM: mvebu: Prevent null pointer dereference caused by kzalloc failure

On Sun, 25 Feb 2024 11:29:53 +0000 Russell King (Oracle) wrote:
> > The kzalloc() in i2c_quirk() will return null if the physical
> > memory has run out. As a result, if we dereference the new_compat
> > pointer, the null pointer dereference bug will happen.
> >
> > This patch adds a check to avoid null pointer dereference.
>
> Yet again another janitorial patch that is Way too focused on a single
> issue, rather than analysing the code and proposing the best fix. :(
>
> What if the two kstrdup() fail?

Thank you for your suggestions, I will also add a check to judge
whether two kstrdup() fail. The detail is shown below:

@@ -125,12 +125,20 @@ static void __init i2c_quirk(void)
struct property *new_compat;

new_compat = kzalloc(sizeof(*new_compat), GFP_KERNEL);
+ if(!new_compat)
+ continue;

new_compat->name = kstrdup("compatible", GFP_KERNEL);
new_compat->length = sizeof("marvell,mv78230-a0-i2c");
new_compat->value = kstrdup("marvell,mv78230-a0-i2c",
GFP_KERNEL);

+ if (!new_compat->name || !new_compat->value) {
+ kfree(new_compat->name);
+ kfree(new_compat->value);
+ kfree(new_compat);
+ continue;
+ }
of_update_property(np, new_compat);
}
}

Best regards,
Duoming Zhou

2024-02-25 11:30:19

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH] ARM: mvebu: Prevent null pointer dereference caused by kzalloc failure

On Sun, Feb 25, 2024 at 02:04:01PM +0800, Duoming Zhou wrote:
> The kzalloc() in i2c_quirk() will return null if the physical
> memory has run out. As a result, if we dereference the new_compat
> pointer, the null pointer dereference bug will happen.
>
> This patch adds a check to avoid null pointer dereference.

Yet again another janitorial patch that is Way too focused on a single
issue, rather than analysing the code and proposing the best fix. :(

What if the two kstrdup() fail?

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!