Index | Thread | Search

From:
"Theo de Raadt" <deraadt@openbsd.org>
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

Download raw body.

Thread
Sensible.

Claudio Jeker <cjeker@diehard.n-r-g.com> 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++];
>