From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 9A9D95A0279 for ; Wed, 1 May 2024 08:33:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1714545205; bh=Z9ZBKc89vTGqkYIaAE3JmKeesCg4rCmKhGtbsuKFk4o=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=m8Xo6UqOoUBouK4XHBn+TS7AM24eyDBIU7e19J7SeRaTz9EqwY3Ue6+W4J8V75Jvd rEGRshJVqeyCCYBFXHAO8OSlgUguvQNqiauB2NE0PNXYfwNH1jkIMQc/bY5GBsPZ/1 FDzvi3JBtAMtdEMOzAMYyIlSuRVFePHpmzooId/YvR9kC4oRGReyk4VnEI6nfKi7r/ 9PU0zPS7qDnohMdoRQox5XThXOJpCYxorfAdr7LjBB9u0/JSYUuS0zbDExqg4S+67S d8883vlKEm/ws1woX5JNSi1FAzyA004D5xW4f1ZgxPd4VupKhljYK+0yn1Swbhpt+P HzvJG96Hr1OMw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4VTnM90621z4x10; Wed, 1 May 2024 16:33:25 +1000 (AEST) Date: Wed, 1 May 2024 16:24:06 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 1/2] netlink: Fix iterations over nexthop objects Message-ID: References: <20240423204125.3424982-1-sbrivio@redhat.com> <20240423204125.3424982-2-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="mCteW70ZYZCBw0u0" Content-Disposition: inline In-Reply-To: <20240423204125.3424982-2-sbrivio@redhat.com> Message-ID-Hash: SBXGU5K32C523TG7MDXCUJMPHLBTSIHG X-Message-ID-Hash: SBXGU5K32C523TG7MDXCUJMPHLBTSIHG X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, runsisi X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --mCteW70ZYZCBw0u0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Apr 23, 2024 at 10:41:24PM +0200, Stefano Brivio wrote: > Somewhat confusingly, RTNH_NEXT(), as defined by , > doesn't take an attribute length parameter like RTA_NEXT() does, and > I just modelled loops over nexthops after RTA loops, forgetting to > decrease the remaining length we pass to RTNH_OK(). >=20 > In practice, this didn't cause issue in any of the combinations I > checked, at least without the next patch. >=20 > We seem to be the only user of RTNH_OK(): even iproute2 has an > open-coded version of it in print_rta_multipath() (ip/iproute.c). >=20 > Introduce RTNH_NEXT_AND_DEC(), similar to RTA_NEXT(), and use it. >=20 > Fixes: 6c7623d07bbd ("netlink: Add support to fetch default gateway from = multipath routes") > Fixes: f4e38b5cd232 ("netlink: Adjust interface index inside copied nexth= op objects too") > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > netlink.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) >=20 > diff --git a/netlink.c b/netlink.c > index 89c0641..a5a4870 100644 > --- a/netlink.c > +++ b/netlink.c > @@ -36,6 +36,10 @@ > #include "ip.h" > #include "netlink.h" > =20 > +/* Same as RTA_NEXT() but for nexthops: RTNH_NEXT() doesn't take 'attrle= n' */ > +#define RTNH_NEXT_AND_DEC(rtnh, attrlen) \ > + ((attrlen) -=3D RTNH_ALIGN((rtnh)->rtnh_len), RTNH_NEXT(rtnh)) > + > /* Netlink expects a buffer of at least 8kiB or the system page size, > * whichever is larger. 32kiB is recommended for more efficient. > * Since the largest page size on any remotely common Linux setup is > @@ -349,12 +353,13 @@ unsigned int nl_get_ext_if(int s, sa_family_t af) > */ > bool nl_route_get_def_multipath(struct rtattr *rta, void *gw) > { > + size_t nh_len =3D RTA_PAYLOAD(rta); > struct rtnexthop *rtnh; > bool found =3D false; > int hops =3D -1; > =20 > for (rtnh =3D (struct rtnexthop *)RTA_DATA(rta); > - RTNH_OK(rtnh, RTA_PAYLOAD(rta)); rtnh =3D RTNH_NEXT(rtnh)) { > + RTNH_OK(rtnh, nh_len); rtnh =3D RTNH_NEXT_AND_DEC(rtnh, nh_len)) { > size_t len =3D rtnh->rtnh_len - sizeof(*rtnh); > struct rtattr *rta_inner; > =20 > @@ -566,11 +571,12 @@ int nl_route_dup(int s_src, unsigned int ifi_src, > if (rta->rta_type =3D=3D RTA_OIF) { > *(unsigned int *)RTA_DATA(rta) =3D ifi_dst; > } else if (rta->rta_type =3D=3D RTA_MULTIPATH) { > + size_t nh_len =3D RTA_PAYLOAD(rta); > struct rtnexthop *rtnh; > =20 > for (rtnh =3D (struct rtnexthop *)RTA_DATA(rta); > - RTNH_OK(rtnh, RTA_PAYLOAD(rta)); > - rtnh =3D RTNH_NEXT(rtnh)) > + RTNH_OK(rtnh, nh_len); > + rtnh =3D RTNH_NEXT_AND_DEC(rtnh, nh_len)) > rtnh->rtnh_ifindex =3D ifi_dst; > } else if (rta->rta_type =3D=3D RTA_PREFSRC) { > /* Host routes might include a preferred source --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --mCteW70ZYZCBw0u0 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmYx3/AACgkQzQJF27ox 2Gd9Cg//YItrW0aHPx9w360pcoHGkU+RrbgAhsOCao4MoBgVMRh/Fn9Vb4T/5P70 3UDk7NtIJ6iG521/NRR+9MSt7bsqW4jdWSe9mPutS8wizIxuTVR1mkPKXCMeFQKp v0t89B9LQUF7DceoPlrv/d35o5VU9lPRXxrPLRug8t0uoLGqzEiz/u+A6Boxxr79 9YZQh/qQoqpTwEXSy3tqjIbAwryJD0dloUyX9u0TJo0ZLfG6fTU167cV9o+rWzmG yqSkWKD6EzJ1qfuT6QwK75bbIFR60kw41ngp3Fp+002H+noUVcv2t/NmZLFTz3fQ WI5oQZ8F6p6ruUq5u4fPLkiFEeS6WYfldd19OSnBQoNEJl790NDMMdgTZuLNJQyT o4p9nnFBK4ubjav4CykEKt2QLNmqR35EXlYm4ZXSd+2nmkubNAQUVTajO1Qp2YPN ofQ0vcUdgycRjIBg+Z5d+snqOAnWRuXem9MpYwxMJdO+Jf5hFSIyCJ+UV7eOvIef 3Hzp/APU3VUf7AGkzOJ+N2xrlSh+lLIP2HX2pDg4F/7tAsHpgnEKpofbvzU/sdrz bXtPunw6/yAwsV2OpawP8tGmQ76j0zdzxML0+9oW6In61bhGWC4dx1L8+27bP0ga qL1yLZEkm592VCoJkXgLp02lRS6kMTSyjf/FvqYO3fp35IPXLuc= =eS+9 -----END PGP SIGNATURE----- --mCteW70ZYZCBw0u0--