[PATCH 09/44] ipv6: __ip6_append_data() don't abuse max_t() casts

david.laight.linux@gmail.com posted 44 patches 1 week, 5 days ago
There is a newer version of this series
[PATCH 09/44] ipv6: __ip6_append_data() don't abuse max_t() casts
Posted by david.laight.linux@gmail.com 1 week, 5 days ago
From: David Laight <david.laight.linux@gmail.com>

The implicit casts done by max_t() should only really be used to
convert positive values to signed or unsigned types.
In the EMSGSIZE error path
	pmtu = max_t(int, mtu - headersize + sizeof(struct ipv6hdr), 0);
is being used to convert a large unsigned value to a signed negative one.
Rework using a signed temporary variable and max(pmtu, 0), as well as
casting sizeof() to (int) - which is where the unsignedness comes from.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 net/ipv6/ip6_output.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index f904739e99b9..6fecf2f2cc9a 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1440,7 +1440,7 @@ static int __ip6_append_data(struct sock *sk,
 	struct sk_buff *skb, *skb_prev = NULL;
 	struct inet_cork *cork = &cork_full->base;
 	struct flowi6 *fl6 = &cork_full->fl.u.ip6;
-	unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu, pmtu;
+	unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu;
 	struct ubuf_info *uarg = NULL;
 	int exthdrlen = 0;
 	int dst_exthdrlen = 0;
@@ -1504,9 +1504,10 @@ static int __ip6_append_data(struct sock *sk,
 		maxnonfragsize = mtu;
 
 	if (cork->length + length > maxnonfragsize - headersize) {
+		int pmtu;
 emsgsize:
-		pmtu = max_t(int, mtu - headersize + sizeof(struct ipv6hdr), 0);
-		ipv6_local_error(sk, EMSGSIZE, fl6, pmtu);
+		pmtu = mtu - headersize + (int)sizeof(struct ipv6hdr);
+		ipv6_local_error(sk, EMSGSIZE, fl6, max(pmtu, 0));
 		return -EMSGSIZE;
 	}
 
-- 
2.39.5
Re: [PATCH 09/44] ipv6: __ip6_append_data() don't abuse max_t() casts
Posted by bot+bpf-ci@kernel.org 1 week, 5 days ago
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index f904739e9..6fecf2f2c 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1440,7 +1440,7 @@ static int __ip6_append_data(struct sock *sk,
>  	struct sk_buff *skb, *skb_prev = NULL;
>  	struct inet_cork *cork = &cork_full->base;
>  	struct flowi6 *fl6 = &cork_full->fl.u.ip6;
> -	unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu, pmtu;
> +	unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu;

[ ... ]

> @@ -1504,9 +1504,10 @@ static int __ip6_append_data(struct sock *sk,
>  		maxnonfragsize = mtu;
>
>  	if (cork->length + length > maxnonfragsize - headersize) {
> +		int pmtu;
>  emsgsize:
> -		pmtu = max_t(int, mtu - headersize + sizeof(struct ipv6hdr), 0);
> -		ipv6_local_error(sk, EMSGSIZE, fl6, pmtu);
> +		pmtu = mtu - headersize + (int)sizeof(struct ipv6hdr);
> +		ipv6_local_error(sk, EMSGSIZE, fl6, max(pmtu, 0));
>  		return -EMSGSIZE;
>  	}

Can the variable pmtu be used uninitialized here?

When reaching the emsgsize label via goto statements from earlier in
__ip6_append_data() (the checks at lines around mtu <= fragheaderlen,
headersize + transhdrlen > mtu, and the dontfrag check), the int pmtu
declaration is skipped. This leaves pmtu uninitialized when it's used
in the calculation that's passed to ipv6_local_error().


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19519124312
Re: [PATCH 09/44] ipv6: __ip6_append_data() don't abuse max_t() casts
Posted by David Laight 1 week, 4 days ago
On Thu, 20 Nov 2025 00:32:34 +0000 (UTC)
bot+bpf-ci@kernel.org wrote:

> > diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> > index f904739e9..6fecf2f2c 100644
> > --- a/net/ipv6/ip6_output.c
> > +++ b/net/ipv6/ip6_output.c
> > @@ -1440,7 +1440,7 @@ static int __ip6_append_data(struct sock *sk,
> >  	struct sk_buff *skb, *skb_prev = NULL;
> >  	struct inet_cork *cork = &cork_full->base;
> >  	struct flowi6 *fl6 = &cork_full->fl.u.ip6;
> > -	unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu, pmtu;
> > +	unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu;  
> 
> [ ... ]
> 
> > @@ -1504,9 +1504,10 @@ static int __ip6_append_data(struct sock *sk,
> >  		maxnonfragsize = mtu;
> >
> >  	if (cork->length + length > maxnonfragsize - headersize) {
> > +		int pmtu;
> >  emsgsize:
> > -		pmtu = max_t(int, mtu - headersize + sizeof(struct ipv6hdr), 0);
> > -		ipv6_local_error(sk, EMSGSIZE, fl6, pmtu);
> > +		pmtu = mtu - headersize + (int)sizeof(struct ipv6hdr);
> > +		ipv6_local_error(sk, EMSGSIZE, fl6, max(pmtu, 0));
> >  		return -EMSGSIZE;
> >  	}  
> 
> Can the variable pmtu be used uninitialized here?

If your AI bot doesn't understand code don't run it.

	David

> 
> When reaching the emsgsize label via goto statements from earlier in
> __ip6_append_data() (the checks at lines around mtu <= fragheaderlen,
> headersize + transhdrlen > mtu, and the dontfrag check), the int pmtu
> declaration is skipped. This leaves pmtu uninitialized when it's used
> in the calculation that's passed to ipv6_local_error().
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19519124312
Re: [PATCH 09/44] ipv6: __ip6_append_data() don't abuse max_t() casts
Posted by Chris Mason 1 week, 4 days ago
On 11/20/25 6:16 AM, David Laight wrote:
> On Thu, 20 Nov 2025 00:32:34 +0000 (UTC)
> bot+bpf-ci@kernel.org wrote:
> 
>>> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
>>> index f904739e9..6fecf2f2c 100644
>>> --- a/net/ipv6/ip6_output.c
>>> +++ b/net/ipv6/ip6_output.c
>>> @@ -1440,7 +1440,7 @@ static int __ip6_append_data(struct sock *sk,
>>>  	struct sk_buff *skb, *skb_prev = NULL;
>>>  	struct inet_cork *cork = &cork_full->base;
>>>  	struct flowi6 *fl6 = &cork_full->fl.u.ip6;
>>> -	unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu, pmtu;
>>> +	unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu;  
>>
>> [ ... ]
>>
>>> @@ -1504,9 +1504,10 @@ static int __ip6_append_data(struct sock *sk,
>>>  		maxnonfragsize = mtu;
>>>
>>>  	if (cork->length + length > maxnonfragsize - headersize) {
>>> +		int pmtu;
>>>  emsgsize:
>>> -		pmtu = max_t(int, mtu - headersize + sizeof(struct ipv6hdr), 0);
>>> -		ipv6_local_error(sk, EMSGSIZE, fl6, pmtu);
>>> +		pmtu = mtu - headersize + (int)sizeof(struct ipv6hdr);
>>> +		ipv6_local_error(sk, EMSGSIZE, fl6, max(pmtu, 0));
>>>  		return -EMSGSIZE;
>>>  	}  
>>
>> Can the variable pmtu be used uninitialized here?
> 
> If your AI bot doesn't understand code don't run it.

I ran this locally three more times and this false positive didn't
reproduce, but I'll see if the CI has enough logs to figure out where it
got confused.

Regardless, I'm doing periodic checks for patterns of false positives
and fine tuning the prompts.

-chris