mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Vadim Fedorenko <vadim.fedorenko@linux.dev>
To: Petr Oros <poros@redhat.com>, netdev@vger.kernel.org
Cc: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>,
	Jiri Pirko <jiri@resnulli.us>,
	Michal Michalik <michal.michalik@intel.com>,
	Milena Olech <milena.olech@intel.com>,
	linux-kernel@vger.kernel.org, ivecera@redhat.com
Subject: Re: [PATCH net v2] dpll: fix NULL deref in dpll_device_ops() during teardown race
Date: Fri, 14 Aug 2026 23:35:57 +0100	[thread overview]
Message-ID: <e28dd09f-be7b-4f74-9d42-95baca307db0@linux.dev> (raw)
In-Reply-To: <20260813140817.1051388-1-poros@redhat.com>

On 13/08/2026 15:08, Petr Oros wrote:
> When the last owner of a dpll device unregisters while a foreign driver
> still holds a pin on it via dpll_pin_on_pin_register(), the dpll object
> stays alive with an empty registration list. A pin notification queued
> before the unregister (e.g. ice reacting to zl3073x_i2c removal) then
> walks pin->dpll_refs into dpll_device_ops(), which trips the WARN_ON and
> dereferences the missing registration. dpll_lock cannot help because the
> notification work was queued before the unregistering driver took the
> lock.
> 
> Treat the empty registration list as a legitimate transient state. Make
> dpll_priv() and dpll_device_ops() return NULL in that case and make
> every pin netlink path that resolves a device from a pin skip such
> dplls. dpll_cmd_pin_get_one() picks a ref with a live registration and
> returns -ENODEV when there is none, the pin dumpit skips such a pin
> instead of aborting the dump, dpll_msg_add_pin_dplls() and the
> frequency, esync, reference sync and phase adjust set paths skip dead
> refs, and dpll_pin_parent_device_set() validates the parent with
> dpll_device_get_by_id(). dpll_pin_register() is the last caller that
> dereferenced the device ops without a check, so move its frequency
> monitor validation under dpll_lock and tolerate a missing registration
> there as well.
> 
> The empty registration list is equivalent to a cleared DPLL_REGISTERED
> mark, both transitions happen under dpll_lock in dpll_device_register()
> and dpll_device_unregister(). A pin notification for a pin whose dplls
> are all gone is now dropped with -ENODEV instead of crashing, all
> callers in the core ignore that return value.
> 
>   WARNING: drivers/dpll/dpll_core.c:1092 at dpll_device_ops+0x24/0x40,
>   CPU#83: kworker/u576:3/23471
>   Modules linked in: ... ice ... zl3073x_i2c(-) ... zl3073x ...
>   Workqueue: ice_dpll_wq ice_dpll_pin_notify_work [ice]
>   RIP: 0010:dpll_device_ops+0x24/0x40
>   Call Trace:
>    <TASK>
>    dpll_cmd_pin_get_one+0x336/0x520
>    dpll_pin_event_send+0x82/0x140
>    dpll_pin_on_pin_unregister+0xbb/0x160
>    ice_dpll_pin_notify_work+0x1bc/0x1f0 [ice]
>    process_one_work+0x19e/0x370
>    worker_thread+0x1a6/0x310
>    kthread+0xe4/0x120
>    ret_from_fork+0x1a1/0x270
>    ret_from_fork_asm+0x1a/0x30
>    </TASK>
>   ---[ end trace 0000000000000000 ]---
>   BUG: kernel NULL pointer dereference, address: 0000000000000010
>   #PF: supervisor read access in kernel mode
>   #PF: error_code(0x0000) - not-present page
> 
> Fixes: 9431063ad323 ("dpll: core: Add DPLL framework base functions")
> Signed-off-by: Petr Oros <poros@redhat.com>
> ---
> v2:
> - guard every path that resolves a device from a pin, not only the
>    first ref in dpll_cmd_pin_get_one(); skip half-dead refs in
>    dpll_msg_add_pin_dplls() and the set paths, select a live
>    representative ref and turn the pin dumpit -ENODEV into a per pin
>    skip (Jakub)
> - validate the parent device in dpll_pin_parent_device_set() via
>    dpll_device_get_by_id()
> - guard the frequency monitor validation in dpll_pin_register() and
>    perform it under dpll_lock, it was the only remaining unchecked
>    dereference of the device ops
> - drop patch 2/2, superseded by commit 32239d600236 ("dpll: fix stale
>    iteration in dpll_pin_on_pin_unregister()")
> 
> v1: https://lore.kernel.org/all/20260516191317.1005612-2-poros@redhat.com/
> ---
>   drivers/dpll/dpll_core.c    | 24 +++++++++------
>   drivers/dpll/dpll_netlink.c | 59 ++++++++++++++++++++++++++++++++-----
>   2 files changed, 67 insertions(+), 16 deletions(-)
> 

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

      parent reply	other threads:[~2026-08-14 22:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-13 14:08 Petr Oros
2026-08-13 15:36 ` Ivan Vecera
2026-08-14 22:35 ` Vadim Fedorenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e28dd09f-be7b-4f74-9d42-95baca307db0@linux.dev \
    --to=vadim.fedorenko@linux.dev \
    --cc=arkadiusz.kubalewski@intel.com \
    --cc=ivecera@redhat.com \
    --cc=jiri@resnulli.us \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.michalik@intel.com \
    --cc=milena.olech@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=poros@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®