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

[PATCH 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 the explicit unregistration calls from carl9170_unregister().

Because carl9170_register() runs asynchronously from the
request_firmware_nowait() callback after probe has returned, and
carl9170_usb_disconnect() frees struct ar9170 immediately in the
interface disconnect callback before devres_release_all() runs, both the
WPS input device (along with its ar->wps.name and ar->wps.phys strings)
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>
---
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 1/2] wifi: carl9170: Revert "carl9170: devres-ing input_allocate_device"

[Dmitry Torokhov]

This reverts commit 87ddb2fc29f10cf689ff0dfb88a19b7d3687006b.

carl9170_register() is invoked asynchronously from the
request_firmware_nowait() callback after carl9170_usb_probe() has
already returned, and carl9170_usb_disconnect() calls
carl9170_unregister() followed immediately by carl9170_free(), which
frees struct ar9170 (including ar->wps.name and ar->wps.phys).

Allocating the WPS button input device via
devm_input_allocate_device(&ar->udev->dev) ties its unregistration to
the parent struct usb_device rather than the driver lifecycle. Even if
it were tied to &ar->intf->dev, devres_release_all() runs after
carl9170_usb_disconnect() has already freed struct ar9170 and
unregistered the parent wiphy device. Consequently, the input device
remains registered on input_dev_list with input->name and input->phys
pointing into freed memory, causing a use-after-free when reading
/proc/bus/input/devices or when generating the KOBJ_REMOVE uevent on
unregistration.

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.rc1.315.gc6ed9934b7-goog

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

[Dmitry Torokhov]

This reverts commit 23de0fa0d2a05ff71c7bc8df9d12c9f23be83f13.

carl9170_register() is invoked asynchronously from the
request_firmware_nowait() callback after carl9170_usb_probe() has
already returned, and carl9170_usb_disconnect() calls
carl9170_unregister() followed immediately by carl9170_free(), which
frees struct ar9170 (including struct hwrng ar->rng.rng and
ar->rng.name).

Registering the hardware random number generator via
devm_hwrng_register(&ar->udev->dev, &ar->rng.rng) ties its
unregistration to the parent struct usb_device rather than the driver
lifecycle. Furthermore, if carl9170_rng_get() fails at the end of
carl9170_register_hwrng(), or when the interface is disconnected and
carl9170_free() frees struct ar9170 before devres_release_all() runs,
the embedded struct hwrng remains registered on the global rng_list in
freed memory, causing a use-after-free.

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.rc1.315.gc6ed9934b7-goog

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

[Christian Lamparter]

On 10/1/26 6:45 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 the explicit unregistration calls from carl9170_unregister().
> 
> Because carl9170_register() runs asynchronously from the
> request_firmware_nowait() callback after probe has returned, and
> carl9170_usb_disconnect() frees struct ar9170 immediately in the
> interface disconnect callback before devres_release_all() runs, both the
> WPS input device (along with its ar->wps.name and ar->wps.phys strings)
> and the embedded struct hwrng remain registered after struct ar9170 has
> been freed, leading to use-after-free bugs.

? Do I have a different source there ?

carl9170_usb_disconnect() does a wait_for_completion(&ar->fw_load_wait)
before doing anything.

For this completion to be "completed" either the firmware loader callback
went as far as running through all the initialization (includes
carl9170_register(), which registers the WPS button + rng) successfully
and the device is up.

or if there was a grave error (usb protocol error, firmware not responding
the way we want) and the driver basically has to give up... (but then
carl9170_register would have never been able to even get as far as
registering the wps + rng)

Cheers,
Christian

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

[Dmitry Torokhov]

Hi Christian,

On Thu, Oct 01, 2026 at 08:30:29PM +0200, Christian Lamparter wrote:
> On 10/1/26 6:45 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 the explicit unregistration calls from carl9170_unregister().
> > 
> > Because carl9170_register() runs asynchronously from the
> > request_firmware_nowait() callback after probe has returned, and
> > carl9170_usb_disconnect() frees struct ar9170 immediately in the
> > interface disconnect callback before devres_release_all() runs, both the
> > WPS input device (along with its ar->wps.name and ar->wps.phys strings)
> > and the embedded struct hwrng remain registered after struct ar9170 has
> > been freed, leading to use-after-free bugs.
>
> ? Do I have a different source there ?
>
> carl9170_usb_disconnect() does a wait_for_completion(&ar->fw_load_wait)
> before doing anything.
>
> For this completion to be "completed" either the firmware loader callback
> went as far as running through all the initialization (includes
> carl9170_register(), which registers the WPS button + rng) successfully
> and the device is up.
>
> or if there was a grave error (usb protocol error, firmware not responding
> the way we want) and the driver basically has to give up... (but then
> carl9170_register would have never been able to even get as far as
> registering the wps + rng)

Sorry for the confusion, mentioning request_firmware_nowait() in the
commit description was a distraction. The issue is not a race with
fw_load_wait, but what happens *after* wait_for_completion() returns in
carl9170_usb_disconnect():

1. Normal disconnect / unbind teardown order:
   In carl9170_usb_disconnect(), right after wait_for_completion(), the
   driver calls carl9170_unregister(ar) and then carl9170_free(ar) ->
   ieee80211_free_hw(ar->hw), which frees struct ar9170 immediately
   inside the .disconnect() callback.

   Because the devres conversions removed input_unregister_device() and
   hwrng_unregister() from carl9170_unregister(), both the WPS input
   device (whose input->name and input->phys point to ar->wps.name and
   ar->wps.phys inside struct ar9170) and ar->rng.rng (which is embedded
   directly inside struct ar9170) are still registered when struct
   ar9170 is freed:

   - The devm_* calls were attached to &ar->udev->dev (the parent
     struct usb_device) rather than &ar->intf->dev (struct
     usb_interface). If the driver is unbound from the interface (via
     sysfs unbind or rmmod) while the USB device stays plugged in,
     devres_release_all(&ar->udev->dev) does not run at all, leaving the
     input device and embedded hwrng registered in global lists after ar
     is freed.
   - Even on a physical USB unplug (and even if &ar->intf->dev had been
     used), the driver core runs .disconnect() before
     devres_release_all(). Thus struct ar9170 is already freed before
     devres runs devm_hwrng_unregister() (which dereferences the freed
     ar->rng.rng to unlink it from rng_list) and
     devm_input_device_unregister() (which reads the freed ar->wps.name
     and ar->wps.phys when generating the KOBJ_REMOVE uevent).

2. Error path in carl9170_register():
   Even during initial bringup, carl9170_register() calls
   carl9170_register_wps_button() and then carl9170_register_hwrng(),
   which calls devm_hwrng_register() *before* carl9170_rng_get(ar)
   issues CARL9170_CMD_RREG over USB.

   If carl9170_rng_get(ar) fails due to a USB or firmware error, both
   the WPS button and the HWRNG have already been registered.
   carl9170_register() then jumps to err_unreg -> carl9170_unregister(),
   and carl9170_usb_firmware_failed() calls
   usb_driver_release_interface(), which runs carl9170_usb_disconnect()
   and frees ar while &ar->udev->dev remains bound.

I can send a v2 with updated commit messages that drop the mention of
request_firmware_nowait() and focus directly on the disconnect teardown
order if you prefer.

Thanks.

-- 
Dmitry