RE: [PATCH net v2 4/4] dpll: hide "zombie" pins for userspace

From: Jiri Pirko <jiri@resnulli.us>
Sent: Thursday, January 4, 2024 4:05 PM
To: Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>
Cc: netdev@vger.kernel.org; vadim.fedorenko@linux.dev;
michal.michalik@intel.com; Olech, Milena [off-list ref];
pabeni@redhat.com; kuba@kernel.org; Glaza, Jan [off-list ref]
Subject: Re: [PATCH net v2 4/4] dpll: hide "zombie" pins for userspace

Thu, Jan 04, 2024 at 12:11:32PM CET, arkadiusz.kubalewski@intel.com wrote:

quoted

If parent pin was unregistered but child pin was not, the userspace
would see the "zombie" pins - the ones that were registered with
parent pin (pins_pin_on_pin_register(..)).
Technically those are not available - as there is no dpll device in the
system. Do not dump those pins and prevent userspace from any
interaction with them.

Ah, here it is :)

:)

quoted

Fixes: 9431063ad323 ("dpll: core: Add DPLL framework base functions")
Fixes: 9d71b54b65b1 ("dpll: netlink: Add DPLL framework base functions")
Reviewed-by: Jan Glaza <redacted>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
drivers/dpll/dpll_netlink.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index f266db8da2f0..495dfc43c0be 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c

@@ -949,6 +949,19 @@ dpll_pin_parent_pin_set(struct dpll_pin *pin, struct

nlattr *parent_nest,

quoted

	return 0;
}

+static bool dpll_pin_parents_registered(struct dpll_pin *pin)
+{
+	struct dpll_pin_ref *par_ref;
+	struct dpll_pin *p;
+	unsigned long i, j;
+
+	xa_for_each(&pin->parent_refs, i, par_ref)
+		xa_for_each_marked(&dpll_pin_xa, j, p, DPLL_REGISTERED)
+			if (par_ref->pin == p)
+				return true;

	if (xa_get_mark(..))
		return true;
?

Sure, will do.

quoted

+	return false;

As I wrote in the reply to the other patch, could you unify the "hide"
behaviour for unregistered parent pin/device?

Yes, in v3.

quoted

+}
+
static int
dpll_pin_set_from_nlattr(struct dpll_pin *pin, struct genl_info *info)
{

@@ -1153,6 +1166,9 @@ int dpll_nl_pin_get_dumpit(struct sk_buff *skb,

struct netlink_callback *cb)

	xa_for_each_marked_start(&dpll_pin_xa, i, pin, DPLL_REGISTERED,
				 ctx->idx) {
+		if (!xa_empty(&pin->parent_refs) &&

This empty check is redundant, remove it.

Ok.

quoted

+		    !dpll_pin_parents_registered(pin))
+			continue;
		hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid,
				  cb->nlh->nlmsg_seq,
				  &dpll_nl_family, NLM_F_MULTI,

@@ -1179,6 +1195,10 @@ int dpll_nl_pin_set_doit(struct sk_buff *skb,

struct genl_info *info)
{
	struct dpll_pin *pin = info->user_ptr[0];

+	if (!xa_empty(&pin->parent_refs) &&

This empty check is redundant, remove it.

Will do.
Thank you!
Arkadiusz

quoted

+	    !dpll_pin_parents_registered(pin))
+		return -ENODEV;
+
	return dpll_pin_set_from_nlattr(pin, info);
}

--
2.38.1