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

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: <DM6PR11MB46575360FC1EC41E4584DF649B682@DM6PR11MB4657.namprd11.prod.outlook.com>
Date: Thu, 11 Jan 2024 09:16:39 +0000
From: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
To: Jiri Pirko <jiri@...nulli.us>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"vadim.fedorenko@...ux.dev" <vadim.fedorenko@...ux.dev>,
	"michal.michalik@...el.com" <michal.michalik@...el.com>, "Olech, Milena"
	<milena.olech@...el.com>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "Glaza, Jan" <jan.glaza@...el.com>
Subject: RE: [PATCH net v2 4/4] dpll: hide "zombie" pins for userspace

>From: Jiri Pirko <jiri@...nulli.us>
>Sent: Thursday, January 4, 2024 4:05 PM
>To: Kubalewski, Arkadiusz <arkadiusz.kubalewski@...el.com>
>Cc: netdev@...r.kernel.org; vadim.fedorenko@...ux.dev;
>michal.michalik@...el.com; Olech, Milena <milena.olech@...el.com>;
>pabeni@...hat.com; kuba@...nel.org; Glaza, Jan <jan.glaza@...el.com>
>Subject: Re: [PATCH net v2 4/4] dpll: hide "zombie" pins for userspace
>
>Thu, Jan 04, 2024 at 12:11:32PM CET, arkadiusz.kubalewski@...el.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@...el.com>
>>Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.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;
>?
>

Sure, will do.

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

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

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

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