[PATCH net 1/2] dpll: fix NULL deref in dpll_device_ops() during teardown race

[Petr Oros <poros@redhat.com>]

When a dpll device driver (zl3073x) is unloaded via rmmod while a
foreign driver (ice) still holds pins on it via
dpll_pin_on_pin_register(), dpll_device_unregister() empties
dpll->registration_list but the dpll object stays alive on ice's
refcount. A subsequent PIN_DELETED
notification on ice's pin reaches dpll_cmd_pin_get_one() which calls
dpll_device_ops() on the still-indexed but half-dead dpll;
dpll_device_registration_first() returns NULL and the accessor
dereferences ops at offset 0x10 (struct dpll_device_registration::ops):

  CPU A (zl3073x rmmod)            CPU B (ice workqueue)
  ---------------------            ---------------------
  dpll_device_unregister():
    drop reg from
    dpll->registration_list
                                   dpll_pin_on_pin_unregister
                                     dpll_pin_delete_ntf
                                       dpll_cmd_pin_get_one
                                         dpll_device_ops()
                                           registration_first() = NULL
                                           NULL deref @ 0x10

  WARNING: CPU: 27 PID: 972 at drivers/dpll/dpll_core.c:1072 dpll_device_ops+0x20/0x30
  Workqueue: ice_dpll_wq ice_dpll_pin_notify_work [ice]
  RIP: 0010:dpll_device_ops+0x20/0x30
  Call Trace:
   <TASK>
   ? __warn+0x84/0x140
   ? dpll_device_ops+0x20/0x30
   ? report_bug+0x16b/0x180
   ? handle_bug+0x3c/0x70
   ? exc_invalid_op+0x14/0x70
   ? asm_exc_invalid_op+0x16/0x20
   ? dpll_device_ops+0x20/0x30
   dpll_cmd_pin_get_one+0x303/0x490
   dpll_pin_event_send+0x93/0x140
   dpll_pin_on_pin_unregister+0x45/0xe0
   ice_dpll_pin_notify_work+0x7b/0x150 [ice]
   process_one_work+0x188/0x3b0
   worker_thread+0x2ef/0x410
   kthread+0x122/0x240
   ret_from_fork+0x28/0x50
   </TASK>
  ---[ end trace 0000000000000000 ]---
  BUG: kernel NULL pointer dereference, address: 0000000000000010
  RIP: 0010:dpll_device_ops+0x24/0x30
  Call Trace:
   dpll_cmd_pin_get_one+0x303/0x490
   dpll_pin_event_send+0x93/0x140
   dpll_pin_on_pin_unregister+0x45/0xe0
   ice_dpll_pin_notify_work+0x7b/0x150 [ice]
   process_one_work+0x188/0x3b0
   worker_thread+0x2ef/0x410
   kthread+0x122/0x240
   ret_from_fork+0x28/0x50

dpll_lock serializes the two paths but cannot reorder them: ice's work
was queued before zl3073x took the lock. "Empty registration_list on a
still-indexed dpll" is a legitimate transient state caused by
peer-driver refcounting; the accessors must tolerate it.

Drop the WARN_ON in dpll_device_registration_first(), make dpll_priv()
and dpll_device_ops() return NULL on the empty-list state, and bail
out cleanly in dpll_cmd_pin_get_one() when the first dpll the pin
references is already gone.

In-tree doit/dumpit callers of dpll_priv() / dpll_device_ops() obtain
their dpll via dpll_pre_doit() -> dpll_device_get_by_id(), which only
returns DPLL_REGISTERED devices; dpll_device_unregister() clears that
mark only after observing list_empty(&dpll->registration_list), so
those callers cannot see the new NULL return. The only path that can
is dpll_cmd_pin_get_one() reached from dpll_pin_event_send(), gated
here.

Fixes: 9431063ad323 ("dpll: core: Add DPLL framework base functions")
Signed-off-by: Petr Oros <poros@redhat.com>
---
 drivers/dpll/dpll_core.c    | 12 ++++++------
 drivers/dpll/dpll_netlink.c |  6 ++++++
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index cbb635db43210f..4a058b46c69d4f 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -1060,12 +1060,8 @@ EXPORT_SYMBOL_GPL(dpll_pin_ref_sync_pair_add);
 static struct dpll_device_registration *
 dpll_device_registration_first(struct dpll_device *dpll)
 {
-	struct dpll_device_registration *reg;
-
-	reg = list_first_entry_or_null((struct list_head *)&dpll->registration_list,
-				       struct dpll_device_registration, list);
-	WARN_ON(!reg);
-	return reg;
+	return list_first_entry_or_null((struct list_head *)&dpll->registration_list,
+					struct dpll_device_registration, list);
 }
 
 void *dpll_priv(struct dpll_device *dpll)
@@ -1073,6 +1069,8 @@ void *dpll_priv(struct dpll_device *dpll)
 	struct dpll_device_registration *reg;
 
 	reg = dpll_device_registration_first(dpll);
+	if (!reg)
+		return NULL;
 	return reg->priv;
 }
 
@@ -1081,6 +1079,8 @@ const struct dpll_device_ops *dpll_device_ops(struct dpll_device *dpll)
 	struct dpll_device_registration *reg;
 
 	reg = dpll_device_registration_first(dpll);
+	if (!reg)
+		return NULL;
 	return reg->ops;
 }
 
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index 0ff1658c2dc1ba..8e7e61982b867c 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -682,6 +682,12 @@ dpll_cmd_pin_get_one(struct sk_buff *msg, struct dpll_pin *pin,
 	ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
 	ASSERT_NOT_NULL(ref);
 
+	/* The first dpll the pin references may be torn down while still
+	 * pinned by foreign-driver refs; drop the notification cleanly.
+	 */
+	if (!dpll_device_ops(ref->dpll))
+		return -ENODEV;
+
 	ret = dpll_msg_add_pin_handle(msg, pin);
 	if (ret)
 		return ret;
-- 
2.53.0