media: i2c: mt9t112: fix missing IS_ERR() check before PTR_ERR()

[PATCH] media: i2c: mt9t112: fix missing IS_ERR() check before PTR_ERR()

Posted by Uday Khare 4 days, 15 hours ago

In mt9t112_probe(), the -ENOENT check after devm_v4l2_sensor_clk_get() is
missing an IS_ERR() guard.

Without IS_ERR(), PTR_ERR(priv->clk) is executed unconditionally,
including on successful clock lookups where priv->clk points to a valid
struct clk pointer.  Calling PTR_ERR() on a valid pointer reinterprets its
address as a signed long integer; the result is arbitrary and leads to an
invalid comparison against -ENOENT.

Fix this by nesting the -ENOENT check inside an IS_ERR() guard block.

Fixes: 6a26f141bf62 ("media: i2c: mt9t112: Remove soc_camera dependencies")
Signed-off-by: Uday Khare <udaykhare77@gmail.com>
---
 drivers/media/i2c/mt9t112.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/mt9t112.c b/drivers/media/i2c/mt9t112.c
index a8e2a3cf3456..56b7c8910112 100644
--- a/drivers/media/i2c/mt9t112.c
+++ b/drivers/media/i2c/mt9t112.c
@@ -1079,10 +1079,12 @@ static int mt9t112_probe(struct i2c_client *client)
 	v4l2_i2c_subdev_init(&priv->subdev, client, &mt9t112_subdev_ops);
 
 	priv->clk = devm_v4l2_sensor_clk_get(&client->dev, "extclk");
-	if (PTR_ERR(priv->clk) == -ENOENT)
-		priv->clk = NULL;
-	else if (IS_ERR(priv->clk))
-		return dev_err_probe(&client->dev, PTR_ERR(priv->clk),
-				     "Unable to get clock \"extclk\"\n");
+	if (IS_ERR(priv->clk)) {
+		if (PTR_ERR(priv->clk) == -ENOENT)
+			priv->clk = NULL;
+		else
+			return dev_err_probe(&client->dev, PTR_ERR(priv->clk),
+					     "Unable to get clock \"extclk\"\n");
+	}
 
 	priv->standby_gpio = devm_gpiod_get_optional(&client->dev, "standby",
-- 
2.55.0

Re: [PATCH] media: i2c: mt9t112: fix missing IS_ERR() check before PTR_ERR()

Posted by Sakari Ailus 4 days, 14 hours ago

Hi Uday,

On Mon, Jul 20, 2026 at 04:29:35PM +0530, Uday Khare wrote:
> In mt9t112_probe(), the -ENOENT check after devm_v4l2_sensor_clk_get() is
> missing an IS_ERR() guard.
> 
> Without IS_ERR(), PTR_ERR(priv->clk) is executed unconditionally,
> including on successful clock lookups where priv->clk points to a valid
> struct clk pointer.  Calling PTR_ERR() on a valid pointer reinterprets its
> address as a signed long integer; the result is arbitrary and leads to an
> invalid comparison against -ENOENT.

How can the result be arbitrary? Both are the same size.

The patch addresses an API usage issue but I don't think there was an
actual bug there.

> 
> Fix this by nesting the -ENOENT check inside an IS_ERR() guard block.
> 
> Fixes: 6a26f141bf62 ("media: i2c: mt9t112: Remove soc_camera dependencies")
> Signed-off-by: Uday Khare <udaykhare77@gmail.com>
> ---
>  drivers/media/i2c/mt9t112.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/mt9t112.c b/drivers/media/i2c/mt9t112.c
> index a8e2a3cf3456..56b7c8910112 100644
> --- a/drivers/media/i2c/mt9t112.c
> +++ b/drivers/media/i2c/mt9t112.c
> @@ -1079,10 +1079,12 @@ static int mt9t112_probe(struct i2c_client *client)
>  	v4l2_i2c_subdev_init(&priv->subdev, client, &mt9t112_subdev_ops);
>  
>  	priv->clk = devm_v4l2_sensor_clk_get(&client->dev, "extclk");
> -	if (PTR_ERR(priv->clk) == -ENOENT)
> -		priv->clk = NULL;
> -	else if (IS_ERR(priv->clk))
> -		return dev_err_probe(&client->dev, PTR_ERR(priv->clk),
> -				     "Unable to get clock \"extclk\"\n");
> +	if (IS_ERR(priv->clk)) {
> +		if (PTR_ERR(priv->clk) == -ENOENT)

You could test for !- -ENOENT and drop the else clause.

> +			priv->clk = NULL;
> +		else
> +			return dev_err_probe(&client->dev, PTR_ERR(priv->clk),
> +					     "Unable to get clock \"extclk\"\n");
> +	}
>  
>  	priv->standby_gpio = devm_gpiod_get_optional(&client->dev, "standby",

-- 
Regards,

Sakari Ailus

[PATCH v2] media: i2c: mt9t112: fix incorrect PTR_ERR() call on non-error pointer

Posted by Uday Khare 4 days, 14 hours ago

In mt9t112_probe(), the clock error check after devm_v4l2_sensor_clk_get()
calls PTR_ERR(priv->clk) unconditionally, before testing IS_ERR().

On a successful lookup, priv->clk is a valid pointer and calling PTR_ERR()
on it is incorrect API usage.  While the comparison against -ENOENT happens
to be harmless in practice (valid kernel pointers never fall in the error
range), this is still a violation of the IS_ERR()/PTR_ERR() contract that
can mislead readers.

Restructure the check to guard PTR_ERR() inside IS_ERR(), using the
simpler != -ENOENT form to avoid an unnecessary else clause.

Signed-off-by: Uday Khare <udaykhare77@gmail.com>
---
 drivers/media/i2c/mt9t112.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/mt9t112.c b/drivers/media/i2c/mt9t112.c
index bd2268154ca7..b3a6c6d76632 100644
--- a/drivers/media/i2c/mt9t112.c
+++ b/drivers/media/i2c/mt9t112.c
@@ -1079,11 +1079,12 @@ static int mt9t112_probe(struct i2c_client *client)
 	v4l2_i2c_subdev_init(&priv->subdev, client, &mt9t112_subdev_ops);
 
 	priv->clk = devm_v4l2_sensor_clk_get(&client->dev, "extclk");
-	if (PTR_ERR(priv->clk) == -ENOENT)
+	if (IS_ERR(priv->clk)) {
+		if (PTR_ERR(priv->clk) != -ENOENT)
+			return dev_err_probe(&client->dev, PTR_ERR(priv->clk),
+					     "Unable to get clock \"extclk\"\n");
 		priv->clk = NULL;
-	else if (IS_ERR(priv->clk))
-		return dev_err_probe(&client->dev, PTR_ERR(priv->clk),
-				     "Unable to get clock \"extclk\"\n");
+	}
 
 	priv->standby_gpio = devm_gpiod_get_optional(&client->dev, "standby",
 						     GPIOD_OUT_HIGH);
-- 
2.55.0