public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] usb: xhci: check Null pointer in segment alloc
@ 2025-12-19  7:18 胡连勤
  2025-12-19 12:48 ` Mathias Nyman
  2025-12-21 16:22 ` Greg Kroah-Hartman
  0 siblings, 2 replies; 21+ messages in thread
From: 胡连勤 @ 2025-12-19  7:18 UTC (permalink / raw)
  To: Mathias Nyman, Greg Kroah-Hartman, Sarah Sharp
  Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	胡连勤

From: Lianqin Hu <hulianqin@vivo.com>

Considering that in some extreme cases,
when a digital headset is connected and a wake-up
operation is performed,if the headset is plug out
or the headset connection is abnormally disconnected at this time,
segment_pool will be set to null, resulting in accessing a null pointer.

So, add null pointer checks to fix the problem.

Call trace:
 dma_pool_alloc+0x3c/0x248
 xhci_segment_alloc+0x9c/0x184
 xhci_alloc_segments_for_ring+0xcc/0x1cc
 xhci_ring_alloc+0xc4/0x1a8
 xhci_endpoint_init+0x36c/0x4ac
 xhci_add_endpoint+0x18c/0x2a4
 usb_hcd_alloc_bandwidth+0x384/0x3e4
 usb_set_interface+0x144/0x510
 usb_reset_and_verify_device+0x248/0x5fc
 usb_port_resume+0x580/0x700
 usb_generic_driver_resume+0x24/0x5c
 usb_resume_both+0x104/0x32c
 usb_runtime_resume+0x18/0x28
 __rpm_callback+0x94/0x3d4
 rpm_resume+0x3f8/0x5fc
 rpm_resume+0x1fc/0x5fc

Fixes: 0ebbab374223 ("USB: xhci: Ring allocation and initialization.")
Cc: stable@vger.kernel.org
Signed-off-by: Lianqin Hu <hulianqin@vivo.com>

 drivers/usb/host/xhci-mem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index c708bdd69f16..2ea5fb810a80 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -35,6 +35,9 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
 	dma_addr_t	dma;
 	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
 
