[PATCH net v2] strparser: Fix signed/unsigned mismatch bug

[PATCH net v2] strparser: Fix signed/unsigned mismatch bug

[Nate Karstens]

The `len` member of the sk_buff is an unsigned int. This is cast to
`ssize_t` (a signed type) for the first sk_buff in the comparison,
but not the second sk_buff. On 32-bit systems, this can result in
an integer underflow for certain values because unsigned arithmetic
is being used.

This appears to be an oversight: if the intention was to use unsigned
arithmetic, then the first cast would have been omitted. The change
ensures both len values are cast to `ssize_t`.

The underflow causes an issue with ktls when multiple TLS PDUs are
included in a single TCP segment. The mainline kernel does not use
strparser for ktls anymore, but this is still useful for other
features that still use strparser, and for backporting.

Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
Cc: stable@vger.kernel.org
Fixes: 43a0c6751a32 ("strparser: Stream parser for messages")
---
 net/strparser/strparser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
index 43b1f558b33d..e659fea2da70 100644
--- a/net/strparser/strparser.c
+++ b/net/strparser/strparser.c
@@ -238,7 +238,7 @@ static int __strp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb,
 				strp_parser_err(strp, -EMSGSIZE, desc);
 				break;
 			} else if (len <= (ssize_t)head->len -
-					  skb->len - stm->strp.offset) {
+					  (ssize_t)skb->len - stm->strp.offset) {
 				/* Length must be into new skb (and also
 				 * greater than zero)
 				 */
-- 
2.34.1

Re: [PATCH net v2] strparser: Fix signed/unsigned mismatch bug

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 1703 bytes --]



On 11/6/2025 2:28 PM, Nate Karstens wrote:
> The `len` member of the sk_buff is an unsigned int. This is cast to
> `ssize_t` (a signed type) for the first sk_buff in the comparison,
> but not the second sk_buff. On 32-bit systems, this can result in
> an integer underflow for certain values because unsigned arithmetic
> is being used.
> 
> This appears to be an oversight: if the intention was to use unsigned
> arithmetic, then the first cast would have been omitted. The change
> ensures both len values are cast to `ssize_t`.
> 
> The underflow causes an issue with ktls when multiple TLS PDUs are
> included in a single TCP segment. The mainline kernel does not use
> strparser for ktls anymore, but this is still useful for other
> features that still use strparser, and for backporting.
> 
> Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
> Cc: stable@vger.kernel.org
> Fixes: 43a0c6751a32 ("strparser: Stream parser for messages")
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  net/strparser/strparser.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
> index 43b1f558b33d..e659fea2da70 100644
> --- a/net/strparser/strparser.c
> +++ b/net/strparser/strparser.c
> @@ -238,7 +238,7 @@ static int __strp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb,
>  				strp_parser_err(strp, -EMSGSIZE, desc);
>  				break;
>  			} else if (len <= (ssize_t)head->len -
> -					  skb->len - stm->strp.offset) {
> +					  (ssize_t)skb->len - stm->strp.offset) {
>  				/* Length must be into new skb (and also
>  				 * greater than zero)
>  				 */


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH net v2] strparser: Fix signed/unsigned mismatch bug

[Sabrina Dubroca]

2025-11-06, 16:28:33 -0600, Nate Karstens wrote:
> The `len` member of the sk_buff is an unsigned int. This is cast to
> `ssize_t` (a signed type) for the first sk_buff in the comparison,
> but not the second sk_buff. On 32-bit systems, this can result in
> an integer underflow for certain values because unsigned arithmetic
> is being used.
> 
> This appears to be an oversight: if the intention was to use unsigned
> arithmetic, then the first cast would have been omitted. The change
> ensures both len values are cast to `ssize_t`.
> 
> The underflow causes an issue with ktls when multiple TLS PDUs are
> included in a single TCP segment. The mainline kernel does not use
> strparser for ktls anymore, but this is still useful for other
> features that still use strparser, and for backporting.
> 
> Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
> Cc: stable@vger.kernel.org
> Fixes: 43a0c6751a32 ("strparser: Stream parser for messages")
> ---
>  net/strparser/strparser.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>

