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

[Jiri Pirko <jiri@resnulli.us>]

Thu, Jan 04, 2024 at 12:11:32PM CET, arkadiusz.kubalewski@intel.com wrote:
>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 :)


>
>Fixes: 9431063ad323 ("dpll: core: Add DPLL framework base functions")
>Fixes: 9d71b54b65b1 ("dpll: netlink: Add DPLL framework base functions")
>Reviewed-by: Jan Glaza <jan.glaza@intel.com>
>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,
> 	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;
?


>+	return false;


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


>+}
>+
> 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.


>+		    !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.


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