| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAFEp6-3n=125v7RDbbJaT2XPyt+adGfgY-pgXguymYVy+oxuaw@mail.gmail.com>
Date: Wed, 14 Jan 2026 22:31:45 +0100
From: Loic Poulain <loic.poulain@....qualcomm.com>
To: Sergey Ryazanov <ryazanov.s.a@...il.com>
Cc: Johannes Berg <johannes@...solutions.net>,
Andrew Lunn <andrew+netdev@...n.ch>,
Eric Dumazet <edumazet@...gle.com>,
"David S . Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
netdev@...r.kernel.org, Daniele Palmas <dnlplm@...il.com>
Subject: Re: [RFC PATCH v5 2/7] net: wwan: core: explicit WWAN device
reference counting
On Fri, Jan 9, 2026 at 2:09 AM Sergey Ryazanov <ryazanov.s.a@...il.com> wrote:
>
> We need information about existing WWAN device children since we remove
> the device after removing the last child. Previously, we tracked users
> implicitly by checking whether ops was registered and existence of a
> child device of the wwan_class class. Upcoming GNSS (NMEA) port type
> support breaks this approach by introducing a child device of the
> gnss_class class.
>
> And a modem driver can easily trigger a kernel Oops by removing regular
> (e.g., MBIM, AT) ports first and then removing a GNSS port. The WWAN
> device will be unregistered on removal of a last regular WWAN port. And
> subsequent GNSS port removal will cause NULL pointer dereference in
> simple_recursive_removal().
>
> In order to support ports of classes other than wwan_class, switch to
> explicit references counting. Introduce a dedicated counter to the WWAN
> device struct, increment it on every wwan_create_dev() call, decrement
> on wwan_remove_dev(), and actually unregister the WWAN device when there
> are no more references.
>
> Run tested with wwan_hwsim with NMEA support patches applied and
> different port removing sequences.
>
> Reported-by: Daniele Palmas <dnlplm@...il.com>
> Closes: https://lore.kernel.org/netdev/CAGRyCJE28yf-rrfkFbzu44ygLEvoUM7fecK1vnrghjG_e9UaRA@mail.gmail.com/
> Suggested-by: Loic Poulain <loic.poulain@....qualcomm.com>
> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@...il.com>
> ---
> Changes:
> * RFCv2->RFCv5: new patch to address modem disconnection / system
> shutdown issues
> ---
> drivers/net/wwan/wwan_core.c | 37 ++++++++++++++++++------------------
> 1 file changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
> index ade8bbffc93e..33f7a140fba9 100644
> --- a/drivers/net/wwan/wwan_core.c
> +++ b/drivers/net/wwan/wwan_core.c
> @@ -42,6 +42,9 @@ static struct dentry *wwan_debugfs_dir;
> * struct wwan_device - The structure that defines a WWAN device
> *
> * @id: WWAN device unique ID.
> + * @refcount: Reference count of this WWAN device. When this refcount reaches
> + * zero, the device is deleted. NB: access is protected by global
> + * wwan_register_lock mutex.
> * @dev: Underlying device.
> * @ops: wwan device ops
> * @ops_ctxt: context to pass to ops
> @@ -49,6 +52,7 @@ static struct dentry *wwan_debugfs_dir;
> */
> struct wwan_device {
> unsigned int id;
> + int refcount;
> struct device dev;
> const struct wwan_ops *ops;
> void *ops_ctxt;
> @@ -222,8 +226,10 @@ static struct wwan_device *wwan_create_dev(struct device *parent)
>
> /* If wwandev already exists, return it */
> wwandev = wwan_dev_get_by_parent(parent);
> - if (!IS_ERR(wwandev))
> + if (!IS_ERR(wwandev)) {
> + wwandev->refcount++;
> goto done_unlock;
> + }
>
> id = ida_alloc(&wwan_dev_ids, GFP_KERNEL);
> if (id < 0) {
> @@ -242,6 +248,7 @@ static struct wwan_device *wwan_create_dev(struct device *parent)
> wwandev->dev.class = &wwan_class;
> wwandev->dev.type = &wwan_dev_type;
> wwandev->id = id;
> + wwandev->refcount = 1;
> dev_set_name(&wwandev->dev, "wwan%d", wwandev->id);
>
> err = device_register(&wwandev->dev);
> @@ -263,30 +270,21 @@ static struct wwan_device *wwan_create_dev(struct device *parent)
> return wwandev;
> }
>
> -static int is_wwan_child(struct device *dev, void *data)
> -{
> - return dev->class == &wwan_class;
> -}
> -
> static void wwan_remove_dev(struct wwan_device *wwandev)
> {
> - int ret;
> -
> /* Prevent concurrent picking from wwan_create_dev */
> mutex_lock(&wwan_register_lock);
FYI, you can use guarded mutex:
guard(mutex)(&wwan_register_lock);
This ensures the lock is 'automatically' released when leaving the
scope/function, and would save the below goto/out_unlock.
>
> - /* WWAN device is created and registered (get+add) along with its first
> - * child port, and subsequent port registrations only grab a reference
> - * (get). The WWAN device must then be unregistered (del+put) along with
> - * its last port, and reference simply dropped (put) otherwise. In the
> - * same fashion, we must not unregister it when the ops are still there.
> - */
> - if (wwandev->ops)
> - ret = 1;
> - else
> - ret = device_for_each_child(&wwandev->dev, NULL, is_wwan_child);
> + if (--wwandev->refcount <= 0) {
> + struct device *child = device_find_any_child(&wwandev->dev);
> +
> + if (WARN_ON(wwandev->ops)) /* Paranoid */
You may keep a reference to child (if existing)
> + goto out_unlock;
> + if (WARN_ON(child)) { /* Paranoid */
> + put_device(child);
> + goto out_unlock;
> + }
Maybe you can simplify that with:
```
struct device *child = device_find_any_child(&wwandev->dev);
put_device(child) /* NULL param is ok */
if (WARN_ON(child || wwandev->ops))
return; /* or goto */
```
>
> - if (!ret) {
> #ifdef CONFIG_WWAN_DEBUGFS
> debugfs_remove_recursive(wwandev->debugfs_dir);
> #endif
> @@ -295,6 +293,7 @@ static void wwan_remove_dev(struct wwan_device *wwandev)
> put_device(&wwandev->dev);
> }
>
> +out_unlock:
> mutex_unlock(&wwan_register_lock);
> }
>
> --
> 2.52.0
>
Regards,
Loic
Powered by blists - more mailing lists