+	if (!xhci->segment_pool)
+		return NULL;
+
 	seg = kzalloc_node(sizeof(*seg), flags, dev_to_node(dev));
 	if (!seg)
 		return NULL;
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-19  7:18 [PATCH] usb: xhci: check Null pointer in segment alloc 胡连勤
@ 2025-12-19 12:48 ` Mathias Nyman
  2025-12-19 15:53   ` 答复: " 胡连勤
  2025-12-21 16:22 ` Greg Kroah-Hartman
  1 sibling, 1 reply; 21+ messages in thread
From: Mathias Nyman @ 2025-12-19 12:48 UTC (permalink / raw)
  To: 胡连勤, Mathias Nyman, Greg Kroah-Hartman,
	Sarah Sharp
  Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org

On 12/19/25 09:18, 胡连勤 wrote:
> From: Lianqin Hu <hulianqin@vivo.com>
> 
> Considering that in some extreme cases,
> when a digital headset is connected and a wake-up
> operation is performed,if the headset is plug out
> or the headset connection is abnormally disconnected at this time,
> segment_pool will be set to null, resulting in accessing a null pointer.
> 
> So, add null pointer checks to fix the problem.
> 
> Call trace:
>   dma_pool_alloc+0x3c/0x248
>   xhci_segment_alloc+0x9c/0x184
>   xhci_alloc_segments_for_ring+0xcc/0x1cc
>   xhci_ring_alloc+0xc4/0x1a8
>   xhci_endpoint_init+0x36c/0x4ac
>   xhci_add_endpoint+0x18c/0x2a4
>   usb_hcd_alloc_bandwidth+0x384/0x3e4
>   usb_set_interface+0x144/0x510
>   usb_reset_and_verify_device+0x248/0x5fc
>   usb_port_resume+0x580/0x700
>   usb_generic_driver_resume+0x24/0x5c
>   usb_resume_both+0x104/0x32c
>   usb_runtime_resume+0x18/0x28
>   __rpm_callback+0x94/0x3d4
>   rpm_resume+0x3f8/0x5fc
>   rpm_resume+0x1fc/0x5fc
> 
> Fixes: 0ebbab374223 ("USB: xhci: Ring allocation and initialization.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Lianqin Hu <hulianqin@vivo.com>
> 
>   drivers/usb/host/xhci-mem.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index c708bdd69f16..2ea5fb810a80 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -35,6 +35,9 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
>   	dma_addr_t	dma;
>   	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
>   
> +	if (!xhci->segment_pool)
> +		return NULL;
> +

The xhci->segment_pool is created in xhci_mem_init() and destroyed in xhci_mem_cleanup().
It should never be NULL when xhci driver tries to allocate a ring segment.

If you can trigger a null pointer dereference here, then please share a backtrace.
There is likely something else is wrong that needs to be fixed.

Thanks
Mathias




^ permalink raw reply	[flat|nested] 21+ messages in thread

* 答复: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-19 12:48 ` Mathias Nyman
@ 2025-12-19 15:53   ` 胡连勤
  2025-12-20  8:08     ` Greg Kroah-Hartman
  2025-12-20 13:15     ` Michal Pecio
  0 siblings, 2 replies; 21+ messages in thread
From: 胡连勤 @ 2025-12-19 15:53 UTC (permalink / raw)
  To: Mathias Nyman, Mathias Nyman, Greg Kroah-Hartman, Sarah Sharp
  Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org

Hello Mathias Nyman:

> > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> > index c708bdd69f16..2ea5fb810a80 100644
> > --- a/drivers/usb/host/xhci-mem.c
> > +++ b/drivers/usb/host/xhci-mem.c
> > @@ -35,6 +35,9 @@ static struct xhci_segment *xhci_segment_alloc(struct
> xhci_hcd *xhci,
> >   	dma_addr_t	dma;
> >   	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
> >
> > +	if (!xhci->segment_pool)
> > +		return NULL;
> > +
> 
> The xhci->segment_pool is created in xhci_mem_init() and destroyed in
> xhci_mem_cleanup().
> It should never be NULL when xhci driver tries to allocate a ring segment.
> 
> If you can trigger a null pointer dereference here, then please share a
> backtrace.
> There is likely something else is wrong that needs to be fixed.
> 
> Thanks
> Mathias
> 


[ 4021.986094][  T332] Unable to handle kernel paging request at virtual address 00000000eff70000
[ 4021.986099][  T332] Mem abort info:
[ 4021.986101][  T332]   ESR = 0x0000000096000005
[ 4021.986104][  T332]   EC = 0x25: DABT (current EL), IL = 32 bits
[ 4021.986108][  T332]   SET = 0, FnV = 0
[ 4021.986110][  T332]   EA = 0, S1PTW = 0
[ 4021.986113][  T332]   FSC = 0x05: level 1 translation fault
[ 4021.986115][  T332] Data abort info:
[ 4021.986117][  T332]   ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
[ 4021.986120][  T332]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[ 4021.986123][  T332]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 4021.986127][  T332] user pgtable: 4k pages, 39-bit VAs, pgdp=00000000a5e7a000
[ 4021.986131][  T332] [00000000eff70000] pgd=0000000000000000, p4d=0000000000000000, pud=0000000000000000
[ 4021.986141][  T332] Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
[ 4021.986147][  T332] dbg_snapshot_backtrace: regs
[ 4021.986341][  T332] Success to add btrace in Minidump
[ 4021.986346][  T332] Skip md ftrace buffer dump for: 0x1609e0
[ 4021.986428][  T332] Modules linked in: cs_press(O) fp_dispatch_event(O) vivo_ts(O) qca_cld3_peach_v2(O) rmnet_shs(O) rmnet_aps(O) rmnet_offload(O) rmnet_wlan(O) rmnet_perf(O) rmnet_perf_tether(O) rmnet_core(O) ipanetm(O) rmnet_ctl(O) camera(O) ipam(O) swr_dmic_dlkm(O) msm_drm(O) machine_dlkm(O) qcom_hv_haptics wcd938x_dlkm(O) wcd9378_dlkm(O) leds_qti_flash swr_haptics_dlkm(O) wcd939x_dlkm(O) msm_kgsl(O) lpass_cdc_rx_macro_dlkm(O) qti_battery_charger lpass_cdc_va_macro_dlkm(O) lpass_cdc_wsa2_macro_dlkm(O) lpass_bt_swr_dlkm(O) lpass_cdc_wsa_macro_dlkm(O) lpass_cdc_tx_macro_dlkm(O) vfcs_cms_core coresight_tmc hdcp_qseecom_dlkm(O) msm_eva(O) audio_pkt_dlkm(O) vivo_chg_cms usb_f_qdss smmu_proxy_dlkm(O) usb_f_gsi pinctrl_lpi_dlkm(O) swr_ctrl_dlkm(O) snd_usb_audio_qmi adsp_loader_dlkm(O) wcd9xxx_dlkm(O) lpass_cdc_dlkm(O) audio_prm_dlkm(O) vivo_tfa986x_dlkm(O) vivo_aw882xx_v4_dlkm(O) vivo_tfa9894_dlkm(O) vivo_tfa9874_dlkm(O) wcd9378_slave_dlkm(O) vivo_cs43130_dlkm(O) vivo_aw882xx_v3_dlkm(O) vivo_tas25xx_dlkm(O) mbhc_dlkm(O)
[ 4021.986541][  T332]  vivo_aw882xx_dlkm(O) wcd939x_slave_dlkm(O) vivo_fs194x_dlkm(O) vivo_aw87319_dlkm(O) vfcs_pi_i2c_dev vivo_fs1599_dlkm(O) spcom(O) vivo_aw87xxx_dlkm(O) vivo_ak4377a_dlkm(O) vivo_sia8159_dlkm(O) vivo_aw87xxx_v2_dlkm(O) cnss2(O) dwc3_msm gsim(O) synx_driver(O) coresight_dummy msm_video(O) mhi_dev_satellite spf_core_dlkm(O) goodix_ts(O) qrtr_mhi mhi_dev_uci coresight_remote_etm qcrypto_msm_dlkm(O) slc_mpam bt_fm_swr(O) spss_utils(O) sys_pm_vx coresight_tmc_sec smcinvoke_dlkm(O) mpam_msc_slc qcedev_mod_dlkm(O) st_fts(O) vfcs_core hdmi_dlkm(O) vivo_codec_common_dlkm(O) btfm_slim_codec(O) phy_msm_m31_eusb2 vivo_max20328_dlkm(O) msm_hw_fence(O) fsa4480_i2c frpc_adsprpc(O) coresight_tpdm qce50_dlkm(O) vfcs_bat_encryptor_core_v2 coresight_cti i2c_msm_geni rmnet_sch(O) i3c_master_msm_geni atmel_mxt_ts(O) qrtr_smd sync_fence(O) gpr_dlkm(O) msm_mmrm(O) audpkt_ion_dlkm(O) stm_st54se_gpio(O) bam_dma vivo_display(O) msm_sharedmem leds_qcom_lpg subsystem_ddrvote_stats qfs2630 swapmem_free leds_qpnp_vibrator_ldo
[ 4021.986662][  T332]  vfcs_fg_bq28z610_dev cdsprm vfcs_sc8510 btfmcodec(O) phy_generic qcom_stats videocc_tuna qcom_iommu_debug drm_dp_aux_bus pinctrl_spmi_gpio wcd938x_slave_dlkm(O) qts(O) tz_log_dlkm(O) guw5228 msm_show_resume_irq vfcs_cp_i2c_dev rmnet_mem(O) mem_object altmode_glink charger_ulog_glink nvmem_qfprom leds_qcom_flash stub_dlkm(O) mem_hooks msm_ext_display(O) usb_f_cdev repeater_qti_pmic_eusb2 gf9578 wcd_usbss_i2c pmic_glink_debug wsa883x_dlkm(O) mhi sensorhub_monitor vivo_chg_fg_bq27426slave qcom_rng qcom_va_minidump shrinker_proxy qti_glink_adc phy_msm_ssusb_qmp qti_battery_debug phy_msm_snps_eusb2 wsa884x_dlkm(O) ucsi_qti_glink_driver focaltech_fts(O) vivo_tshell qcom_spss leds_lm3644 btpower(O) q2spi_geni glink_probe si_core_module thermal_config dynpf_scmi qmp_dlkm(O) qti_amoled_ecm cnss_utils(O) panel_event_notifier adsp_sleepmon q6_notifier_dlkm(O) boot_stats coresight_replicator qcom_edac drm_display_helper xhci_sideband gh_ctrl c1dcvs_scmi_v2 q6_dlkm(O) qseecom_proxy swr_dlkm(O) gc1_driver(O)
[ 4021.986805][  T332]  snd_event_dlkm(O) sysmon_subsystem_stats qrng_dlkm(O) cnss_nl(O) radio_i2c_rtc6226_qca(O) q6_pdr_dlkm(O) wcd_core_dlkm(O) max31760_fan vivo_tas2562_dlkm(O) vfcs_max77929 mac80211 smem_mailbox(O) wlan_firmware_service(O) sps_drv qcom_q6v5_pas jv0301 qcom_spmi_adc5_gen3 spmi_pmic_arb_debug cnss_plat_ipc_qmi_svc(O) ddr_cdev spi_msm_geni vivo_wls_charger_monitor cnss_prealloc(O) coresight_trace_noc stm_heartbeat vivo_wls_rx_p9415 vfcs_fg_common coresight_tpda fpid msm_geni_serial gpu_dump_skip_cdev usb_f_ccid qcom_lpm vfcs_bat_encryptor_core qcom_cpuss_sleep_stats_v4 coresight_stm gh_rm_booster slim_qcom_ngd_ctrl vfcs_fg_sh366009_dev qcom_dynamic_ramoops qti_cpufreq_cdev qcom_cpuss_sleep_stats cpucp_log governor_msm_adreno_tz qcom_q6v5 pm8941_pwrkey hwmon smp2p vivo_haptics_boost gh_tlmm_vm_mem_access msm_memshare bwmon vivo_spi_ir qcom_glink_spss hung_task_enh coresight_uetm rdbg dmesg_dumper hvc_gunyah qfprom_sys qcom_amoled_regulator evacc_tuna qcom_cpufreq_hw_debug coresight_qmi clk_scmi
[ 4021.986930][  T332]  vivo_chg_fg_bq28z610 qcom_pil_info vfcs_fg_sh366003_dev qcom_vadc_common mpam_msc coresight_funnel cpufreq_stats_scmi_v2 gh_irq_lend debugcc_tuna lt9611uxc mem_offline eud stm_ftrace debugcc_kera cfg80211 llcc_heuristics qti_qmi_cdev vfcs_fchg_core pinctrl_spmi_mpp stm_console cpu_mpam governor_gpubw_mon coresight_tgu smp2p_sleepstate snd_soc_hdmi_codec f_fs_ipc_log debugcc_sun coresight_csr lz4m qrtr_tun vivo_chg_fg_bq27426master pci_msm_drv platform_mpam stm_p_ost slimbus qti_pmic_glink msm_show_epoch memlat glink_pkt heap_mem_ext_v01 gh_mem_notifier qcom_sysmon qti_fixed_regulator qti_dmof_scmi qcom_ramdump repeater gh_panic_notifier tmecom_intf qcom_spmi_temp_alarm vfcs_ex_bms msm_sysstats coresight phy_qcom_emu mem_ddr_size stm_core pdr_interface pcie_pdc mpam qti_ocp_notifier kprobe_fs_special qsee_ipc_irq_bridge vivo_wls_rx_cps4041 qti_devfreq_cdev vfcs_misc_core qti_userspace_cdev rproc_qcom_common msm_gpi qcom_smd bcl_soc nb7vpq904m vfcs_bigdata_core redriver qcom_glink_smem cpufreq_stats_scmi_v3
[ 4021.987069][  T332]  qcom_glink qti_qmi_sensor_v2 qmi_helpers virtio_pci r8153_ecm l2tp_ppp ptp diag aqc111 hci_uart cdc_ncm cdc_eem pptp clk_test soc_utils_test vmw_vsock_virtio_transport input_test ax88179_178a dev_addr_lists_test fat_test asix cdc_ether iio_test_format virtio_pci_legacy_dev 9pnet_fd lib_test l2tp_core virtio_pci_modern_dev zram hidp nhc_mobility rfcomm usbnet nhc_routing mac802154 btbcm btqca ext4_inode_test btsdio ieee802154_6lowpan wwan regmap_kunit slcan kunit_test rtl8150 macsec tls pppox tipc ieee802154_socket r8152 kunit_example_test ftdi_sio clk_gate_test time_test ppp_mppe virtio_blk zsmalloc nhc_hop vcpu_stall_detector nhc_dest bluetooth regmap_raw_ram nhc_fragment nhc_ipv6 usbmon soc_topology_test pps_core virtio_console hid_uclogic_test open_dice bsd_comp regmap_ram libarc4 virtio_balloon vcan ppp_deflate nhc_udp ieee802154 kunit gzvm usbserial nfc can_dev mii 9pnet cdc_acm ppp_generic can cctrng slhc rfkill 8021q 6lowpan aw86938 vivo_haptic_core bsplog qrtr_gunyah handshake_counter pinctrl_kera
[ 4021.987251][  T332]  gpucc_tuna tcsrcc_sun dispcc_sun cambistmclkcc_tuna gpucc_kera vivo_bsp_engine gpucc_sun gcc_tuna vkyber_iosched dispcc_tuna gcc_kera clk_dummy vivo_tcp cambistmclkcc_sun qnoc_kera nfc_i2c qnoc_sun camcc_kera msm_qmp arm_smmu pinctrl_tuna pinctrl_sun gunyah_loader qnoc_tuna vivo_rms tcsrcc_tuna st21nfc camcc_sun camcc_tuna clk_rpmh mem_buf evacc_sun gcc_sun videocc_sun vr sdhci_msm ufs_qcom ufshcd_crypto_qti clk_qcom pinctrl_msm vne memory_dump_v2 phy_qcom_ufs_qmp_v4_niobe qcom_spmi_pmic symphony gdsc_regulator qcom_pdc pmic_pon_log csc_driver qcom_ice stub_regulator vivo_wifi_driver phy_qcom_ufs_qmp_v4_sun qrtr rpmh_regulator bbklog_enable(C) qcom_i2c_pmic sch_uprio icc_rpmh mem_buf_msgq qcom_tsens qcom_iommu_util qcom_aoss phy_qcom_ufs_qrbtc_sdm845 qcom_logbuf_vendor_hooks mdt_loader sch_ufifo zcache board_info bcl_pmic5 qcom_logbuf_boot_log dcc_v2 vivo_netstats vivo_slowpath_opt card_detect vpsnh cpucp_fast qcom_pm8008_regulator gh_virt_wdt thermal_pause msm_performance sched_walt socinfo vivo_csc
[ 4021.987408][  T332]  vivo_rsc qcom_dcvs dcvs_fp icc_bcm_voter qcom_rpmh crm_v2 vklp msm_poweroff qcom_pmu_lib qcom_wdt_core qcom_dload_mode qcom_llcc_pmu qcom_ipc_logging qcom_pon minidump qcom_dma_heaps mem_buf_dev secure_buffer qcom_scm gh_rm_drv thp_pool vsed qcom_et5904_regulator sch_cfg debug_regulator gh_arm_drv sch_eval logd_kernel(C) phy_qcom_ufs debug_symbol vivo_rsmc_driver qcom_scmi_client gh_dbl smem fuse_info ipclite(O) sched_penalty sensors_class blk_enhance reboot_mode icc_debug qti_thermal_vendor_hooks rtc_pm8xxx qti_regmap_debugfs spmi_pmic_arb sg iommu_logger qcom_scmi_vendor qcom_cpu_vendor_hooks cmd_db vhp bootprof qcom_hwspinlock cpu_hotplug tmf8801 cpu_phys_log_map qcom_cpufreq_thermal llcc_qcom max77812_regulator pem_driver qcom_reboot_reason qcom_cpufreq_hw v_zsmalloc nvmem_qcom_spmi_sdam qcom_cpucp wl2866d qnoc_qos tango32 vivo_fs_trace qcom_ipcc pie_driver cqhci proxy_consumer gh_msgq wl28681 drivers_info leds_aw22xxx pmc gic_intr_routing msm_dma_iommu_mapping
[ 4021.987553][  T332] CPU: 0 PID: 332 Comm: init Tainted: G        WC O       6.6.89-android15-8-g42db9ecb036b-ab14487600-4k #1 396856ca684d560f9c295b5f1feeed469ef25794
[ 4021.987561][  T332] Hardware name: Qualcomm Technologies, Inc. Sun QRD SKU1 V8 Power Grid PD2408F_EX (DT)
[ 4021.987564][  T332] pstate: 614000c5 (nZCv daIF +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[ 4021.987571][  T332] pc : dma_pool_alloc+0x3c/0x248
[ 4021.987580][  T332] lr : dma_pool_alloc+0x30/0x248
[ 4021.987588][  T332] sp : ffffffc08091b570
[ 4021.987591][  T332] x29: ffffffc08091b570 x28: ffffff8a00d4ae00 x27: 0000000000000001
[ 4021.987599][  T332] x26: 0000000000000c00 x25: 0000000000000002 x24: 0000000000000d00
[ 4021.987607][  T332] x23: 00000000eff70000 x22: ffffff8a10350490 x21: ffffffc08091b5b0
[ 4021.987614][  T332] x20: 0000000000000d00 x19: ffffff8a10350480 x18: ffffffe1606fc5c0
[ 4021.987621][  T332] x17: 000000000c9745e1 x16: 000000000c9745e1 x15: 0000000000000000
[ 4021.987629][  T332] x14: 0000000000000000 x13: ffffffc08104b1c0 x12: ffffffa912252000
[ 4021.987636][  T332] x11: 000000000040003b x10: d00fd549715d6176 x9 : 0000000000000000
[ 4021.987644][  T332] x8 : 0000000000000001 x7 : 0000000000000000 x6 : 0000000000000040
[ 4021.987651][  T332] x5 : 0000000000000001 x4 : 0000000000000000 x3 : 0000000000000001
[ 4021.987658][  T332] x2 : ffffffc08091b5b0 x1 : 0000000000000000 x0 : 0000000000000000
[ 4021.987665][  T332] Call trace:
[ 4021.987668][  T332]  dma_pool_alloc+0x3c/0x248
[ 4021.987676][  T332]  xhci_segment_alloc+0x9c/0x184
[ 4021.987682][  T332]  xhci_alloc_segments_for_ring+0xcc/0x1cc
[ 4021.987688][  T332]  xhci_ring_alloc+0xc4/0x1a8
[ 4021.987693][  T332]  xhci_endpoint_init+0x36c/0x4ac
[ 4021.987698][  T332]  xhci_add_endpoint+0x18c/0x2a4
[ 4021.987702][  T332]  usb_hcd_alloc_bandwidth+0x384/0x3e4
[ 4021.987711][  T332]  usb_set_interface+0x144/0x510
[ 4021.987716][  T332]  usb_reset_and_verify_device+0x248/0x5fc
[ 4021.987723][  T332]  usb_port_resume+0x580/0x700
[ 4021.987730][  T332]  usb_generic_driver_resume+0x24/0x5c
[ 4021.987735][  T332]  usb_resume_both+0x104/0x32c
[ 4021.987740][  T332]  usb_runtime_resume+0x18/0x28
[ 4021.987746][  T332]  __rpm_callback+0x94/0x3d4
[ 4021.987754][  T332]  rpm_resume+0x3f8/0x5fc
[ 4021.987762][  T332]  rpm_resume+0x1fc/0x5fc
[ 4021.987769][  T332]  __pm_runtime_resume+0x4c/0x90
[ 4021.987777][  T332]  usb_autopm_get_interface+0x20/0x4c
[ 4021.987783][  T332]  snd_usb_autoresume+0x68/0x124
[ 4021.987792][  T332]  suspend_resume_store+0x2a0/0x2b4 [dwc3_msm a4b7997a2e35cfe1a4a429762003b34dd4e85076]
[ 4021.987867][  T332]  dev_attr_store+0x30/0x48
[ 4021.987874][  T332]  sysfs_kf_write+0x54/0x6c
[ 4021.987883][  T332]  kernfs_fop_write_iter+0x104/0x1a8
[ 4021.987889][  T332]  vfs_write+0x24c/0x2f4
[ 4021.987898][  T332]  ksys_write+0x78/0xe8
[ 4021.987902][  T332]  __arm64_sys_write+0x1c/0x2c
[ 4021.987907][  T332]  invoke_syscall+0x58/0x114
[ 4021.987915][  T332]  el0_svc_common+0x80/0xe0
[ 4021.987923][  T332]  do_el0_svc+0x1c/0x28
[ 4021.987930][  T332]  el0_svc+0x38/0x68
[ 4021.987938][  T332]  el0t_64_sync_handler+0x68/0xbc
[ 4021.987945][  T332]  el0t_64_sync+0x1a8/0x1ac
[ 4021.987952][  T332] Code: 94338aea f9400e77 aa0003e9 b40002f7 (f94002e8) 
[ 4021.987954][  T332] ---[ end trace 0000000000000000 ]---
[ 4021.987958][  T332] Kernel panic - not syncing: Oops: Fatal exception

Thanks

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: 答复: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-19 15:53   ` 答复: " 胡连勤
@ 2025-12-20  8:08     ` Greg Kroah-Hartman
  2025-12-20 11:34       ` 答复: " 胡连勤
  2025-12-20 13:15     ` Michal Pecio
  1 sibling, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2025-12-20  8:08 UTC (permalink / raw)
  To: 胡连勤
  Cc: Mathias Nyman, Mathias Nyman, Sarah Sharp,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org

On Fri, Dec 19, 2025 at 03:53:08PM +0000, 胡连勤 wrote:
> [ 4021.987553][  T332] CPU: 0 PID: 332 Comm: init Tainted: G        WC O       6.6.89-android15-8-g42db9ecb036b-ab14487600-4k #1 396856ca684d560f9c295b5f1feeed469ef25794

6.6?  Does this still happen on 6.18?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* 答复: 答复: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-20  8:08     ` Greg Kroah-Hartman
@ 2025-12-20 11:34       ` 胡连勤
  0 siblings, 0 replies; 21+ messages in thread
From: 胡连勤 @ 2025-12-20 11:34 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Mathias Nyman, Mathias Nyman, Sarah Sharp,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org

Hi Greg:
 
> On Fri, Dec 19, 2025 at 03:53:08PM +0000, 胡连勤 wrote:
> > [ 4021.987553][  T332] CPU: 0 PID: 332 Comm: init Tainted: G        WC O
> 6.6.89-android15-8-g42db9ecb036b-ab14487600-4k #1
> 396856ca684d560f9c295b5f1feeed469ef25794
> 
> 6.6?  Does this still happen on 6.18?
> 
> thanks,
> 
> greg k-h

Our current testing shows that this issue exists in both kernel 6.6 and kernel 6.12.
We do not currently have any projects using kernel 6.18, so we are unaware of the situation at this time.

Thanks

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-19 15:53   ` 答复: " 胡连勤
  2025-12-20  8:08     ` Greg Kroah-Hartman
@ 2025-12-20 13:15     ` Michal Pecio
  2025-12-21  5:48       ` 答复: " 胡连勤
  2025-12-22  6:42       ` Lee Jones
  1 sibling, 2 replies; 21+ messages in thread
