kernel/drivers/usb/gadget/function/f_ncm.c, branch linux-6.9.y

usb: gadget: f_ncm: Fix UAF ncm object at re-bind after usb ep transport error

2024-04-04T14:55:53Z

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 <-- eth_start_xmit() --> dev->wrap() 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 Cc: stable Link: https://lore.kernel.org/r/20240327023550.51214-1-Norihiko.Hama@alpsalpine.com Signed-off-by: Greg Kroah-Hartman

usb: gadget: ncm: Fix handling of zero block length packets

2024-03-05T13:20:40Z

While connecting to a Linux host with CDC_NCM_NTB_DEF_SIZE_TX set to 65536, it has been observed that we receive short packets, which come at interval of 5-10 seconds sometimes and have block length zero but still contain 1-2 valid datagrams present. According to the NCM spec: "If wBlockLength = 0x0000, the block is terminated by a short packet. In this case, the USB transfer must still be shorter than dwNtbInMaxSize or dwNtbOutMaxSize. If exactly dwNtbInMaxSize or dwNtbOutMaxSize bytes are sent, and the size is a multiple of wMaxPacketSize for the given pipe, then no ZLP shall be sent. wBlockLength= 0x0000 must be used with extreme care, because of the possibility that the host and device may get out of sync, and because of test issues. wBlockLength = 0x0000 allows the sender to reduce latency by starting to send a very large NTB, and then shortening it when the sender discovers that there’s not sufficient data to justify sending a large NTB" However, there is a potential issue with the current implementation, as it checks for the occurrence of multiple NTBs in a single giveback by verifying if the leftover bytes to be processed is zero or not. If the block length reads zero, we would process the same NTB infintely because the leftover bytes is never zero and it leads to a crash. Fix this by bailing out if block length reads zero. Cc: stable@vger.kernel.org Fixes: 427694cfaafa ("usb: gadget: ncm: Handle decoding of multiple NTB's in unwrap call") Signed-off-by: Krishna Kurapati Reviewed-by: Maciej Żenczykowski Link: https://lore.kernel.org/r/20240228115441.2105585-1-quic_kriskura@quicinc.com Signed-off-by: Greg Kroah-Hartman

usb: gadget: ncm: Avoid dropping datagrams of properly parsed NTBs

2024-02-19T08:33:27Z

It is observed sometimes when tethering is used over NCM with Windows 11 as host, at some instances, the gadget_giveback has one byte appended at the end of a proper NTB. When the NTB is parsed, unwrap call looks for any leftover bytes in SKB provided by u_ether and if there are any pending bytes, it treats them as a separate NTB and parses it. But in case the second NTB (as per unwrap call) is faulty/corrupt, all the datagrams that were parsed properly in the first NTB and saved in rx_list are dropped. Adding a few custom traces showed the following: [002] d..1 7828.532866: dwc3_gadget_giveback: ep1out: req 000000003868811a length 1025/16384 zsI ==> 0 [002] d..1 7828.532867: ncm_unwrap_ntb: K: ncm_unwrap_ntb toprocess: 1025 [002] d..1 7828.532867: ncm_unwrap_ntb: K: ncm_unwrap_ntb nth: 1751999342 [002] d..1 7828.532868: ncm_unwrap_ntb: K: ncm_unwrap_ntb seq: 0xce67 [002] d..1 7828.532868: ncm_unwrap_ntb: K: ncm_unwrap_ntb blk_len: 0x400 [002] d..1 7828.532868: ncm_unwrap_ntb: K: ncm_unwrap_ntb ndp_len: 0x10 [002] d..1 7828.532869: ncm_unwrap_ntb: K: Parsed NTB with 1 frames In this case, the giveback is of 1025 bytes and block length is 1024. The rest 1 byte (which is 0x00) won't be parsed resulting in drop of all datagrams in rx_list. Same is case with packets of size 2048: [002] d..1 7828.557948: dwc3_gadget_giveback: ep1out: req 0000000011dfd96e length 2049/16384 zsI ==> 0 [002] d..1 7828.557949: ncm_unwrap_ntb: K: ncm_unwrap_ntb nth: 1751999342 [002] d..1 7828.557950: ncm_unwrap_ntb: K: ncm_unwrap_ntb blk_len: 0x800 Lecroy shows one byte coming in extra confirming that the byte is coming in from PC: Transfer 2959 - Bytes Transferred(1025) Timestamp((18.524 843 590) - Transaction 8391 - Data(1025 bytes) Timestamp(18.524 843 590) --- Packet 4063861 Data(1024 bytes) Duration(2.117us) Idle(14.700ns) Timestamp(18.524 843 590) --- Packet 4063863 Data(1 byte) Duration(66.160ns) Time(282.000ns) Timestamp(18.524 845 722) According to Windows driver, no ZLP is needed if wBlockLength is non-zero, because the non-zero wBlockLength has already told the function side the size of transfer to be expected. However, there are in-market NCM devices that rely on ZLP as long as the wBlockLength is multiple of wMaxPacketSize. To deal with such devices, it pads an extra 0 at end so the transfer is no longer multiple of wMaxPacketSize. Cc: Fixes: 9f6ce4240a2b ("usb: gadget: f_ncm.c added") Signed-off-by: Krishna Kurapati Reviewed-by: Maciej Żenczykowski Link: https://lore.kernel.org/r/20240205074650.200304-1-quic_kriskura@quicinc.com Signed-off-by: Greg Kroah-Hartman

