[PATCH v2 0/2] wifi: carl9170: revert broken devres conversions for input and hwrng

[PATCH v2 0/2] wifi: carl9170: revert broken devres conversions for input and hwrng

[Dmitry Torokhov]

Commits 23de0fa0d2a0 ("carl9170: devres-ing hwrng_register usage") and
87ddb2fc29f1 ("carl9170: devres-ing input_allocate_device") converted
the HWRNG and WPS button input device registrations in carl9170 to
devres attached to the parent struct usb_device (&ar->udev->dev) and
removed explicit unregistration from carl9170_unregister().

In carl9170_usb_disconnect(), the driver calls carl9170_unregister()
followed immediately by carl9170_free(), which frees struct ar9170
inside the interface .disconnect() callback before devres_release_all()
runs. Furthermore, devres on &ar->udev->dev is not released on
interface unbind or registration failure in carl9170_register().
As a result, both the WPS input device (whose input->name and
input->phys point into freed memory) and the embedded struct hwrng
remain registered after struct ar9170 has been freed, leading to
use-after-free bugs.

Revert both commits to restore explicit lifecycle management in
carl9170_unregister().

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Changes in v2:
- Clarify commit messages to focus on the teardown order in
  carl9170_usb_disconnect() and the error path in carl9170_register()
  rather than request_firmware_nowait().
- Link to v1: https://patch.msgid.link/20260930-carl9170-reverts-v1-0-7033b5716c14@gmail.com

---
Dmitry Torokhov (2):
      wifi: carl9170: Revert "carl9170: devres-ing input_allocate_device"
      wifi: carl9170: Revert "carl9170: devres-ing hwrng_register usage"

 drivers/net/wireless/ath/carl9170/carl9170.h |  1 +
 drivers/net/wireless/ath/carl9170/main.c     | 42 ++++++++++++++++++++++++----
 2 files changed, 38 insertions(+), 5 deletions(-)
---
base-commit: 6474fa070f2b8013b4b87350b775b8c3be6e8aac
change-id: 20260930-carl9170-reverts-bc4b3b6a1a25

Thanks.

-- 
Dmitry

[PATCH v2 1/2] wifi: carl9170: Revert "carl9170: devres-ing input_allocate_device"

[Dmitry Torokhov]

This reverts commit 87ddb2fc29f10cf689ff0dfb88a19b7d3687006b.

In carl9170_usb_disconnect(), the driver calls carl9170_unregister()
followed immediately by carl9170_free(), freeing struct ar9170 inside
.disconnect(), while devres resources are released only after
.disconnect() returns (and devres on &ar->udev->dev is not released at
all on interface unbind or registration failure), leaving the WPS input
device registered with input->name and input->phys pointing into freed
memory.

Fixes: 87ddb2fc29f1 ("carl9170: devres-ing input_allocate_device")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/net/wireless/ath/carl9170/main.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
index 61c7a1288743..702b6d113b18 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -1500,7 +1500,7 @@ static int carl9170_register_wps_button(struct ar9170 *ar)
 	if (!(ar->features & CARL9170_WPS_BUTTON))
 		return 0;
 
-	input = devm_input_allocate_device(&ar->udev->dev);
+	input = input_allocate_device();
 	if (!input)
 		return -ENOMEM;
 
@@ -1518,8 +1518,10 @@ static int carl9170_register_wps_button(struct ar9170 *ar)
 	input_set_capability(input, EV_KEY, KEY_WPS_BUTTON);
 
 	err = input_register_device(input);
-	if (err)
+	if (err) {
+		input_free_device(input);
 		return err;
+	}
 
 	ar->wps.pbc = input;
 	return 0;
@@ -2044,6 +2046,13 @@ void carl9170_unregister(struct ar9170 *ar)
 	carl9170_debugfs_unregister(ar);
 #endif /* CONFIG_CARL9170_DEBUGFS */
 
+#ifdef CONFIG_CARL9170_WPC
+	if (ar->wps.pbc) {
+		input_unregister_device(ar->wps.pbc);
+		ar->wps.pbc = NULL;
+	}
+#endif /* CONFIG_CARL9170_WPC */
+
 	carl9170_cancel_worker(ar);
 	cancel_work_sync(&ar->restart_work);
 

