I want to use this code also in bgpctl (like util.c) and since bgpctl
has no fatalx() and "library" code should not abort.

The comparison function can not return an error so instead sort invalid
objects in a deterministic way. flowspec_cmp() should only be called on
flowspec NLRI that have previously passed flowspec_valid(). Doing that is
enough to ensure that neither extract_prefix() nor
flowspec_next_component() fail.

The overflow check in flowspec_valid() is mostly an assert stile check but
it is totally fine to return an error there.
-- 
:wq Claudio

Index: flowspec.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/flowspec.c,v
retrieving revision 1.3
diff -u -p -r1.3 flowspec.c
--- flowspec.c  19 Apr 2023 07:07:58 -0000      1.3
+++ flowspec.c  19 Apr 2023 08:53:34 -0000
@@ -105,10 +105,8 @@ flowspec_cmp_prefix4(const uint8_t *abuf
        clen = MINIMUM(alen, blen);
 
        /* only extract the common prefix */
-       if (extract_prefix(abuf + 2, ablen - 2, &a, clen, sizeof(a)) == -1)
-               fatalx("bad flowspec prefix encoding");
-       if (extract_prefix(bbuf + 2, bblen - 2, &b, clen, sizeof(b)) == -1)
-               fatalx("bad flowspec prefix encoding");
+       extract_prefix(abuf + 2, ablen - 2, &a, clen, sizeof(a));
+       extract_prefix(bbuf + 2, bblen - 2, &b, clen, sizeof(b));
 
        /* lowest IP value has precedence */
        cmp = memcmp(a, b, sizeof(a));
@@ -149,10 +147,8 @@ flowspec_cmp_prefix6(const uint8_t *abuf
        clen = MINIMUM(alen, blen);
 
        /* only extract the common prefix */
-       if (extract_prefix(abuf + 3, ablen - 3, &a, clen, sizeof(a)) == -1)
-               fatalx("bad flowspec prefix encoding");
-       if (extract_prefix(bbuf + 3, bblen - 3, &b, clen, sizeof(b)) == -1)
-               fatalx("bad flowspec prefix encoding");
+       extract_prefix(abuf + 3, ablen - 3, &a, clen, sizeof(a));
+       extract_prefix(bbuf + 3, bblen - 3, &b, clen, sizeof(b));
 
        /* lowest IP value has precedence */
        cmp = memcmp(a, b, sizeof(a));
@@ -193,7 +189,7 @@ flowspec_valid(const uint8_t *buf, int l
                len -= l;
        }
        if (len < 0)
-               fatalx("flowspec overflow");
+               return -1;
        return 0;
 }
 
@@ -211,11 +207,12 @@ flowspec_cmp(const uint8_t *a, int alen,
 
        while (alen > 0 && blen > 0) {
                acomplen = flowspec_next_component(a, alen, is_v6, &atype);
-               if (acomplen == -1)
-                       fatalx("bad flowspec component");
                bcomplen = flowspec_next_component(b, blen, is_v6, &btype);
+               /* should not happen */
                if (acomplen == -1)
-                       fatalx("bad flowspec component");
+                       return 1;
+               if (bcomplen == -1)
+                       return -1;
 
                /* If types differ, lowest type wins. */
                if (atype < btype)

Reply via email to