diff mbox series

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

Message ID 20230927105858.12950-1-quic_kriskura@quicinc.com (mailing list archive)
State Accepted
Commit 427694cfaafa565a3db5c5ea71df6bc095dca92f
Headers show
Series [v4] usb: gadget: ncm: Handle decoding of multiple NTB's in unwrap call | expand

Commit Message

Krishna Kurapati Sept. 27, 2023, 10:58 a.m. UTC 
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 <quic_kriskura@quicinc.com>
---
Changes in v4: Replaced void* with __le16* typecast for tmp variable
Changes in v3: Removed explicit void* typecast for ntb_ptr variable

 drivers/usb/gadget/function/f_ncm.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

Comments

Maciej Żenczykowski Sept. 27, 2023, 11:13 a.m. UTC | #1 
On Wed, Sep 27, 2023 at 3:59 AM Krishna Kurapati
<quic_kriskura@quicinc.com> wrote:
>
> 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 <quic_kriskura@quicinc.com>
> ---
> Changes in v4: Replaced void* with __le16* typecast for tmp variable
> Changes in v3: Removed explicit void* typecast for ntb_ptr variable
>
>  drivers/usb/gadget/function/f_ncm.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
> index 424bb3b666db..faf90a217419 100644
> --- a/drivers/usb/gadget/function/f_ncm.c
> +++ b/drivers/usb/gadget/function/f_ncm.c
> @@ -1171,7 +1171,8 @@ static int ncm_unwrap_ntb(struct gether *port,
>                           struct sk_buff_head *list)
>  {
>         struct f_ncm    *ncm = func_to_ncm(&port->func);
> -       __le16          *tmp = (void *) skb->data;
> +       unsigned char   *ntb_ptr = skb->data;
> +       __le16          *tmp;
>         unsigned        index, index2;
>         int             ndp_index;
>         unsigned        dg_len, dg_len2;
> @@ -1184,6 +1185,10 @@ static int ncm_unwrap_ntb(struct gether *port,
>         const struct ndp_parser_opts *opts = ncm->parser_opts;
>         unsigned        crc_len = ncm->is_crc ? sizeof(uint32_t) : 0;
>         int             dgram_counter;
> +       int             to_process = skb->len;
> +
> +parse_ntb:
> +       tmp = (__le16 *)ntb_ptr;
>
>         /* dwSignature */
>         if (get_unaligned_le32(tmp) != opts->nth_sign) {
> @@ -1230,7 +1235,7 @@ static int ncm_unwrap_ntb(struct gether *port,
>                  * walk through NDP
>                  * dwSignature
>                  */
> -               tmp = (void *)(skb->data + ndp_index);
> +               tmp = (__le16 *)(ntb_ptr + ndp_index);
>                 if (get_unaligned_le32(tmp) != ncm->ndp_sign) {
>                         INFO(port->func.config->cdev, "Wrong NDP SIGN\n");
>                         goto err;
> @@ -1287,11 +1292,11 @@ static int ncm_unwrap_ntb(struct gether *port,
>                         if (ncm->is_crc) {
>                                 uint32_t crc, crc2;
>
> -                               crc = get_unaligned_le32(skb->data +
> +                               crc = get_unaligned_le32(ntb_ptr +
>                                                          index + dg_len -
>                                                          crc_len);
>                                 crc2 = ~crc32_le(~0,
> -                                                skb->data + index,
> +                                                ntb_ptr + index,
>                                                  dg_len - crc_len);
>                                 if (crc != crc2) {
>                                         INFO(port->func.config->cdev,
> @@ -1318,7 +1323,7 @@ static int ncm_unwrap_ntb(struct gether *port,
>                                                          dg_len - crc_len);
>                         if (skb2 == NULL)
>                                 goto err;
> -                       skb_put_data(skb2, skb->data + index,
> +                       skb_put_data(skb2, ntb_ptr + index,
>                                      dg_len - crc_len);
>
>                         skb_queue_tail(list, skb2);
> @@ -1331,10 +1336,17 @@ static int ncm_unwrap_ntb(struct gether *port,
>                 } while (ndp_len > 2 * (opts->dgram_item_len * 2));
>         } while (ndp_index);
>
> -       dev_consume_skb_any(skb);
> -
>         VDBG(port->func.config->cdev,
>              "Parsed NTB with %d frames\n", dgram_counter);
> +
> +       to_process -= block_len;
> +       if (to_process != 0) {
> +               ntb_ptr = (unsigned char *)(ntb_ptr + block_len);
> +               goto parse_ntb;
> +       }
> +
> +       dev_consume_skb_any(skb);
> +
>         return 0;
>  err:
>         skb_queue_purge(list);
> --
> 2.42.0

Reviewed-by: Maciej Żenczykowski <maze@google.com>
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 424bb3b666db..faf90a217419 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1171,7 +1171,8 @@  static int ncm_unwrap_ntb(struct gether *port,
 			  struct sk_buff_head *list)
 {
 	struct f_ncm	*ncm = func_to_ncm(&port->func);
-	__le16		*tmp = (void *) skb->data;
+	unsigned char	*ntb_ptr = skb->data;
+	__le16		*tmp;
 	unsigned	index, index2;
 	int		ndp_index;
 	unsigned	dg_len, dg_len2;
@@ -1184,6 +1185,10 @@  static int ncm_unwrap_ntb(struct gether *port,
 	const struct ndp_parser_opts *opts = ncm->parser_opts;
 	unsigned	crc_len = ncm->is_crc ? sizeof(uint32_t) : 0;
 	int		dgram_counter;
+	int		to_process = skb->len;
+
+parse_ntb:
+	tmp = (__le16 *)ntb_ptr;
 
 	/* dwSignature */
 	if (get_unaligned_le32(tmp) != opts->nth_sign) {
@@ -1230,7 +1235,7 @@  static int ncm_unwrap_ntb(struct gether *port,
 		 * walk through NDP
 		 * dwSignature
 		 */
-		tmp = (void *)(skb->data + ndp_index);
+		tmp = (__le16 *)(ntb_ptr + ndp_index);
 		if (get_unaligned_le32(tmp) != ncm->ndp_sign) {
 			INFO(port->func.config->cdev, "Wrong NDP SIGN\n");
 			goto err;
@@ -1287,11 +1292,11 @@  static int ncm_unwrap_ntb(struct gether *port,
 			if (ncm->is_crc) {
 				uint32_t crc, crc2;
 
-				crc = get_unaligned_le32(skb->data +
+				crc = get_unaligned_le32(ntb_ptr +
 							 index + dg_len -
 							 crc_len);
 				crc2 = ~crc32_le(~0,
-						 skb->data + index,
+						 ntb_ptr + index,
 						 dg_len - crc_len);
 				if (crc != crc2) {
 					INFO(port->func.config->cdev,
@@ -1318,7 +1323,7 @@  static int ncm_unwrap_ntb(struct gether *port,
 							 dg_len - crc_len);
 			if (skb2 == NULL)
 				goto err;
-			skb_put_data(skb2, skb->data + index,
+			skb_put_data(skb2, ntb_ptr + index,
 				     dg_len - crc_len);
 
 			skb_queue_tail(list, skb2);
@@ -1331,10 +1336,17 @@  static int ncm_unwrap_ntb(struct gether *port,
 		} while (ndp_len > 2 * (opts->dgram_item_len * 2));
 	} while (ndp_index);
 
-	dev_consume_skb_any(skb);
-
 	VDBG(port->func.config->cdev,
 	     "Parsed NTB with %d frames\n", dgram_counter);
+
+	to_process -= block_len;
+	if (to_process != 0) {
+		ntb_ptr = (unsigned char *)(ntb_ptr + block_len);
+		goto parse_ntb;
+	}
+
+	dev_consume_skb_any(skb);
+
 	return 0;
 err:
 	skb_queue_purge(list);