-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH v2 2/2] wifi: carl9170: Revert "carl9170: devres-ing hwrng_register usage"

[Dmitry Torokhov]

This reverts commit 23de0fa0d2a05ff71c7bc8df9d12c9f23be83f13.

carl9170_usb_disconnect() frees struct ar9170 inside .disconnect()
before devres release (and devres on &ar->udev->dev is not released
on interface unbind), leaving the embedded struct hwrng registered
on rng_list in freed memory. Furthermore, if carl9170_rng_get() fails
in carl9170_register_hwrng(), the HWRNG also remains registered while
ar is freed on the error path. Revert the devres conversion to restore
explicit HWRNG unregistration.

Fixes: 23de0fa0d2a0 ("carl9170: devres-ing hwrng_register usage")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/net/wireless/ath/carl9170/carl9170.h |  1 +
 drivers/net/wireless/ath/carl9170/main.c     | 29 +++++++++++++++++++++++++---
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/carl9170.h b/drivers/net/wireless/ath/carl9170/carl9170.h
index e66e3e2ae952..03a94a5a209b 100644
--- a/drivers/net/wireless/ath/carl9170/carl9170.h
+++ b/drivers/net/wireless/ath/carl9170/carl9170.h
@@ -454,6 +454,7 @@ struct ar9170 {
 # define CARL9170_HWRNG_CACHE_SIZE	CARL9170_MAX_CMD_PAYLOAD_LEN
 	struct {
 		struct hwrng rng;
+		bool initialized;
 		char name[30 + 1];
 		u16 cache[CARL9170_HWRNG_CACHE_SIZE / sizeof(u16)];
 		unsigned int cache_idx;
diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
index 702b6d113b18..8ca76e9044de 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -1545,7 +1545,7 @@ static int carl9170_rng_get(struct ar9170 *ar)
 
 	BUILD_BUG_ON(RB > CARL9170_MAX_CMD_PAYLOAD_LEN);
 
-	if (!IS_ACCEPTING_CMD(ar))
+	if (!IS_ACCEPTING_CMD(ar) || !ar->rng.initialized)
 		return -EAGAIN;
 
 	count = ARRAY_SIZE(ar->rng.cache);
@@ -1591,6 +1591,14 @@ static int carl9170_rng_read(struct hwrng *rng, u32 *data)
 	return sizeof(u16);
 }
 
+static void carl9170_unregister_hwrng(struct ar9170 *ar)
+{
+	if (ar->rng.initialized) {
+		hwrng_unregister(&ar->rng.rng);
+		ar->rng.initialized = false;
+	}
+}
+
 static int carl9170_register_hwrng(struct ar9170 *ar)
 {
 	int err;
@@ -1601,14 +1609,25 @@ static int carl9170_register_hwrng(struct ar9170 *ar)
 	ar->rng.rng.data_read = carl9170_rng_read;
 	ar->rng.rng.priv = (unsigned long)ar;
 
-	err = devm_hwrng_register(&ar->udev->dev, &ar->rng.rng);
+	if (WARN_ON(ar->rng.initialized))
+		return -EALREADY;
+
+	err = hwrng_register(&ar->rng.rng);
 	if (err) {
 		dev_err(&ar->udev->dev, "Failed to register the random "
 			"number generator (%d)\n", err);
 		return err;
 	}
 
-	return carl9170_rng_get(ar);
+	ar->rng.initialized = true;
+
+	err = carl9170_rng_get(ar);
+	if (err) {
+		carl9170_unregister_hwrng(ar);
+		return err;
+	}
+
+	return 0;
 }
 #endif /* CONFIG_CARL9170_HWRNG */
 
@@ -2053,6 +2072,10 @@ void carl9170_unregister(struct ar9170 *ar)
 	}
 #endif /* CONFIG_CARL9170_WPC */
 
