On Fri, Apr 10, 2026 at 11:20:34AM +0200, Otto Moerbeek wrote:
> From: Otto Moerbeek <[email protected]>
> To: Alexandr Nedvedicky <[email protected]>
> Cc: Renaud Allard <[email protected]>, [email protected]
> Subject: Re: pf: broken RB comparator drops IPv6 fragments
> Date: Fri, 10 Apr 2026 11:20:34 +0200
>
> On Fri, Apr 10, 2026 at 10:41:32AM +0200, Alexandr Nedvedicky wrote:
>
> > Hello,
> >
> > On Fri, Apr 10, 2026 at 08:56:33AM +0200, Renaud Allard wrote:
> > > pf_frag_compare() in sys/net/pf_norm.c uses subtraction to compare
> > > u_int32_t fragment IDs:
> > >
> > > if ((diff = a->fr_id - b->fr_id) != 0)
> > > return (diff);
> > >
> >
> > </snip>
> >
> > I wonder if diff below would work too here...
>
> Maybe, but it's hard to reason about that as it requires intimate
> knowledge of the (implicit) type conversion rules. I much prefer
> writing it out in a clear way as Renaud did. See also the comment in
> qsort(3).
>
> -Otto
>
The subtraction a->fr_id - b->fr_id would still done in u_int32_t
even if diff is int64_t as far as I know or you will have to cast
them first.
> >
> > thanks and
> > regards
> > sashan
> >
> > --------8<---------------8<---------------8<------------------8<--------
> > diff --git a/sys/net/pf_norm.c b/sys/net/pf_norm.c
> > index 93b98db1d22..f7daa8a0c41 100644
> > --- a/sys/net/pf_norm.c
> > +++ b/sys/net/pf_norm.c
> > @@ -112,7 +112,7 @@ RB_HEAD(pf_frnode_tree, pf_frnode) pf_frnode_tree;
> > RB_PROTOTYPE(pf_frnode_tree, pf_frnode, fn_entry, pf_frnode_compare);
> > RB_GENERATE(pf_frnode_tree, pf_frnode, fn_entry, pf_frnode_compare);
> >
> > -static __inline int pf_frag_compare(struct pf_fragment *,
> > +static __inline int64_t pf_frag_compare(struct pf_fragment *,
> > struct pf_fragment *);
> > RB_PROTOTYPE(pf_frag_tree, pf_fragment, fr_entry, pf_frag_compare);
> > RB_GENERATE(pf_frag_tree, pf_fragment, fr_entry, pf_frag_compare);
> > @@ -184,10 +184,10 @@ pf_frnode_compare(struct pf_frnode *a, struct
> > pf_frnode *b)
> > return (0);
> > }
> >
> > -static __inline int
> > +static __inline int64_t
> > pf_frag_compare(struct pf_fragment *a, struct pf_fragment *b)
> > {
> > - int diff;
> > + int64_t diff;
> >
> > if ((diff = a->fr_id - b->fr_id) != 0)
> > return (diff);
> >
