[PATCH 0/2] Error checking in rndis

[PATCH 0/2] Error checking in rndis

[Griffin Kroah-Hartman]

Small patch series for two changes in
drivers/usb/gadget/function/rndis.c, both related to error checking.

Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
---
Griffin Kroah-Hartman (2):
      usb: gadget: function: rndis: add length check to response query
      usb: gadget: function: rndis: add length check for header

 drivers/usb/gadget/function/rndis.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
---
base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
change-id: 20260708-usb-gadget-rndis-b80cbb3b4572

Best regards,
--  
Griffin Kroah-Hartman <griffin@kroah.com>

[PATCH 1/2] usb: gadget: function: rndis: add length check to response query

[Griffin Kroah-Hartman]

Add variable representations for BufLength and BufOffset in
rndis_query_response(), and perform a length check on them.

This is identical to how rndis_set_response() handles these parameters.

Assisted-by: gkh_clanker_2000
Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
---
 drivers/usb/gadget/function/rndis.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/function/rndis.c b/drivers/usb/gadget/function/rndis.c
index 3da54a7d7aba..fe2018ff071a 100644
--- a/drivers/usb/gadget/function/rndis.c
+++ b/drivers/usb/gadget/function/rndis.c
@@ -591,6 +591,7 @@ static int rndis_init_response(struct rndis_params *params,
 static int rndis_query_response(struct rndis_params *params,
 				rndis_query_msg_type *buf)
 {
+	u32 BufLength, BufOffset;
 	rndis_query_cmplt_type *resp;
 	rndis_resp_t *r;
 
@@ -598,6 +599,13 @@ static int rndis_query_response(struct rndis_params *params,
 	if (!params->dev)
 		return -ENOTSUPP;
 
+	BufLength = le32_to_cpu(buf->InformationBufferLength);
+	BufOffset = le32_to_cpu(buf->InformationBufferOffset);
+	if ((BufLength > RNDIS_MAX_TOTAL_SIZE) ||
+	    (BufOffset > RNDIS_MAX_TOTAL_SIZE) ||
+	    (BufOffset + 8 >= RNDIS_MAX_TOTAL_SIZE))
+		return -EINVAL;
+
 	/*
 	 * we need more memory:
 	 * gen_ndis_query_resp expects enough space for
@@ -614,10 +622,8 @@ static int rndis_query_response(struct rndis_params *params,
 	resp->RequestID = buf->RequestID; /* Still LE in msg buffer */
 
 	if (gen_ndis_query_resp(params, le32_to_cpu(buf->OID),
-			le32_to_cpu(buf->InformationBufferOffset)
-					+ 8 + (u8 *)buf,
-			le32_to_cpu(buf->InformationBufferLength),
-			r)) {
+				BufOffset + 8 + (u8 *)buf,
+				BufLength, r)) {
 		/* OID not supported */
 		resp->Status = cpu_to_le32(RNDIS_STATUS_NOT_SUPPORTED);
 		resp->MessageLength = cpu_to_le32(sizeof *resp);

-- 
2.55.0

Re: [PATCH 1/2] usb: gadget: function: rndis: add length check to response query

[Oliver Neukum]

Hi,

On 08.07.26 13:08, Griffin Kroah-Hartman wrote:
   
> @@ -598,6 +599,13 @@ static int rndis_query_response(struct rndis_params *params,
>   	if (!params->dev)
>   		return -ENOTSUPP;
>   
> +	BufLength = le32_to_cpu(buf->InformationBufferLength);
> +	BufOffset = le32_to_cpu(buf->InformationBufferOffset);
> +	if ((BufLength > RNDIS_MAX_TOTAL_SIZE) ||
> +	    (BufOffset > RNDIS_MAX_TOTAL_SIZE) ||
> +	    (BufOffset + 8 >= RNDIS_MAX_TOTAL_SIZE))

Wouldn't those last two lines be more efficiently done with:

BufOffset >= RNDIS_MAX_TOTAL_SIZE - 8

	Regards
		Oliver

Re: [PATCH 1/2] usb: gadget: function: rndis: add length check to response query

[Greg Kroah-Hartman]

On Wed, Jul 08, 2026 at 06:24:16PM +0200, Oliver Neukum wrote:
> Hi,
> 
> On 08.07.26 13:08, Griffin Kroah-Hartman wrote:
> > @@ -598,6 +599,13 @@ static int rndis_query_response(struct rndis_params *params,
> >   	if (!params->dev)
> >   		return -ENOTSUPP;
> > +	BufLength = le32_to_cpu(buf->InformationBufferLength);
> > +	BufOffset = le32_to_cpu(buf->InformationBufferOffset);
> > +	if ((BufLength > RNDIS_MAX_TOTAL_SIZE) ||
> > +	    (BufOffset > RNDIS_MAX_TOTAL_SIZE) ||
> > +	    (BufOffset + 8 >= RNDIS_MAX_TOTAL_SIZE))
> 
> Wouldn't those last two lines be more efficiently done with:
> 
> BufOffset >= RNDIS_MAX_TOTAL_SIZE - 8

Probably, but doesn't the compiler turn them into the same result?
Also, rndis_set_response() has this same check, so at least everything
is consistent :)

thanks,

greg k-h

[PATCH 2/2] usb: gadget: function: rndis: add length check for header

[Griffin Kroah-Hartman]

Add a length check for the rndis header in rndis_rm_hdr, to ensure that
MessageType, MessageLength, DataOffset, and DataLength fields are
present before they are accessed.

Assisted-by: gkh_clanker_2000
Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
---
 drivers/usb/gadget/function/rndis.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/gadget/function/rndis.c b/drivers/usb/gadget/function/rndis.c
index fe2018ff071a..a2fd239b7ad3 100644
--- a/drivers/usb/gadget/function/rndis.c
+++ b/drivers/usb/gadget/function/rndis.c
@@ -1080,6 +1080,12 @@ int rndis_rm_hdr(struct gether *port,
 	/* tmp points to a struct rndis_packet_msg_type */
 	__le32 *tmp = (void *)skb->data;
 
+	/* Need at least MessageType, MessageLength, DataOffset, DataLength */
+	if (skb->len < 16) {
+		dev_kfree_skb_any(skb);
+		return -EINVAL;
+	}
+
 	/* MessageType, MessageLength */
 	if (cpu_to_le32(RNDIS_MSG_PACKET)
 			!= get_unaligned(tmp++)) {

-- 
2.55.0