usb: gadget: ncm: Fix endianness of wMaxSegmentSize variable in ecm_desc

2024-01-28T00:28:28Z

Recent commit [1] added support for changing max segment size of the NCM interface via configfs. But the value of segment size value stored in ncm_opts need to be converted to little endian before saving it in ecm_desc. Also while initialising the value of segment size in opts during instance allocation, the value ETH_FRAME_LEN needs to be assigned directly without any conversion as ETH_FRAME_LEN and the variable max_segment_size are native endian. The current implementaion modifies it into little endian thus breaking things for big endian targets. Fix endianness while assigning these variables. While at it, fix up some stray spaces in comments added in code. [1]: https://lore.kernel.org/all/20231221153216.18657-1-quic_kriskura@quicinc.com/ Fixes: 1900daeefd3e ("usb: gadget: ncm: Add support to update wMaxSegmentSize via configfs") Signed-off-by: Krishna Kurapati Reviewed-by: Maciej Żenczykowski Link: https://lore.kernel.org/r/20240118154910.8765-1-quic_kriskura@quicinc.com Signed-off-by: Greg Kroah-Hartman

usb: gadget: ncm: Add support to update wMaxSegmentSize via configfs

2024-01-04T15:01:14Z

The max segment size is currently limited to the ethernet frame length of the kernel which happens to be 1514 at this point in time. However the NCM specification limits it to 64K for sixtenn bit NTB's. For peer to peer connections, increasing the segment size gives better throughput. Add support to configure this value before configfs symlink is created. Also since the NTB Out/In buffer sizes are fixed at 16384 bytes, limit the segment size to an upper cap of 8000 to allow at least a minimum of 2 MTU sized datagrams to be aggregated. Set the default MTU size for the ncm interface during function bind before network interface is registered allowing MTU to be set in parity with wMaxSegmentSize. Update gadget documentation describing the new configfs property. Signed-off-by: Krishna Kurapati Link: https://lore.kernel.org/r/20231221153216.18657-1-quic_kriskura@quicinc.com Signed-off-by: Greg Kroah-Hartman

usb: gadget: f_ncm: Always set current gadget in ncm_bind()

2023-10-21T10:54:53Z