From: Michal Pecio @ 2025-12-20 13:15 UTC (permalink / raw)
  To: 胡连勤
  Cc: Mathias Nyman, Mathias Nyman, Greg Kroah-Hartman, Sarah Sharp,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org

Hi,

On Fri, 19 Dec 2025 15:53:08 +0000, 胡连勤 wrote:
> [ 4021.987665][  T332] Call trace:
> [ 4021.987668][  T332]  dma_pool_alloc+0x3c/0x248
> [ 4021.987676][  T332]  xhci_segment_alloc+0x9c/0x184
> [ 4021.987682][  T332]  xhci_alloc_segments_for_ring+0xcc/0x1cc
> [ 4021.987688][  T332]  xhci_ring_alloc+0xc4/0x1a8
> [ 4021.987693][  T332]  xhci_endpoint_init+0x36c/0x4ac
> [ 4021.987698][  T332]  xhci_add_endpoint+0x18c/0x2a4
> [ 4021.987702][  T332]  usb_hcd_alloc_bandwidth+0x384/0x3e4
> [ 4021.987711][  T332]  usb_set_interface+0x144/0x510
> [ 4021.987716][  T332]  usb_reset_and_verify_device+0x248/0x5fc
> [ 4021.987723][  T332]  usb_port_resume+0x580/0x700
> [ 4021.987730][  T332]  usb_generic_driver_resume+0x24/0x5c
> [ 4021.987735][  T332]  usb_resume_both+0x104/0x32c
> [ 4021.987740][  T332]  usb_runtime_resume+0x18/0x28
> [ 4021.987746][  T332]  __rpm_callback+0x94/0x3d4
> [ 4021.987754][  T332]  rpm_resume+0x3f8/0x5fc
> [ 4021.987762][  T332]  rpm_resume+0x1fc/0x5fc
> [ 4021.987769][  T332]  __pm_runtime_resume+0x4c/0x90
> [ 4021.987777][  T332]  usb_autopm_get_interface+0x20/0x4c
> [ 4021.987783][  T332]  snd_usb_autoresume+0x68/0x124
> [ 4021.987792][  T332]  suspend_resume_store+0x2a0/0x2b4 [dwc3_msm a4b7997a2e35cfe1a4a429762003b34dd4e85076]

