From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Mike Isely at pobox <isely@pobox.com>
Cc: mike.isely@cobaltdigital.com, Daniel Scally <djrscally@gmail.com>,
Heikki Krogerus <heikki.krogerus@linux.intel.com>,
Sakari Ailus <sakari.ailus@linux.intel.com>,
linux-acpi@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node
Date: Thu, 26 Feb 2026 09:16:47 +0200 [thread overview]
Message-ID: <aZ_zX-jQ7T8LVeSU@smile.fi.intel.com> (raw)
In-Reply-To: <5c5f39f4-9797-4d84-eaf2-3fb7a830cbbf@isely.net>
On Wed, Feb 25, 2026 at 02:16:39PM -0600, Mike Isely wrote:
> On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > On Wed, Feb 25, 2026 at 01:42:30PM -0600, Mike Isely wrote:
> > > On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > > > On Tue, Feb 24, 2026 at 01:19:22PM -0600, mike.isely@cobaltdigital.com wrote:
...
> > > > > A scenario exists where device_create_managed_software_node() is used
> > > > > to create an swnode instance that will be implicitly shared to a child
> > > > > device despite best intentions not to permit such sharing (per the
> > > > > comment in device_create_managed_software_node()). I encountered this
> > > > > with the sfp kernel module when it was instantiated with properties
> > > >
> > > > SFP? Or is it the name of the actual module in the kernel?
> > >
> > > Actual kernel module name, sfp.ko, CONFIG_SFP in .config, named after
> > > the piece of hardware it works with, an SFP cage. This is logic which
> > > monitors SFP cages for hotplug appearance / removal of SFP transceivers.
> > > When a transceiver appears, the sfp kernel module will create a child
> > > hwmon device instance to monitor various bits of metadata from the
> > > transceiver. When that transceiver goes away, the sfp kernel module
> > > will tear down that child hwmon device instance.
> > >
> > > The sfp kernel module needs resources configured to know where to
> > > monitor; in our case that is set up dynamically by another locally
> > > written kernel module (which iteracts with an FPGA we have where the SFP
> > > hardware elements reside), and that kernel module will combine
> >
> > > devicetree information with some run-time information to generate the
> > > properties handed off to the sfp kernel module instantiation.
> >
> > What runtime information? Why this can't be done via DT overlay as others do?
>
> I don't recall the specifics. It might be calculation of a unit name.
> The connectivity in this case is I2C so that should be a constant. We
> have some variants where the FPGA is PCIE-connected to the host and so
> the memory map is a run-time calculation. We have other drivers that
> have to be instantiated with run-time computed properties. So we handle
> this as a general case.
But the configurations are semi-static, right? For the contents of FPGA we have
a specific manager that reloads the FPGA configuration.
Using swnode for dynamically calculated data seems weird. The data in swnodes
is usually static (const), I can't remember the case where we need to supply
run-time calculated values.
Since that, DT overlay approach seems fine, no?
> > > We use platform_device_register_full() to instantiate it, and that in turn
> > > causes the swnode instance to be created. The hwmon child instance
> > > later created by the sfp module ends up inheriting those resources,
> > > including the swnode instance, from the sfp parent device, and when the
> > > hwmon instance is later torn down, we end up with the use-after-free
> > > problem due to the swnode instance's reference count being incorrectly
> > > decremented by that child device due to the managed flag being set.
> > >
> > > If the term SFP is unfamiliar, an explanation can be found here:
> > >
> > > https://en.wikipedia.org/wiki/Small_Form-factor_Pluggable
> > >
> > > > > via a call to platform_device_register_full() - it will create hwmon
> > > > > child devices which get all property references. Unfortunately with
> > > > > just a "managed" boolean in struct swnode handling this, then
> > > > > kobject_put() gets called for the managed aspect when the child device
> > > > > goes away instead of the parent. This leads to premature freeing of
> > > > > the swnode structure, followed by use-after-free problems, heap
> > > > > corruption, and generally chaos / crashes / misbehavior in the kernel.
> > > > >
> > > > > This commit changes that boolean into a pointer to the actual managing
> > > > > struct device, which is then checked against the struct device
> > > > > instance that is actually going away (via the usual call back into
> > > > > software_node_notify_remove()). Thus the child device removal is
> > > > > ignored as it should, and we only do the kobject_put() when the actual
> > > > > managing struct device instance goes away. We effectively carry a
> > > > > little bit more information now so that we can be sure to clean up
> > > > > only when the correct struct device instance is actually going away.
> > > > >
> > > > > Note that while we are now keeping a pointer to a struct device here,
> > > > > this is safe to do because the pointer itself only stays in use while
> > > > > the pointed-to device remains valid. (So no need to be concerned
> > > > > about additional reference counting.)
> > > >
> > > > The term is called "object lifetime".
> > >
> > > Meh
...
> Would SoB lines by *both* addresses (even though it's the same person)
> clear that hurdle?
Yes.
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2026-02-26 7:16 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-24 19:19 [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c mike.isely
2026-02-24 19:19 ` [PATCH 1/1] sofware node: Only the managing device can unreference managed software node mike.isely
2026-02-25 11:22 ` Andy Shevchenko
2026-02-25 19:42 ` Mike Isely
2026-02-25 20:01 ` Andy Shevchenko
2026-02-25 20:16 ` Mike Isely
2026-02-26 7:16 ` Andy Shevchenko [this message]
2026-02-26 19:06 ` Mike Isely
2026-02-26 20:42 ` Andy Shevchenko
2026-02-27 17:55 ` Mike Isely
2026-02-28 11:02 ` Andy Shevchenko
2026-02-28 16:34 ` Mike Isely
2026-02-25 9:46 ` [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c Andy Shevchenko
2026-02-25 18:59 ` Mike Isely
2026-02-25 19:17 ` Andy Shevchenko
2026-02-25 19:48 ` Mike Isely
2026-02-25 20:05 ` Andy Shevchenko
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=aZ_zX-jQ7T8LVeSU@smile.fi.intel.com \
--to=andriy.shevchenko@linux.intel.com \
--cc=djrscally@gmail.com \
--cc=heikki.krogerus@linux.intel.com \
--cc=isely@pobox.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mike.isely@cobaltdigital.com \
--cc=sakari.ailus@linux.intel.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®