Remove net lock from DIOC{S,G}ETLIMIT

Grab the pf lock for pf_pool_limits[] in pfsync such that all access is
covered by the pf lock;  document accordingly.

Hard memory pool limits don't need the net lock for protection, pool(9)s
have their own internal lock and the pf lock fully covers limit values.

(pf_pool_limits[] access in DIOCXCOMMIT remains under pf *and net* lock
 until the rest in there gets pulled out of the net lock.)

OK sashan

diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
index 71ca9f2..2bf9330 100644 (file)
--- a/sys/net/if_pfsync.c
+++ b/sys/net/if_pfsync.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_pfsync.c,v 1.315 2023/05/18 12:10:04 sashan Exp $  */
+/*     $OpenBSD: if_pfsync.c,v 1.316 2023/05/26 12:13:26 kn Exp $      */
 
 /*
  * Copyright (c) 2002 Michael Shalayeff
@@ -1009,10 +1009,12 @@ pfsync_in_bus(caddr_t buf, int len, int count, int flags)
 
        switch (bus->status) {
        case PFSYNC_BUS_START:
+               PF_LOCK();
                timeout_add(&sc->sc_bulkfail_tmo, 4 * hz +
                    pf_pool_limits[PF_LIMIT_STATES].limit /
                    ((sc->sc_if.if_mtu - PFSYNC_MINPKT) /
                    sizeof(struct pfsync_state)));
+               PF_UNLOCK();
                DPFPRINTF(LOG_INFO, "received bulk update start");
                break;
 
@@ -2037,10 +2039,12 @@ pfsync_request_full_update(struct pfsync_softc *sc)
 #endif
                pfsync_sync_ok = 0;
                DPFPRINTF(LOG_INFO, "requesting bulk update");
+               PF_LOCK();
                timeout_add(&sc->sc_bulkfail_tmo, 4 * hz +
                    pf_pool_limits[PF_LIMIT_STATES].limit /
                    ((sc->sc_if.if_mtu - PFSYNC_MINPKT) /
                    sizeof(struct pfsync_state)));
+               PF_UNLOCK();
                pfsync_request_update(0, 0);
        }
 }

diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index f70796c..e5b930a 100644 (file)
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pf_ioctl.c,v 1.404 2023/05/11 12:36:22 kn Exp $ */
+/*     $OpenBSD: pf_ioctl.c,v 1.405 2023/05/26 12:13:26 kn Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -2094,44 +2094,37 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
                        error = EINVAL;
                        goto fail;
                }
-               NET_LOCK();
                PF_LOCK();
                pl->limit = pf_pool_limits[pl->index].limit;
                PF_UNLOCK();
-               NET_UNLOCK();
                break;
        }
 
        case DIOCSETLIMIT: {
                struct pfioc_limit      *pl = (struct pfioc_limit *)addr;
 
-               NET_LOCK();
                PF_LOCK();
                if (pl->index < 0 || pl->index >= PF_LIMIT_MAX) {
                        error = EINVAL;
                        PF_UNLOCK();
-                       NET_UNLOCK();
                        goto fail;
                }
                if (((struct pool *)pf_pool_limits[pl->index].pp)->pr_nout >
                    pl->limit) {
                        error = EBUSY;
                        PF_UNLOCK();
-                       NET_UNLOCK();
                        goto fail;
                }
                /* Fragments reference mbuf clusters. */
                if (pl->index == PF_LIMIT_FRAGS && pl->limit > nmbclust) {
                        error = EINVAL;
                        PF_UNLOCK();
-                       NET_UNLOCK();
                        goto fail;
                }
 
                pf_pool_limits[pl->index].limit_new = pl->limit;
                pl->limit = pf_pool_limits[pl->index].limit;
                PF_UNLOCK();
-               NET_UNLOCK();
                break;
        }
 

diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 2e808e1..c37b5f8 100644 (file)
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pfvar.h,v 1.530 2023/04/28 14:08:38 sashan Exp $ */
+/*     $OpenBSD: pfvar.h,v 1.531 2023/05/26 12:13:26 kn Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1795,10 +1795,16 @@ const struct pfq_ops
 extern struct pf_status        pf_status;
 extern struct pool     pf_frent_pl, pf_frag_pl;
 
+/*
+ * Protection/ownership of pf_pool_limit:
+ *     I       immutable after pfattach()
+ *     p       pf_lock
+ */
+
 struct pf_pool_limit {
-       void            *pp;
-       unsigned         limit;
-       unsigned         limit_new;
+       void            *pp;            /* [I] */
+       unsigned         limit;         /* [p] */
+       unsigned         limit_new;     /* [p] */
 };
 extern struct pf_pool_limit    pf_pool_limits[PF_LIMIT_MAX];