+#ifdef CONFIG_CARL9170_HWRNG
+	carl9170_unregister_hwrng(ar);
+#endif /* CONFIG_CARL9170_HWRNG */
+
 	carl9170_cancel_worker(ar);
 	cancel_work_sync(&ar->restart_work);
 

-- 
2.56.0.385.gd3acb90ef8-goog

Re: [PATCH v2 0/2] wifi: carl9170: revert broken devres conversions for input and hwrng

[Christian Lamparter]

On 10/8/26 11:39 AM, Dmitry Torokhov wrote:
> Commits 23de0fa0d2a0 ("carl9170: devres-ing hwrng_register usage") and
> 87ddb2fc29f1 ("carl9170: devres-ing input_allocate_device") converted
> the HWRNG and WPS button input device registrations in carl9170 to
> devres attached to the parent struct usb_device (&ar->udev->dev) and
> removed explicit unregistration from carl9170_unregister().
> 
> In carl9170_usb_disconnect(), the driver calls carl9170_unregister()
> followed immediately by carl9170_free(), which frees struct ar9170
> inside the interface .disconnect() callback before devres_release_all()
> runs. Furthermore, devres on &ar->udev->dev is not released on
> interface unbind or registration failure in carl9170_register().
> As a result, both the WPS input device (whose input->name and
> input->phys point into freed memory) and the embedded struct hwrng
> remain registered after struct ar9170 has been freed, leading to
> use-after-free bugs.

Ok, so you just reworded your patch? Sight...

looking at the WPS input


|        snprintf(ar->wps.name, sizeof(ar->wps.name), "%s WPS Button",
|                 wiphy_name(ar->hw->wiphy));
|
|        snprintf(ar->wps.phys, sizeof(ar->wps.phys),
|                 "ieee80211/%s/input0", wiphy_name(ar->hw->wiphy));
|
|        input->name = ar->wps.name;
|        input->phys = ar->wps.phys;
|        input->id.bustype = BUS_USB;
|        input->dev.parent = &ar->hw->wiphy->dev;
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

the input's dev.parent is set to ar->hw->wiphy->dev and not ar->udev->dev, right?
Does this do anything at all? If not, why? The wiphy gets shutdown by
ieee80211_unregister() and having the "freeing" stick around after the USB device
is gone should not hurt, right?

As for the hwrng, wouldn't it make sense to use the wiphy dev there as well?
So the whole reverting can be sidestepped by simply going with wiphy dev.

Cheers,
Christian

Re: [PATCH v2 0/2] wifi: carl9170: revert broken devres conversions for input and hwrng

[Dmitry Torokhov]

Hi Christian,

On Fri, Oct 09, 2026 at 09:37:36PM +0200, Christian Lamparter wrote:
> On 10/8/26 11:39 AM, Dmitry Torokhov wrote:
> > Commits 23de0fa0d2a0 ("carl9170: devres-ing hwrng_register usage") and
> > 87ddb2fc29f1 ("carl9170: devres-ing input_allocate_device") converted
> > the HWRNG and WPS button input device registrations in carl9170 to
> > devres attached to the parent struct usb_device (&ar->udev->dev) and
> > removed explicit unregistration from carl9170_unregister().
> > 
> > In carl9170_usb_disconnect(), the driver calls carl9170_unregister()
> > followed immediately by carl9170_free(), which frees struct ar9170
> > inside the interface .disconnect() callback before devres_release_all()
> > runs. Furthermore, devres on &ar->udev->dev is not released on
> > interface unbind or registration failure in carl9170_register().
> > As a result, both the WPS input device (whose input->name and
> > input->phys point into freed memory) and the embedded struct hwrng
> > remain registered after struct ar9170 has been freed, leading to
> > use-after-free bugs.
>
> Ok, so you just reworded your patch? Sight...

Yes, as I promised I would.

> looking at the WPS input
>
> |        input->name = ar->wps.name;
> |        input->phys = ar->wps.phys;
> |        input->id.bustype = BUS_USB;
> |        input->dev.parent = &ar->hw->wiphy->dev;
>           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> the input's dev.parent is set to ar->hw->wiphy->dev and not ar->udev->dev, right?
> Does this do anything at all? If not, why? The wiphy gets shutdown by
> ieee80211_unregister() and having the "freeing" stick around after the USB device
> is gone should not hurt, right?

