netdev - Re: [RFC PATCH 1/1] net: wwan: core: explicit WWAN device reference counting

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFEp6-17zHKA+88FfTqUKV44O18sg7Ow2HAt05ucucaXjXbSKA@mail.gmail.com>
Date: Thu, 8 Jan 2026 09:59:10 +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 1/1] net: wwan: core: explicit WWAN device reference counting

On Thu, Jan 8, 2026 at 3:05 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>
> ---
>  drivers/net/wwan/wwan_core.c | 29 +++++++++--------------------
>  1 file changed, 9 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
> index ade8bbffc93e..d24f7b2b435b 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;
> +       unsigned 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,12 @@ 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);
>
> -       /* 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 (!ret) {
> +       if (--wwandev->refcount == 0) {

Looks good to me, though I’m not sure why this wasn’t the initial
solution. I’d suggest adding a paranoid WARN here, just in the
unlikely case there are still ops or wwan children attached.


>  #ifdef CONFIG_WWAN_DEBUGFS
>                 debugfs_remove_recursive(wwandev->debugfs_dir);
>  #endif
> --
> 2.52.0
>