This looks like some out of tree driver tries to resume a sound device,
and apparently it's doing it while xhci_hcd isn't ready, perhaps during
the power_lost branch in xhci_resume() after full system suspend.

I suppose dynamic debug could show better what's going on:
echo 'module usbcore +p' >/proc/dynamic_debug/control
echo 'module xhci_hcd +p' >/proc/dynamic_debug/control

If my guess is right then USB core is failing to prevent device resume
during HC resume, but IDK whether it's supposed to prevent that or if
the out of tree driver simply shouldn't be trying such things.

Regards,
Michal

^ permalink raw reply	[flat|nested] 21+ messages in thread

* 答复: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-20 13:15     ` Michal Pecio
@ 2025-12-21  5:48       ` 胡连勤
  2025-12-22  6:42       ` Lee Jones
  1 sibling, 0 replies; 21+ messages in thread
From: 胡连勤 @ 2025-12-21  5:48 UTC (permalink / raw)
  To: Michal Pecio
  Cc: Mathias Nyman, Mathias Nyman, Greg Kroah-Hartman, Sarah Sharp,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org

Hi Michal:

> On Fri, 19 Dec 2025 15:53:08 +0000, 胡连勤 wrote:
> > [ 4021.987665][  T332] Call trace:
> > [ 4021.987668][  T332]  dma_pool_alloc+0x3c/0x248 [ 4021.987676][
> > T332]  xhci_segment_alloc+0x9c/0x184 [ 4021.987682][  T332]
> > xhci_alloc_segments_for_ring+0xcc/0x1cc
> > [ 4021.987688][  T332]  xhci_ring_alloc+0xc4/0x1a8 [ 4021.987693][
> > T332]  xhci_endpoint_init+0x36c/0x4ac [ 4021.987698][  T332]
> > xhci_add_endpoint+0x18c/0x2a4 [ 4021.987702][  T332]
> > usb_hcd_alloc_bandwidth+0x384/0x3e4
> > [ 4021.987711][  T332]  usb_set_interface+0x144/0x510 [ 4021.987716][
> > T332]  usb_reset_and_verify_device+0x248/0x5fc
> > [ 4021.987723][  T332]  usb_port_resume+0x580/0x700 [ 4021.987730][
> > T332]  usb_generic_driver_resume+0x24/0x5c
> > [ 4021.987735][  T332]  usb_resume_both+0x104/0x32c [ 4021.987740][
> > T332]  usb_runtime_resume+0x18/0x28 [ 4021.987746][  T332]
> > __rpm_callback+0x94/0x3d4 [ 4021.987754][  T332]
> > rpm_resume+0x3f8/0x5fc [ 4021.987762][  T332]  rpm_resume+0x1fc/0x5fc
> > [ 4021.987769][  T332]  __pm_runtime_resume+0x4c/0x90 [ 4021.987777][
> > T332]  usb_autopm_get_interface+0x20/0x4c
> > [ 4021.987783][  T332]  snd_usb_autoresume+0x68/0x124 [ 4021.987792][
> > T332]  suspend_resume_store+0x2a0/0x2b4 [dwc3_msm
> > a4b7997a2e35cfe1a4a429762003b34dd4e85076]
> 
> This looks like some out of tree driver tries to resume a sound device, and
> apparently it's doing it while c isn't ready, perhaps during the
> power_lost branch in xhci_resume() after full system suspend.

Yes, the logic in the ko part wants to wake up the connected audio device.
Is there a way to determine if xhci_hcd is ready before waking it up?


> I suppose dynamic debug could show better what's going on:
> echo 'module usbcore +p' >/proc/dynamic_debug/control echo 'module
> xhci_hcd +p' >/proc/dynamic_debug/control
>
I enabled dynamic debugging and tried to reproduce the problem again.

> If my guess is right then USB core is failing to prevent device resume during
> HC resume, but IDK whether it's supposed to prevent that or if the out of
> tree driver simply shouldn't be trying such things.
> 
When the connected digital headphones are in sleep mode, 
if there is audio data (notification tone) that needs to be played, 
the audio device needs to be woken up, but an error occurs during the wake-up process.

Thanks



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-19  7:18 [PATCH] usb: xhci: check Null pointer in segment alloc 胡连勤
  2025-12-19 12:48 ` Mathias Nyman
