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

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

[Fang Jiabao]

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(-)

diff --git a/ip/iproute.c b/ip/iproute.c
index 1ce71f78..35c23801 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -1525,9 +1525,10 @@ static int iproute_modify(int cmd, unsigned int flags, int argc, char **argv)
 			lwt_parse_encap(rta, sizeof(buf), &argc, &argv,
 					RTA_ENCAP, RTA_ENCAP_TYPE);
 
-			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;
 		} else if (strcmp(*argv, "ttl-propagate") == 0) {
 			__u8 ttl_prop;
 
-- 
2.43.0

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

[Stephen Hemminger]

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.