Message-ID: <CADYyEwQsA2KR+LbO5-mkR+s6DpL6fkvB1jrYoWOrROzetvhoWQ@mail.gmail.com>
Date: Tue, 18 Nov 2025 15:31:03 +0800
From: Zhengqiao Xia <xiazhengqiao@...qin.corp-partner.google.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: rafael@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] device_core: check null pointer

Greg KH <gregkh@...uxfoundation.org> 于2025年11月17日周一 20:37写道：

>
> On Mon, Nov 17, 2025 at 10:55:05AM +0800, Zhengqiao Xia wrote:
> > Hi Greg,
> >
> > Based on my analysis,
>
> Please do not top-post :(

got it

>
> >
> > usb_new_device ->  device_add -> device_private_init
> >                                                     ->
> > bus_probe_device -> usb_generic_driver_probe  ->
> > usb_set_configuration  -> "add each interface"  ->
> > device_add(&intf->dev);
> >
> >
> >                                     --> if of_node status is disabled
> > -> of_device_is_available is false -> not running  device_add ->
>
> Why is of_node related to USB interfaces?

you can see usb/core/message.c
```
for (i = 0; i < nintf; ++i) {
struct usb_interface *intf = cp->interface[i];

dump_usb_intf_info(intf);

   if (intf->dev.of_node &&
               !of_device_is_available(intf->dev.of_node)) {
          dev_info(&dev->dev, "skipping disabled interface %d\n",
          intf->cur_altsetting->desc.bInterfaceNumber);
          continue;
     }
     ...
     ret = device_add(&intf->dev);
     ...
}
```

>
> > interface dev->p is null
> > usbdev_ioctl -> proc_ioctl -> device_attach -> __device_attach ->
> > device_attach ->  if (dev->p->dead)
> >
> > Since dev->p has not been created at this time, dev->p is NULL,  So I
> > think we should add a check here.
> >
> > Here is my current analysis, but there may be some misunderstandings
> > or mistakes in it. Could you please help clarify my confusion?
>
> As this code is very old, USB was one of the first subsystems to get
> driver core support decades ago, what has changed recently to require
> this check?  Have you seen crashes here?  If so, what is the traceback
> for them and with what hardware/drivers?

```
[   93.530302] usb 4-1: USB disconnect, device number 2
[   93.535637] cdc_mbim 4-1:1.0 wwan0: unregister 'cdc_mbim'
usb-11260000.usb-1, CDC MBIM
[   93.564041] option1 ttyUSB0: GSM modem (1-port) converter now
disconnected from ttyUSB0
[   93.574619] option 4-1:1.2: device disconnected
[   93.948062] usb 4-1: new high-speed USB device number 3 using xhci-mtk
[   94.084037] usb 4-1: New USB device found, idVendor=05c6,
idProduct=9008, bcdDevice= 0.00
[   94.092275] usb 4-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[   94.099455] usb 4-1: Product: QUSB__BULK
[   94.103407] usb 4-1: Manufacturer: Qualcomm CDMA Technologies MSM
[   94.114020] [USB-ATTACH] dev=4-1 dev->p=ffffff811faec600
[   94.122394] ===== USB Interface Dump =====
[   94.122394] dev name:        4-1:1.0
[   94.122394] dev of_node:     hub
[   94.122394] dev->p:          0000000000000000 (NULL)
[   94.122394] parent name:     4-1
[   94.122394] parent of_node:  hub
[   94.122394] ==============================
[   94.148881] usb 4-1: skipping disabled interface 0

--> 4-1:1.0 doesn't run device_add

[   94.161987] [USB-ATTACH] dev=4-1:1.0 dev->p=0000000000000000

-->  usbdev_ioctl -> __device_attach -->
```
static int __device_attach(struct device *dev, bool allow_async)
{
int ret = 0;
bool async = false;

device_lock(dev);

if (dev->bus && !strcmp(dev->bus->name, "usb")) {
pr_info("[USB-ATTACH] dev=%s dev->p=%px, of_node=%s\n",
dev_name(dev), dev->p, dev->of_node? dev->of_node->name: "(none)");
}

if (dev->p && dev->p->dead) {
goto out_unlock;
} else if (dev->driver) {
```

[   94.167858] Unable to handle kernel NULL pointer dereference at
virtual address 00000000000000d0
[   94.178558] Mem abort info:
[   94.181370]   ESR = 0x0000000096000005
[   94.185117]   EC = 0x25: DABT (current EL), IL = 32 bits
[   94.190431]   SET = 0, FnV = 0
[   94.193481]   EA = 0, S1PTW = 0
[   94.196624]   FSC = 0x05: level 1 translation fault
[   94.201498] Data abort info:
[   94.204409]   ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
[   94.209920]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[   94.214980]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[   94.220287] user pgtable: 4k pages, 39-bit VAs, pgdp=0000000161f08000
[   94.226723] [00000000000000d0] pgd=0000000000000000,
p4d=0000000000000000, pud=0000000000000000
[   94.235418] Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
[   94.241674] Modules linked in: cdc_mbim cdc_ncm option cdc_wdm
usb_wwan cdc_ether usbserial usbnet dm_integrity async_xor xor
xor_neon async_tx zram zsmalloc zstd_compress lz4_compress uinput
mtk_vcodec_dec_hwe
[   94.241779]  industrialio_triggered_buffer kfifo_buf xt_state
xt_conntrack cfg80211 nf_conntrack cros_ec_sensorhub r8152 mii
dm_default_key joydev
[   94.342977] CPU: 7 PID: 3596 Comm: fwupd Not tainted
6.6.99-08966-g207d9b4854da-dirty #1
01d7582e29ae61230a949cd03fcbe14466fd99cb
[   94.354612] Hardware name: Google Padme sku546/sku1058 board (DT)
[   94.360692] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   94.367641] pc : __device_attach+0x58/0x1c8
[   94.371816] lr : __device_attach+0x1b8/0x1c8
[   94.376075] sp : ffffffc086303bc0
[   94.379378] pmr_save: 000000e0
[   94.382420] x29: ffffffc086303be0 x28: ffffff8123952000 x27: 0000000000000000
[   94.389545] x26: 0000000000000000 x25: 0000000000000000 x24: 000000000000000d
[   94.396669] x23: ffffff80c264b000 x22: ffffffc086303c68 x21: 0000000000000000
[   94.403793] x20: 0000000000000000 x19: ffffff8121f24850 x18: 00000002adfe0242
[   94.410917] x17: 0000000000000400 x16: 0000000000000000 x15: 0000000121f08000
[   94.418041] x14: 0000000000000001 x13: 2ba0000000000000 x12: ffffffd4bcf0225c
[   94.425164] x11: 0000000000000000 x10: 0000000000000be0 x9 : 75370bc53c647f00
[   94.432288] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
[   94.439412] x5 : 0000000000000000 x4 : 0000000000000001 x3 : 0000000000000000
[   94.446536] x2 : ffffff81f6b552f8 x1 : ffffff81f6b48cc8 x0 : 0000000000000030
[   94.453659] Call trace:
[   94.456095]  __device_attach+0x58/0x1c8
[   94.459920]  device_attach+0x18/0x28
[   94.463485]  proc_ioctl+0x190/0x208
[   94.466967]  proc_ioctl_default+0x50/0x80
[   94.470968]  usbdev_ioctl+0x88c/0xdc8
[   94.474621]  __arm64_sys_ioctl+0x8c/0xd0
[   94.478536]  invoke_syscall+0x6c/0xf8
[   94.482190]  el0_svc_common+0x84/0xe0
[   94.485843]  do_el0_svc+0x20/0x30
[   94.489148]  el0_svc+0x34/0x88
[   94.492192]  el0t_64_sync_handler+0x88/0xf0
[   94.496364]  el0t_64_sync+0x180/0x188
[   94.500017] Code: 91000821 9417e359 34000a60 f9402668 (39434109)
[   94.506097] ---[ end trace 0000000000000000 ]---
[   94.516632] Kernel panic - not syncing: Oops: Fatal exception
[   94.522368] SMP: stopping secondary CPUs
[   94.526368] Kernel Offset: 0x143c000000 from 0xffffffc080000000
[   94.532275] PHYS_OFFSET: 0x40000000
[   94.535751] CPU features: 0x1,c0000001,70028143,7000720b
[   94.541051] Memory Limit: none

```

>
> thanks,
>
> greg k-h

Currently, LTE USB devices are directly connected to the CPU port. In
the DTS, a disabled hub is connected to this port.

So there are a few questions about this issue:

1. Why is the of_node name of USB LTE "hub"?
2. Why is the device structure not cleared even when the USB interface
is disabled?
3. When usbdev_ioctl calls 4-1, it will also search for the driver for
4-1:1.0. In this case, is it reasonable to encounter a null pointer
exception in __device_attach?

In my opinion, a null pointer check should be added to __device_attach.

thanks

Powered by blists - more mailing lists