Re: [PATCH iproute2] ip route: check return value of addraw_l() when adding encap

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon,  2 Nov 2026 17:05:34 +0800
Fang Jiabao <kaquoin@163.com> wrote:

> iproute_modify() ignores the return value of addraw_l() when it adds
> the encap attribute. If the attribute does not fit into the request,
> for example a seg6 encap with 60 segments, addraw_l() prints
> "addraw_l ERROR: message exceeded bound of 1024" and returns -1, but
> ip sends the request without the encap attribute and exits with
> status 0. A route without encap is then installed silently.
> 
> Return an error instead of sending the incomplete request.
> 
> Fixes: 1e5293056a02 ("lwtunnel: Add encapsulation support to ip route")
> Assisted-by: Claude:claude-sonnet-5-5
> Signed-off-by: Fang Jiabao <kaquoin@163.com>
> ---
> Tested on current main (e4cd9f1d, "json_writer: fix pretty printing of
> arrays"), in a network namespace with a dummy device:
> 
>   ip -6 route add fc00:ff::60/128 encap seg6 mode encap segs <60 SIDs> dev d0
> 
> Before: "addraw_l ERROR: message exceeded bound of 1024", exit status 0,
> and a route without encap was installed.
> After: same message, exit status 1, no route installed.
> With 59 segments the route is still added with encap, exit status 0.
> 
> With 63 or more segments the failure happens earlier, in the encap
> parsing, with a different error. That is not addressed here. The same
> addraw_l() pattern exists in ip/ipnexthop.c and is not changed here.
> 
>  ip/iproute.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

I agree with AI review here:

The bug is real and worth fixing - thanks. I reproduced the
error-then-send behavior: with 60 segments the unpatched ip prints the
addraw_l error and still hands the request to the kernel (the reply
proves the send). Exit 0 follows from the ignored return value, matching
your test. Silently installing a route without the requested encap is
the worst possible outcome here.

But please fix the cause rather than the symptom.

> -			if (rta->rta_len > RTA_LENGTH(0))
> -				addraw_l(&req.n, 1024
> -					 , RTA_DATA(rta), RTA_PAYLOAD(rta));
> +			if (rta->rta_len > RTA_LENGTH(0) &&
> +			    addraw_l(&req.n, 1024, RTA_DATA(rta),
> +				     RTA_PAYLOAD(rta)) < 0)
> +				return -1;

The request buffer is 4096 bytes:

	struct {
		struct nlmsghdr	n;
		struct rtmsg		r;
		char			buf[4096];
	} req = { ... };

so the hardcoded 1024 is simply wrong - it is a leftover that bears no
relation to the actual buffer. Every other attribute in this function
passes sizeof(req); this call site is the only one using a magic number.

That matters because the 60-segment case you hit is not actually too
big for the message. It only exceeds the bogus 1024 limit. Using the
correct bound:

			if (rta->rta_len > RTA_LENGTH(0) &&
			    addraw_l(&req.n, sizeof(req), RTA_DATA(rta),
				     RTA_PAYLOAD(rta)) < 0)
				return -1;

I tested this and the 60-segment route is accepted and passed to the
kernel instead of being rejected. Segment counts up to 62 work; at 63
the 1024-byte rta buffer in iproute_modify becomes the limiting factor
and it fails there with a clear error - which is the earlier failure you
noted.

So your patch makes a working command fail loudly, while fixing the
bound makes it work. Keep the return-value check - it is correct and
still needed for genuinely oversized attributes - but combine it with
sizeof(req).

> The same addraw_l() pattern exists in ip/ipnexthop.c and is not
> changed here.

Please fix it in the same series; it is the identical bug with the
identical 1024, and leaving one copy behind just means someone
rediscovers it later. Two small patches are fine.

Otherwise the patch is well formed - builds clean, the Fixes tag
resolves correctly, and the commit message states the problem and the
reproducer clearly. Please respin.