On Fri, Aug 06, 2021 at 04:53:26PM +0900, Daehwan Jung wrote:
> From: taehyun cho <taehyun.cho@xxxxxxxxxxx>
>
> Endpoint complete function in u_serial can be executed
> when 'gs_port' is Null. This situation happens when
> 'dwc3_gadget_pullup' returns ETIMEDOUT. The reason why
> ETIMEDOUT is returned is that whole system is out of order
> including interrupt regardless of USB.
>
> pc : __lock_acquire+0x54/0x5ec
> lr : lock_acquire+0xe8/0x198
> sp : ffffffc03914b9d0
> x29: ffffffc03914b9d0 x28: ffffff895f13b680
> x27: 0000000000000000 x26: 0000000000000000
> x25: 00000000000003c8 x24: 0000000000000080
> x23: ffffffc010a8f650 x22: 0000000000000000
> x21: 0000000000000000 x20: 0000000000000000
> x19: ffffffc010a8f650 x18: ffffffc02d70a0b0
> x17: 0000000000000000 x16: 00000000000229e0
> x15: 0000000000000004 x14: 00000000000004f2
> x13: ffffffc0120fe178 x12: 0000000000000003
> x11: 00000000ffffffff x10: 0000000100000001
> x9 : 0000000000000001 x8 : 00000000000003c8
> x7 : 0000000000000000 x6 : ffffffc010a8f650
> x5 : 0000000000000000 x4 : 0000000000000080
> x3 : 0000000000000000 x2 : 0000000000000000
> x1 : 0000000000000000 x0 : 00000000000003c8
> Call trace:
> __lock_acquire+0x54/0x5ec
> lock_acquire+0xe8/0x198
> _raw_spin_lock+0x70/0x88
> gs_read_complete+0x48/0xac
> usb_gadget_giveback_request+0x48/0x80
> dwc3_gadget_giveback+0xcc/0xe8
> dwc3_remove_requests+0xa8/0x188
> __dwc3_gadget_ep_disable+0x98/0x110
> dwc3_gadget_ep_disable+0x50/0xbc
> usb_ep_disable+0x44/0x94
> gserial_disconnect+0xc0/0x250
> acm_free_func+0x30/0x48
> usb_put_function+0x34/0x68
> config_usb_cfg_unlink+0xdc/0xf8
> configfs_unlink+0x144/0x264
> vfs_unlink+0x134/0x218
> do_unlinkat+0x13c/0x2a0
> __arm64_sys_unlinkat+0x48/0x60
> el0_svc_common.llvm.10277270529376503802+0xb8/0x1b4
> do_el0_svc+0x24/0x8c
> el0_svc+0x10/0x1c
> el0_sync_handler+0x68/0xac
> el0_sync+0x18c/0x1c0
>
> Signed-off-by: taehyun cho <taehyun.cho@xxxxxxxxxxx>
> ---
> drivers/usb/gadget/function/u_serial.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
> index 6f68cbe..af08a18 100644
> --- a/drivers/usb/gadget/function/u_serial.c
> +++ b/drivers/usb/gadget/function/u_serial.c
> @@ -450,6 +450,15 @@ static void gs_read_complete(struct usb_ep *ep, struct usb_request *req)
> {
> struct gs_port *port = ep->driver_data;
>
> + /*
> + * Port became NULL when 'dwc3_gadget_pullup' returns ETIMEDOUT.
> + * Return here to avoid panic.
> + */
> + if (!port) {
> + pr_err("%s, failed to get port\n", __func__);
> + return;
> + }
> +
What prevents port from being null right after checking this? Where is
the lock to prevent this?
> /* Queue all received data until the tty layer is ready for it. */
> spin_lock(&port->port_lock);
> list_add_tail(&req->list, &port->read_queue);
> @@ -461,6 +470,15 @@ static void gs_write_complete(struct usb_ep *ep, struct usb_request *req)
> {
> struct gs_port *port = ep->driver_data;
>
> + /*
> + * port became NULL when 'dwc3_gadget_pullup' returns ETIMEDOUT.
> + * Return here to avoid panic.
> + */
> + if (!port) {
> + pr_err("%s, failed to get port\n", __func__);
> + return;
> + }
Same here, where is the lock?
And why report an error, what can a user do about it?
thanks,
greg k-h
