pf: add anchor support for ether rules

Support anchors in ether rules.

Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D32482

pf: Support clearing ether counters

Allow the evaluations/packets/bytes counters on Ethernet rules to be
cleared.

Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https:

pf: Support clearing ether counters

Allow the evaluations/packets/bytes counters on Ethernet rules to be
cleared.

Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31748

show more ...

pf: Only hook the Ethernet pfil hook when we have rules

Avoid the overhead of the Ethernet pfil hooks if we don't have any
Ethernet rules.

Sponsored by: Rubicon Communications, LLC ("Netgate")
Diff

pf: Only hook the Ethernet pfil hook when we have rules

Avoid the overhead of the Ethernet pfil hooks if we don't have any
Ethernet rules.

Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31742

show more ...

pf: Do not hold PF_RULES_RLOCK while processing Ethernet rules

Avoid the overhead of acquiring a (read) RULES lock when processing the
Ethernet rules.
We can get away with that because when rules ar

pf: Do not hold PF_RULES_RLOCK while processing Ethernet rules

Avoid the overhead of acquiring a (read) RULES lock when processing the
Ethernet rules.
We can get away with that because when rules are modified they're staged
in V_pf_keth_inactive. We take care to ensure the swap to V_pf_keth is
atomic, so that pf_test_eth_rule() always sees either the old rules, or
the new ruleset.

We need to take care not to delete the old ruleset until we're sure no
pf_test_eth_rule() is still running with those. We accomplish that by
using NET_EPOCH_CALL() to actually free the old rules.

Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31739

show more ...

pf: Initial Ethernet level filtering code

This is the kernel side of stateless Ethernel level filtering for pf.

The primary use case for this is to enable captive portal functionality
to allow/deny

pf: Initial Ethernet level filtering code

This is the kernel side of stateless Ethernel level filtering for pf.

The primary use case for this is to enable captive portal functionality
to allow/deny access by MAC address, rather than per IP address.

Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31737

show more ...

pf: Initialize pf_kpool mutexes earlier

There are some error paths in ioctl handlers that will call
pf_krule_free() before the rule's rpool.mtx field is initialized,
causing a panic with INVARIANTS

pf: Initialize pf_kpool mutexes earlier

There are some error paths in ioctl handlers that will call
pf_krule_free() before the rule's rpool.mtx field is initialized,
causing a panic with INVARIANTS enabled.

Fix the problem by introducing pf_krule_alloc() and initializing the
mutex there. This does mean that the rule->krule and pool->kpool
conversion functions need to stop zeroing the input structure, but I
don't see a nicer way to handle this except perhaps by guarding the
mtx_destroy() with a mtx_initialized() check.

Constify some related functions while here and add a regression test
based on a syzkaller reproducer.

Reported by: syzbot+77cd12872691d219c158@syzkaller.appspotmail.com
Reviewed by: kp
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D34115

show more ...

pf: ensure we don't destroy an uninitialised lock

The new lock introduced in 5f5e32f1b3 needs to be initialised early so
that it can be safely destroyed if we error out.

Reported-by: syzbot+d76113e

pf: ensure we don't destroy an uninitialised lock

The new lock introduced in 5f5e32f1b3 needs to be initialised early so
that it can be safely destroyed if we error out.

Reported-by: syzbot+d76113e9a4ae0c0fcac2@syzkaller.appspotmail.com
MFC after: 3 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")

show more ...

pf: protect the rpool from races

The roundrobin pool stores its state in the rule, which could
potentially lead to invalid addresses being returned.

