Index | Thread | Search

From:
Denis Fondras <denis@openbsd.org>
Subject:
Re: bgpd: EVPN, don't overflow vni in log_evpnaddr
To:
tech@openbsd.org
Cc:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Date:
Thu, 14 May 2026 10:11:20 +0200

Download raw body.

Thread
Le Wed, May 13, 2026 at 04:59:35PM +0200, Claudio Jeker a écrit :
> In log_evpnaddr() the labellen for EVPN_ROUTE_TYPE_2 can either be 3 or 6.
> Fitting 6 bytes into a uint32_t is tight.
> 
> I think this code just wants to print the first label (which always
> exists).
> 
> Also I dislike this memcpy, lets just use the old school shift and or
> method. We can certainly keep the memcpy but then with a fixed length of
> 3.
> 

No regress so far :)
OK denis@

> -- 
> :wq Claudio
> 
> Index: util.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
> diff -u -p -r1.101 util.c
> --- util.c	8 May 2026 12:03:50 -0000	1.101
> +++ util.c	13 May 2026 14:34:05 -0000
> @@ -95,9 +95,11 @@ log_evpnaddr(const struct bgpd_addr *add
>  
>  	switch (addr->evpn.type) {
>  	case EVPN_ROUTE_TYPE_2:
> -		memcpy(&vni, addr->labelstack, addr->labellen);
> +		vni = addr->labelstack[0];
> +		vni = vni << 8 | addr->labelstack[1];
> +		vni = vni << 8 | addr->labelstack[2];
>  		snprintf(buf, sizeof(buf), "[2]:[%s]:[%s]:[%d]:[48]:[%s]",
> -		    log_rd(addr->rd), log_esi(addr->evpn.esi), htonl(vni) >> 8,
> +		    log_rd(addr->rd), log_esi(addr->evpn.esi), vni,
>  		    log_mac(addr->evpn.mac));
>  		if (sa != NULL) {
>  			len = strlen(buf);
> @@ -108,7 +110,6 @@ log_evpnaddr(const struct bgpd_addr *add
>  		break;
>  	case EVPN_ROUTE_TYPE_3:
>  		if (sa != NULL) {
> -			memcpy(&vni, addr->labelstack, addr->labellen);
>  			snprintf(buf, sizeof(buf), "[3]:[%s]:[%d]:[%s]",
>  			    log_rd(addr->rd),
>  			    sa->sa_family == AF_INET ? 32 : 128,