On Tue, Mar 28, 2023 at 06:49:38PM +0200, Claudio Jeker wrote:
> I almost stepped into this trap and tried to pt_ref the static memory
> returned by pt_fill(). That wont work so better make the code explode.
> By setting the refcnt to USHRT_MAX a following pr_ref() call will fail.
> Since pt_alloc copies the passed data structure reset the refcnt to 0 there.
>
> I think this is a reasonable change to protect from silly but hard to spot
> mistakes.
Yes. That absolutely makes sense.
ok tb
> --
> :wq Claudio
>
> Index: rde_prefix.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde_prefix.c,v
> retrieving revision 1.45
> diff -u -p -r1.45 rde_prefix.c
> --- rde_prefix.c 28 Mar 2023 15:17:34 - 1.45
> +++ rde_prefix.c 28 Mar 2023 16:36:33 -
> @@ -21,6 +21,7 @@
>
> #include
> #include
> +#include
> #include
> #include
>
> @@ -162,6 +163,7 @@ pt_fill(struct bgpd_addr *prefix, int pr
> switch (prefix->aid) {
> case AID_INET:
> memset(, 0, sizeof(pte4));
> + pte4.refcnt = USHRT_MAX;
> pte4.aid = prefix->aid;
> if (prefixlen > 32)
> fatalx("pt_fill: bad IPv4 prefixlen");
> @@ -170,6 +172,7 @@ pt_fill(struct bgpd_addr *prefix, int pr
> return ((struct pt_entry *));
> case AID_INET6:
> memset(, 0, sizeof(pte6));
> + pte6.refcnt = USHRT_MAX;
> pte6.aid = prefix->aid;
> if (prefixlen > 128)
> fatalx("pt_fill: bad IPv6 prefixlen");
> @@ -178,6 +181,7 @@ pt_fill(struct bgpd_addr *prefix, int pr
> return ((struct pt_entry *));
> case AID_VPN_IPv4:
> memset(_vpn4, 0, sizeof(pte_vpn4));
> + pte_vpn4.refcnt = USHRT_MAX;
> pte_vpn4.aid = prefix->aid;
> if (prefixlen > 32)
> fatalx("pt_fill: bad IPv4 prefixlen");
> @@ -190,6 +194,7 @@ pt_fill(struct bgpd_addr *prefix, int pr
> return ((struct pt_entry *)_vpn4);
> case AID_VPN_IPv6:
> memset(_vpn6, 0, sizeof(pte_vpn6));
> + pte_vpn6.refcnt = USHRT_MAX;
> pte_vpn6.aid = prefix->aid;
> if (prefixlen > 128)
> fatalx("pt_get: bad IPv6 prefixlen");
> @@ -360,6 +365,7 @@ pt_alloc(struct pt_entry *op)
> rdemem.pt_cnt[op->aid]++;
> rdemem.pt_size[op->aid] += pt_sizes[op->aid];
> memcpy(p, op, pt_sizes[op->aid]);
> + p->refcnt = 0;
>
> return (p);
> }
>