@ 2025-12-21 16:22 ` Greg Kroah-Hartman
  1 sibling, 0 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2025-12-21 16:22 UTC (permalink / raw)
  To: 胡连勤
  Cc: Mathias Nyman, Sarah Sharp, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Fri, Dec 19, 2025 at 07:18:10AM +0000, 胡连勤 wrote:
> From: Lianqin Hu <hulianqin@vivo.com>
> 
> Considering that in some extreme cases,
> when a digital headset is connected and a wake-up
> operation is performed,if the headset is plug out
> or the headset connection is abnormally disconnected at this time,
> segment_pool will be set to null, resulting in accessing a null pointer.

Nit, please wrap your changelog at 72 columns.

> So, add null pointer checks to fix the problem.
> 
> Call trace:
>  dma_pool_alloc+0x3c/0x248
>  xhci_segment_alloc+0x9c/0x184
>  xhci_alloc_segments_for_ring+0xcc/0x1cc
>  xhci_ring_alloc+0xc4/0x1a8
>  xhci_endpoint_init+0x36c/0x4ac
>  xhci_add_endpoint+0x18c/0x2a4
>  usb_hcd_alloc_bandwidth+0x384/0x3e4
>  usb_set_interface+0x144/0x510
>  usb_reset_and_verify_device+0x248/0x5fc
>  usb_port_resume+0x580/0x700
>  usb_generic_driver_resume+0x24/0x5c
>  usb_resume_both+0x104/0x32c
>  usb_runtime_resume+0x18/0x28
>  __rpm_callback+0x94/0x3d4
>  rpm_resume+0x3f8/0x5fc
>  rpm_resume+0x1fc/0x5fc
> 
> Fixes: 0ebbab374223 ("USB: xhci: Ring allocation and initialization.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Lianqin Hu <hulianqin@vivo.com>
> 
>  drivers/usb/host/xhci-mem.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index c708bdd69f16..2ea5fb810a80 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -35,6 +35,9 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
>  	dma_addr_t	dma;
>  	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
>  
> +	if (!xhci->segment_pool)
> +		return NULL;

What prevents segment_pool from being set to NULL right after checking
this?

And what happens when you return "out of memory" like this is doing?
Doesn't that cause problems with the caller?  Looking at the callbacks
it seems like the whole ring will then be torn down, is that the proper
thing to do on system resume?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-20 13:15     ` Michal Pecio
  2025-12-21  5:48       ` 答复: " 胡连勤
@ 2025-12-22  6:42       ` Lee Jones
  2025-12-22  7:13         ` Greg Kroah-Hartman
  1 sibling, 1 reply; 21+ messages in thread
From: Lee Jones @ 2025-12-22  6:42 UTC (permalink / raw)
  To: Michal Pecio
  Cc: 胡连勤, Mathias Nyman, Mathias Nyman,
	Greg Kroah-Hartman, Sarah Sharp, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Sat, 20 Dec 2025, Michal Pecio wrote:

> Hi,
> 
> On Fri, 19 Dec 2025 15:53:08 +0000, 胡连勤 wrote:
> > [ 4021.987665][  T332] Call trace:
> > [ 4021.987668][  T332]  dma_pool_alloc+0x3c/0x248
> > [ 4021.987676][  T332]  xhci_segment_alloc+0x9c/0x184
> > [ 4021.987682][  T332]  xhci_alloc_segments_for_ring+0xcc/0x1cc
> > [ 4021.987688][  T332]  xhci_ring_alloc+0xc4/0x1a8
> > [ 4021.987693][  T332]  xhci_endpoint_init+0x36c/0x4ac
> > [ 4021.987698][  T332]  xhci_add_endpoint+0x18c/0x2a4
> > [ 4021.987702][  T332]  usb_hcd_alloc_bandwidth+0x384/0x3e4
> > [ 4021.987711][  T332]  usb_set_interface+0x144/0x510
> > [ 4021.987716][  T332]  usb_reset_and_verify_device+0x248/0x5fc
> > [ 4021.987723][  T332]  usb_port_resume+0x580/0x700
> > [ 4021.987730][  T332]  usb_generic_driver_resume+0x24/0x5c
> > [ 4021.987735][  T332]  usb_resume_both+0x104/0x32c
> > [ 4021.987740][  T332]  usb_runtime_resume+0x18/0x28
> > [ 4021.987746][  T332]  __rpm_callback+0x94/0x3d4
> > [ 4021.987754][  T332]  rpm_resume+0x3f8/0x5fc
> > [ 4021.987762][  T332]  rpm_resume+0x1fc/0x5fc
> > [ 4021.987769][  T332]  __pm_runtime_resume+0x4c/0x90
> > [ 4021.987777][  T332]  usb_autopm_get_interface+0x20/0x4c
> > [ 4021.987783][  T332]  snd_usb_autoresume+0x68/0x124
> > [ 4021.987792][  T332]  suspend_resume_store+0x2a0/0x2b4 [dwc3_msm a4b7997a2e35cfe1a4a429762003b34dd4e85076]
> 
> This looks like some out of tree driver tries to resume a sound device,
> and apparently it's doing it while xhci_hcd isn't ready, perhaps during
> the power_lost branch in xhci_resume() after full system suspend.
> 
> I suppose dynamic debug could show better what's going on:
> echo 'module usbcore +p' >/proc/dynamic_debug/control
> echo 'module xhci_hcd +p' >/proc/dynamic_debug/control
> 
> If my guess is right then USB core is failing to prevent device resume
> during HC resume, but IDK whether it's supposed to prevent that or if
> the out of tree driver simply shouldn't be trying such things.

Lower-level functionality shouldn't be able to attack / fuzz core-code
in this way.  Shouldn't the core be resistant to any possible mistakes
or a lack of education exhibited by it's consumers?  An API that insists
on its users exercising care, knowledge and cognisance sounds fragile
and vulnerable.

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-22  6:42       ` Lee Jones
@ 2025-12-22  7:13         ` Greg Kroah-Hartman
  2025-12-22  7:55           ` Michal Pecio
  0 siblings, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2025-12-22  7:13 UTC (permalink / raw)
  To: Lee Jones
  Cc: Michal Pecio, 胡连勤, Mathias Nyman,
	Mathias Nyman, Sarah Sharp, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Mon, Dec 22, 2025 at 06:42:52AM +0000, Lee Jones wrote:
> On Sat, 20 Dec 2025, Michal Pecio wrote:
> 
> > Hi,
> > 
> > On Fri, 19 Dec 2025 15:53:08 +0000, 胡连勤 wrote:
> > > [ 4021.987665][  T332] Call trace:
> > > [ 4021.987668][  T332]  dma_pool_alloc+0x3c/0x248
> > > [ 4021.987676][  T332]  xhci_segment_alloc+0x9c/0x184
> > > [ 4021.987682][  T332]  xhci_alloc_segments_for_ring+0xcc/0x1cc
> > > [ 4021.987688][  T332]  xhci_ring_alloc+0xc4/0x1a8
> > > [ 4021.987693][  T332]  xhci_endpoint_init+0x36c/0x4ac
> > > [ 4021.987698][  T332]  xhci_add_endpoint+0x18c/0x2a4
> > > [ 4021.987702][  T332]  usb_hcd_alloc_bandwidth+0x384/0x3e4
> > > [ 4021.987711][  T332]  usb_set_interface+0x144/0x510
> > > [ 4021.987716][  T332]  usb_reset_and_verify_device+0x248/0x5fc
> > > [ 4021.987723][  T332]  usb_port_resume+0x580/0x700
> > > [ 4021.987730][  T332]  usb_generic_driver_resume+0x24/0x5c
> > > [ 4021.987735][  T332]  usb_resume_both+0x104/0x32c
> > > [ 4021.987740][  T332]  usb_runtime_resume+0x18/0x28
> > > [ 4021.987746][  T332]  __rpm_callback+0x94/0x3d4
> > > [ 4021.987754][  T332]  rpm_resume+0x3f8/0x5fc
> > > [ 4021.987762][  T332]  rpm_resume+0x1fc/0x5fc
> > > [ 4021.987769][  T332]  __pm_runtime_resume+0x4c/0x90
> > > [ 4021.987777][  T332]  usb_autopm_get_interface+0x20/0x4c
> > > [ 4021.987783][  T332]  snd_usb_autoresume+0x68/0x124
> > > [ 4021.987792][  T332]  suspend_resume_store+0x2a0/0x2b4 [dwc3_msm a4b7997a2e35cfe1a4a429762003b34dd4e85076]
> > 
> > This looks like some out of tree driver tries to resume a sound device,
> > and apparently it's doing it while xhci_hcd isn't ready, perhaps during
> > the power_lost branch in xhci_resume() after full system suspend.
> > 
> > I suppose dynamic debug could show better what's going on:
> > echo 'module usbcore +p' >/proc/dynamic_debug/control
> > echo 'module xhci_hcd +p' >/proc/dynamic_debug/control
> > 
> > If my guess is right then USB core is failing to prevent device resume
> > during HC resume, but IDK whether it's supposed to prevent that or if
> > the out of tree driver simply shouldn't be trying such things.
> 
> Lower-level functionality shouldn't be able to attack / fuzz core-code
> in this way.  Shouldn't the core be resistant to any possible mistakes
> or a lack of education exhibited by it's consumers?

Not always, we rely on drivers "doing the right thing" in almost all of
our in-kernel apis because we have access to the source of those drivers
to fix them to do the right thing.

> An API that insists on its users exercising care, knowledge and
> cognisance sounds fragile and vulnerable.

Fragile yes, vulnerable no.  Let's fix the fragility then, but as has
been pointed out in this thread, we don't know the root cause, and I
don't even think this "fix" would do the right thing anyway.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-22  7:13         ` Greg Kroah-Hartman
@ 2025-12-22  7:55           ` Michal Pecio
  2025-12-22 12:21             ` 答复: " 胡连勤
  0 siblings, 1 reply; 21+ messages in thread
From: Michal Pecio @ 2025-12-22  7:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Lee Jones, 胡连勤, Mathias Nyman, Mathias Nyman,
	Sarah Sharp, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Mon, 22 Dec 2025 08:13:21 +0100, Greg Kroah-Hartman wrote:
> > An API that insists on its users exercising care, knowledge and
> > cognisance sounds fragile and vulnerable.  
> 
> Fragile yes, vulnerable no.  Let's fix the fragility then, but as has
> been pointed out in this thread, we don't know the root cause, and I
> don't even think this "fix" would do the right thing anyway.

The patch looks wrong. I suspect this happens when add_endpoint() is
called concurrently with resume(), which makes little sense. And it
means the same code can probably call add_endpoint() before resume(),
which makes no sense either. We can't do that with suspended HW.

