Re: [PATCH net v2] net/x25: Fix overflow when accumulating packets

[Martin Schiller]

On 2026-04-02 04:48, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net/x25: Fix overflow when accumulating packets
>
> This commit adds a check to prevent x25_sock.fraglen from overflowing
> during packet accumulation. It also resets the fragment length when
> purging the fragment queue.
>
> > diff --git a/net/x25/x25_in.c b/net/x25/x25_in.c
> > index b981a4828d08..cb84c683d249 100644
> > --- a/net/x25/x25_in.c
> > +++ b/net/x25/x25_in.c
> > @@ -34,6 +34,12 @@ static int x25_queue_rx_frame(struct sock *sk, 
> > struct sk_buff *skb, int more)
> >  	struct sk_buff *skbo, *skbn = skb;
> >  	struct x25_sock *x25 = x25_sk(sk);
> >
> > +	/* make sure we don't overflow */
> > +	if (x25->fraglen + skb->len > USHRT_MAX) {
> > +		kfree_skb(skb);
> > +		return 1;
> > +	}
>
> Does this cause the skb to be freed twice?
>
> When x25_queue_rx_frame() frees the skb and returns 1, the caller
> x25_state3_machine() leaves its local queued variable as 0 and returns 
> 0.
>
> This 0 value propagates up through x25_process_rx_frame() to callers
> like x25_backlog_rcv() or x25_lapb_receive_frame(). Since the return
> value indicates the packet was not consumed by the protocol stack, 
> these
> upper-level functions will call kfree_skb(skb) again on the already
> freed skb.
>
> Would it be safer to return 1 without calling kfree_skb(skb) here so
> the callers can handle freeing the unconsumed packet?

This is already fixed / changed in the v4 patch set.