pf: Don't hold PF_RULES_WLOCK during copyin() on DIOCRCLRTSTATS

We cannot hold a non-sleepable lock during copyin(). This means we can't
safely count the table, so instead we fall back to the pf_ioc

pf: Don't hold PF_RULES_WLOCK during copyin() on DIOCRCLRTSTATS

We cannot hold a non-sleepable lock during copyin(). This means we can't
safely count the table, so instead we fall back to the pf_ioctl_maxcount
used in other ioctls to protect against overly large requests.

Reported by: syzbot+81e380344d4a6c37d78a@syzkaller.appspotmail.com
MFC after: 1 week

show more ...

pf: Allocate and free pfi_kkif in separate functions

Factor out allocating and freeing pfi_kkif structures. This will be
useful when we change the counters to be counter_u64, so we don't have
to dea

pf: Allocate and free pfi_kkif in separate functions

Factor out allocating and freeing pfi_kkif structures. This will be
useful when we change the counters to be counter_u64, so we don't have
to deal with that complexity in the multiple locations where we allocate
pfi_kkif structures.

No functional change.

MFC after: 2 weeks
Sponsored by: Orange Business Services
Differential Revision: https://reviews.freebsd.org/D27762

show more ...

pf: Split pfi_kif into a user and kernel space structure

No functional change.

MFC after: 2 weeks
Sponsored by: Orange Business Services
Differential Revision: https://reviews.freebsd.org/D27761

pf: Change pf_krule counters to use counter_u64

This improves the cache behaviour of pf and results in improved
throughput.

MFC after: 2 weeks
Sponsored by: Orange Business Services
Differential Re

pf: Change pf_krule counters to use counter_u64

This improves the cache behaviour of pf and results in improved
throughput.

MFC after: 2 weeks
Sponsored by: Orange Business Services
Differential Revision: https://reviews.freebsd.org/D27760

show more ...

pf: Split pf_rule into kernel and user space versions

No functional change intended.

MFC after: 2 weeks
Sponsored by: Orange Business Services
Differential Revision: https://reviews.freebsd.org/D27

pf: Split pf_rule into kernel and user space versions

No functional change intended.

MFC after: 2 weeks
Sponsored by: Orange Business Services
Differential Revision: https://reviews.freebsd.org/D27758

show more ...

pf: Use counter_u64 in pf_src_node

Reviewd by: philip
MFC after: 2 weeks
Sponsored by: Orange Business Services
Differential Revision: https://reviews.freebsd.org/D27756

pf: Split pf_src_node into a kernel and userspace struct

Introduce a kernel version of struct pf_src_node (pf_ksrc_node).

This will allow us to improve the in-kernel data structure without
breaking

pf: Split pf_src_node into a kernel and userspace struct

Introduce a kernel version of struct pf_src_node (pf_ksrc_node).

This will allow us to improve the in-kernel data structure without
breaking userspace compatibility.

Reviewed by: philip
MFC after: 2 weeks
Sponsored by: Orange Business Services
Differential Revision: https://reviews.freebsd.org/D27707

show more ...

pf: Use counter(9) for pf_state byte/packet tracking

This improves cache behaviour by not writing to the same variable from
multiple cores simultaneously.

pf_state is only used in the kernel, so ca

pf: Use counter(9) for pf_state byte/packet tracking

This improves cache behaviour by not writing to the same variable from
multiple cores simultaneously.

pf_state is only used in the kernel, so can be safely modified.

Reviewed by: Lutz Donnerhacke, philip
MFC after: 1 week
Sponsed by: Orange Business Services
Differential Revision: https://reviews.freebsd.org/D27661

show more ...

pf: Make tag hashing more robust

tagname2tag() hashes the tag name before truncating it to 63 characters.
tag_unref() removes the tag from the name hash by computing the hash
over the truncated name

pf: Make tag hashing more robust

tagname2tag() hashes the tag name before truncating it to 63 characters.
tag_unref() removes the tag from the name hash by computing the hash
over the truncated name. Ensure that both operations compute the same
hash for a given tag.

The larger issue is a lack of string validation in pf(4) ioctl handlers.
This is intended to be fixed with some future work, but an extra safety
belt in tagname2hashindex() is worthwhile regardless.

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

show more ...

net: clean up empty lines in .c and .h files

pf: Improve DIOCADDRULE validation

We expect the addrwrap.p.dyn value to be set to NULL (and assert such),
but do not verify it on input.

Reported-by: syzbot+936a89182e7d8f927de1@syzkaller.appspotm

pf: Improve DIOCADDRULE validation

We expect the addrwrap.p.dyn value to be set to NULL (and assert such),
but do not verify it on input.

Reported-by: syzbot+936a89182e7d8f927de1@syzkaller.appspotmail.com
Reviewed by: melifaro (previous version)
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D24538

show more ...

pf: Improve input validation

If we pass an anchor name which doesn't exist pfr_table_count() returns
-1, which leads to an overflow in mallocarray() and thus a panic.

