Re: [PATCH v7] usb: gadget: u_serial: Add null pointer check in gs_read_complete & gs_write_complete

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 25-08-24 01:58 pm, 胡连勤 wrote:
> From: Lianqin Hu <hulianqin@xxxxxxxx>
> 
> Considering that in some extreme cases, when the unbind operation
> is being executed, gserial_disconnect has already cleared gser->ioport,
> and the controller has not stopped & pullup 0, sys.usb.config is reset

As mentioned by Michael earlier, avoid android specific (sys.usb.config)
operations.

> and the bind operation will be re-executed, calling gs_read_complete,
> which will result in accessing gser->iport, resulting in a null pointer
> dereference, add a null pointer check to prevent this situation.
> 
> Added a static spinlock to prevent gser->ioport from becoming
> null after the newly added check.
> 
> Unable to handle kernel NULL pointer dereference at

[...]
> 
> Fixes: c1dca562be8a ("usb gadget: split out serial core")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Lianqin Hu <hulianqin@xxxxxxxx>
> ---
> v7:
>   - Remove code comments
>   - Update the commit text
>   - Add the Fixes tag
>   - CC stable kernel
>   - Add serial_port_lock protection when checking port pointer
>   - Optimize code comments
>   - Delete log printing

We usually indicate what had changed in each versions.
v7: Remove code comments
v6: xx
v5: xx
v4: xx
v3: xx
v2: Delete log printing

> ---
No need for this '---' here, just leave an empty line
>  drivers/usb/gadget/function/u_serial.c | 33 ++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
> index b394105e55d6..e43d8065f7ec 100644
> --- a/drivers/usb/gadget/function/u_serial.c
> +++ b/drivers/usb/gadget/function/u_serial.c
> @@ -452,20 +452,43 @@ 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);
> +	spin_unlock_irqrestore(&port->port_lock, flags);
>  }
>  
>  static void gs_write_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;
> +	}
>  
>  	spin_lock(&port->port_lock);
> +	spin_unlock(&serial_port_lock);
>  	list_add(&req->list, &port->write_pool);
>  	port->write_started--;
>  
> @@ -486,7 +509,7 @@ static void gs_write_complete(struct usb_ep *ep, struct usb_request *req)
>  		break;
>  	}
>  
> -	spin_unlock(&port->port_lock);
> +	spin_unlock_irqrestore(&port->port_lock, flags);
>  }
>  
>  static void gs_free_requests(struct usb_ep *ep, struct list_head *head,
Regards,




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux