From 999a6f611a4e145fb42cda51197cf146eb7623f5 Mon Sep 17 00:00:00 2001 From: sashan Date: Tue, 16 Nov 2021 20:51:30 +0000 Subject: [PATCH] move memory allocations in pfr_add_addrs() outside of NET_LOCK()/PF_LOCK() scope. feedback by bluhm@ OK bluhm@ --- sys/net/pf_ioctl.c | 6 +- sys/net/pf_table.c | 260 ++++++++++++++++++++++++++++++++++++++++----- sys/net/pfvar.h | 9 +- 3 files changed, 242 insertions(+), 33 deletions(-) diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c index e960f770a46..d61face1558 100644 --- a/sys/net/pf_ioctl.c +++ b/sys/net/pf_ioctl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf_ioctl.c,v 1.366 2021/11/11 12:35:01 sashan Exp $ */ +/* $OpenBSD: pf_ioctl.c,v 1.367 2021/11/16 20:51:30 sashan Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -2270,13 +2270,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) error = ENODEV; break; } - NET_LOCK(); - PF_LOCK(); error = pfr_add_addrs(&io->pfrio_table, io->pfrio_buffer, io->pfrio_size, &io->pfrio_nadd, io->pfrio_flags | PFR_FLAG_USERIOCTL); - PF_UNLOCK(); - NET_UNLOCK(); break; } diff --git a/sys/net/pf_table.c b/sys/net/pf_table.c index 3a136a8da4f..3f895b4429d 100644 --- a/sys/net/pf_table.c +++ b/sys/net/pf_table.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf_table.c,v 1.137 2021/11/11 12:35:01 sashan Exp $ */ +/* $OpenBSD: pf_table.c,v 1.138 2021/11/16 20:51:31 sashan Exp $ */ /* * Copyright (c) 2002 Cedric Berger @@ -155,8 +155,13 @@ void pfr_enqueue_addrs(struct pfr_ktable *, void pfr_mark_addrs(struct pfr_ktable *); struct pfr_kentry *pfr_lookup_addr(struct pfr_ktable *, struct pfr_addr *, int); +struct pfr_kentry *pfr_lookup_kentry(struct pfr_ktable *, + struct pfr_kentry *, int); struct pfr_kentry *pfr_create_kentry(struct pfr_addr *); +struct pfr_kentry *pfr_create_kentry_unlocked(struct pfr_addr *, int); +void pfr_kentry_kif_ref(struct pfr_kentry *); void pfr_destroy_kentries(struct pfr_kentryworkq *); +void pfr_destroy_ioq(struct pfr_kentryworkq *, int); void pfr_destroy_kentry(struct pfr_kentry *); void pfr_insert_kentries(struct pfr_ktable *, struct pfr_kentryworkq *, time_t); @@ -265,13 +270,50 @@ pfr_clr_addrs(struct pfr_table *tbl, int *ndel, int flags) return (0); } +void +pfr_fill_feedback(struct pfr_kentry_all *ke, struct pfr_addr *ad) +{ + ad->pfra_type = ke->pfrke_type; + + switch (ke->pfrke_type) { + case PFRKE_PLAIN: + break; + case PFRKE_COST: + ((struct pfr_kentry_cost *)ke)->weight = ad->pfra_weight; + /* FALLTHROUGH */ + case PFRKE_ROUTE: + if (ke->pfrke_rifname[0]) + strlcpy(ad->pfra_ifname, ke->pfrke_rifname, IFNAMSIZ); + break; + } + + switch (ke->pfrke_af) { + case AF_INET: + ad->pfra_ip4addr = ke->pfrke_sa.sin.sin_addr; + break; +#ifdef INET6 + case AF_INET6: + ad->pfra_ip6addr = ke->pfrke_sa.sin6.sin6_addr; + break; +#endif /* INET6 */ + default: + unhandled_af(ke->pfrke_af); + } + ad->pfra_weight = ((struct pfr_kentry_cost *)ke)->weight; + ad->pfra_af = ke->pfrke_af; + ad->pfra_net = ke->pfrke_net; + if (ke->pfrke_flags & PFRKE_FLAG_NOT) + ad->pfra_not = 1; + ad->pfra_fback = ke->pfrke_fb; +} + int pfr_add_addrs(struct pfr_table *tbl, struct pfr_addr *addr, int size, int *nadd, int flags) { struct pfr_ktable *kt, *tmpkt; - struct pfr_kentryworkq workq; - struct pfr_kentry *p, *q; + struct pfr_kentryworkq workq, ioq; + struct pfr_kentry *p, *q, *ke; struct pfr_addr ad; int i, rv, xadd = 0; time_t tzero = gettime(); @@ -279,9 +321,6 @@ pfr_add_addrs(struct pfr_table *tbl, struct pfr_addr *addr, int size, ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY | PFR_FLAG_FEEDBACK); if (pfr_validate_table(tbl, 0, flags & PFR_FLAG_USERIOCTL)) return (EINVAL); - kt = pfr_lookup_table(tbl); - if (kt == NULL || !(kt->pfrkt_flags & PFR_TFLAG_ACTIVE)) - return (ESRCH); if (kt->pfrkt_flags & PFR_TFLAG_CONST) return (EPERM); tmpkt = pfr_create_ktable(&pfr_nulltable, 0, 0, @@ -289,53 +328,97 @@ pfr_add_addrs(struct pfr_table *tbl, struct pfr_addr *addr, int size, if (tmpkt == NULL) return (ENOMEM); SLIST_INIT(&workq); + SLIST_INIT(&ioq); for (i = 0; i < size; i++) { YIELD(flags & PFR_FLAG_USERIOCTL); if (COPYIN(addr+i, &ad, sizeof(ad), flags)) senderr(EFAULT); if (pfr_validate_addr(&ad)) senderr(EINVAL); - p = pfr_lookup_addr(kt, &ad, 1); - q = pfr_lookup_addr(tmpkt, &ad, 1); + + ke = pfr_create_kentry_unlocked(&ad, flags); + if (ke == NULL) + senderr(ENOMEM); + ke->pfrke_fb = PFR_FB_NONE; + SLIST_INSERT_HEAD(&ioq, ke, pfrke_ioq); + } + + NET_LOCK(); + PF_LOCK(); + kt = pfr_lookup_table(tbl); + if (kt == NULL || !(kt->pfrkt_flags & PFR_TFLAG_ACTIVE)) { + PF_UNLOCK(); + NET_UNLOCK(); + senderr(ESRCH); + } + SLIST_FOREACH(ke, &ioq, pfrke_ioq) { + pfr_kentry_kif_ref(ke); + p = pfr_lookup_kentry(kt, ke, 1); + q = pfr_lookup_kentry(tmpkt, ke, 1); if (flags & PFR_FLAG_FEEDBACK) { if (q != NULL) - ad.pfra_fback = PFR_FB_DUPLICATE; + ke->pfrke_fb = PFR_FB_DUPLICATE; else if (p == NULL) - ad.pfra_fback = PFR_FB_ADDED; + ke->pfrke_fb = PFR_FB_ADDED; else if ((p->pfrke_flags & PFRKE_FLAG_NOT) != - ad.pfra_not) - ad.pfra_fback = PFR_FB_CONFLICT; + (ke->pfrke_flags & PFRKE_FLAG_NOT)) + ke->pfrke_fb = PFR_FB_CONFLICT; else - ad.pfra_fback = PFR_FB_NONE; + ke->pfrke_fb = PFR_FB_NONE; } if (p == NULL && q == NULL) { - p = pfr_create_kentry(&ad); - if (p == NULL) - senderr(ENOMEM); - if (pfr_route_kentry(tmpkt, p)) { - pfr_destroy_kentry(p); - ad.pfra_fback = PFR_FB_NONE; + if (pfr_route_kentry(tmpkt, ke)) { + /* defer destroy after feedback is processed */ + ke->pfrke_fb = PFR_FB_NONE; } else { - SLIST_INSERT_HEAD(&workq, p, pfrke_workq); + /* + * mark entry as added to table, so we won't + * kill it with rest of the ioq + */ + ke->pfrke_fb = PFR_FB_ADDED; + SLIST_INSERT_HEAD(&workq, ke, pfrke_workq); xadd++; } } - if (flags & PFR_FLAG_FEEDBACK) - if (COPYOUT(&ad, addr+i, sizeof(ad), flags)) - senderr(EFAULT); } + /* remove entries, which we will insert from tmpkt */ pfr_clean_node_mask(tmpkt, &workq); - if (!(flags & PFR_FLAG_DUMMY)) { + if (!(flags & PFR_FLAG_DUMMY)) pfr_insert_kentries(kt, &workq, tzero); + + PF_UNLOCK(); + NET_UNLOCK(); + + if (flags & PFR_FLAG_FEEDBACK) { + i = 0; + while ((ke = SLIST_FIRST(&ioq)) != NULL) { + YIELD(flags & PFR_FLAG_USERIOCTL); + pfr_fill_feedback((struct pfr_kentry_all *)ke, &ad); + if (COPYOUT(&ad, addr+i, sizeof(ad), flags)) + senderr(EFAULT); + i++; + SLIST_REMOVE_HEAD(&ioq, pfrke_ioq); + switch (ke->pfrke_fb) { + case PFR_FB_CONFLICT: + case PFR_FB_DUPLICATE: + case PFR_FB_NONE: + pfr_destroy_kentry(ke); + break; + case PFR_FB_ADDED: + if (flags & PFR_FLAG_DUMMY) + pfr_destroy_kentry(ke); + } + } } else - pfr_destroy_kentries(&workq); + pfr_destroy_ioq(&ioq, flags); + if (nadd != NULL) *nadd = xadd; + pfr_destroy_ktable(tmpkt, 0); return (0); _bad: - pfr_clean_node_mask(tmpkt, &workq); - pfr_destroy_kentries(&workq); + pfr_destroy_ioq(&ioq, flags); if (flags & PFR_FLAG_FEEDBACK) pfr_reset_feedback(addr, size, flags); pfr_destroy_ktable(tmpkt, 0); @@ -822,6 +905,37 @@ pfr_lookup_addr(struct pfr_ktable *kt, struct pfr_addr *ad, int exact) return (ke); } +struct pfr_kentry * +pfr_lookup_kentry(struct pfr_ktable *kt, struct pfr_kentry *key, int exact) +{ + union sockaddr_union mask; + struct radix_node_head *head; + struct pfr_kentry *ke; + + switch (key->pfrke_af) { + case AF_INET: + head = kt->pfrkt_ip4; + break; +#ifdef INET6 + case AF_INET6: + head = kt->pfrkt_ip6; + break; +#endif /* INET6 */ + default: + unhandled_af(key->pfrke_af); + } + if (KENTRY_NETWORK(key)) { + pfr_prepare_network(&mask, key->pfrke_af, key->pfrke_net); + ke = (struct pfr_kentry *)rn_lookup(&key->pfrke_sa, &mask, + head); + } else { + ke = (struct pfr_kentry *)rn_match(&key->pfrke_sa, head); + if (exact && ke && KENTRY_NETWORK(ke)) + ke = NULL; + } + return (ke); +} + struct pfr_kentry * pfr_create_kentry(struct pfr_addr *ad) { @@ -873,6 +987,81 @@ pfr_create_kentry(struct pfr_addr *ad) return ((struct pfr_kentry *)ke); } +struct pfr_kentry * +pfr_create_kentry_unlocked(struct pfr_addr *ad, int flags) +{ + struct pfr_kentry_all *ke; + int mflags = PR_ZERO; + + if (ad->pfra_type >= PFRKE_MAX) + panic("unknown pfra_type %d", ad->pfra_type); + + if (flags & PFR_FLAG_USERIOCTL) + mflags |= PR_WAITOK; + else + mflags |= PR_NOWAIT; + + ke = pool_get(&pfr_kentry_pl[ad->pfra_type], mflags); + if (ke == NULL) + return (NULL); + + ke->pfrke_type = ad->pfra_type; + + /* set weight allowing implicit weights */ + if (ad->pfra_weight == 0) + ad->pfra_weight = 1; + + switch (ke->pfrke_type) { + case PFRKE_PLAIN: + break; + case PFRKE_COST: + ((struct pfr_kentry_cost *)ke)->weight = ad->pfra_weight; + /* FALLTHROUGH */ + case PFRKE_ROUTE: + if (ad->pfra_ifname[0]) + (void) strlcpy(ke->pfrke_rifname, ad->pfra_ifname, + IFNAMSIZ); + break; + } + + switch (ad->pfra_af) { + case AF_INET: + FILLIN_SIN(ke->pfrke_sa.sin, ad->pfra_ip4addr); + break; +#ifdef INET6 + case AF_INET6: + FILLIN_SIN6(ke->pfrke_sa.sin6, ad->pfra_ip6addr); + break; +#endif /* INET6 */ + default: + unhandled_af(ad->pfra_af); + } + ke->pfrke_af = ad->pfra_af; + ke->pfrke_net = ad->pfra_net; + if (ad->pfra_not) + ke->pfrke_flags |= PFRKE_FLAG_NOT; + return ((struct pfr_kentry *)ke); +} + +void +pfr_kentry_kif_ref(struct pfr_kentry *ke_all) +{ + struct pfr_kentry_all *ke = (struct pfr_kentry_all *)ke_all; + + NET_ASSERT_LOCKED(); + switch (ke->pfrke_type) { + case PFRKE_PLAIN: + break; + case PFRKE_COST: + case PFRKE_ROUTE: + if (ke->pfrke_rifname[0]) + ke->pfrke_rkif = pfi_kif_get(ke->pfrke_rifname, NULL); + if (ke->pfrke_rkif) + pfi_kif_ref(ke->pfrke_rkif, PFI_KIF_REF_ROUTE); + break; + } +} + void pfr_destroy_kentries(struct pfr_kentryworkq *workq) { @@ -885,6 +1074,23 @@ pfr_destroy_kentries(struct pfr_kentryworkq *workq) } } +void +pfr_destroy_ioq(struct pfr_kentryworkq *ioq, int flags) +{ + struct pfr_kentry *p; + + while ((p = SLIST_FIRST(ioq)) != NULL) { + YIELD(flags & PFR_FLAG_USERIOCTL); + SLIST_REMOVE_HEAD(ioq, pfrke_ioq); + /* + * we destroy only those entries, which did not make it to + * table + */ + if ((p->pfrke_fb != PFR_FB_ADDED) || (flags & PFR_FLAG_DUMMY)) + pfr_destroy_kentry(p); + } +} + void pfr_destroy_kentry(struct pfr_kentry *ke) { diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index 1baee0f474a..514b9e156f3 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -1,4 +1,4 @@ -/* $OpenBSD: pfvar.h,v 1.503 2021/11/11 12:35:01 sashan Exp $ */ +/* $OpenBSD: pfvar.h,v 1.504 2021/11/16 20:51:31 sashan Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -1043,12 +1043,14 @@ struct _pfr_kentry { struct radix_node _pfrke_node[2]; union pfsockaddr_union _pfrke_sa; SLIST_ENTRY(pfr_kentry) _pfrke_workq; + SLIST_ENTRY(pfr_kentry) _pfrke_ioq; struct pfr_kcounters *_pfrke_counters; time_t _pfrke_tzero; u_int8_t _pfrke_af; u_int8_t _pfrke_net; u_int8_t _pfrke_flags; u_int8_t _pfrke_type; + u_int8_t _pfrke_fb; }; #define PFRKE_FLAG_NOT 0x01 #define PFRKE_FLAG_MARK 0x02 @@ -1064,12 +1066,14 @@ struct pfr_kentry { #define pfrke_node u._ke._pfrke_node #define pfrke_sa u._ke._pfrke_sa #define pfrke_workq u._ke._pfrke_workq +#define pfrke_ioq u._ke._pfrke_ioq #define pfrke_counters u._ke._pfrke_counters #define pfrke_tzero u._ke._pfrke_tzero #define pfrke_af u._ke._pfrke_af #define pfrke_net u._ke._pfrke_net #define pfrke_flags u._ke._pfrke_flags #define pfrke_type u._ke._pfrke_type +#define pfrke_fb u._ke._pfrke_fb struct pfr_kentry_route { union { @@ -1077,6 +1081,7 @@ struct pfr_kentry_route { } u; struct pfi_kif *kif; + char ifname[IFNAMSIZ]; }; struct pfr_kentry_cost { @@ -1085,6 +1090,7 @@ struct pfr_kentry_cost { } u; struct pfi_kif *kif; + char ifname[IFNAMSIZ]; /* Above overlaps with pfr_kentry route */ u_int16_t weight; @@ -1098,6 +1104,7 @@ struct pfr_kentry_all { } u; }; #define pfrke_rkif u.kr.kif +#define pfrke_rifname u.kr.ifname SLIST_HEAD(pfr_ktableworkq, pfr_ktable); RB_HEAD(pfr_ktablehead, pfr_ktable); -- 2.20.1