Chances are that this crash isn't even the only thing that could go
wrong when such calls are attempted. For one, xhci_resume() drops
the spinlock after reporting usb_root_hub_lost_power(), so your guess
elsewhere was correct - this code isn't even locked properly.

It seems no operations on USB devices during resume() are expected.

Regards,
Michal

^ permalink raw reply	[flat|nested] 21+ messages in thread

* 答复: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-22  7:55           ` Michal Pecio
@ 2025-12-22 12:21             ` 胡连勤
  2025-12-22 13:34               ` Alan Stern
  2025-12-22 14:00               ` 答复: " Greg Kroah-Hartman
  0 siblings, 2 replies; 21+ messages in thread
From: 胡连勤 @ 2025-12-22 12:21 UTC (permalink / raw)
  To: Michal Pecio, Greg Kroah-Hartman
  Cc: Lee Jones, Mathias Nyman, Mathias Nyman, Sarah Sharp,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org

Hi Michal:

> On Mon, 22 Dec 2025 08:13:21 +0100, Greg Kroah-Hartman wrote:
> > > An API that insists on its users exercising care, knowledge and
> > > cognisance sounds fragile and vulnerable.
> >
> > Fragile yes, vulnerable no.  Let's fix the fragility then, but as has
> > been pointed out in this thread, we don't know the root cause, and I
> > don't even think this "fix" would do the right thing anyway.
> 
> The patch looks wrong. I suspect this happens when add_endpoint() is called
> concurrently with resume(), which makes little sense. And it means the same
> code can probably call add_endpoint() before resume(), which makes no
> sense either. We can't do that with suspended HW.
> 
> Chances are that this crash isn't even the only thing that could go wrong
> when such calls are attempted. For one, xhci_resume() drops the spinlock
> after reporting usb_root_hub_lost_power(), so your guess elsewhere was
> correct - this code isn't even locked properly.
> 
> It seems no operations on USB devices during resume() are expected.

Currently, after checking the logic of our KO section, 
we found that there might be two places simultaneously calling snd_usb_autoresume to wake up the headset device.

Thanks

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: 答复: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-22 12:21             ` 答复: " 胡连勤
@ 2025-12-22 13:34               ` Alan Stern
  2025-12-22 16:49                 ` Michal Pecio
  2025-12-22 14:00               ` 答复: " Greg Kroah-Hartman
  1 sibling, 1 reply; 21+ messages in thread
From: Alan Stern @ 2025-12-22 13:34 UTC (permalink / raw)
  To: 胡连勤
  Cc: Michal Pecio, Greg Kroah-Hartman, Lee Jones, Mathias Nyman,
	Mathias Nyman, Sarah Sharp, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Mon, Dec 22, 2025 at 12:21:09PM +0000, 胡连勤 wrote:
> Hi Michal:
> 
> > On Mon, 22 Dec 2025 08:13:21 +0100, Greg Kroah-Hartman wrote:
> > > > An API that insists on its users exercising care, knowledge and
> > > > cognisance sounds fragile and vulnerable.
> > >
> > > Fragile yes, vulnerable no.  Let's fix the fragility then, but as has
> > > been pointed out in this thread, we don't know the root cause, and I
> > > don't even think this "fix" would do the right thing anyway.
> > 
> > The patch looks wrong. I suspect this happens when add_endpoint() is called
> > concurrently with resume(), which makes little sense. And it means the same
> > code can probably call add_endpoint() before resume(), which makes no
> > sense either. We can't do that with suspended HW.
> > 
> > Chances are that this crash isn't even the only thing that could go wrong
> > when such calls are attempted. For one, xhci_resume() drops the spinlock
> > after reporting usb_root_hub_lost_power(), so your guess elsewhere was
> > correct - this code isn't even locked properly.
> > 
> > It seems no operations on USB devices during resume() are expected.

Let's be more precise.  No extraneous operations are expected on a USB 
device while that device is being resumed (i.e., no operations other 
than those directly involved with the resume itself).  However, 
operations on a USB hub or controller are expected and allowed while a 
downstream device is being resumed.

> Currently, after checking the logic of our KO section, 
> we found that there might be two places simultaneously calling snd_usb_autoresume to wake up the headset device.

That should not make any difference, because the PM core serializes 
runtime resume calls.  If concurrent autoresume calls can cause a crash 
then there is a bug somewhere.  Maybe in a sound driver, maybe in a USB 
driver, and maybe in the PM core.

To track down the bug, it would help to log the start and end of the 
resume call, as well as the add_endpoint() call that gets into trouble.

Alan Stern

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: 答复: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-22 12:21             ` 答复: " 胡连勤
  2025-12-22 13:34               ` Alan Stern
@ 2025-12-22 14:00               ` Greg Kroah-Hartman
  1 sibling, 0 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2025-12-22 14:00 UTC (permalink / raw)
  To: 胡连勤
  Cc: Michal Pecio, Lee Jones, Mathias Nyman, Mathias Nyman,
	Sarah Sharp, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Mon, Dec 22, 2025 at 12:21:09PM +0000, 胡连勤 wrote:
> Hi Michal:
> 
> > On Mon, 22 Dec 2025 08:13:21 +0100, Greg Kroah-Hartman wrote:
> > > > An API that insists on its users exercising care, knowledge and
> > > > cognisance sounds fragile and vulnerable.
> > >
> > > Fragile yes, vulnerable no.  Let's fix the fragility then, but as has
> > > been pointed out in this thread, we don't know the root cause, and I
> > > don't even think this "fix" would do the right thing anyway.
> > 
> > The patch looks wrong. I suspect this happens when add_endpoint() is called
> > concurrently with resume(), which makes little sense. And it means the same
> > code can probably call add_endpoint() before resume(), which makes no
> > sense either. We can't do that with suspended HW.
> > 
> > Chances are that this crash isn't even the only thing that could go wrong
> > when such calls are attempted. For one, xhci_resume() drops the spinlock
> > after reporting usb_root_hub_lost_power(), so your guess elsewhere was
> > correct - this code isn't even locked properly.
> > 
> > It seems no operations on USB devices during resume() are expected.
> 
> Currently, after checking the logic of our KO section, 
> we found that there might be two places simultaneously calling snd_usb_autoresume to wake up the headset device.

Can you provide a link to the source for your kernel code so that we can
review it as well?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-22 13:34               ` Alan Stern
@ 2025-12-22 16:49                 ` Michal Pecio
  2025-12-22 17:03                   ` Alan Stern
  0 siblings, 1 reply; 21+ messages in thread
From: Michal Pecio @ 2025-12-22 16:49 UTC (permalink / raw)
  To: Alan Stern
  Cc: 胡连勤, Greg Kroah-Hartman, Lee Jones,
	Mathias Nyman, Mathias Nyman, Sarah Sharp,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org

On Mon, 22 Dec 2025 08:34:14 -0500, Alan Stern wrote:
> On Mon, Dec 22, 2025 at 12:21:09PM +0000, 胡连勤 wrote:
> > Hi Michal:
> >   
> > > On Mon, 22 Dec 2025 08:13:21 +0100, Greg Kroah-Hartman wrote:  
> > > > > An API that insists on its users exercising care, knowledge
> > > > > and cognisance sounds fragile and vulnerable.  
> > > >
> > > > Fragile yes, vulnerable no.  Let's fix the fragility then, but
> > > > as has been pointed out in this thread, we don't know the root
> > > > cause, and I don't even think this "fix" would do the right
> > > > thing anyway.  
> > > 
> > > The patch looks wrong. I suspect this happens when add_endpoint()
> > > is called concurrently with resume(), which makes little sense.
> > > And it means the same code can probably call add_endpoint()
> > > before resume(), which makes no sense either. We can't do that
> > > with suspended HW.
> > > 
> > > Chances are that this crash isn't even the only thing that could
> > > go wrong when such calls are attempted. For one, xhci_resume()
> > > drops the spinlock after reporting usb_root_hub_lost_power(), so
> > > your guess elsewhere was correct - this code isn't even locked
> > > properly.
> > > 
> > > It seems no operations on USB devices during resume() are
> > > expected.  
> 
> Let's be more precise.  No extraneous operations are expected on a
> USB device while that device is being resumed (i.e., no operations
> other than those directly involved with the resume itself).  However, 
> operations on a USB hub or controller are expected and allowed while
> a downstream device is being resumed.

That's not the situation here. The problematic resume is that of the
host controller itself, it's the only place I see which is expected to
destroy the segment pool at runtime (other than stop()) and possibly
cause the reported NULL derefence.

It is not expected that anyone will add endpoints (and probably do
anything) while the HC is resuming. No sanity checks for that either,
the driver just does stupid things. It likely does stupid things too
if you try to manipulate devices while the HC is suspended.

And it looks like somebody found a way of doing just that, by calling
snd_usb_autoresume() at inappropriate time for some reason. The export
was added by Wesley Chang, so I guess it was part of "audio offload".
IDK if offload uses it correctly. Somebody uses it incorrectly.

