From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: ifa_load() pfctl parser should adhere interface flags
To: Alexandr Nedvedicky <sashan@fastmail.net>
Cc: tech@openbsd.org
Date: Sun, 30 Nov 2025 13:12:06 +0100

On Sun, Nov 30, 2025 at 11:57:57AM +0100, Alexandr Nedvedicky wrote:
> Hello,
> 
> ping. any objections here? any OKs?

OK claudio@
 
> thanks and
> regards
> sashan
> 
> On Tue, Nov 25, 2025 at 09:24:53AM +0100, Alexandr Nedvedicky wrote:
> > Hello,
> > 
> > the issue has been pointed by claudio@ off-list.
> > 
> > the ifa_load9() function does not distinct between
> > broadcast address and peer address when it processes
> > interface item (`ifa`) obtained by getifaddrs(3) from
> > kernel.
> > 
> > as I understand it the IFF_BROADCAST and IFF_POINTOPOINT
> > flags are mutually exclusive so ifa_load() should use
> > the address either as brodacst or as peer (when dealing
> > with ppp interface).
> > 
> > the change makes code more correct. I could not spot
> > any change on pfctl behavior.
> > 
> > this is interface list on machine where I test the change;
> </snip>
> > 
> > --------8<---------------8<---------------8<------------------8<--------
> > diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
> > index 3a7cc6885b3..eb29edb900a 100644
> > --- a/sbin/pfctl/pfctl_parser.c
> > +++ b/sbin/pfctl/pfctl_parser.c
> > @@ -1459,13 +1459,14 @@ ifa_load(void)
> >  			copy_satopfaddr(&n->addr.v.a.addr, ifa->ifa_addr);
> >  			ifa->ifa_netmask->sa_family = ifa->ifa_addr->sa_family;
> >  			copy_satopfaddr(&n->addr.v.a.mask, ifa->ifa_netmask);
> > -			if (ifa->ifa_broadaddr != NULL &&
> > +			if (ifa->ifa_flags & IFF_BROADCAST &&
> > +			    ifa->ifa_broadaddr != NULL &&
> >  			    ifa->ifa_broadaddr->sa_len != 0) {
> >  				ifa->ifa_broadaddr->sa_family =
> >  				    ifa->ifa_addr->sa_family;
> >  				copy_satopfaddr(&n->bcast, ifa->ifa_broadaddr);
> > -			}
> > -			if (ifa->ifa_dstaddr != NULL &&
> > +			} else if (ifa->ifa_flags & IFF_POINTOPOINT &&
> > +			    ifa->ifa_dstaddr != NULL &&
> >  			    ifa->ifa_dstaddr->sa_len != 0) {
> >  				ifa->ifa_dstaddr->sa_family =
> >  				    ifa->ifa_addr->sa_family;
> > 
> 

-- 
:wq Claudio