linux-kernel - Re: [RFC 04/12] staging: media: max96712: change DT parsing routine

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: <e2b7b98d-fb16-497e-9102-ba49e04e1748@stanley.mountain>
Date: Sat, 1 Feb 2025 13:30:48 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Laurentiu Palcu <laurentiu.palcu@....nxp.com>
Cc: Niklas Söderlund <niklas.soderlund@...natech.se>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
	linux-staging@...ts.linux.dev
Subject: Re: [RFC 04/12] staging: media: max96712: change DT parsing routine

On Fri, Jan 31, 2025 at 06:33:58PM +0200, Laurentiu Palcu wrote:
> -static int max96712_parse_dt(struct max96712_priv *priv)
> +static int max96712_parse_rx_ports(struct max96712_priv *priv, struct device_node *node,
> +				   struct of_endpoint *ep)
> +{
> +	struct device *dev = &priv->client->dev;
> +	struct max96712_rx_port *source;
> +	struct fwnode_handle *remote_port_parent;
> +
> +	if (priv->rx_ports[ep->port].fwnode) {
> +		dev_info(dev, "Multiple port endpoints are not supported: %d", ep->port);
> +		return 0;
> +	}
> +
> +	source = &priv->rx_ports[ep->port];
> +	source->fwnode = fwnode_graph_get_remote_endpoint(of_fwnode_handle(node));
> +	if (!source->fwnode) {
> +		dev_info(dev, "Endpoint %pOF has no remote endpoint connection\n", ep->local_node);
> +		return 0;
> +	}
> +
> +	remote_port_parent = fwnode_graph_get_remote_port_parent(of_fwnode_handle(node));
> +
> +	if (!fwnode_device_is_available(remote_port_parent)) {
> +		dev_dbg(dev, "Skipping port %d as remote port parent is disabled.\n",
> +			ep->port);
> +		source->fwnode = NULL;

I don't understand the refcounting in this function.  Should we call
fwnode_handle_put(source->fwnode); before setting this to NULL?

> +		goto fwnode_put;
> +	}
> +
> +	priv->rx_port_mask |= BIT(ep->port);
> +	priv->n_rx_ports++;
> +
> +fwnode_put:
> +	fwnode_handle_put(remote_port_parent);
> +	fwnode_handle_put(source->fwnode);
> +	return 0;

I don't understand why we save source->fwnode but drop the refcount on
the success path.  My instinct says that it should be a source_fwnode
local stack variable instead.  (But again, I've only glanced at this
code so I could be wrong).


> +}
> +

regards,
dan carpenter