From: "Theo de Raadt" Subject: Re: bgpd: use unsigned int instead of uint8_t as loop iterator var To: tech@openbsd.org Date: Thu, 07 May 2026 13:22:16 -0600 Sensible. Claudio Jeker wrote: > I like to move away from using size restricted variables for loop indexes > and length variables. > > Too often the limited range cause overflows in unexpeced ways. It is > better to use a default int type instead. > > This converts the uint8_t length vars for iterating over the others > attribute array to unsigned int. I also added early breaks in the loop in > two places since after the first NULL all values will be NULL as well. > > -- > :wq Claudio > > Index: mrt.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/mrt.c,v > diff -u -p -r1.134 mrt.c > --- mrt.c 17 Feb 2026 14:06:44 -0000 1.134 > +++ mrt.c 7 May 2026 18:48:22 -0000 > @@ -222,9 +222,10 @@ mrt_attr_dump(struct ibuf *buf, struct r > struct attr *oa; > u_char *pdata; > uint32_t tmp; > + unsigned int l; > int neednewpath = 0; > uint16_t plen, afi; > - uint8_t l, safi; > + uint8_t safi; > > /* origin */ > if (attr_writebuf(buf, ATTR_WELL_KNOWN, ATTR_ORIGIN, > Index: rde.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v > diff -u -p -r1.696 rde.c > --- rde.c 7 May 2026 09:42:26 -0000 1.696 > +++ rde.c 7 May 2026 18:47:04 -0000 > @@ -2923,7 +2923,7 @@ rde_dump_rib_as(struct prefix *p, struct > struct rde_peer *peer; > monotime_t staletime; > size_t aslen; > - uint8_t l; > + unsigned int l; > > nexthop = prefix_nexthop(p); > peer = prefix_peer(p); > @@ -3041,7 +3041,7 @@ rde_dump_adjout_as(struct rde_peer *peer > struct rde_aspath *asp; > struct nexthop *nexthop; > size_t aslen; > - uint8_t l; > + unsigned int l; > > nexthop = attrs->nexthop; > asp = attrs->aspath; > Index: rde_attr.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/rde_attr.c,v > diff -u -p -r1.143 rde_attr.c > --- rde_attr.c 5 May 2026 08:37:45 -0000 1.143 > +++ rde_attr.c 7 May 2026 18:52:37 -0000 > @@ -152,7 +152,7 @@ attr_optadd(struct rde_aspath *asp, uint > struct attr * > attr_optget(const struct rde_aspath *asp, uint8_t type) > { > - uint8_t l; > + unsigned int l; > > for (l = 0; l < asp->others_len; l++) { > if (asp->others[l] == NULL) > @@ -168,7 +168,7 @@ attr_optget(const struct rde_aspath *asp > void > attr_copy(struct rde_aspath *t, const struct rde_aspath *s) > { > - uint8_t l; > + unsigned int l; > > if (t->others != NULL) > attr_freeall(t); > @@ -208,22 +208,25 @@ attr_eq(const struct attr *oa, const str > int > attr_equal(const struct rde_aspath *a, const struct rde_aspath *b) > { > - uint8_t l; > + unsigned int l; > > if (a->others_len != b->others_len) > return (0); > - for (l = 0; l < a->others_len; l++) > + for (l = 0; l < a->others_len; l++) { > if (a->others[l] != b->others[l]) > return (0); > + if (a->others[l] == NULL) > + break; > + } > return (1); > } > > void > attr_free(struct rde_aspath *asp, struct attr *attr) > { > - uint8_t l; > + unsigned int l; > > - for (l = 0; l < asp->others_len; l++) > + for (l = 0; l < asp->others_len; l++) { > if (asp->others[l] == attr) { > attr_put(asp->others[l]); > for (++l; l < asp->others_len; l++) > @@ -231,6 +234,9 @@ attr_free(struct rde_aspath *asp, struct > asp->others[asp->others_len - 1] = NULL; > return; > } > + if (asp->others[l] == NULL) > + break; > + } > > /* no realloc() because the slot may be reused soon */ > } > @@ -238,7 +244,7 @@ attr_free(struct rde_aspath *asp, struct > void > attr_freeall(struct rde_aspath *asp) > { > - uint8_t l; > + unsigned int l; > > for (l = 0; l < asp->others_len; l++) > attr_put(asp->others[l]); > Index: rde_update.c > =================================================================== > RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v > diff -u -p -r1.193 rde_update.c > --- rde_update.c 11 Feb 2026 10:24:57 -0000 1.193 > +++ rde_update.c 7 May 2026 18:49:19 -0000 > @@ -591,9 +591,10 @@ up_generate_attr(struct ibuf *buf, struc > struct attr *oa = NULL, *newaggr = NULL; > u_char *pdata; > uint32_t tmp32; > + unsigned int oalen = 0; > int flags, neednewpath = 0, rv; > uint16_t plen; > - uint8_t oalen = 0, type; > + uint8_t type; > > if (asp->others_len > 0) > oa = asp->others[oalen++]; >