Previously, gadget assignment to the net device occurred exclusively during the initial binding attempt. Nevertheless, the gadget pointer could change during bind/unbind cycles due to various conditions, including the unloading/loading of the UDC device driver or the detachment/reconnection of an OTG-capable USB hub device. This patch relocates the gether_set_gadget() function out from ncm_opts->bound condition check, ensuring that the correct gadget is assigned during each bind request. The provided logs demonstrate the consistency of ncm_opts throughout the power cycle, while the gadget may change. * OTG hub connected during boot up and assignment of gadget and ncm_opts pointer [ 2.366301] usb 2-1.5: New USB device found, idVendor=2996, idProduct=0105 [ 2.366304] usb 2-1.5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 2.366306] usb 2-1.5: Product: H2H Bridge [ 2.366308] usb 2-1.5: Manufacturer: Aptiv [ 2.366309] usb 2-1.5: SerialNumber: 13FEB2021 [ 2.427989] usb 2-1.5: New USB device found, VID=2996, PID=0105 [ 2.428959] dabridge 2-1.5:1.0: dabridge 2-4 total endpoints=5, 0000000093a8d681 [ 2.429710] dabridge 2-1.5:1.0: P(0105) D(22.06.22) F(17.3.16) H(1.1) high-speed [ 2.429714] dabridge 2-1.5:1.0: Hub 2-2 P(0151) V(06.87) [ 2.429956] dabridge 2-1.5:1.0: All downstream ports in host mode [ 2.430093] gadget 000000003c414d59 ------> gadget pointer * NCM opts and associated gadget pointer during First ncm_bind [ 34.763929] NCM opts 00000000aa304ac9 [ 34.763930] NCM gadget 000000003c414d59 * OTG capable hub disconnecte or assume driver unload. [ 97.203114] usb 2-1: USB disconnect, device number 2 [ 97.203118] usb 2-1.1: USB disconnect, device number 3 [ 97.209217] usb 2-1.5: USB disconnect, device number 4 [ 97.230990] dabr_udc deleted * Reconnect the OTG hub or load driver assaign new gadget pointer. [ 111.534035] usb 2-1.1: New USB device found, idVendor=2996, idProduct=0120, bcdDevice= 6.87 [ 111.534038] usb 2-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 111.534040] usb 2-1.1: Product: Vendor [ 111.534041] usb 2-1.1: Manufacturer: Aptiv [ 111.534042] usb 2-1.1: SerialNumber: Superior [ 111.535175] usb 2-1.1: New USB device found, VID=2996, PID=0120 [ 111.610995] usb 2-1.5: new high-speed USB device number 8 using xhci-hcd [ 111.630052] usb 2-1.5: New USB device found, idVendor=2996, idProduct=0105, bcdDevice=21.02 [ 111.630055] usb 2-1.5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 111.630057] usb 2-1.5: Product: H2H Bridge [ 111.630058] usb 2-1.5: Manufacturer: Aptiv [ 111.630059] usb 2-1.5: SerialNumber: 13FEB2021 [ 111.687464] usb 2-1.5: New USB device found, VID=2996, PID=0105 [ 111.690375] dabridge 2-1.5:1.0: dabridge 2-8 total endpoints=5, 000000000d87c961 [ 111.691172] dabridge 2-1.5:1.0: P(0105) D(22.06.22) F(17.3.16) H(1.1) high-speed [ 111.691176] dabridge 2-1.5:1.0: Hub 2-6 P(0151) V(06.87) [ 111.691646] dabridge 2-1.5:1.0: All downstream ports in host mode [ 111.692298] gadget 00000000dc72f7a9 --------> new gadget ptr on connect * NCM opts and associated gadget pointer during second ncm_bind [ 113.271786] NCM opts 00000000aa304ac9 -----> same opts ptr used during first bind [ 113.271788] NCM gadget 00000000dc72f7a9 ----> however new gaget ptr, that will not set in net_device due to ncm_opts->bound = true Signed-off-by: Hardik Gajjar Link: https://lore.kernel.org/r/20231020153324.82794-1-hgajjar@de.adit-jv.com Signed-off-by: Greg Kroah-Hartman

usb: gadget: ncm: Handle decoding of multiple NTB's in unwrap call

2023-10-02T11:38:10Z

When NCM is used with hosts like Windows PC, it is observed that there are multiple NTB's contained in one usb request giveback. Since the driver unwraps the obtained request data assuming only one NTB is present, we loose the subsequent NTB's present resulting in data loss. Fix this by checking the parsed block length with the obtained data length in usb request and continue parsing after the last byte of current NTB. Cc: stable@vger.kernel.org Fixes: 9f6ce4240a2b ("usb: gadget: f_ncm.c added") Signed-off-by: Krishna Kurapati Reviewed-by: Maciej Żenczykowski Link: https://lore.kernel.org/r/20230927105858.12950-1-quic_kriskura@quicinc.com Signed-off-by: Greg Kroah-Hartman

usb: gadget: remove max support speed info in bind operation

2023-08-22T12:48:33Z

Take ecm_bind() for example, it call gadget_is_{*}speed() API to show gadget max support speed, it is not much help, remove the API usage here is safe. Similar change apply to acm,eem,loopback,ncm,obex,rndis,serial, sourcesink,subset functions. Signed-off-by: Linyu Yuan Link: https://lore.kernel.org/r/20230803091053.9714-8-quic_linyyuan@quicinc.com Signed-off-by: Greg Kroah-Hartman

usb: gadget: add a inline function gether_bitrate()

2023-08-22T12:48:33Z

In function ecm_bitrate(), it is not support report bit rate for super speed plus mode, but it can use same bit rate value defined in ncm and rndis. Add a common inline function gether_bitrate() which report different for all possible speeds, it can be used by ecm, ncm and rndis, also remove old function from them. Signed-off-by: Linyu Yuan Link: https://lore.kernel.org/r/20230803091053.9714-3-quic_linyyuan@quicinc.com Signed-off-by: Greg Kroah-Hartman

usb: gadget: use working speed to calcaulate network bitrate and qlen

2023-08-22T12:48:33Z

Take ecm_bitrate() as example, it will be called after gadget device link speed negotiation, consider code if (gadget_is_superspeed(g) && g->speed == USB_SPEED_SUPER), if a gadget device link speed is USB_SPEED_SUPER, gadget_is_superspeed(g) must be true, or not it is a wrong configuration of gadget max support speed. Remove gadget_is_superspeed(g) checking should be safe, and remove other similar operation in ncm, rndis, u_ether. Signed-off-by: Linyu Yuan Link: https://lore.kernel.org/r/20230803091053.9714-2-quic_linyyuan@quicinc.com Signed-off-by: Greg Kroah-Hartman