Re: [PATCH v2] usb: chipidea: udc: fix DMA and SG cleanup in _ep_nuke()

["Peter Chen (CIX)" <peter.chen@kernel.org>]

On 26-01-08 16:59:02, Mario Peter wrote:
> The ChipIdea UDC driver can encounter "not page aligned sg buffer"
> errors when a USB device is reconnected after being disconnected
> during an active transfer. This occurs because _ep_nuke() returns
> requests to the gadget layer without properly unmapping DMA buffers
> or cleaning up scatter-gather bounce buffers.
> 
> Root cause:
> When a disconnect happens during a multi-segment DMA transfer, the
> request's num_mapped_sgs field and sgt.sgl pointer remain set with
> stale values. The request is returned to the gadget driver with status
> -ESHUTDOWN but still has active DMA state. If the gadget driver reuses
> this request on reconnect without reinitializing it, the stale DMA
> state causes _hardware_enqueue() to skip DMA mapping (seeing non-zero
> num_mapped_sgs) and attempt to use freed/invalid DMA addresses,
> leading to alignment errors and potential memory corruption.
> 
> The normal completion path via _hardware_dequeue() properly calls
> usb_gadget_unmap_request_by_dev() and sglist_do_debounce() before
> returning the request. The _ep_nuke() path must do the same cleanup
> to ensure requests are returned in a clean, reusable state.
> 
> Fix:
> Add DMA unmapping and bounce buffer cleanup to _ep_nuke() to mirror
> the cleanup sequence in _hardware_dequeue():
> - Call usb_gadget_unmap_request_by_dev() if num_mapped_sgs is set
> - Call sglist_do_debounce() with copy=false if bounce buffer exists
> 
> This ensures that when requests are returned due to endpoint shutdown,
> they don't retain stale DMA mappings. The 'false' parameter to
> sglist_do_debounce() prevents copying data back (appropriate for
> shutdown path where transfer was aborted).
> 
> Signed-off-by: Mario Peter <mario.peter@leica-geosystems.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> ---
> 
> v1: submitted (https://lore.kernel.org/linux-usb/ofw7h7s7jbhzjzbj6fpkzp5m2av25ovlyfp7fdlcvt2dd6x6a5@kx3rnlojdzdy/)
> v2:
> * dropped redundant req.num_mapped_sgs check
> * removed obsolete brackets
> 
>  drivers/usb/chipidea/udc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 64a421ae0f05..c8d931d9d433 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -931,6 +931,13 @@ __acquires(hwep->lock)
>  		list_del_init(&hwreq->queue);
>  		hwreq->req.status = -ESHUTDOWN;
> 
> +		/* Unmap DMA and clean up bounce buffers before giving back */
> +		usb_gadget_unmap_request_by_dev(hwep->ci->dev->parent,
> +					&hwreq->req, hwep->dir);
> +
> +		if (hwreq->sgt.sgl)
> +			sglist_do_debounce(hwreq, false);
> +
>  		if (hwreq->req.complete != NULL) {
>  			spin_unlock(hwep->lock);
>  			usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
> --
> 2.43.0
> 

-- 

Best regards,
Peter