[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