diff mbox series

[net,v2,1/4] dpll: fix pin dump crash after module unbind

Message ID 20240104111132.42730-2-arkadiusz.kubalewski@intel.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series dpll: fix unordered unbind/bind registerer issues | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net
netdev/ynl success SINGLE THREAD; Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1115 this patch: 1115
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 1141 this patch: 1141
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1142 this patch: 1142
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Kubalewski, Arkadiusz Jan. 4, 2024, 11:11 a.m. UTC
Disallow dump of unregistered parent pins, it is possible when parent
pin and dpll device registerer kernel module instance unbinds, and
other kernel module instances of the same dpll device have pins
registered with the parent pin. The user can invoke a pin-dump but as
the parent was unregistered, those shall not be accessed by the
userspace, prevent that by checking if parent pin is still registered.

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 | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jiri Pirko Jan. 4, 2024, 3:08 p.m. UTC | #1
Thu, Jan 04, 2024 at 12:11:29PM CET, arkadiusz.kubalewski@intel.com wrote:
>Disallow dump of unregistered parent pins, it is possible when parent
>pin and dpll device registerer kernel module instance unbinds, and
>other kernel module instances of the same dpll device have pins
>registered with the parent pin. The user can invoke a pin-dump but as
>the parent was unregistered, those shall not be accessed by the
>userspace, prevent that by checking if parent pin is still registered.
>
>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 | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>index ce7cf736f020..b53478374a38 100644
>--- a/drivers/dpll/dpll_netlink.c
>+++ b/drivers/dpll/dpll_netlink.c
>@@ -328,6 +328,8 @@ dpll_msg_add_pin_parents(struct sk_buff *msg, struct dpll_pin *pin,
> 		void *parent_priv;
> 
> 		ppin = ref->pin;
>+		if (!xa_get_mark(&dpll_pin_xa, ppin->id, DPLL_REGISTERED))
>+			continue;

If you make sure to hide the pin properly with the last patch, you don't
need this one. Drop it.


> 		parent_priv = dpll_pin_on_dpll_priv(dpll_ref->dpll, ppin);
> 		ret = ops->state_on_pin_get(pin,
> 					    dpll_pin_on_pin_priv(ppin, pin),
>-- 
>2.38.1
>
Kubalewski, Arkadiusz Jan. 11, 2024, 9:06 a.m. UTC | #2
>From: Jiri Pirko <jiri@resnulli.us>
>Sent: Thursday, January 4, 2024 4:09 PM
>
>Thu, Jan 04, 2024 at 12:11:29PM CET, arkadiusz.kubalewski@intel.com wrote:
>>Disallow dump of unregistered parent pins, it is possible when parent
>>pin and dpll device registerer kernel module instance unbinds, and
>>other kernel module instances of the same dpll device have pins
>>registered with the parent pin. The user can invoke a pin-dump but as
>>the parent was unregistered, those shall not be accessed by the
>>userspace, prevent that by checking if parent pin is still registered.
>>
>>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 | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>>diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>>index ce7cf736f020..b53478374a38 100644
>>--- a/drivers/dpll/dpll_netlink.c
>>+++ b/drivers/dpll/dpll_netlink.c
>>@@ -328,6 +328,8 @@ dpll_msg_add_pin_parents(struct sk_buff *msg, struct
>>dpll_pin *pin,
>> 		void *parent_priv;
>>
>> 		ppin = ref->pin;
>>+		if (!xa_get_mark(&dpll_pin_xa, ppin->id, DPLL_REGISTERED))
>>+			continue;
>
>If you make sure to hide the pin properly with the last patch, you don't
>need this one. Drop it.
>

Makes sense, will do.

Thank you!
Arkadiusz

>
>> 		parent_priv = dpll_pin_on_dpll_priv(dpll_ref->dpll, ppin);
>> 		ret = ops->state_on_pin_get(pin,
>> 					    dpll_pin_on_pin_priv(ppin, pin),
>>--
>>2.38.1
>>
diff mbox series

Patch

diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index ce7cf736f020..b53478374a38 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -328,6 +328,8 @@  dpll_msg_add_pin_parents(struct sk_buff *msg, struct dpll_pin *pin,
 		void *parent_priv;
 
 		ppin = ref->pin;
+		if (!xa_get_mark(&dpll_pin_xa, ppin->id, DPLL_REGISTERED))
+			continue;
 		parent_priv = dpll_pin_on_dpll_priv(dpll_ref->dpll, ppin);
 		ret = ops->state_on_pin_get(pin,
 					    dpll_pin_on_pin_priv(ppin, pin),