[PATCH] device_core: check null pointer

[PATCH] device_core: check null pointer

[Zhengqiao Xia]

Add null pointer check to avoid null pointer.
When the USB device's interface is disabled, 'device_add' will not
be called, and 'dev->p' will be NULL. When you use 'usbdev_ioctl' to
call this USB interface at this point,'__device_attach' will be invoked.
Then a null pointer will be generated.

Signed-off-by: Zhengqiao Xia <xiazhengqiao@huaqin.corp-partner.google.com>
---
 drivers/base/dd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 3328101e0e106..cfdeb420fd12a 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -1033,7 +1033,7 @@ static int __device_attach(struct device *dev, bool allow_async)
 	bool async = false;
 
 	device_lock(dev);
-	if (dev->p->dead) {
+	if (dev->p && dev->p->dead) {
 		goto out_unlock;
 	} else if (dev->driver) {
 		if (device_is_bound(dev)) {
-- 
2.34.1

Re: [PATCH] device_core: check null pointer

[Greg KH]

On Fri, Nov 14, 2025 at 10:18:21PM +0800, Zhengqiao Xia wrote:
> Add null pointer check to avoid null pointer.
> When the USB device's interface is disabled, 'device_add' will not
> be called, and 'dev->p' will be NULL.

Wait, how does that happen?  Shouldn't we prevent that?

> When you use 'usbdev_ioctl' to
> call this USB interface at this point,'__device_attach' will be invoked.
> Then a null pointer will be generated.
> 
> Signed-off-by: Zhengqiao Xia <xiazhengqiao@huaqin.corp-partner.google.com>

Not a valid email address :( 

> ---
>  drivers/base/dd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 3328101e0e106..cfdeb420fd12a 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -1033,7 +1033,7 @@ static int __device_attach(struct device *dev, bool allow_async)
>  	bool async = false;
>  
>  	device_lock(dev);
> -	if (dev->p->dead) {
> +	if (dev->p && dev->p->dead) {

If dev->p is NULL here, something else went wrong with the caller,
please, let's fix the root problem here instead.

What suddenly changed to cause this to happen?

thanks,

greg k-h

Re: [PATCH] device_core: check null pointer

[Zhengqiao Xia]

Hi Greg,

Based on my analysis,

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 ->
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?

thanks

Greg KH <gregkh@linuxfoundation.org> 于2025年11月16日周日 04:16写道：


>
> On Fri, Nov 14, 2025 at 10:18:21PM +0800, Zhengqiao Xia wrote:
> > Add null pointer check to avoid null pointer.
> > When the USB device's interface is disabled, 'device_add' will not
> > be called, and 'dev->p' will be NULL.
>
> Wait, how does that happen?  Shouldn't we prevent that?
>
> > When you use 'usbdev_ioctl' to
> > call this USB interface at this point,'__device_attach' will be invoked.
> > Then a null pointer will be generated.
> >
> > Signed-off-by: Zhengqiao Xia <xiazhengqiao@huaqin.corp-partner.google.com>
>
> Not a valid email address :(
>
> > ---
> >  drivers/base/dd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > index 3328101e0e106..cfdeb420fd12a 100644
> > --- a/drivers/base/dd.c
> > +++ b/drivers/base/dd.c
> > @@ -1033,7 +1033,7 @@ static int __device_attach(struct device *dev, bool allow_async)
> >       bool async = false;
> >
> >       device_lock(dev);
> > -     if (dev->p->dead) {
> > +     if (dev->p && dev->p->dead) {
>
> If dev->p is NULL here, something else went wrong with the caller,
> please, let's fix the root problem here instead.
>
> What suddenly changed to cause this to happen?
>
> thanks,
>
> greg k-h

Re: [PATCH] device_core: check null pointer

[Greg KH]

On Mon, Nov 17, 2025 at 10:55:05AM +0800, Zhengqiao Xia wrote:
> Hi Greg,
> 
> Based on my analysis,

Please do not top-post :(

> 
> 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?

> 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?

thanks,

greg k-h

Re: [PATCH] device_core: check null pointer

[Zhengqiao Xia]

Greg KH <gregkh@linuxfoundation.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

Re: [PATCH] device_core: check null pointer

[Greg KH]

On Tue, Nov 18, 2025 at 03:31:03PM +0800, Zhengqiao Xia wrote:
> Greg KH <gregkh@linuxfoundation.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);

Yes, device_add() is skipped, so that means we should not ever be seeing
this device in the device tree anywhere, so that the driver core
shouldn't be seeing it anywhere else.

This device_add() call is not made, so what is attempting to register
this "disabled" interface?

And the more and more I see this "tacked on" ability to disable
interfaces based on firmware node, the more I dislike it (we are having
other discussions on the linux-usb list about it.)

So perhaps we just missed a path where the interface is not really known
to be disabled and we try to do something with it later on?

>      ...
> }
> ```
> 
> >
> > > 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

Good.

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

Wait, why are you attempting to talk to this interface through usbfs?

Maybe that's the path here, we need to disable that "for real" and
prevent usbfs from accessing the interface as well.

What userspace tool attempts to call this "by default" or are you just
running fuzzing tools?

Again, I think this needs to be fixed properly, not papered over in the
driver core.  So let's either rip the ability to disable interfaces out
of the usb core, or fix up where we missed that a device was not
registered.

thanks,

greg k-h

Re: [PATCH] device_core: check null pointer

[Zhengqiao Xia]

Greg KH <gregkh@linuxfoundation.org> 于2025年11月27日周四 17:54写道：
>
> On Tue, Nov 18, 2025 at 03:31:03PM +0800, Zhengqiao Xia wrote:
> > Greg KH <gregkh@linuxfoundation.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);
>
> Yes, device_add() is skipped, so that means we should not ever be seeing
> this device in the device tree anywhere, so that the driver core
> shouldn't be seeing it anywhere else.
>
> This device_add() call is not made, so what is attempting to register
> this "disabled" interface?
>
> And the more and more I see this "tacked on" ability to disable
> interfaces based on firmware node, the more I dislike it (we are having
> other discussions on the linux-usb list about it.)
>
> So perhaps we just missed a path where the interface is not really known
> to be disabled and we try to do something with it later on?
>
> >      ...
> > }
> > ```
> >
> > >
> > > > 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
>
> Good.
>
> > [   94.161987] [USB-ATTACH] dev=4-1:1.0 dev->p=0000000000000000
> >
> > -->  usbdev_ioctl -> __device_attach -->
>
> Wait, why are you attempting to talk to this interface through usbfs?

When LTE enters QUSB__BULK mode, it indicates that the
userspace(modemfwd) needs to update the firmware via usbfs.
Therefore, this device cannot be disabled.

>
> Maybe that's the path here, we need to disable that "for real" and
> prevent usbfs from accessing the interface as well.
>
> What userspace tool attempts to call this "by default" or are you just
> running fuzzing tools?
>
> Again, I think this needs to be fixed properly, not papered over in the
> driver core.  So let's either rip the ability to disable interfaces out
> of the usb core, or fix up where we missed that a device was not
> registered.

this is why I submit a new patch
https://lore.kernel.org/all/CAEXTbpeN0Mk+Y-UeV79JzE547UCa_Fhh7T75L+2mhoSq3ark8g@mail.gmail.com/
 .

> thanks,
>
> greg k-h

Re: [PATCH] device_core: check null pointer

[Greg KH]

On Thu, Nov 27, 2025 at 07:37:34PM +0800, Zhengqiao Xia wrote:
> Greg KH <gregkh@linuxfoundation.org> 于2025年11月27日周四 17:54写道：
> >
> > On Tue, Nov 18, 2025 at 03:31:03PM +0800, Zhengqiao Xia wrote:
> > > Greg KH <gregkh@linuxfoundation.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);
> >
> > Yes, device_add() is skipped, so that means we should not ever be seeing
> > this device in the device tree anywhere, so that the driver core
> > shouldn't be seeing it anywhere else.
> >
> > This device_add() call is not made, so what is attempting to register
> > this "disabled" interface?
> >
> > And the more and more I see this "tacked on" ability to disable
> > interfaces based on firmware node, the more I dislike it (we are having
> > other discussions on the linux-usb list about it.)
> >
> > So perhaps we just missed a path where the interface is not really known
> > to be disabled and we try to do something with it later on?
> >
> > >      ...
> > > }
> > > ```
> > >
> > > >
> > > > > 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
> >
> > Good.
> >
> > > [   94.161987] [USB-ATTACH] dev=4-1:1.0 dev->p=0000000000000000
> > >
> > > -->  usbdev_ioctl -> __device_attach -->
> >
> > Wait, why are you attempting to talk to this interface through usbfs?
> 
> When LTE enters QUSB__BULK mode, it indicates that the
> userspace(modemfwd) needs to update the firmware via usbfs.
> Therefore, this device cannot be disabled.

Then this should not be disabled in the firmware, please fix your dt.

But again, let's fix the real problem here.

> > Maybe that's the path here, we need to disable that "for real" and
> > prevent usbfs from accessing the interface as well.
> >
> > What userspace tool attempts to call this "by default" or are you just
> > running fuzzing tools?
> >
> > Again, I think this needs to be fixed properly, not papered over in the
> > driver core.  So let's either rip the ability to disable interfaces out
> > of the usb core, or fix up where we missed that a device was not
> > registered.
> 
> this is why I submit a new patch
> https://lore.kernel.org/all/CAEXTbpeN0Mk+Y-UeV79JzE547UCa_Fhh7T75L+2mhoSq3ark8g@mail.gmail.com/

Does that really solve this problem?  How?

confused,

greg k-h

Re: [PATCH] device_core: check null pointer

[Zhengqiao Xia]

Greg KH <gregkh@linuxfoundation.org> 于2025年11月27日周四 19:51写道：
>
> On Thu, Nov 27, 2025 at 07:37:34PM +0800, Zhengqiao Xia wrote:
> > Greg KH <gregkh@linuxfoundation.org> 于2025年11月27日周四 17:54写道：
> > >
> > > On Tue, Nov 18, 2025 at 03:31:03PM +0800, Zhengqiao Xia wrote:
> > > > Greg KH <gregkh@linuxfoundation.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);
> > >
> > > Yes, device_add() is skipped, so that means we should not ever be seeing
> > > this device in the device tree anywhere, so that the driver core
> > > shouldn't be seeing it anywhere else.
> > >
> > > This device_add() call is not made, so what is attempting to register
> > > this "disabled" interface?
> > >
> > > And the more and more I see this "tacked on" ability to disable
> > > interfaces based on firmware node, the more I dislike it (we are having
> > > other discussions on the linux-usb list about it.)
> > >
> > > So perhaps we just missed a path where the interface is not really known
> > > to be disabled and we try to do something with it later on?
> > >
> > > >      ...
> > > > }
> > > > ```
> > > >
> > > > >
> > > > > > 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
> > >
> > > Good.
> > >
> > > > [   94.161987] [USB-ATTACH] dev=4-1:1.0 dev->p=0000000000000000
> > > >
> > > > -->  usbdev_ioctl -> __device_attach -->
> > >
> > > Wait, why are you attempting to talk to this interface through usbfs?
> >
> > When LTE enters QUSB__BULK mode, it indicates that the
> > userspace(modemfwd) needs to update the firmware via usbfs.
> > Therefore, this device cannot be disabled.
>
> Then this should not be disabled in the firmware, please fix your dt.
>
> But again, let's fix the real problem here.
As this（https://lore.kernel.org/all/CAEXTbpeN0Mk+Y-UeV79JzE547UCa_Fhh7T75L+2mhoSq3ark8g@mail.gmail.com/）
says：

We have device A that has an on-board USB hub, so we describe it in the DTS.
Then, another device B is similar to device A but does not have the USB hub.
So, we inherit device A's DTS and disable the hub nodes.

LTE device is connected to device B' port and no hub, so we disabled
the usb hub' node.
I think this is reasonable.

>
> > > Maybe that's the path here, we need to disable that "for real" and
> > > prevent usbfs from accessing the interface as well.
> > >
> > > What userspace tool attempts to call this "by default" or are you just
> > > running fuzzing tools?
> > >
> > > Again, I think this needs to be fixed properly, not papered over in the
> > > driver core.  So let's either rip the ability to disable interfaces out
> > > of the usb core, or fix up where we missed that a device was not
> > > registered.
> >
> > this is why I submit a new patch
> > https://lore.kernel.org/all/CAEXTbpeN0Mk+Y-UeV79JzE547UCa_Fhh7T75L+2mhoSq3ark8g@mail.gmail.com/
>
> Does that really solve this problem?  How?

yes, this prevents the LTE device's of_node from pointing to the USB hub.

@@ -31,6 +31,9 @@ struct device_node *usb_of_get_device_node(struct
usb_device *hub, int port1)
                if (of_property_read_u32(node, "reg", &reg))
                        continue;

+               if (!of_device_is_available(node))
+                       continue;
+
                if (reg == port1)
                        return node;
        }

    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;
     }
so usb interface can continue to register.
>
> confused,
>
> greg k-h

Re: [PATCH] device_core: check null pointer

[Greg KH]

On Thu, Nov 27, 2025 at 08:34:45PM +0800, Zhengqiao Xia wrote:
> Greg KH <gregkh@linuxfoundation.org> 于2025年11月27日周四 19:51写道：
> >
> > On Thu, Nov 27, 2025 at 07:37:34PM +0800, Zhengqiao Xia wrote:
> > > Greg KH <gregkh@linuxfoundation.org> 于2025年11月27日周四 17:54写道：
> > > >
> > > > On Tue, Nov 18, 2025 at 03:31:03PM +0800, Zhengqiao Xia wrote:
> > > > > Greg KH <gregkh@linuxfoundation.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);
> > > >
> > > > Yes, device_add() is skipped, so that means we should not ever be seeing
> > > > this device in the device tree anywhere, so that the driver core
> > > > shouldn't be seeing it anywhere else.
> > > >
> > > > This device_add() call is not made, so what is attempting to register
> > > > this "disabled" interface?
> > > >
> > > > And the more and more I see this "tacked on" ability to disable
> > > > interfaces based on firmware node, the more I dislike it (we are having
> > > > other discussions on the linux-usb list about it.)
> > > >
> > > > So perhaps we just missed a path where the interface is not really known
> > > > to be disabled and we try to do something with it later on?
> > > >
> > > > >      ...
> > > > > }
> > > > > ```
> > > > >
> > > > > >
> > > > > > > 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
> > > >
> > > > Good.
> > > >
> > > > > [   94.161987] [USB-ATTACH] dev=4-1:1.0 dev->p=0000000000000000
> > > > >
> > > > > -->  usbdev_ioctl -> __device_attach -->
> > > >
> > > > Wait, why are you attempting to talk to this interface through usbfs?
> > >
> > > When LTE enters QUSB__BULK mode, it indicates that the
> > > userspace(modemfwd) needs to update the firmware via usbfs.
> > > Therefore, this device cannot be disabled.
> >
> > Then this should not be disabled in the firmware, please fix your dt.
> >
> > But again, let's fix the real problem here.
> As this（https://lore.kernel.org/all/CAEXTbpeN0Mk+Y-UeV79JzE547UCa_Fhh7T75L+2mhoSq3ark8g@mail.gmail.com/）
> says：
> 
> We have device A that has an on-board USB hub, so we describe it in the DTS.
> Then, another device B is similar to device A but does not have the USB hub.
> So, we inherit device A's DTS and disable the hub nodes.
> 
> LTE device is connected to device B' port and no hub, so we disabled
> the usb hub' node.
> I think this is reasonable.
> 
> >
> > > > Maybe that's the path here, we need to disable that "for real" and
> > > > prevent usbfs from accessing the interface as well.
> > > >
> > > > What userspace tool attempts to call this "by default" or are you just
> > > > running fuzzing tools?
> > > >
> > > > Again, I think this needs to be fixed properly, not papered over in the
> > > > driver core.  So let's either rip the ability to disable interfaces out
> > > > of the usb core, or fix up where we missed that a device was not
> > > > registered.
> > >
> > > this is why I submit a new patch
> > > https://lore.kernel.org/all/CAEXTbpeN0Mk+Y-UeV79JzE547UCa_Fhh7T75L+2mhoSq3ark8g@mail.gmail.com/
> >
> > Does that really solve this problem?  How?
> 
> yes, this prevents the LTE device's of_node from pointing to the USB hub.
> 
> @@ -31,6 +31,9 @@ struct device_node *usb_of_get_device_node(struct
> usb_device *hub, int port1)
>                 if (of_property_read_u32(node, "reg", &reg))
>                         continue;
> 
> +               if (!of_device_is_available(node))
> +                       continue;
> +
>                 if (reg == port1)
>                         return node;
>         }
> 
>     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;
>      }
> so usb interface can continue to register.

But where does this change usbfs's access to this interface?

thanks,

greg k-h

Re: [PATCH] device_core: check null pointer

[Zhengqiao Xia]

Greg KH <gregkh@linuxfoundation.org> 于2025年11月27日周四 20:43写道：
>
> On Thu, Nov 27, 2025 at 08:34:45PM +0800, Zhengqiao Xia wrote:
> > Greg KH <gregkh@linuxfoundation.org> 于2025年11月27日周四 19:51写道：
> > >
> > > On Thu, Nov 27, 2025 at 07:37:34PM +0800, Zhengqiao Xia wrote:
> > > > Greg KH <gregkh@linuxfoundation.org> 于2025年11月27日周四 17:54写道：
> > > > >
> > > > > On Tue, Nov 18, 2025 at 03:31:03PM +0800, Zhengqiao Xia wrote:
> > > > > > Greg KH <gregkh@linuxfoundation.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);
> > > > >
> > > > > Yes, device_add() is skipped, so that means we should not ever be seeing
> > > > > this device in the device tree anywhere, so that the driver core
> > > > > shouldn't be seeing it anywhere else.
> > > > >
> > > > > This device_add() call is not made, so what is attempting to register
> > > > > this "disabled" interface?
> > > > >
> > > > > And the more and more I see this "tacked on" ability to disable
> > > > > interfaces based on firmware node, the more I dislike it (we are having
> > > > > other discussions on the linux-usb list about it.)
> > > > >
> > > > > So perhaps we just missed a path where the interface is not really known
> > > > > to be disabled and we try to do something with it later on?
> > > > >
> > > > > >      ...
> > > > > > }
> > > > > > ```
> > > > > >
> > > > > > >
> > > > > > > > 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
> > > > >
> > > > > Good.
> > > > >
> > > > > > [   94.161987] [USB-ATTACH] dev=4-1:1.0 dev->p=0000000000000000
> > > > > >
> > > > > > -->  usbdev_ioctl -> __device_attach -->
> > > > >
> > > > > Wait, why are you attempting to talk to this interface through usbfs?
> > > >
> > > > When LTE enters QUSB__BULK mode, it indicates that the
> > > > userspace(modemfwd) needs to update the firmware via usbfs.
> > > > Therefore, this device cannot be disabled.
> > >
> > > Then this should not be disabled in the firmware, please fix your dt.
> > >
> > > But again, let's fix the real problem here.
> > As this（https://lore.kernel.org/all/CAEXTbpeN0Mk+Y-UeV79JzE547UCa_Fhh7T75L+2mhoSq3ark8g@mail.gmail.com/）
> > says：
> >
> > We have device A that has an on-board USB hub, so we describe it in the DTS.
> > Then, another device B is similar to device A but does not have the USB hub.
> > So, we inherit device A's DTS and disable the hub nodes.
> >
> > LTE device is connected to device B' port and no hub, so we disabled
> > the usb hub' node.
> > I think this is reasonable.
> >
> > >
> > > > > Maybe that's the path here, we need to disable that "for real" and
> > > > > prevent usbfs from accessing the interface as well.
> > > > >
> > > > > What userspace tool attempts to call this "by default" or are you just
> > > > > running fuzzing tools?
> > > > >
> > > > > Again, I think this needs to be fixed properly, not papered over in the
> > > > > driver core.  So let's either rip the ability to disable interfaces out
> > > > > of the usb core, or fix up where we missed that a device was not
> > > > > registered.
> > > >
> > > > this is why I submit a new patch
> > > > https://lore.kernel.org/all/CAEXTbpeN0Mk+Y-UeV79JzE547UCa_Fhh7T75L+2mhoSq3ark8g@mail.gmail.com/
> > >
> > > Does that really solve this problem?  How?
> >
> > yes, this prevents the LTE device's of_node from pointing to the USB hub.
> >
> > @@ -31,6 +31,9 @@ struct device_node *usb_of_get_device_node(struct
> > usb_device *hub, int port1)
> >                 if (of_property_read_u32(node, "reg", &reg))
> >                         continue;
> >
> > +               if (!of_device_is_available(node))
> > +                       continue;
> > +
> >                 if (reg == port1)
> >                         return node;
> >         }
> >
> >     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;
> >      }
> > so usb interface can continue to register.
>
> But where does this change usbfs's access to this interface?

Since the USB interface's of_node does not point to the USB hub, the
device can continue to register.

usbfs can access this interface normally, there will be no null
pointers. usbfs uses this interface to upgrade the firmware.
>
> thanks,
>
> greg k-h