linux-kernel - 答复: [PATCH v2] usb: gadget: u_serial: check Null pointer in EP callback

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <TYUPR06MB621757ABDEF795FB39B912FED28D2@TYUPR06MB6217.apcprd06.prod.outlook.com>
Date: Tue, 20 Aug 2024 10:52:47 +0000
From: 胡连勤 <hulianqin@...o.com>
To: Prashanth K <quic_prashk@...cinc.com>, "gregkh@...uxfoundation.org"
	<gregkh@...uxfoundation.org>
CC: "quic_jjohnson@...cinc.com" <quic_jjohnson@...cinc.com>,
	"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>, "akpm@...ux-foundation.org"
	<akpm@...ux-foundation.org>, Michael Nazzareno Trimarchi
	<michael@...rulasolutions.com>
Subject:
 答复: [PATCH v2] usb: gadget: u_serial: check Null pointer in EP callback

Hello linux community expert:

>> diff --git a/drivers/usb/gadget/function/u_serial.c 
>> b/drivers/usb/gadget/function/u_serial.c
>> index b394105e55d6..65637d53bf02
>> --- a/drivers/usb/gadget/function/u_serial.c
>> +++ b/drivers/usb/gadget/function/u_serial.c
>> @@ -454,6 +454,14 @@ static void gs_read_complete(struct usb_ep *ep, 
>> struct usb_request *req)  {
>>  	struct gs_port	*port = ep->driver_data;
>>  
>> +	/* When port is NULL, return to avoid panic. */
>> +	if (!port)
>> +		return;
>> +

>This doesn't protect the port from going to NULL right after the above the check. Since you mentioned gser_disconnect is making port to NULL, add the serial_port_lock to protect port (just like its done on gser_disconnect/suspend/resume). Something like this would do.
  OK, it is more reasonable to modify it as follows, I will modify it again.

>diff --git a/drivers/usb/gadget/function/u_serial.c
>b/drivers/usb/gadget/function/u_serial.c
>index f975dc03a190..a33f8cd871ac 100644
>--- a/drivers/usb/gadget/function/u_serial.c
>+++ b/drivers/usb/gadget/function/u_serial.c
>@@ -451,10 +451,21 @@ static void gs_rx_push(struct work_struct *work)

>static void gs_read_complete(struct usb_ep *ep, struct usb_request *req) {
>-       struct gs_port  *port = ep->driver_data;
>+       struct gs_port  *port;
>+       unsigned long   flags;
>+
>+       spin_lock_irqsave(&serial_port_lock, flags);
>+       port = ep->driver_data;
>+
>+       if (!port) {
>+               spin_unlock_irqrestore(&serial_port_lock, flags);
>+               return;
>+       }

>-       /* Queue all received data until the tty layer is ready for it. */
>       spin_lock(&port->port_lock);
>+       spin_unlock(&serial_port_lock);
>+
>+       /* Queue all received data until the tty layer is ready for it. 
>+ */
>       list_add_tail(&req->list, &port->read_queue);
>       schedule_delayed_work(&port->push, 0);
>       spin_unlock(&port->port_lock);
>
>>  	/* 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); @@ -465,6 +473,14 @@ 
>> static void gs_write_complete(struct usb_ep *ep, struct usb_request 
>> *req)  {
>>  	struct gs_port	*port = ep->driver_data;
>>  
>> +   /* When port is NULL, return to avoid panic. */
>> +	if (!port)
>> +		return;
>> +
>>  	spin_lock(&port->port_lock);
>>  	list_add(&req->list, &port->write_pool);
>>  	port->write_started--;
>Same here also

Thanks