| Message ID | 20240115085241.312144-4-arkadiusz.kubalewski@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | dpll: fix unordered unbind/bind registerer issues | expand |
Mon, Jan 15, 2024 at 09:52:41AM 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 >a parent pin (dpll_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. Provide a unified function to determine if the >pin is available and use it before acting/responding for user requests. > >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> The patch looks fine, but it should come before the previous one. pw-bot: cr Feel free to attach the review tag to the next version you send: Reviewed-by: Jiri Pirko <jiri@nvidia.com>
>From: Jiri Pirko <jiri@resnulli.us> >Sent: Tuesday, January 16, 2024 8:47 AM > >Mon, Jan 15, 2024 at 09:52:41AM 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 >>a parent pin (dpll_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. Provide a unified function to determine if the >>pin is available and use it before acting/responding for user requests. >> >>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> > >The patch looks fine, but it should come before the previous one. > >pw-bot: cr > >Feel free to attach the review tag to the next version you send: >Reviewed-by: Jiri Pirko <jiri@nvidia.com> Fixed in v4. Thank you! Arkadiusz
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c index 108c002537e6..7cc99d627942 100644 --- a/drivers/dpll/dpll_netlink.c +++ b/drivers/dpll/dpll_netlink.c @@ -525,6 +525,24 @@ __dpll_device_change_ntf(struct dpll_device *dpll) return dpll_device_event_send(DPLL_CMD_DEVICE_CHANGE_NTF, dpll); } +static bool dpll_pin_available(struct dpll_pin *pin) +{ + struct dpll_pin_ref *par_ref; + unsigned long i; + + if (!xa_get_mark(&dpll_pin_xa, pin->id, DPLL_REGISTERED)) + return false; + xa_for_each(&pin->parent_refs, i, par_ref) + if (xa_get_mark(&dpll_pin_xa, par_ref->pin->id, + DPLL_REGISTERED)) + return true; + xa_for_each(&pin->dpll_refs, i, par_ref) + if (xa_get_mark(&dpll_device_xa, par_ref->dpll->id, + DPLL_REGISTERED)) + return true; + return false; +} + /** * dpll_device_change_ntf - notify that the dpll device has been changed * @dpll: registered dpll pointer @@ -551,7 +569,7 @@ dpll_pin_event_send(enum dpll_cmd event, struct dpll_pin *pin) int ret = -ENOMEM; void *hdr; - if (!xa_get_mark(&dpll_pin_xa, pin->id, DPLL_REGISTERED)) + if (!dpll_pin_available(pin)) return -ENODEV; msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL); @@ -1102,6 +1120,10 @@ int dpll_nl_pin_id_get_doit(struct sk_buff *skb, struct genl_info *info) } pin = dpll_pin_find_from_nlattr(info); if (!IS_ERR(pin)) { + if (!dpll_pin_available(pin)) { + nlmsg_free(msg); + return -ENODEV; + } ret = dpll_msg_add_pin_handle(msg, pin); if (ret) { nlmsg_free(msg); @@ -1151,6 +1173,8 @@ 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 (!dpll_pin_available(pin)) + continue; hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq, &dpll_nl_family, NLM_F_MULTI, @@ -1413,7 +1437,8 @@ int dpll_pin_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb, } info->user_ptr[0] = xa_load(&dpll_pin_xa, nla_get_u32(info->attrs[DPLL_A_PIN_ID])); - if (!info->user_ptr[0]) { + if (!info->user_ptr[0] || + !dpll_pin_available(info->user_ptr[0])) { NL_SET_ERR_MSG(info->extack, "pin not found"); ret = -ENODEV; goto unlock_dev;
If parent pin was unregistered but child pin was not, the userspace would see the "zombie" pins - the ones that were registered with a parent pin (dpll_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. Provide a unified function to determine if the pin is available and use it before acting/responding for user requests. 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> --- v3: - introduce separated function to determine pin availability - move check for pin availablity to dpll_pin_pre_doit(..) - check if pin available within pin-id-get flow - rename patch title drivers/dpll/dpll_netlink.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)