devm_input_allocate_device() explicitly states:

	NOTE: the owner device is set up as parent of input device and
	users should not override it.

That is because managed input devices split teardown across two separate
devres entries: devm_input_allocate_device(dev) registers
devm_input_device_release() on dev, whereas input_register_device()
registers devm_input_device_unregister() on input->dev.parent.

Overriding input->dev.parent splits those two actions across different
devices: if carl9170 is unbound from the USB interface (via sysfs unbind
or rmmod) without physically unplugging the USB device, or if
input_register_device() fails, &ar->udev->dev is never unbound and
devm_input_device_release never runs, leaking struct input_dev and an
input-core module reference on every unbind/rebind cycle.

I'll look into how to make input core reject such overrides. So far
there are 4 drivers that do that and I'll fix them up.

Also, using ar->hw->wiphy->dev as the input device's parent has
its own issues: wiphy devices can be renamed ("iw phy phy0 set name
...") or moved between network namespaces ("iw phy phy0 set netns ...").
Because phyX sits under a netns-tagged ieee80211 glue directory whereas
input_class is not namespace-tagged:

- renaming phyX changes the input device's sysfs path without emitting
  KOBJ_MOVE uevents for child input/event devices (leaving udev's cached
  DEVPATH out of date); and

- moving phyX into another netns re-tags the phyX directory to that
  namespace, turning /sys/class/input/inputN into a dangling symlink
  in init_net.

> As for the hwrng, wouldn't it make sense to use the wiphy dev there as well?
> So the whole reverting can be sidestepped by simply going with wiphy dev.

If we do that then unregistering of input device and hwrng will happen
inside ieee80211_unregister_hw() -> wiphy_unregister() -> device_del(),
which will hold both global rtnl_lock() and wiphy_lock(). Unregistering
the HWRNG there stalls global RTNL while hwrng_unregister() waits on
cleanup_done for any in-flight read to finish, whereas explicit
carl9170_unregister() tears down both WPS input and HWRNG upfront before
wiphy_unregister() acquires rtnl_lock() and wiphy_lock().

Until carl9170's own lifecycle (carl9170_alloc()/carl9170_free()) is
managed via devres on &ar->intf->dev, explicit unregistration in
carl9170_unregister() is the right way to manage these sub-devices.

Thanks.

-- 
Dmitry

Re: [PATCH v2 0/2] wifi: carl9170: revert broken devres conversions for input and hwrng

[Christian Lamparter]

On 10/10/26 1:13 AM, Dmitry Torokhov wrote:
> Hi Christian,
> 
> On Fri, Oct 09, 2026 at 09:37:36PM +0200, Christian Lamparter wrote:
>> On 10/8/26 11:39 AM, Dmitry Torokhov wrote:
>>> Commits 23de0fa0d2a0 ("carl9170: devres-ing hwrng_register usage") and
>>> 87ddb2fc29f1 ("carl9170: devres-ing input_allocate_device") converted
>>> the HWRNG and WPS button input device registrations in carl9170 to
>>> devres attached to the parent struct usb_device (&ar->udev->dev) and
>>> removed explicit unregistration from carl9170_unregister().
>>>
>>> In carl9170_usb_disconnect(), the driver calls carl9170_unregister()
>>> followed immediately by carl9170_free(), which frees struct ar9170
>>> inside the interface .disconnect() callback before devres_release_all()
>>> runs. Furthermore, devres on &ar->udev->dev is not released on
>>> interface unbind or registration failure in carl9170_register().
>>> As a result, both the WPS input device (whose input->name and
>>> input->phys point into freed memory) and the embedded struct hwrng
>>> remain registered after struct ar9170 has been freed, leading to
>>> use-after-free bugs.
>>
>> Ok, so you just reworded your patch? Sight...
> 
> Yes, as I promised I would.
> 
>> looking at the WPS input
>>
>> |        input->name = ar->wps.name;
>> |        input->phys = ar->wps.phys;
>> |        input->id.bustype = BUS_USB;
>> |        input->dev.parent = &ar->hw->wiphy->dev;
>>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> the input's dev.parent is set to ar->hw->wiphy->dev and not ar->udev->dev, right?
>> Does this do anything at all? If not, why? The wiphy gets shutdown by
>> ieee80211_unregister() and having the "freeing" stick around after the USB device
>> is gone should not hurt, right?
> 
> devm_input_allocate_device() explicitly states:
> 
> 	NOTE: the owner device is set up as parent of input device and
> 	users should not override it.

Yeah, and this is something you added in 2012... The carl9170 code driver
is back from 2010.

So, was this changed to "break it", rather than the driver?

 From my POV: What's the point of devres then? And why are you so "eager"
to revert the changes.

Cheers,
Christian

Re: [PATCH v2 0/2] wifi: carl9170: revert broken devres conversions for input and hwrng

[Dmitry Torokhov]

On Sun, Oct 11, 2026 at 04:41:02PM +0200, Christian Lamparter wrote:
> On 10/10/26 1:13 AM, Dmitry Torokhov wrote:
> > Hi Christian,
> > 
> > On Fri, Oct 09, 2026 at 09:37:36PM +0200, Christian Lamparter wrote:
> > > On 10/8/26 11:39 AM, Dmitry Torokhov wrote:
> > > > Commits 23de0fa0d2a0 ("carl9170: devres-ing hwrng_register usage") and
> > > > 87ddb2fc29f1 ("carl9170: devres-ing input_allocate_device") converted
> > > > the HWRNG and WPS button input device registrations in carl9170 to
> > > > devres attached to the parent struct usb_device (&ar->udev->dev) and
> > > > removed explicit unregistration from carl9170_unregister().
> > > > 
> > > > In carl9170_usb_disconnect(), the driver calls carl9170_unregister()
> > > > followed immediately by carl9170_free(), which frees struct ar9170
> > > > inside the interface .disconnect() callback before devres_release_all()
> > > > runs. Furthermore, devres on &ar->udev->dev is not released on
> > > > interface unbind or registration failure in carl9170_register().
> > > > As a result, both the WPS input device (whose input->name and
> > > > input->phys point into freed memory) and the embedded struct hwrng
> > > > remain registered after struct ar9170 has been freed, leading to
> > > > use-after-free bugs.
> > > 
> > > Ok, so you just reworded your patch? Sight...
> > 
> > Yes, as I promised I would.
> > 
> > > looking at the WPS input
> > > 
> > > |        input->name = ar->wps.name;
> > > |        input->phys = ar->wps.phys;
> > > |        input->id.bustype = BUS_USB;
> > > |        input->dev.parent = &ar->hw->wiphy->dev;
> > >            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > 
> > > the input's dev.parent is set to ar->hw->wiphy->dev and not ar->udev->dev, right?
> > > Does this do anything at all? If not, why? The wiphy gets shutdown by
> > > ieee80211_unregister() and having the "freeing" stick around after the USB device
> > > is gone should not hurt, right?
> > 
> > devm_input_allocate_device() explicitly states:
> > 
> > 	NOTE: the owner device is set up as parent of input device and
> > 	users should not override it.
> 
> Yeah, and this is something you added in 2012... 

Yes, in the same patch where devm_input_allocate_device() was
introduced. This requirement was there from the very beginning.

> The carl9170 code driver is back from 2010.

No, the devm conversion was done in 2022, 10 years later.

> 
> So, was this changed to "break it", rather than the driver?

The conversion to devm in the driver was broken on arrival.

> 
> From my POV: What's the point of devres then?

In certain situation it allows to reduce amount of boilerplate code in
the driver. But it has be to be used properly. One of the most important
thinks it attaching resources to the right device and not mixing managed
and noon managed resources in the same driver, as that very often causes
incorrect order on unwinding.

> And why are you so "eager" to revert the changes.

Because I have to deal with syzhot reports routed to me and I would
rather not.

Humor me: why are you so against reverting the change? I have shown that
it is problematic. You can go back and re-introduce devm for input once
the rest of the driver is ready.

Thanks.

-- 
Dmitry