On Tue, Dec 29, 2020 at 05:01:27PM +0100, Florian Westphal wrote:
> Visa Hankala <v...@hankala.org> wrote:
> > Use three-way comparison for address elements to avoid integer
> > wraparound in the result of xfrm_policy_addr_delta().
> >
> > This ensures that the search trees are built and traversed correctly
> > when the difference between compared address elements is larger
> > than INT_MAX.
>
> Please provide an update to tools/testing/selftests/net/xfrm_policy.sh
> that shows that this is a problem.
I will do that in the next revision.
> > switch (family) {
> > case AF_INET:
> > - if (sizeof(long) == 4 && prefixlen == 0)
> > - return ntohl(a->a4) - ntohl(b->a4);
> > - return (ntohl(a->a4) & ((~0UL << (32 - prefixlen)))) -
> > - (ntohl(b->a4) & ((~0UL << (32 - prefixlen))));
> > + mask = ~0U << (32 - prefixlen);
> > + ma = ntohl(a->a4) & mask;
> > + mb = ntohl(b->a4) & mask;
>
> This is suspicious. Is prefixlen == 0 impossible?
>
> If not, then after patch
> mask = ~0U << 32;
>
> ... and function returns 0.
prefixlen == 0 is possible. However, I now realize that left shift
by 32 should be avoided with 32-bit integers.
With prefixlen == 0, there is only one equivalence class, so
returning 0 seems reasonable to me.
Is there a reason why the function has treated /0 prefix as /32
with IPv4? IPv6 does not have this treatment.
