[PATCH] caif: fix integer underflow in cffrml_receive()

[PATCH] caif: fix integer underflow in cffrml_receive()

[Junrui Luo]

The cffrml_receive() function extracts a length field from the packet
header and, when FCS is disabled, subtracts 2 from this length without
validating that len >= 2.

If an attacker sends a malicious packet with a length field of 0 or 1
to an interface with FCS disabled, the subtraction causes an integer
underflow.

This can lead to memory exhaustion and kernel instability, potential
information disclosure if padding contains uninitialized kernel memory.

Fix this by validating that len >= 2 before performing the subtraction.

Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Reported-by: Junrui Luo <moonafterrain@outlook.com>
Fixes: b482cd2053e3 ("net-caif: add CAIF core protocol stack")
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 net/caif/cffrml.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/caif/cffrml.c b/net/caif/cffrml.c
index 6651a8dc62e0..d4d63586053a 100644
--- a/net/caif/cffrml.c
+++ b/net/caif/cffrml.c
@@ -92,8 +92,15 @@ static int cffrml_receive(struct cflayer *layr, struct cfpkt *pkt)
 	len = le16_to_cpu(tmp);
 
 	/* Subtract for FCS on length if FCS is not used. */
-	if (!this->dofcs)
+	if (!this->dofcs) {
+		if (len < 2) {
+			++cffrml_rcv_error;
+			pr_err("Invalid frame length (%d)\n", len);
+			cfpkt_destroy(pkt);
+			return -EPROTO;
+		}
 		len -= 2;
+	}
 
 	if (cfpkt_setlen(pkt, len) < 0) {
 		++cffrml_rcv_error;

---
base-commit: 559e608c46553c107dbba19dae0854af7b219400
change-id: 20251204-fixes-23393d72bfc8

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>

Re: [PATCH] caif: fix integer underflow in cffrml_receive()

[Simon Horman]

On Thu, Dec 04, 2025 at 09:30:47PM +0800, Junrui Luo wrote:
> The cffrml_receive() function extracts a length field from the packet
> header and, when FCS is disabled, subtracts 2 from this length without
> validating that len >= 2.
> 
> If an attacker sends a malicious packet with a length field of 0 or 1
> to an interface with FCS disabled, the subtraction causes an integer
> underflow.
> 
> This can lead to memory exhaustion and kernel instability, potential
> information disclosure if padding contains uninitialized kernel memory.
> 
> Fix this by validating that len >= 2 before performing the subtraction.
> 
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Reported-by: Junrui Luo <moonafterrain@outlook.com>
> Fixes: b482cd2053e3 ("net-caif: add CAIF core protocol stack")
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>

Hi Junrui,

I agree with your analysis and that the problem was introduced
by the cited commit.

I think that this function could benefit with a goto label that is jumped
to by all of the cases that follow the same error handling logic as this
one - I count 4 including this one.  But as a minimal bug fix I agree this
is a good approach.

No need to repost, but in future please consider targeting networking
bug fixes at the net tree like this:

Subject: [PATCH net] ...

Reviewed-by: Simon Horman <horms@kernel.org>

...

Re: [PATCH] caif: fix integer underflow in cffrml_receive()

[patchwork-bot+netdevbpf]

Hello:

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

On Thu, 04 Dec 2025 21:30:47 +0800 you wrote:
> The cffrml_receive() function extracts a length field from the packet
> header and, when FCS is disabled, subtracts 2 from this length without
> validating that len >= 2.
> 
> If an attacker sends a malicious packet with a length field of 0 or 1
> to an interface with FCS disabled, the subtraction causes an integer
> underflow.
> 
> [...]

Here is the summary with links:
  - caif: fix integer underflow in cffrml_receive()
    https://git.kernel.org/netdev/net/c/8a11ff0948b5

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

Re: [PATCH] caif: fix integer underflow in cffrml_receive()

[David Laight]

On Thu, 04 Dec 2025 21:30:47 +0800
Junrui Luo <moonafterrain@outlook.com> wrote:

> The cffrml_receive() function extracts a length field from the packet
> header and, when FCS is disabled, subtracts 2 from this length without
> validating that len >= 2.
> 
> If an attacker sends a malicious packet with a length field of 0 or 1
> to an interface with FCS disabled, the subtraction causes an integer
> underflow.
> 
> This can lead to memory exhaustion and kernel instability, potential
> information disclosure if padding contains uninitialized kernel memory.
> 
> Fix this by validating that len >= 2 before performing the subtraction.
> 
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Reported-by: Junrui Luo <moonafterrain@outlook.com>
> Fixes: b482cd2053e3 ("net-caif: add CAIF core protocol stack")
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> ---
>  net/caif/cffrml.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/net/caif/cffrml.c b/net/caif/cffrml.c
> index 6651a8dc62e0..d4d63586053a 100644
> --- a/net/caif/cffrml.c
> +++ b/net/caif/cffrml.c
> @@ -92,8 +92,15 @@ static int cffrml_receive(struct cflayer *layr, struct cfpkt *pkt)
>  	len = le16_to_cpu(tmp);
>  
>  	/* Subtract for FCS on length if FCS is not used. */
> -	if (!this->dofcs)
> +	if (!this->dofcs) {
> +		if (len < 2) {
> +			++cffrml_rcv_error;
> +			pr_err("Invalid frame length (%d)\n", len);

Doesn't that let the same remote attacker flood the kernel message buffer?

	David

> +			cfpkt_destroy(pkt);
> +			return -EPROTO;
> +		}
>  		len -= 2;
> +	}
>  
>  	if (cfpkt_setlen(pkt, len) < 0) {
>  		++cffrml_rcv_error;
> 
> ---
> base-commit: 559e608c46553c107dbba19dae0854af7b219400
> change-id: 20251204-fixes-23393d72bfc8
> 
> Best regards,

Re: [PATCH] caif: fix integer underflow in cffrml_receive()

[Junrui Luo]

On Thu, Dec 11, 2025 at 01:26:16PM +0000, David Laight wrote:
> On Thu, 04 Dec 2025 21:30:47 +0800
> Junrui Luo <moonafterrain@outlook.com> wrote:
> 
> > The cffrml_receive() function extracts a length field from the packet
> > header and, when FCS is disabled, subtracts 2 from this length without
> > validating that len >= 2.
> > 
> > If an attacker sends a malicious packet with a length field of 0 or 1
> > to an interface with FCS disabled, the subtraction causes an integer
> > underflow.
> > 
> > This can lead to memory exhaustion and kernel instability, potential
> > information disclosure if padding contains uninitialized kernel memory.
> > 
> > Fix this by validating that len >= 2 before performing the subtraction.
> > 
> > Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> > Reported-by: Junrui Luo <moonafterrain@outlook.com>
> > Fixes: b482cd2053e3 ("net-caif: add CAIF core protocol stack")
> > Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> > ---
> >  net/caif/cffrml.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/caif/cffrml.c b/net/caif/cffrml.c
> > index 6651a8dc62e0..d4d63586053a 100644
> > --- a/net/caif/cffrml.c
> > +++ b/net/caif/cffrml.c
> > @@ -92,8 +92,15 @@ static int cffrml_receive(struct cflayer *layr, struct cfpkt *pkt)
> >  	len = le16_to_cpu(tmp);
> >  
> >  	/* Subtract for FCS on length if FCS is not used. */
> > -	if (!this->dofcs)
> > +	if (!this->dofcs) {
> > +		if (len < 2) {
> > +			++cffrml_rcv_error;
> > +			pr_err("Invalid frame length (%d)\n", len);
> 
> Doesn't that let the same remote attacker flood the kernel message buffer?

Thanks for the review and suggestion.  Please let me know if you'd like me to
repost a patch removing pr_err(), or if there is a preferred alternative.

> 
> 	David
> 
> > +			cfpkt_destroy(pkt);
> > +			return -EPROTO;
> > +		}
> >  		len -= 2;
> > +	}
> >  
> >  	if (cfpkt_setlen(pkt, len) < 0) {
> >  		++cffrml_rcv_error;
> > 
> > ---
> > base-commit: 559e608c46553c107dbba19dae0854af7b219400
> > change-id: 20251204-fixes-23393d72bfc8
> > 
> > Best regards,
>

Thanks,
Junrui Luo