On Thu, 6 May 2021 11:18:14 +0200
Phil Sutter <phil@xxxxxx> wrote:
> On Thu, May 06, 2021 at 12:23:13AM +0200, Stefano Brivio wrote:
> > As concatenated ranges are fetched from kernel sets and displayed to
> > the user, range_mask_len() evaluates whether the range is suitable for
> > display as netmask, and in that case it calculates the mask length by
> > right-shifting the endpoints until no set bits are left, but in the
> > existing version the temporary copies of the endpoints are derived by
> > copying their unsigned int representation, which doesn't suffice for
> > IPv6 netmask lengths, in general.
> >
> > PetrB reports that, after inserting a /56 subnet in a concatenated set
> > element, it's listed as a /64 range. In fact, this happens for any
> > IPv6 mask shorter than 64 bits.
> >
> > Fix this issue by simply sourcing the range endpoints provided by the
> > caller and setting the temporary copies with mpz_init_set(), instead
> > of fetching the unsigned int representation. The issue only affects
> > displaying of the masks, setting elements already works as expected.
>
> Fixes: 8ac2f3b2fca38 ("src: Add support for concatenated set ranges")
Thanks Phil! I even looked it up and forgot to paste it ;)
> > Reported-by: PetrB <petr.boltik@xxxxxxxxx>
> > Bugzilla: https://bugzilla.netfilter.org/show_bug.cgi?id=1520
> > Signed-off-by: Stefano Brivio <sbrivio@xxxxxxxxxx>
> > ---
> > src/segtree.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/segtree.c b/src/segtree.c
> > index ad199355532e..353a0053ebc0 100644
> > --- a/src/segtree.c
> > +++ b/src/segtree.c
> > @@ -838,8 +838,8 @@ static int range_mask_len(const mpz_t start, const mpz_t end, unsigned int len)
> > mpz_t tmp_start, tmp_end;
> > int ret;
> >
> > - mpz_init_set_ui(tmp_start, mpz_get_ui(start));
> > - mpz_init_set_ui(tmp_end, mpz_get_ui(end));
> > + mpz_init_set(tmp_start, start);
> > + mpz_init_set(tmp_end, end);
>
> The old code is a bit funny, was there a specific reason why you
> exported the values into a C variable intermediately?
Laziness, ultimately: I didn't remember the name of gmp_printf(),
didn't look it up, and used a fprintf() instead to check 'start' and
'end'... and then whoops, I left the mpz_get_ui() calls there.
--
Stefano
