[PATCH 1/1] drivers/i2c: fix a potential null pointer dereference

[PATCH 1/1] drivers/i2c: fix a potential null pointer dereference

[Yuanjun Gong]

Add check to *priv to make sure the input *dev is not NULL,
and to avoid a potential null pointer dereference.

Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index d2877e4cc28d..ebc600f3a7c8 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -968,6 +968,9 @@ static int nmk_i2c_runtime_resume(struct device *dev)
 	struct nmk_i2c_dev *priv = amba_get_drvdata(adev);
 	int ret;
 
+	if (!priv)
+		return -EINVAL;
+
 	ret = clk_prepare_enable(priv->clk);
 	if (ret) {
 		dev_err(dev, "can't prepare_enable clock\n");
-- 
2.25.1

Re: [PATCH 1/1] drivers/i2c: fix a potential null pointer dereference

[Linus Walleij]

On Tue, May 13, 2025 at 2:38 PM Yuanjun Gong <ruc_gongyuanjun@163.com> wrote:

> Add check to *priv to make sure the input *dev is not NULL,
> and to avoid a potential null pointer dereference.
>
> Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>

Why is this a problem? Reading nmk_i2c_probe():

        priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
        if (!priv)
                return -ENOMEM;

OK so priv is not NULL.

        amba_set_drvdata(adev, priv);

It is unconditionally assigned to the AMBA device.

Surely runtime resume isn't called before probe() has commenced
successfully?

I don't understand when this condition can occur.

Also including Theo on this, he is the major industrial user of this driver now.

Yours,
Linus Walleij

Re: [PATCH 1/1] drivers/i2c: fix a potential null pointer dereference

[Théo Lebrun]

Hello Linus, Yuanjun,

On Wed May 14, 2025 at 12:10 AM CEST, Linus Walleij wrote:
> On Tue, May 13, 2025 at 2:38 PM Yuanjun Gong <ruc_gongyuanjun@163.com> wrote:
>
>> Add check to *priv to make sure the input *dev is not NULL,
>> and to avoid a potential null pointer dereference.
>>
>> Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
>
> Why is this a problem? Reading nmk_i2c_probe():
>
>         priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>         if (!priv)
>                 return -ENOMEM;
>
> OK so priv is not NULL.
>
>         amba_set_drvdata(adev, priv);
>
> It is unconditionally assigned to the AMBA device.
>
> Surely runtime resume isn't called before probe() has commenced
> successfully?
>
> I don't understand when this condition can occur.
>
> Also including Theo on this, he is the major industrial user of this driver now.

Thanks for the Cc; I agree with your reading of the code and don't see
the issue.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 1/1] drivers/i2c: fix a potential null pointer dereference

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 236 bytes --]


> > Also including Theo on this, he is the major industrial user of this driver now.
> 
> Thanks for the Cc; I agree with your reading of the code and don't see
> the issue.

Likely a false positive from static code analysis.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]