Thanks Nate.

-- 
Sabrina

Re: [PATCH net v2] strparser: Fix signed/unsigned mismatch bug

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 6 Nov 2025 16:28:33 -0600 you wrote:
> The `len` member of the sk_buff is an unsigned int. This is cast to
> `ssize_t` (a signed type) for the first sk_buff in the comparison,
> but not the second sk_buff. On 32-bit systems, this can result in
> an integer underflow for certain values because unsigned arithmetic
> is being used.
> 
> This appears to be an oversight: if the intention was to use unsigned
> arithmetic, then the first cast would have been omitted. The change
> ensures both len values are cast to `ssize_t`.
> 
> [...]

Here is the summary with links:
  - [net,v2] strparser: Fix signed/unsigned mismatch bug
    https://git.kernel.org/netdev/net/c/4da4e4bde1c4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH] strparser: Fix signed/unsigned mismatch bug

[Nate Karstens]

Thanks, Sabrina!

> Are you testing on some 32b arch? Otherwise ssize_t would be s64 and
> int/unsigned int should be 32b so the missing cast would not matter?

Yes, that is a good point. I tested this on a 32-bit architecture. On a 64-bit system, the u32 would be put into an s64 because all possible values for the u32 can fit into the s64. Signed arithmetic is used and you would get the correct result.

> Agree. And adding a summary of the information in this thread to the
> commit message would be really useful

Sounds good!

> Agree. I didn't mean to dismiss the presence of a bug, sorry if it
> sounded like that. But I was a bit unclear on the conditions, this
> discussion is helpful.

No worries, I didn't take it as being dismissive at all. You had great questions and I agree that the discussion has been really helpful!

Cheers,

Nate

________________________________

CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.

________________________________

CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.

[PATCH net v2] strparser: Fix signed/unsigned mismatch bug

[Nate Karstens]

The `len` member of the sk_buff is an unsigned int. This is cast to
`ssize_t` (a signed type) for the first sk_buff in the comparison,
but not the second sk_buff. On 32-bit systems, this can result in
an integer underflow for certain values because unsigned arithmetic
is being used.

This appears to be an oversight: if the intention was to use unsigned
arithmetic, then the first cast would have been omitted. The change
ensures both len values are cast to `ssize_t`.

The underflow causes an issue with ktls when multiple TLS PDUs are
included in a single TCP segment. The mainline kernel does not use
strparser for ktls anymore, but this is still useful for other
features that still use strparser, and for backporting.

Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
Cc: stable@vger.kernel.org
Fixes: 43a0c6751a32 ("strparser: Stream parser for messages")
---
 net/strparser/strparser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
index 43b1f558b33d..e659fea2da70 100644
--- a/net/strparser/strparser.c
+++ b/net/strparser/strparser.c
@@ -238,7 +238,7 @@ static int __strp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb,
                                strp_parser_err(strp, -EMSGSIZE, desc);
                                break;
                        } else if (len <= (ssize_t)head->len -
-                                         skb->len - stm->strp.offset) {
+                                         (ssize_t)skb->len - stm->strp.offset) {
                                /* Length must be into new skb (and also
                                 * greater than zero)
                                 */
--
2.34.1


________________________________

CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.

Re: [PATCH net v2] strparser: Fix signed/unsigned mismatch bug

[Jakub Kicinski]

On Thu, 6 Nov 2025 10:51:17 -0600 Nate Karstens wrote:
> CONFIDENTIALITY NOTICE: This email and any attachments are for the
> sole use of the intended recipient(s) and contain information that
> may be Garmin confidential and/or Garmin legally privileged. If you
> have received this email in error, please notify the sender by reply
> email and delete the message. Any disclosure, copying, distribution
> or use of this communication (including attachments) by someone other
> than the intended recipient is prohibited. Thank you.

This notice prevents us from doing anything with the patch.

Also please do _not_ send the patches in reply to existing threads.