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] [day] [month] [year] [list]
Message-ID: <62406CC3-02F6-4D9D-9A0B-63C07B162AA6@gmail.com>
Date: Thu, 08 Jan 2026 15:01:37 +0200
From: Sergey Ryazanov <ryazanov.s.a@...il.com>
To: Loic Poulain <loic.poulain@....qualcomm.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 January 8, 2026 10:59:10 AM, Loic Poulain <loic.poulain@....qualcomm.com> wrote:
>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.

Good point. Will put BUG_ON for both: ops and *any* child existence.

--
Sergey

Hi Loic