netdev - Re: [RFC net-next 02/15] net: cpc: add endpoint infrastructure

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: <e038e209-dc4a-4dc4-9356-cd3a54535856@lunn.ch>
Date: Mon, 12 May 2025 04:28:48 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Damien Riégel <damien.riegel@...abs.com>
Cc: Andrew Lunn <andrew+netdev@...n.ch>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Silicon Labs Kernel Team <linux-devel@...abs.com>,
	netdev@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC net-next 02/15] net: cpc: add endpoint infrastructure

> +/**
> + * cpc_endpoint_register() - Register an endpoint.
> + * @ep: Endpoint to register.
> + *
> + * Companion function of cpc_endpoint_alloc(). This function adds the endpoint, making it usable by
> + * CPC drivers. As this ensures that endpoint ID is unique within a CPC interface and then adds the
> + * endpoint, the lock interface is held to prevent concurrent additions.
> + *
> + * Context: Lock "add_lock" of endpoint's interface.
> + *
> + * Return: 0 on success, negative errno otherwise.
> + */
> +int cpc_endpoint_register(struct cpc_endpoint *ep)
> +{
> +	int err;
> +
> +	if (!ep || !ep->intf)
> +		return -EINVAL;
> +
> +	mutex_lock(&ep->intf->add_lock);
> +	err = __cpc_endpoint_register(ep);
> +	mutex_unlock(&ep->intf->add_lock);

What exactly is add_lock protecting?

> +void cpc_endpoint_unregister(struct cpc_endpoint *ep)
> +{
> +	device_del(&ep->dev);
> +	put_device(&ep->dev);
> +}

Register needs a lock, but unregister does not?

> +/**
> + * cpc_interface_get_endpoint() - get endpoint registered in CPC device with this id
> + * @intf: CPC device to probe
> + * @ep_id: endpoint ID that's being looked for
> + *
> + * Context: This function locks device's endpoint list.
> + *
> + * Return: a struct cpc_endpoint pointer or NULL if not found.
> + */
> +struct cpc_endpoint *cpc_interface_get_endpoint(struct cpc_interface *intf, u8 ep_id)
> +{
> +	struct cpc_endpoint *ep;
> +
> +	mutex_lock(&intf->lock);
> +	ep = __cpc_interface_get_endpoint(intf, ep_id);
> +	mutex_unlock(&intf->lock);
> +
> +	return ep;
> +}

cpc_interface_get_endpoint() but no cpc_interface_put_endpoint() ? Is
this not taking a reference on the end point? Maybe this should not be
called _get_.

	Andrew