From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 632CD330B0B for ; Thu, 27 Nov 2025 11:51:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764244281; cv=none; b=HIuvQDs0x6YMZaX2liAzRGIb6FRdqxT+EcOYyjX0+a/PBxMCf1w4iVahcMwdIYFRb+rpJYVpnlQ8ERkukTLWBcBYzGDFA1D0vyR49CVG4ovmBoqpZEOhOeaxdfQfYEpxqXWNAfvObGW0b9IB9ttbbrW+ByS3qCruUYdojn0hIbU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764244281; c=relaxed/simple; bh=YeDaz91PyrTxTUmkEYaXDQ7LSKtuJHLThyXbhmB1jkA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=AEKWECsYRlzxyM/ee3tKT32StmaVjgX0WeOXqBm2ONIh8vsms+FpkPWZLDicVqgbo1LGasE4mYSCpamKtVDwxj7CsVxzLRqile6ivj/b2scRdfJZvszowvT+8wgwd4ArI0cGRhG9dMZTUV+IMDpS70C5Pplhh3AFJqo5c1jhSb4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=JnlxKLDT; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="JnlxKLDT" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5AD4BC4CEF8; Thu, 27 Nov 2025 11:51:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1764244280; bh=YeDaz91PyrTxTUmkEYaXDQ7LSKtuJHLThyXbhmB1jkA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=JnlxKLDTYPL9HaC8DJtT89haIONP3wuSUC1FeKHTzrjix/v9ABef7xddPp0uS8q20 Iaxqlr3hdGgsWye5XNy9aOwKnS6FTy0zlyXkQq3cOvea5+x7f2oanZFU5DNC572dgt z7CApoSvIguamSMBWHIHKy9RHWFb2Nd9wZawLo+0= Date: Thu, 27 Nov 2025 12:51:17 +0100 From: Greg KH To: Zhengqiao Xia Cc: treapking@chromium.org, rafael@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] device_core: check null pointer Message-ID: <2025112721-savanna-overlabor-1269@gregkh> References: <20251114141821.416835-1-xiazhengqiao@huaqin.corp-partner.google.com> <2025111523-preface-scoreless-b478@gregkh> <2025111720-sensitize-schnapps-3189@gregkh> <2025112725-comrade-willfully-67f1@gregkh> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Thu, Nov 27, 2025 at 07:37:34PM +0800, Zhengqiao Xia wrote: > Greg KH 于2025年11月27日周四 17:54写道： > > > > On Tue, Nov 18, 2025 at 03:31:03PM +0800, Zhengqiao Xia wrote: > > > Greg KH 于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