Explicitly check that pfr_tabl

pf: Improve input validation

If we pass an anchor name which doesn't exist pfr_table_count() returns
-1, which leads to an overflow in mallocarray() and thus a panic.

Explicitly check that pfr_table_count() does not return an error.

Reported-by: syzbot+bd09d55d897d63d5f4f4@syzkaller.appspotmail.com
Reviewed by: melifaro
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D24539

show more ...

pf: Improve ioctl() input validation

Both DIOCCHANGEADDR and DIOCADDADDR take a struct pf_pooladdr from
userspace. They failed to validate the dyn pointer contained in its
struct pf_addr_wrap member

pf: Improve ioctl() input validation

Both DIOCCHANGEADDR and DIOCADDADDR take a struct pf_pooladdr from
userspace. They failed to validate the dyn pointer contained in its
struct pf_addr_wrap member structure.

This triggered assertion failures under fuzz testing in
pfi_dynaddr_setup(). Happily the dyn variable was overruled there, but
we should verify that it's set to NULL anyway.

Reported-by: syzbot+93e93150bc29f9b4b85f@syzkaller.appspotmail.com
Reviewed by: emaste
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D24431

show more ...

pf: Do not allow negative ps_len in DIOCGETSTATES

Userspace may pass a negative ps_len value to us, which causes an
assertion failure in malloc().
Treat negative values as zero, i.e. return the requ

pf: Do not allow negative ps_len in DIOCGETSTATES

Userspace may pass a negative ps_len value to us, which causes an
assertion failure in malloc().
Treat negative values as zero, i.e. return the required size.

Reported-by: syzbot+53370d9d0358ee2a059a@syzkaller.appspotmail.com
Reviewed by: lutz at donnerhacke.de
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D24447

show more ...

pf: zero (another) output buffer in pfioctl

Avoid potential structure padding leak. r350294 identified a leak via
static analysis; although there's no report of a leak with the
DIOCGETSRCNODES ioct

pf: zero (another) output buffer in pfioctl

Avoid potential structure padding leak. r350294 identified a leak via
static analysis; although there's no report of a leak with the
DIOCGETSRCNODES ioctl it's a good practice to zero the memory.

Suggested by: kp
MFC after: 3 days
Sponsored by: The FreeBSD Foundation

show more ...

MFHead @r350386

Sponsored by: The FreeBSD Foundation

pf: zero output buffer in pfioctl

Avoid potential structure padding leak.

Reported by: Vlad Tsyrklevich <vlad@tsyrklevich.net>
Reviewed by: kp
MFC after: 3 days
Security: Potential kernel memory di

pf: zero output buffer in pfioctl

Avoid potential structure padding leak.

Reported by: Vlad Tsyrklevich <vlad@tsyrklevich.net>
Reviewed by: kp
MFC after: 3 days
Security: Potential kernel memory disclosure
Sponsored by: The FreeBSD Foundation

show more ...

MFHead @347527

Sponsored by: The FreeBSD Foundation

pf: No need to M_NOWAIT in DIOCRSETTFLAGS

Now that we don't hold a lock during DIOCRSETTFLAGS memory allocation we can
use M_WAITOK.

MFC after: 1 week
Event: Aberdeen hackathon 2019
Pointed out by

pf: No need to M_NOWAIT in DIOCRSETTFLAGS

Now that we don't hold a lock during DIOCRSETTFLAGS memory allocation we can
use M_WAITOK.

MFC after: 1 week
Event: Aberdeen hackathon 2019
Pointed out by: glebius@

show more ...

pf: Fix panic on invalid DIOCRSETTFLAGS

If during DIOCRSETTFLAGS pfrio_buffer is NULL copyin() will fault, which we're
not allowed to do with a lock held.
We must count the number of entries in the

pf: Fix panic on invalid DIOCRSETTFLAGS

If during DIOCRSETTFLAGS pfrio_buffer is NULL copyin() will fault, which we're
not allowed to do with a lock held.
We must count the number of entries in the table and release the lock during
copyin(). Only then can we re-acquire the lock. Note that this is safe, because
pfr_set_tflags() will check if the table and entries exist.

This was discovered by a local syzcaller instance.

MFC after: 1 week
Event: Aberdeen hackathon 2019

show more ...

MFHead@r345677

pf: use UID_ROOT and GID_WHEEL named constants in make_dev

No functional change but improves consistency and greppability of
make_dev calls.

Discussed with: kp

MFHead@r345275

MFhead@r344996

pf: Fix DIOCGETSRCNODES

r343295 broke DIOCGETSRCNODES by failing to reset 'nr' after counting the
number of source tracking nodes.
This meant that we never copied the information to userspace, leadi

pf: Fix DIOCGETSRCNODES

r343295 broke DIOCGETSRCNODES by failing to reset 'nr' after counting the
number of source tracking nodes.
This meant that we never copied the information to userspace, leading to '? ->
?' output from pfctl.

PR: 236368
MFC after: 1 week

show more ...