| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2025011633-cavity-earthworm-2b5e@gregkh>
Date: Thu, 16 Jan 2025 14:28:32 +0100
From: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
To: Jon Hunter <jonathanh@...dia.com>
Cc: 胡连勤 <hulianqin@...o.com>,
Prashanth K <quic_prashk@...cinc.com>,
"mwalle@...nel.org" <mwalle@...nel.org>,
"quic_jjohnson@...cinc.com" <quic_jjohnson@...cinc.com>,
David Brownell <dbrownell@...rs.sourceforge.net>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"opensource.kernel" <opensource.kernel@...o.com>,
"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
Brad Griffis <bgriffis@...dia.com>
Subject: Re: [PATCH v3] usb: gadget: u_serial: Disable ep before setting port
to null to fix the crash caused by port being null
On Thu, Jan 16, 2025 at 01:11:36PM +0000, Jon Hunter wrote:
> Hi Greg, Lianqin,
>
> On 17/12/2024 07:58, 胡连勤 wrote:
> > From: Lianqin Hu <hulianqin@...o.com>
> >
> > Considering that in some extreme cases, when performing the
> > unbinding operation, gserial_disconnect has cleared gser->ioport,
> > which triggers gadget reconfiguration, and then calls gs_read_complete,
> > resulting in access to a null pointer. Therefore, ep is disabled before
> > gserial_disconnect sets port to null to prevent this from happening.
> >
> > Call trace:
> > gs_read_complete+0x58/0x240
> > usb_gadget_giveback_request+0x40/0x160
> > dwc3_remove_requests+0x170/0x484
> > dwc3_ep0_out_start+0xb0/0x1d4
> > __dwc3_gadget_start+0x25c/0x720
> > kretprobe_trampoline.cfi_jt+0x0/0x8
> > kretprobe_trampoline.cfi_jt+0x0/0x8
> > udc_bind_to_driver+0x1d8/0x300
> > usb_gadget_probe_driver+0xa8/0x1dc
> > gadget_dev_desc_UDC_store+0x13c/0x188
> > configfs_write_iter+0x160/0x1f4
> > vfs_write+0x2d0/0x40c
> > ksys_write+0x7c/0xf0
> > __arm64_sys_write+0x20/0x30
> > invoke_syscall+0x60/0x150
> > el0_svc_common+0x8c/0xf8
> > do_el0_svc+0x28/0xa0
> > el0_svc+0x24/0x84
> >
> > Fixes: c1dca562be8a ("usb gadget: split out serial core")
> > Cc: stable@...r.kernel.org
> > Suggested-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > Signed-off-by: Lianqin Hu <hulianqin@...o.com>
> > ---
> >
> > Changes in v3:
> > - Add --- line above the version tag information
> > - Remove extra blank lines in commit messages
> > - Version tag information from v2 to changes in v2
> > - Link to v2: https://lore.kernel.org/all/TYUPR06MB6217DAA095A9863D4B58D57CD23B2@TYUPR06MB6217.apcprd06.prod.outlook.com/
> >
> > Changes in v2:
> > - Remove some address information from patch descriptions
> > - Link to v1: https://lore.kernel.org/all/TYUPR06MB621763AB815989161F4033AFD2762@TYUPR06MB6217.apcprd06.prod.outlook.com/
> > - Link to suggestions: https://lore.kernel.org/all/TYUPR06MB6217DE28012FFEC5E808DD64D2962@TYUPR06MB6217.apcprd06.prod.outlook.com/
> >
> > drivers/usb/gadget/function/u_serial.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
> > index 53d9fc41acc5..bc143a86c2dd 100644
> > --- a/drivers/usb/gadget/function/u_serial.c
> > +++ b/drivers/usb/gadget/function/u_serial.c
> > @@ -1420,6 +1420,10 @@ void gserial_disconnect(struct gserial *gser)
> > /* REVISIT as above: how best to track this? */
> > port->port_line_coding = gser->port_line_coding;
> > + /* disable endpoints, aborting down any active I/O */
> > + usb_ep_disable(gser->out);
> > + usb_ep_disable(gser->in);
> > +
> > port->port_usb = NULL;
> > gser->ioport = NULL;
> > if (port->port.count > 0) {
> > @@ -1431,10 +1435,6 @@ void gserial_disconnect(struct gserial *gser)
> > spin_unlock(&port->port_lock);
> > spin_unlock_irqrestore(&serial_port_lock, flags);
> > - /* disable endpoints, aborting down any active I/O */
> > - usb_ep_disable(gser->out);
> > - usb_ep_disable(gser->in);
> > -
> > /* finally, free any unused/unusable I/O buffers */
> > spin_lock_irqsave(&port->port_lock, flags);
> > if (port->port.count == 0)
>
>
> We have observed a reboot regression on Tegra234 (I have not tried other
> boards) and bisect is pointing to this commit. Reverting this on top of
> mainline is fixing the problem.
>
> With this change, when the board reboots we see ...
>
> [ 59.918177] tegra-xudc 3550000.usb: ep 3 disabled
> [ 59.923097] tegra-xudc 3550000.usb: ep 2 disabled
> [ 59.927955] tegra-xudc 3550000.usb: ep 5 disabled
> [ 80.911432] rcu: INFO: rcu_preempt self-detected stall on CPU
> [ 80.917354] rcu: 6-....: (5248 ticks this GP) idle=ec24/1/0x4000000000000000 softirq=1213/1213 fqs=2623
> [ 80.927146] rcu: (t=5253 jiffies g=3781 q=1490 ncpus=12)
> [ 80.932704] Sending NMI from CPU 6 to CPUs 2:
> [ 90.981555] CPU: 6 UID: 0 PID: 18 Comm: rcu_exp_gp_kthr Not tainted 6.13.0-rc7-00043-g619f0b6fad52 #1
> [ 90.981558] Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer Kit/Jetson, BIOS 00.0.0-dev-main_92e5ae_88fd1_296de 12/16/2024
> [ 90.981559] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 90.981562] pc : smp_call_function_single+0xdc/0x1a0
> [ 90.981574] lr : __sync_rcu_exp_select_node_cpus+0x228/0x3c0
> [ 90.981578] sp : ffff800082eb3cd0
> [ 90.981579] x29: ffff800082eb3cd0 x28: 0000000000000010 x27: ffff0000802933c0
> [ 90.981582] x26: ffff0007a8a1d700 x25: ffff800082895500 x24: ffff800080132018
> [ 90.981584] x23: 0000000000000014 x22: ffff800081fb7700 x21: ffff80008280d970
> [ 90.981586] x20: 0000000000000feb x19: ffff800082eb3d00 x18: 0000000000000000
> [ 90.981588] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> [ 90.981590] x14: ffff000080293440 x13: 0000000000000001 x12: 0000000000000000
> [ 90.981591] x11: ffff800081fb2388 x10: ffff0000802933c0 x9 : 0000000000000001
> [ 90.981593] x8 : 0000000000000040 x7 : 0000000000017068 x6 : ffff800080132018
> [ 90.981595] x5 : 0000000000000000 x4 : ffff0007a8a4f9c8 x3 : 0000000000000001
> [ 90.981597] x2 : 0000000000000000 x1 : ffff0007a8a4f9c0 x0 : 0000000000000004
> [ 90.981599] Call trace:
> [ 90.981601] smp_call_function_single+0xdc/0x1a0 (P)
> [ 90.981605] __sync_rcu_exp_select_node_cpus+0x228/0x3c0
> [ 90.981607] sync_rcu_exp_select_cpus+0x13c/0x2a0
> [ 90.981609] wait_rcu_exp_gp+0x18/0x30
> [ 90.981611] kthread_worker_fn+0xd0/0x188
> [ 90.981614] kthread+0x118/0x11c
> [ 90.981619] ret_from_fork+0x10/0x20
> [ 101.416347] sched: DL replenish lagged too much
>
Odd, you have a usb-serial gadget device in this system that is
disconnecting somehow? That oops doesn't point to anything in the usb
gadget codebase, "all" we have done is move the call to shutdown the
endpoints to earlier in the disconnect function.
I'm glad to revert this, but it feels really odd that this is causing
you an rcu stall issue.
confused,
greg k-h
Powered by blists - more mailing lists