linux-kernel - Re: [PATCH v3 01/15] media: i2c: ds90ub9x3: Fix extra fwnode_handle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z1azue3G14MQpfiI@kekkonen.localdomain>
Date: Mon, 9 Dec 2024 09:09:13 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
	Hans Verkuil <hverkuil-cisco@...all.nl>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Jai Luthra <jai.luthra@...asonboard.com>,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH v3 01/15] media: i2c: ds90ub9x3: Fix extra
 fwnode_handle_put()

Huomenta,

On Wed, Dec 04, 2024 at 01:05:15PM +0200, Tomi Valkeinen wrote:
> The ub913 and ub953 drivers call fwnode_handle_put(priv->sd.fwnode) as
> part of their remove process, and if the driver is removed multiple
> times, eventually leads to put "overflow", possibly causing memory

This is, in fact, an extra put. It'll lead to underflow, not overflow.
I'd expect removing it once would be already too much.

> corruption or crash.
> 
> The fwnode_handle_put() is a leftover from commit 905f88ccebb1 ("media:
> i2c: ds90ub9x3: Fix sub-device matching"), which changed the code
> related to the sd.fwnode, but missed removing these fwnode_handle_put()
> calls.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
> Cc: stable@...r.kernel.org
> Fixes: 905f88ccebb1 ("media: i2c: ds90ub9x3: Fix sub-device matching")
> ---
>  drivers/media/i2c/ds90ub913.c | 1 -
>  drivers/media/i2c/ds90ub953.c | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/ds90ub913.c b/drivers/media/i2c/ds90ub913.c
> index 8eed4a200fd8..b5375d736629 100644
> --- a/drivers/media/i2c/ds90ub913.c
> +++ b/drivers/media/i2c/ds90ub913.c
> @@ -793,7 +793,6 @@ static void ub913_subdev_uninit(struct ub913_data *priv)
>  	v4l2_async_unregister_subdev(&priv->sd);
>  	ub913_v4l2_nf_unregister(priv);
>  	v4l2_subdev_cleanup(&priv->sd);
> -	fwnode_handle_put(priv->sd.fwnode);
>  	media_entity_cleanup(&priv->sd.entity);
>  }
>  
> diff --git a/drivers/media/i2c/ds90ub953.c b/drivers/media/i2c/ds90ub953.c
> index 16f88db14981..10daecf6f457 100644
> --- a/drivers/media/i2c/ds90ub953.c
> +++ b/drivers/media/i2c/ds90ub953.c
> @@ -1291,7 +1291,6 @@ static void ub953_subdev_uninit(struct ub953_data *priv)
>  	v4l2_async_unregister_subdev(&priv->sd);
>  	ub953_v4l2_notifier_unregister(priv);
>  	v4l2_subdev_cleanup(&priv->sd);
> -	fwnode_handle_put(priv->sd.fwnode);
>  	media_entity_cleanup(&priv->sd.entity);
>  }
>  
> 

-- 
Terveisin,

Sakari Ailus