> > Currently, after checking the logic of our KO section, we found that
> > there might be two places simultaneously calling snd_usb_autoresume
> > to wake up the headset device.

Resuming some USB device wouldn't destroy the segment pool and cause
this crash. Here, device resume tries to add an endpoint and crashes
because something else has destroyed the pool.

Regards,
Michal

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-22 16:49                 ` Michal Pecio
@ 2025-12-22 17:03                   ` Alan Stern
  2025-12-22 21:03                     ` Michal Pecio
  0 siblings, 1 reply; 21+ messages in thread
From: Alan Stern @ 2025-12-22 17:03 UTC (permalink / raw)
  To: Michal Pecio
  Cc: 胡连勤, Greg Kroah-Hartman, Lee Jones,
	Mathias Nyman, Mathias Nyman, Sarah Sharp,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org

On Mon, Dec 22, 2025 at 05:49:34PM +0100, Michal Pecio wrote:
> That's not the situation here. The problematic resume is that of the
> host controller itself, it's the only place I see which is expected to
> destroy the segment pool at runtime (other than stop()) and possibly
> cause the reported NULL derefence.

Well, there are two resumes in question: The problematic resume of the 
xhci-hcd device (which you deduce happened because the segment pool was 
destroyed) and the resume of the sound device (which caused the crash by 
trying to add a new endpoint).

> It is not expected that anyone will add endpoints (and probably do
> anything) while the HC is resuming. No sanity checks for that either,
> the driver just does stupid things. It likely does stupid things too
> if you try to manipulate devices while the HC is suspended.

By the time the sound device's resume routine runs, the HC should be 
fully resumed.  That's why I wrote earlier that it would help to get 
logs showing exactly when all the suspends and resumes take place.

> And it looks like somebody found a way of doing just that, by calling
> snd_usb_autoresume() at inappropriate time for some reason. The export
> was added by Wesley Chang, so I guess it was part of "audio offload".
> IDK if offload uses it correctly. Somebody uses it incorrectly.

There's not supposed to be an inappropriate time for doing an 
autoresume.  But I haven't specifically checked the code in 
snd_usb_autoresume().

> Resuming some USB device wouldn't destroy the segment pool and cause
> this crash. Here, device resume tries to add an endpoint and crashes
> because something else has destroyed the pool.

Exactly.  It would be nice to know just what that something else was.

Alan Stern

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-22 17:03                   ` Alan Stern
@ 2025-12-22 21:03                     ` Michal Pecio
  2025-12-23  3:24                       ` Alan Stern
  0 siblings, 1 reply; 21+ messages in thread
From: Michal Pecio @ 2025-12-22 21:03 UTC (permalink / raw)
  To: Alan Stern
  Cc: 胡连勤, Greg Kroah-Hartman, Lee Jones,
	Mathias Nyman, Mathias Nyman, Sarah Sharp,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org

On Mon, 22 Dec 2025 12:03:45 -0500, Alan Stern wrote:
> There's not supposed to be an inappropriate time for doing an 
> autoresume.

> By the time the sound device's resume routine runs, the HC should be 
> fully resumed.

OK, if "should" means "supposed to" then somebody needs to check it.
Is this the HCD_FLAG_HW_ACCESSIBLE flag by any chance?

I see that devices recursively call bus_resume() before resuming, and
this fails with -ESHUTDOWN if the flag is unset, which seems to prevent
device resume from progressing further and crashing. Is this what is
meant to happen in such case?

So I guess it's not happening because xhci_resume() sets this flag
right away and then it may drop the lock and start deallocating memory
to reset everything. So we can "successfully" complete bus_resume()
and allow USB devices to resume while HC resume is still in progress.

Looks dodgy and I suspect this is the bug.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-22 21:03                     ` Michal Pecio
@ 2025-12-23  3:24                       ` Alan Stern
  2025-12-23 10:06                         ` Michal Pecio
  0 siblings, 1 reply; 21+ messages in thread
From: Alan Stern @ 2025-12-23  3:24 UTC (permalink / raw)
  To: Michal Pecio
  Cc: 胡连勤, Greg Kroah-Hartman, Lee Jones,
	Mathias Nyman, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org

[Redundant and invalid entries removed from CC: list]

On Mon, Dec 22, 2025 at 10:03:49PM +0100, Michal Pecio wrote:
> On Mon, 22 Dec 2025 12:03:45 -0500, Alan Stern wrote:
> > There's not supposed to be an inappropriate time for doing an 
> > autoresume.
> 
> > By the time the sound device's resume routine runs, the HC should be 
> > fully resumed.
> 
> OK, if "should" means "supposed to" then somebody needs to check it.
> Is this the HCD_FLAG_HW_ACCESSIBLE flag by any chance?

HCD_FLAG_HW_ACCESSIBLE means the HC is powered and can respond to I/O.  
If the flag is clear, it means the HC is unpowered or in a low-power 
mode or dead, and I/O accesses are likely to cause some sort of hardware 
exception.  At a minimum, they probably won't work.  (Think of a PCI HC 
in D3.  It won't respond to PCI traffic to any but a couple of its 
registers until it is put back into D0.)

> I see that devices recursively call bus_resume() before resuming, and

Are you talking about hcd_bus_resume()?  (There is no function named 
bus_resume() in usbcore.)  That's the routine in charge of resuming root 
hubs.  The PM core ensures that all of a device's ancestors are at full 
power before the device is resumed, so it would (indirectly) call this 
routine if an entire USB bus was suspended when a resume was requested 
for one of the devices on that bus.

I can't see it being an autoresume in that situation, though.  An 
autoresume is one that was requested by the device itself -- a wakeup 
request -- and that can't happen if the HC is suspended.  First there 
would be an autoresume of the HC, and only when that was finished and 
the hub driver started checking the status of the HC's ports would the 
device's wakeup request be noticed and acted on.

> this fails with -ESHUTDOWN if the flag is unset, which seems to prevent
> device resume from progressing further and crashing. Is this what is
> meant to happen in such case?

I'm not sure what code in what routine you're talking about.  However, 
it's obvious that if the host controller's registers can't be accessed 
then no downstream device resume is going to work.  On the other hand, 
the result should just be a normal failure, with an error code return -- 
not a crash.

> So I guess it's not happening because xhci_resume() sets this flag
> right away and then it may drop the lock and start deallocating memory
> to reset everything. So we can "successfully" complete bus_resume()
> and allow USB devices to resume while HC resume is still in progress.

The root-hub resume (i.e., the ->bus_resume() callback) does not occur 
until after the HC's own resume handler returns.

> Looks dodgy and I suspect this is the bug.

No, there should never be a device resume while the HC resume is still 
in progress.  If one happens, it means there must be a bug somewhere.

Is it possible that the HC's resume handler decided that the HC was 
dead, and so started deallocating stuff, but failed to call 
usb_hc_died()?  (But note that resume_common() in hcd-pci.c calls 
usb_hc_died() automatically on the HCD's behalf when a resume fails.)

Alan Stern

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-23  3:24                       ` Alan Stern
@ 2025-12-23 10:06                         ` Michal Pecio
  2025-12-23 18:37                           ` Alan Stern
  0 siblings, 1 reply; 21+ messages in thread
From: Michal Pecio @ 2025-12-23 10:06 UTC (permalink / raw)
  To: Alan Stern
  Cc: 胡连勤, Greg Kroah-Hartman, Lee Jones,
	Mathias Nyman, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Mon, 22 Dec 2025 22:24:35 -0500, Alan Stern wrote:
> > I see that devices recursively call bus_resume() before resuming,
> 
> Are you talking about hcd_bus_resume()?  (There is no function named 
> bus_resume() in usbcore.)  That's the routine in charge of resuming
> root hubs.  The PM core ensures that all of a device's ancestors are
> at full power before the device is resumed, so it would (indirectly)
> call this routine if an entire USB bus was suspended when a resume
> was requested for one of the devices on that bus.

Yes, I mean this function and the bus_resume() method of the HCD,
which the function calls.

> I can't see it being an autoresume in that situation, though.  An 
> autoresume is one that was requested by the device itself -- a wakeup 
> request -- and that can't happen if the HC is suspended.

Indeed, the crashing call stack looks like some driver manually
resuming a USB device.

[ 4021.987702][  T332]  usb_hcd_alloc_bandwidth+0x384/0x3e4
[ 4021.987711][  T332]  usb_set_interface+0x144/0x510
[ 4021.987716][  T332]  usb_reset_and_verify_device+0x248/0x5fc
[ 4021.987723][  T332]  usb_port_resume+0x580/0x700
[ 4021.987730][  T332]  usb_generic_driver_resume+0x24/0x5c
[ 4021.987735][  T332]  usb_resume_both+0x104/0x32c
[ 4021.987740][  T332]  usb_runtime_resume+0x18/0x28
[ 4021.987746][  T332]  __rpm_callback+0x94/0x3d4
[ 4021.987754][  T332]  rpm_resume+0x3f8/0x5fc
[ 4021.987762][  T332]  rpm_resume+0x1fc/0x5fc
[ 4021.987769][  T332]  __pm_runtime_resume+0x4c/0x90
[ 4021.987777][  T332]  usb_autopm_get_interface+0x20/0x4c
[ 4021.987783][  T332]  snd_usb_autoresume+0x68/0x124
[ 4021.987792][  T332]  suspend_resume_store+0x2a0/0x2b4 [dwc3_msm a4b7997a2e35cfe1a4a429762003b34dd4e85076]

