On Mon, Mar 25, 2024 at 06:45:43PM +0900, Norihiko Hama wrote:
> When ncm function is working and then stop usb0 interface for link down,
> eth_stop() is called. At this piont, accidentally if usb transport error
> should happen in usb_ep_enable(), 'in_ep' and/or 'out_ep' may not be enabled.
>
> After that, ncm_disable() is called to disable for ncm unbind
> but gether_disconnect() is never called since 'in_ep' is not enabled.
>
> As the result, ncm object is released in ncm unbind
> but 'dev->port_usb' associated to 'ncm->port' is not NULL.
>
> And when ncm bind again to recover netdev, ncm object is reallocated
> but usb0 interface is already associated to previous released ncm object.
>
> Therefore, once usb0 interface is up and eth_start_xmit() is called,
> released ncm object is dereferrenced and it might cause use-after-free memory.
>
> [function unlink via configfs]
> usb0: eth_stop dev->port_usb=ffffff9b179c3200
> --> error happens in usb_ep_enable().
> NCM: ncm_disable: ncm=ffffff9b179c3200
> --> no gether_disconnect() since ncm->port.in_ep->enabled is false.
> NCM: ncm_unbind: ncm unbind ncm=ffffff9b179c3200
> NCM: ncm_free: ncm free ncm=ffffff9b179c3200 <-- released ncm
>
> [function link via configfs]
> NCM: ncm_alloc: ncm alloc ncm=ffffff9ac4f8a000
> NCM: ncm_bind: ncm bind ncm=ffffff9ac4f8a000
> NCM: ncm_set_alt: ncm=ffffff9ac4f8a000 alt=0
> usb0: eth_open dev->port_usb=ffffff9b179c3200 <-- previous released ncm
> usb0: eth_start dev->port_usb=ffffff9b179c3200 <--
>
> Unable to handle kernel paging request at virtual address dead00000000014f
>
> This patch addresses the issue by checking if 'ncm->netdev' is not NULL at
> ncm_disable() to call gether_disconnect() to deassociate 'dev->port_usb'.
> It's more reasonable to check 'ncm->netdev' to call gether_connect/disconnect
> rather than check 'ncm->port.in_ep->enabled' since it might not be enabled
> but the gether connection might be established.
>
> Signed-off-by: Norihiko Hama <Norihiko.Hama@xxxxxxxxxxxxxx>
> ---
> drivers/usb/gadget/function/f_ncm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
> index bd095ae569ed..23960cd16463 100644
> --- a/drivers/usb/gadget/function/f_ncm.c
> +++ b/drivers/usb/gadget/function/f_ncm.c
> @@ -888,7 +888,7 @@ static int ncm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
> if (alt > 1)
> goto fail;
>
> - if (ncm->port.in_ep->enabled) {
> + if (ncm->netdev) {
> DBG(cdev, "reset ncm\n");
> ncm->netdev = NULL;
> gether_disconnect(&ncm->port);
> @@ -1365,7 +1365,7 @@ static void ncm_disable(struct usb_function *f)
>
> DBG(cdev, "ncm deactivated\n");
>
> - if (ncm->port.in_ep->enabled) {
> + if (ncm->netdev) {
> ncm->netdev = NULL;
> gether_disconnect(&ncm->port);
> }
> --
> 2.17.1
>
What commit id does this change fix?
thanks,
greg k-h