For example, thread A just executed PF_AINC(&rpo

pf: protect the rpool from races

The roundrobin pool stores its state in the rule, which could
potentially lead to invalid addresses being returned.

For example, thread A just executed PF_AINC(&rpool->counter) and
immediately afterwards thread B executes PF_ACPY(naddr, &rpool->counter)
(i.e. after the pf_match_addr() check of rpool->counter).

Lock the rpool with its own mutex to prevent these races. The
performance impact of this is expected to be low, as each rule has its
own lock, and the lock is also only relevant when state is being created
(so only for the initial packets of a connection, not for all traffic).

See also: https://redmine.pfsense.org/issues/12660
Reviewed by: glebius
MFC after: 3 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D33874

show more ...

pf: add COMPAT_FREEBSD13 for DIOCKEEPCOUNTERS

DIOCKEEPCOUNTERS used to overlap with DIOCGIFSPEEDV0, which has been
fixed in 14, but remains in stable/12 and stable/13.
Support the old, overlapping,

pf: add COMPAT_FREEBSD13 for DIOCKEEPCOUNTERS

DIOCKEEPCOUNTERS used to overlap with DIOCGIFSPEEDV0, which has been
fixed in 14, but remains in stable/12 and stable/13.
Support the old, overlapping, call under COMPAT_FREEBSD13.

Reviewed by: jhb
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D33001

show more ...

pf: remove the flags argument from pf_unlink_state

All consumers call it with PF_ENTER_LOCKED.

Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")

pf: fix use-after-free from pf_find_state_all

state was returned without any locks nor references held

Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")

pf: fix some cc --analyze warnings

Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")

pf: do not copy anchor_wildcard / anchor_relative from userspace

We overwrite these fields again in pf_kanchor_setup() anyway.

MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate

pf: do not copy anchor_wildcard / anchor_relative from userspace

We overwrite these fields again in pf_kanchor_setup() anyway.

MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")

show more ...

pf: fix pagefault in pf_getstatus()

We can't copyout() while holding a lock, in case it triggers a page
fault.
Release the lock before copyout, which is safe because we've already
copied all the dat

pf: fix pagefault in pf_getstatus()

We can't copyout() while holding a lock, in case it triggers a page
fault.
Release the lock before copyout, which is safe because we've already
copied all the data into the nvlist.

PR: 258601
Reviewed by: mjg
MFC after: 1 week
Sponsored by: Modirum MDPay
Differential Revision: https://reviews.freebsd.org/D32076

show more ...

pf: Remove duplicate declaration of pf_ioctl_maxcount.

Fixes a -Wredundant-decls warning with GCC 9.

Reviewed by: kp
Differential Revision: https://reviews.freebsd.org/D31944

pf: qid and pqid can be uint16_t

tag2name() returns a uint16_t, so we don't need to use uint32_t for the
qid (or pqid). This reduces the size of struct pf_kstate slightly. That
in turn buys us space

pf: qid and pqid can be uint16_t

tag2name() returns a uint16_t, so we don't need to use uint32_t for the
qid (or pqid). This reduces the size of struct pf_kstate slightly. That
in turn buys us space to add extra fields for dummynet later.

Happily these fields are not exposed to user space (there are user space
versions of them, but they can just stay uint32_t), so there's no ABI
breakage in modifying this.

MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31873

show more ...

pf: Add counters for syncookies

Count when we send a syncookie, receive a valid syncookie or detect a
synflood.

Reviewed by: kbowling
MFC after: 1 week
Sponsored by: Modirum MDPay
Differential Revi

pf: Add counters for syncookies

Count when we send a syncookie, receive a valid syncookie or detect a
synflood.

Reviewed by: kbowling
MFC after: 1 week
Sponsored by: Modirum MDPay
Differential Revision: https://reviews.freebsd.org/D31713

show more ...

pf: Introduce nvlist variant of DIOCGETSTATUS

Make it possible to extend the GETSTATUS call (e.g. when we want to add
new counters, such as for syncookie support) by introducing an
nvlist-based alte

pf: Introduce nvlist variant of DIOCGETSTATUS

Make it possible to extend the GETSTATUS call (e.g. when we want to add
new counters, such as for syncookie support) by introducing an
nvlist-based alternative.

MFC after: 1 week
Sponsored by: Modirum MDPay
Differential Revision: https://reviews.freebsd.org/D31694

show more ...

pf: bound DIOCGETSTATES memory use

Similar to what we did earlier for DIOCGETSTATESV2 we only allocate
enough memory for a handful of states and copy those out, bit by bit,
rather than allocating me

pf: bound DIOCGETSTATES memory use

Similar to what we did earlier for DIOCGETSTATESV2 we only allocate
enough memory for a handful of states and copy those out, bit by bit,
rather than allocating memory for all states in one go.

MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")

show more ...

pf: remove DIOCGETSTATESNV

While nvlists are very useful in maximising flexibility for future
extensions their performance is simply unacceptably bad for the
getstates feature, where we can easily w

pf: remove DIOCGETSTATESNV

While nvlists are very useful in maximising flexibility for future
extensions their performance is simply unacceptably bad for the
getstates feature, where we can easily want to export a million states
or more.

The DIOCGETSTATESNV call has been MFCd, but has not hit a release on any
branch, so we can still remove it everywhere.

Reviewed by: mjg
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31099

show more ...

pf: Validate user string nul-termination before copying

Some pf ioctl handlers use strlcpy() to copy strings when converting
from user structures to their in-kernel representations. strlcpy()
ensur

pf: Validate user string nul-termination before copying

Some pf ioctl handlers use strlcpy() to copy strings when converting
from user structures to their in-kernel representations. strlcpy()
ensures that the destination will be nul-terminated, but it assumes that
the source is nul-terminated. In particular, it returns the full length
of the source string, so if the source is not nul-terminated, strlcpy()
will keep scanning until it finds a nul byte, and it may encounter an
unmapped page first. Add a helper to validate user strings before
copying.

There are also places where we look up a ruleset using a user-provided
anchor string. In some ioctl handlers we were already nul-terminating
the string, avoiding the same problem, but in other places we were not.
Fix those by nul-terminating as well. Aside from being consistent,
anchors have a maximum length of MAXPATHLEN - 1 so calling strnlen()
might not be so desirable.

Reported by: syzbot+35a1549b4663e9483dd1@syzkaller.appspotmail.com
Reviewed by: kp
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D31169

show more ...

pf: Initialize arrays before copying out to userland

A number of pf ioctls populate an array of structures and copy it out.
They have the following structures:
- caller specifies the size of its out

pf: Initialize arrays before copying out to userland

A number of pf ioctls populate an array of structures and copy it out.
They have the following structures:
- caller specifies the size of its output buffer
- ioctl handler allocates a kernel buffer of the same size
- ioctl handler populates the buffer, possibly leaving some items
initialized if the caller provided more space than needed
- ioctl handler copies the entire buffer out to userland

Thus, if more space was provided than is required, we end up copying out
uninitialized kernel memory. Simply zero the buffer at allocation time
to prevent this.

Reported by: KMSAN
Reviewed by: kp
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D31313

show more ...

pf: remove duplicate ERROUT_FUNCTION definition

Sponsored by: Modirum MDPay

pf: batch critical section for several counters

Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")

pf: switch rule counters to pf_counter_u64

Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")