Before we got here, we should have attempted an hcd_bus_resume().
If xhci_hcd tracked its HW_ACCESSIBLE state better, that would have
failed and hopefully aborted device resume before it crashed.

> > this fails with -ESHUTDOWN if the flag is unset, which seems to
> > prevent device resume from progressing further and crashing. Is
> > this what is meant to happen in such case?  
> 
> I'm not sure what code in what routine you're talking about.
> However, it's obvious that if the host controller's registers can't
> be accessed then no downstream device resume is going to work.

If HW_ACCESSIBLE isn't set then xhci_bus_resume() returns -ESHUTDOWN.
It always returns zero otherwise.

So in the light of your explanation, the fact that xhci_resume() sets
HW_ACCESSIBLE before actually completing resume and thus allows root
hub resume to pretend to work, is obviously a bug.

> > So I guess it's not happening because xhci_resume() sets this flag
> > right away and then it may drop the lock and start deallocating
> > memory to reset everything. So we can "successfully" complete
> > bus_resume() and allow USB devices to resume while HC resume is
> > still in progress.  
> 
> The root-hub resume (i.e., the ->bus_resume() callback) does not
> occur until after the HC's own resume handler returns.

If PM core is supposed to prevent it then this is getting weird.
If not, then I'm not sure what else can prevent it.

> Is it possible that the HC's resume handler decided that the HC was 
> dead, and so started deallocating stuff, but failed to call 
> usb_hc_died()?  (But note that resume_common() in hcd-pci.c calls 
> usb_hc_died() automatically on the HCD's behalf when a resume fails.)

Hopefully not.

xhci->segment_pool is only modified by xhci_mem_cleanup() and by
xhci_mem_init() if allocation fails. And those functions are only
called at probe time, during HC resume and by hc_driver->stop().

I'm out of ideas without more logs. The xhci HW_ACCESSIBLE bug should
be fixed, but I'm not sure about correct ordering of setting this bit
wrt some calls done by xhci_resume(), so no patch from me.

Regards,
Michal



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-23 10:06                         ` Michal Pecio
@ 2025-12-23 18:37                           ` Alan Stern
  2025-12-23 19:43                             ` Michal Pecio
  0 siblings, 1 reply; 21+ messages in thread
From: Alan Stern @ 2025-12-23 18:37 UTC (permalink / raw)
  To: Michal Pecio
  Cc: 胡连勤, Greg Kroah-Hartman, Lee Jones,
	Mathias Nyman, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Tue, Dec 23, 2025 at 11:06:21AM +0100, Michal Pecio wrote:
> Indeed, the crashing call stack looks like some driver manually
> resuming a USB device.
> 
> [ 4021.987702][  T332]  usb_hcd_alloc_bandwidth+0x384/0x3e4
> [ 4021.987711][  T332]  usb_set_interface+0x144/0x510
> [ 4021.987716][  T332]  usb_reset_and_verify_device+0x248/0x5fc
> [ 4021.987723][  T332]  usb_port_resume+0x580/0x700
> [ 4021.987730][  T332]  usb_generic_driver_resume+0x24/0x5c
> [ 4021.987735][  T332]  usb_resume_both+0x104/0x32c
> [ 4021.987740][  T332]  usb_runtime_resume+0x18/0x28
> [ 4021.987746][  T332]  __rpm_callback+0x94/0x3d4
> [ 4021.987754][  T332]  rpm_resume+0x3f8/0x5fc
> [ 4021.987762][  T332]  rpm_resume+0x1fc/0x5fc

rpm_resume() is in the PM core.  So this isn't just a USB thing.

> [ 4021.987769][  T332]  __pm_runtime_resume+0x4c/0x90
> [ 4021.987777][  T332]  usb_autopm_get_interface+0x20/0x4c
> [ 4021.987783][  T332]  snd_usb_autoresume+0x68/0x124
> [ 4021.987792][  T332]  suspend_resume_store+0x2a0/0x2b4 [dwc3_msm a4b7997a2e35cfe1a4a429762003b34dd4e85076]
> 
> Before we got here, we should have attempted an hcd_bus_resume().
> If xhci_hcd tracked its HW_ACCESSIBLE state better, that would have
> failed and hopefully aborted device resume before it crashed.

The reason we didn't is because the PM core thought the HC and root hub 
were already at full power.  Possibly because they were resumed before 
the start of the log, or possibly because they were never suspended.

We really need to know what happened leading up to this crash.

> If HW_ACCESSIBLE isn't set then xhci_bus_resume() returns -ESHUTDOWN.
> It always returns zero otherwise.
> 
> So in the light of your explanation, the fact that xhci_resume() sets
> HW_ACCESSIBLE before actually completing resume and thus allows root
> hub resume to pretend to work, is obviously a bug.

No, not really.  The proper time to set HW_ACCESSIBLE is when it becomes 
possible to do I/O to the HC's registers, i.e., when the controller 
changes from D3 to D0 (and maybe a few other things like 
pci_set_master() have been done).  By the time xhci_resume() gets called 
this should already have happened, so setting the flag immediately is 
the right thing for it to do.

> xhci->segment_pool is only modified by xhci_mem_cleanup() and by
> xhci_mem_init() if allocation fails. And those functions are only
> called at probe time, during HC resume and by hc_driver->stop().
> 
> I'm out of ideas without more logs. The xhci HW_ACCESSIBLE bug should
> be fixed, but I'm not sure about correct ordering of setting this bit
> wrt some calls done by xhci_resume(), so no patch from me.

Agreed, we can't do anything without more and better logs.  Adding 
dev_info() lines to the start and end of the various xhci-hcd suspend 
and resume routines, as well as xhci_mem_cleanup() and xhci_mem_init() 
and whatever else you can think of, would be a good start.

Can you write a patch that does this, and can 胡连勤 test it?

Alan Stern

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] usb: xhci: check Null pointer in segment alloc
  2025-12-23 18:37                           ` Alan Stern
@ 2025-12-23 19:43                             ` Michal Pecio
  0 siblings, 0 replies; 21+ messages in thread
From: Michal Pecio @ 2025-12-23 19:43 UTC (permalink / raw)
  To: Alan Stern
  Cc: 胡连勤, Greg Kroah-Hartman, Lee Jones,
	Mathias Nyman, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Tue, 23 Dec 2025 13:37:41 -0500, Alan Stern wrote:
> > So in the light of your explanation, the fact that xhci_resume()
> > sets HW_ACCESSIBLE before actually completing resume and thus
> > allows root hub resume to pretend to work, is obviously a bug.  
> 
> No, not really.  The proper time to set HW_ACCESSIBLE is when it
> becomes possible to do I/O to the HC's registers, i.e., when the
> controller changes from D3 to D0 (and maybe a few other things like 
> pci_set_master() have been done).  By the time xhci_resume() gets
> called this should already have happened, so setting the flag
> immediately is the right thing for it to do.

OK, so no problem here, thanks.

> Agreed, we can't do anything without more and better logs.  Adding 
> dev_info() lines to the start and end of the various xhci-hcd suspend 
> and resume routines, as well as xhci_mem_cleanup() and
> xhci_mem_init() and whatever else you can think of, would be a good
> start.

That code is packed with dev_dbg(), so seeing this reproduced with
dynamic debug enabled would be a good start. It was the first thing
I suggested before trying to find problems with USB or xHCI code.

Regards,
Michal

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2025-12-23 19:44 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-19  7:18 [PATCH] usb: xhci: check Null pointer in segment alloc 胡连勤
2025-12-19 12:48 ` Mathias Nyman
2025-12-19 15:53   ` 答复: " 胡连勤
2025-12-20  8:08     ` Greg Kroah-Hartman
2025-12-20 11:34       ` 答复: " 胡连勤
2025-12-20 13:15     ` Michal Pecio
2025-12-21  5:48       ` 答复: " 胡连勤
2025-12-22  6:42       ` Lee Jones
2025-12-22  7:13         ` Greg Kroah-Hartman
2025-12-22  7:55           ` Michal Pecio
2025-12-22 12:21             ` 答复: " 胡连勤
2025-12-22 13:34               ` Alan Stern
2025-12-22 16:49                 ` Michal Pecio
2025-12-22 17:03                   ` Alan Stern
2025-12-22 21:03                     ` Michal Pecio
2025-12-23  3:24                       ` Alan Stern
2025-12-23 10:06                         ` Michal Pecio
2025-12-23 18:37                           ` Alan Stern
2025-12-23 19:43                             ` Michal Pecio
2025-12-22 14:00               ` 答复: " Greg Kroah-Hartman
2025-12-21 16:22 ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox