linux-kernel - Re: [PATCH] platform/chrome: cross_ec_type: fix missing fwnode reference decrement

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] [day] [month] [year] [list]

Message-ID: <7767afd2-0ada-4cca-8861-ccdc874d555b@gmail.com>
Date: Wed, 9 Oct 2024 21:00:41 +0200
From: Javier Carrasco <javier.carrasco.cruz@...il.com>
To: Prashant Malani <pmalani@...omium.org>, Benson Leung
 <bleung@...omium.org>, Tzung-Bi Shih <tzungbi@...nel.org>,
 Guenter Roeck <groeck@...omium.org>,
 Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
 Enric Balletbo i Serra <eballetbo@...nel.org>
Cc: chrome-platform@...ts.linux.dev, linux-kernel@...r.kernel.org,
 stable@...r.kernel.org
Subject: Re: [PATCH] platform/chrome: cross_ec_type: fix missing fwnode
 reference decrement

On 09/10/2024 20:55, Javier Carrasco wrote:
> The device_for_each_child_node() macro requires explicit calls to
> fwnode_handle_put() upon early exits (return, break, goto) to decrement
> the fwnode's refcount, and avoid levaing a node reference behind.
> 
> Add the missing fwnode_handle_put() after the common label for all error
> paths.
> 
> Cc: stable@...r.kernel.org
> Fixes: fdc6b21e2444 ("platform/chrome: Add Type C connector class driver")
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@...il.com>
> ---
> I usually switch to the scoped variant of the macro to fix such issues,
> but given that the fix is relevant for stable kernels, I have provided
> the "classical" approach by adding the missing fwnode_handle_put().
> 
> If switching to the scoped variant is desired, please let me know.
> This driver and cross_typec_switch could be easily converted.
> 
> By the way, I wonder why all error paths are redirected to the same
> label to unregister ports, even before registering them (which seems to
> be harmless because unregistered ports are ignored, but still). With this
> fix, that jump to the label is definitely required, but if the scoped
> variant is used, maybe some simple returns would be enough.
> ---
>  drivers/platform/chrome/cros_ec_typec.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index c7781aea0b88..f1324466efac 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -409,6 +409,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
>  	return 0;
>  
>  unregister_ports:
> +	fwnode_handle_put(fwnode);
>  	cros_unregister_ports(typec);
>  	return ret;
>  }
> 
> ---
> base-commit: b6270c3bca987530eafc6a15f9d54ecd0033e0e3
> change-id: 20241009-cross_ec_typec_fwnode_handle_put-9f13b4bd467f
> 
> Best regards,

Small typo in the description, should be cross_ec_typec (last c is
missing). I will fix that for v2, but I will wait for feedback and
reviews to this first version.

Best regards,
Javier Carrasco