If we delay route-to/dup-to/reply-to through dummynet we are eventually
returned to pf_test(). At that point we no longer have the context for
the route-to destination. We'd just skip the pf_test() and continue
processing. This means that route-to did not work as expected.
Extend pf_mtag to carry the route-to destination so we can apply it when
we re-enter pf_test().
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D35159
If packets are processed by a route-to/dup-to/reply-to rule (i.e. they
pass through pf_route(6)) dummynet was not applied to them.
This is because pf_route(6) passes packets directly to ifp->if_output(),
so the dummynet functions were never called.
Factor out the dummynet code and call dummynet prior to
ifp->if_output(). This has a secondary benefit of reducing some code
duplication between the IPv4 and IPv6 paths.
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D35158
If we pass a packet to dummynet we should indicate we've passed it (but
keep m0 == NULL). Otherwise we'll indicate to the calling layers that
the packet has been rejected.
Sponsored by: Rubicon Communications, LLC ("Netgate")
If we don't have a pipe set we shouldn't feed packets into dummynet.
This could occur if we have a 'dnpipe (0, 100)' configuration, for
example. We do want to feed the packet to dummynet in the return
direction, but not in the forward direction. In that case
pf_pdesc_to_dnflow() should return false, rather than pass a pipe number
of 0 to dummynet.
Sponsored by: Rubicon Communications, LLC ("Netgate")
Due to a typo dnrpipe (i.e. the pipe for reverse direction traffic) was
nevern assigned, preventing it from working correctly.
Sponsored by: Rubicon Communications, LLC ("Netgate")
This fixed panic with interface being removed while packet
was sitting on a queue. This allows to pass all dummynet
tests including forthcoming dummynet:ipfw_interface_removal
and dummynet:pf_interface_removal and demonstrates use of
m_rcvif_serialize() and m_rcvif_restore().
Reviewed by: kp
Differential revision: https://reviews.freebsd.org/D33267
(cherry picked from commit 165746f4e4)
ip_dn_io_ptr() (i.e. dummynet_io()) can return the mbuf immediately (as
opposed to owning it and later passing it through dummynet_send(), which
returns it to pf_test()). In that case we must clear the PF_TAG_DUMMYNET
flag to ensure we don't skip any subsequent firewall passes.
This can happen if we process a packet in PFIL_IN, set PF_TAG_DUMMYNET
on it, pass it to ip_dn_io_ptr() but have it returned immediately. The
packet continues its normal path, eventually hitting
pf_test(dir=PFIL_OUT), where we'd skip when we're not supposed to.
Sponsored by: Rubicon Communications, LLC ("Netgate")
Similar to ipfw rule timestamps, these timestamps internally are
uint32_t snaps of the system time in seconds. The timestamp is CPU local
and updated each time a rule or a state associated with a rule or state
is matched.
Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D34970
Coverity points out that if counter was NULL when passed to
pfr_pool_get() we could potentially end up dereferencing it.
Happily all users of the function pass a non-NULL pointer. Enforce this
by assertion and remove the pointless NULL check.
Reported by: Coverity (CID 273309)
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Move the use of 'sc' to after the NULL check.
It's very unlikely that we'd actually hit this, but Coverity is correct
that it's not a good idea to dereference the pointer and only then NULL
check it.
Reported by: Coverity (CID 1398362)
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Allow tables to be used for the l3 source/destination matching.
This requires taking the PF_RULES read lock.
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D34917
Inactive Ethernet rules get cleaned by a net_epoch callback. This
callback may still be pending when we try to start a new (pf rules)
transaction, causing it to fail.
This is especially likely to occur in scripted scenarios, such as the
regression tests.
Drain the epoch callbacks before starting a new transaction, ensuring
we've had the opportunity to clean up the inactive rules.
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D34846
Matching for DSCP codes has used incorrect bits. Use IPV6_DSCP()
macro for matching opcodes to fix this. Also this leads to always
use value from a mbuf instead of cached value.
Previously different opcodes have used both cached in f_id value
and stored in the mbuf, and it did not always work after setdscp
action, since cached value was not updated.
Update IPv6 flowid value cached in the f_id.flow_id6 when we do
modification of DSCP value in O_SETDSCP opcode, it may be used by
external modules.
Also added logging support for O_SETDSCP opcode.
Reviewed by: kp
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D34807
The RB_NEXT macro does not use its middle argument since commit
5fce408cc4 in 2004 (which ironically
fixed an "unused parameter" warning by introducing this warning in all
consumers). RB_PREV has also copied this unfortunate behavior of an
unused argument.
This results in 'parent' not being used. To workaround, inline the
value of 'parent' as the second argument to RB_NEXT.
Reviewed by: kp
Differential Revision: https://reviews.freebsd.org/D34833
We forgot to free the nvlist (and packed nvlist) on success.
While here start using the ERROUT macro to clean up error handling, and
to add SDTs for better debugging.
Reported by: Coverity
CID: 1473150
The nvlist is allocated in pf_keth_rule_to_nveth_rule(). There's no need
to allocate one in the calling function. Especially not as we overwrite
the pointer to the new nvlist with the one allocated by
pf_keth_rule_to_nveth_rule(), leaking memory.
Reported by: Coverity
CID: 1476128
Sponsored by: Rubicon Communications, LLC ("Netgate")
Use ERROUT_IOCTL() rather than hand-rolling the macro. This adds DTrace
SDTs in the error path, making debugging ioctl errors easier.
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
When trying to avoid a CARP demotion during a pfsync service restart, I
noticed that a non-default value for the net.pfsync.carp_demotion_factor
sysctl was not being applied during the demotion. The CARP was always
demoted by 240.
After investigating, I realized that the sysctl was using VNET_NAME()
without the CTLFLAG_VNET.
PR: 262983
Reviewed by: kp
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D34737
The limit may later be updated by the "set limit" directive in pf.conf.
UMA does not permit a limit to be set on a zone after any items have
been allocated from a zone.
Other UMA zones used by pf do not appear to be susceptible to this
problem: they either set a limit at zone creation time or never set one
at all.
PR: 260406
Reviewed by: kp
MFC after: 3 days
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D34713
Possibility to do it was always a bug, but it runs into crashes
since recent introduction of a per-ruleset RB tree.
Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")
Reported by: syzbot+665b700afc6f69f1766a@syzkaller.appspotmail.com
with md5 sum used as key.
This gets rid of the quadratic rule traversal when "keep_counters" is
set.
Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")
Makes it cheaper to compare rules when "keep_counters" is set.
This also sets up keeping them in a RB tree.
Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")
For now only protects rule creation/destruction, but will allow
gradually reducing the scope of rules lock when changing the
rules.
Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")
Otherwise all anchors hash to the same value.
Note this can result in checksum mismatches between pfsynced hosts,
but it has to be sorted out as the previously computed checksum
would fail to indicate changed anchors.
Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")
Disallow the use of tables in ethernet rules. Using tables requires
taking the PF_RULES lock. Moreover, the current table code isn't ready
to deal with ethernet rules.
Disallow their use for now.
Sponsored by: Rubicon Communications, LLC ("Netgate")
Allow filtering based on the source or destination IP/IPv6 address in
the Ethernet layer rules.
Reviewed by: pauamma_gundo.com (man), debdrup (man)
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D34482
Newly allocated counters are guaranteed to be 0.
This removes 5 IPIs for each loaded rule.
Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")
When filtering Ethernet packets allow rules to specify a mac address
with a mask. This indicates which bits of the specified address are
significant. This allows users to do things like filter based on device
manufacturer.
Sponsored by: Rubicon Communications, LLC ("Netgate")
Allow packets to be tagged with dummynet information. Note that we do
not apply dummynet shaping on the L2 traffic, but instead mark it for
dummynet processing in the L3 code. This is the same approach as we take
for ALTQ.
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D32222
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
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
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
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
The ref_count counter is global (i.e. not per-vnet) so we can't use a
per-vnet lock to protect it. Moreover, in callouts curvnet is not set,
so we'd end up panicing when trying to use DN_BH_WLOCK().
Instead we use the global sched_lock, which is already used when
evaluating ref_count (in unload_dn_aqm()).
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D34059
When we create a table without counters, add an entry and later
re-define the table to have counters we wound up trying to read
non-existent counters.
We now cope with this by attempting to add them if needed, removing them
when they're no longer needed and not trying to read from counters that
are not present.
MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D34131
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
If there are no more entries, or if we fail to restore the rcvif of a
queued mbuf dn_dequeue() can return NULL.
Cope with this.
Reviewed by: glebius
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D34078
This fixed panic with interface being removed while packet
was sitting on a queue. This allows to pass all dummynet
tests including forthcoming dummynet:ipfw_interface_removal
and dummynet:pf_interface_removal and demonstrates use of
m_rcvif_serialize() and m_rcvif_restore().
Reviewed by: kp
Differential revision: https://reviews.freebsd.org/D33267
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")
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
Postpone zeroing out pd until after the PFI_IFLAG_SKIP/M_SKIP_FIREWALL
checks. We don't need it until then, and it saves us a few CPU cycles in
some cases.
This isn't expected to make a measurable performance change though.
Reviewed by: mjg, glebius
Pointed out by: markj
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D33815
It's never set, so we can remove both the check for it and the
definition.
Reviewed by: mjg, glebius
Pointed out by: markj
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D33814
SDT probe frb_natv4in is only available when an error is encountered.
Make it also available when no error is encountered, i.e. NATed and
not translated.
MFC after: 1 week
Replace the INLINE macro with inline. Some ancient compilers supported
__inline__ instead of inline. The INLINE hack compensated for it.
Ancient compilers are history.
Reported by: glebius
MFC after: 1 month
Convert ipfilter kernel function declarations from K&R to ANSI. This
syncs our function declarations with NetBSD hg commit 75edcd7552a0
(apply our changes). Though not copied from NetBSD, this change was
partially inspired by NetBSD's work and inspired by style(9).
Reviewed by: glebius (for #network)
MFC after: 1 month
Differential Revision: https://reviews.freebsd.org/D33595
Through fixes and improvements our ipfilter sources have diverged
enough to warrant move from contrib into sys/netpil. Now that I'm
planning on implementing MSS clamping as in iptables it makes more
sense to move ipfilter to netpfil.
This is the first of three commits the ipfilter move.
Suggested by glebius on two occaions.
Suggested by and discussed with: glebius
Reviewed by: glebius, kp (for #network)
MFC after: 1 month
Differential Revision: https://reviews.freebsd.org/D33510
When we exposed the PFSYNCF_OK flag to userspace in 5f5bf88949 we
unintentionally caused defer mode to always be enabled.
The ioctl check only looked for nonzero, not for the PFSYNCF_DEFER flag.
Fix this check and ensure ifconfig sets the flag.
Reviewed by: glebius
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D33244
* Ensure we unlock the pfsync lock in pfsync_defer()
* We must hold the bucket lock when calling pfsync_push()
* The pfsync_defer_tmo() callout locks the bucket lock, not the pfsync
lock
Reviewed by: glebius
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D33243
Don't use a fixed number of ticks, but take hz into account so we have a
consistent timeout, regardless of what hz is set up.
Use a 20ms timeout, becaues that's what OpenBSD uses.
Reviewed by: glebius
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D33242
This flag is stored in if_drv_flags, not if_flags.
Reviewed by: glebius
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D33241
In pfsync_defer() we must wait to lock sc until we've ensured it's not
NULL.
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D33240
There were two issues with the new pflog packet length.
The first is that the length is expected to be a multiple of
sizeof(long), but we'd assumed it had to be a multiple of
sizeof(uint32_t).
The second is that there's some broken software out there (such as
Wireshark) that makes incorrect assumptions about the amount of padding.
That is, Wireshark assumes there's always three bytes of padding, rather
than however much is needed to get to a multiple of sizeof(long).
Fix this by adding extra padding, and a fake field to maintain
Wireshark's assumption.
Reported by: Ozkan KIRIK <ozkan.kirik@gmail.com>
Tested by: Ozkan KIRIK <ozkan.kirik@gmail.com>
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D33236
The socket option handler tries to ensure that the option length is no
larger than some reasonable maximum, and no smaller than sizeof(struct
dn_id). But the loaded option length is stored in an int, which is
converted to an unsigned integer for the comparison with a size_t, so
negative values are not caught and instead get passed to malloc().
Change the code to use a size_t for the buffer size.
Reviewed by: kp
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D33133
do_config() processes a buffer of variable-length dummynet commands.
The loop which processes this buffer loads the fixed-length header
before checking whether there are any bytes left to read, so it performs
a 4-byte read past the end of the buffer before terminating.
Restructure the loop to avoid this.
Reported by: Jenkins (KASAN job)
Reviewed by: kp
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D33132
This test failed if ipfw was loaded (as well as pf). pf used the same
tag as dummynet to indicate a packet had already gone through dummynet.
However, ipfw removes this tag, so pf didn't realise the packet had
already gone through dummynet.
Introduce a separate flag, in the existing pf mtag rather than re-using
the ipfw tag. There were no free flag bits, but PF_TAG_FRAGCACHE is no
longer used so its bit can be re-purposed.
MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D33087
In e5c4987e3f we fixed issues with nat and dummynet, but only changed
the IPv4 code. Make the same change for IPv6 as well.
Reviewed by: glebius
MFC after: 3 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D33086
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
Allow users to set a number on rules which will be exposed as part of
the pflog header.
The intent behind this is to allow users to correlate rules across
updates (remember that pf rules continue to exist and match existing
states, even if they're removed from the active ruleset) and pflog.
Obtained from: pfSense
MFC after: 3 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D32750
Dummynet differs from ALTQ in that ALTQ schedules packets after they
leave pf. Dummynet schedules them after they leave pf, but then
re-injects them.
We currently deal with this by ensuring we don't re-schedule a packet we
get from dummynet, but this produces unexpected results when combined
with NAT, as dummynet processing is done after the NAT transformation.
In other words, the second time the packet is handed to pf it may have a
different source and destination address.
Simplify this by moving dummynet processing to after all other pf
processing, and not re-processing (but always passing) packets from
dummynet.
This fixes NAT of dummynet delayed packets, and also reduces processing
overhead (because we only do state/rule lookup for each dummynet packet
once, rather than twice).
MFC after: 3 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D32665
The modification to the hash are already naturally locked by
in_control_sx. Convert the hash lists to CK lists. Remove the
in_ifaddr_rmlock. Assert the network epoch where necessary.
Most cases when the hash lookup is done the epoch is already entered.
Cover a few cases, that need entering the epoch, which mostly is
initial configuration of tunnel interfaces and multicast addresses.
Reviewed by: melifaro
Differential revision: https://reviews.freebsd.org/D32584
When we route-to a packet that later turns out to not fit in the
outbound interface MTU we generate an ICMP error.
However, if we've already changed those (i.e. we've passed through a NAT
rule) we have to undo the transformation first.
Obtained from: pfSense
MFC after: 3 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D32571
For some traffic there might be no matching rule in the current ruleset,
for example when a state was imported via pfsync from a sytem with a
different ruleset checksum. In this case pf_route uses s->rt_addr for
routing target instead of r->rpool.cur but r->rpool is checked anyway,
resulting in dropped packets.
PR: 259183
Submitted by: Kajetan Staszkiewicz <vegeta tuxpowered.net>
Sponsored by: InnoGames GmbH
When a redirection/nat IP address is selected by pf_map_addr it is
logged with PF_DEBUG_MISC level. This one according to the manual means
"Generate debug messages for various errors". Selecting an IP address is
not an error, it's a normal function of pf for route-to, nat and some
other operations. Therefore PF_DEBUG_NOISY level should be choosen which
is means "Generate debug messages for common conditions".
PR: 259184
Submitted by: Kajetan Staszkiewicz <vegeta tuxpowered.net>
Sponsored by: InnoGames GmbH
Similar to the existing functions for strings and ints, this lets us
simplify some of the nvlist conversion code.
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Use atomic counters to ensure that we correctly track the number of half
open states and syncookie responses in-flight.
This determines if we activate or deactivate syncookies in adaptive
mode.
MFC after: 1 week
Sponsored by: Modirum MDPay
Differential Revision: https://reviews.freebsd.org/D32134
Allow pf to use dummynet pipes and queues.
We re-use the currently unused IPFW_IS_DUMMYNET flag to allow dummynet
to tell us that a packet is being re-injected after being delayed. This
is needed to avoid endlessly looping the packet between pf and dummynet.
MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31904
Allow the dummynet module to be loaded without ipfw, as a first step
towards making pf use it for packet scheduling.
Reviewed by: donner
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31903
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
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
When we're synproxy-ing a connection that's going to us (as opposed to a
forwarded one) we wound up trying to send out the pf-generated tcp
packets through pf_intr(), which called ip(6)_output(). That doesn't
work all that well for packets that are destined for us, so in that case
we must call ip(6)_input() instead.
MFC after: 1 week
Sponsored by: Modirum MDPay
Differential Revision: https://reviews.freebsd.org/D31853
In pf_create_state() we can end up deleting the state immediately. This
can happen if we fail to map the relevant addresses or fail
normalization or fail to insert it into the state table.
If that happens we delete the state again with pf_free_state(). However,
this asserts that the state must be unlinked.
It's correct to simply set the state to PFTM_UNLINKED because we've not
yet linked it.
Submitted by: Mateusz Guzik <mjg@FreeBSD.org>
Reviewed by: scottl
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31752
to change a state's state (that term is overloaded in pf, protocol state
like ESTABLISHED for tcp here), don't do it directly, but go through a newly
introduced pf_set_protostate()
Reviewed by: kbowling
Obtainted from: OpenBSD
MFC after: 1 week
Sponsored by: Modirum MDPay
Differential Revision: https://reviews.freebsd.org/D31729
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
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
In nat64_find_route[46] handle NHF_GATEWAY flag and use destination
address from next hop to do link layer address lookup.
PR: 255928
Reviewed by: melifaro
Obtained from: Yandex LLC
MFC after: 1 week
Sponsored by: Yandex LLC
Differential Revision: https://reviews.freebsd.org/D31680
Extend the existing set-tos keyword to also be able to set traffic class
on IPv6 traffic.
Add tests for this as well.
Reviewed by: kp
Differential Revision: https://reviews.freebsd.org/D31564
This allows the maximum value of 4294967295 (~4Gb/s) instead of previous
value of 2147483647 (~2Gb/s).
Reviewed by: np, scottl
Obtained from: pfSense
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31582
The intent is to line up various enums so that branching in the lines of:
idx = (dir == PF_IN ? PF_SK_WIRE : PF_SK_STACK);
is avoided.
Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")
Jump cache is used to reduce the cost of rule lookup for O_SKIPTO and
O_CALLRETURN actions. It uses rules chain id to check correctness of
cached value. But due to the possible race, there is the chance that
one thread can read invalid value. In some cases this can lead to out
of bounds access and panic.
Use thread fence operations to constrain the reordering of accesses.
Also rename jump_fast and jump_linear functions to jump_cached and
jump_lookup_pos respectively.
Submitted by: Arseny Smalyuk
Reviewed by: melifaro
Obtained from: Yandex LLC
MFC after: 1 week
Sponsored by: Yandex LLC
Differential Revision: https://reviews.freebsd.org/D31484
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")
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
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
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
The introduction of synproxy support changed the size of struct
pf_status, which in turn broke the userspace ABI.
Revert the relevant change. More work is needed on the synproxy code to
keep and expose the counters, but in the mean time this restores the
ABI.
PR: 257469
MFC after: 3 days
Sponsored by: Modirum MDPay
These two fuctions were identical, so move them into the common
vlan_set_pcp() function, exposed in the if_vlan_var.h header.
Reviewed by: donner
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31275
Numerous counters got migrated from straight uint64_t to the counter(9)
API. Unfortunately the implementation comes with a significiant
performance hit on some platforms and cannot be easily fixed.
Work around the problem by implementing a pf-specific variant.
Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")
This shaves calculation which in particular helps on arm.
Note using the & hack instead would still be more work.
Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")
Kernel side implementation to allow switching between on and off modes,
and allow this configuration to be retrieved.
MFC after: 1 week
Sponsored by: Modirum MDPay
Differential Revision: https://reviews.freebsd.org/D31139
Import OpenBSD's syncookie support for pf. This feature help pf resist
TCP SYN floods by only creating states once the remote host completes
the TCP handshake rather than when the initial SYN packet is received.
This is accomplished by using the initial sequence numbers to encode a
cookie (hence the name) in the SYN+ACK response and verifying this on
receipt of the client ACK.
Reviewed by: kbowling
Obtained from: OpenBSD
MFC after: 1 week
Sponsored by: Modirum MDPay
Differential Revision: https://reviews.freebsd.org/D31138
Similar to the REPLY_TO shortcut (6d786845cf) we also can't shortcut
ROUTE_TO. If we do we will fail to apply transformations or update the
state, which can lead to premature termination of the connections.
PR: 257106
MFC after: 3 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31177
struct mbuf *replyto is not actually used (and only rarely provided).
The same applies to struct ifnet *ifp.
No functional change.
Reviewed by: mjg
MFC after: 1 week
Sponsored by: Modirum MDPay
Differential Revision: https://reviews.freebsd.org/D31136
Support the 'match' keyword.
Note that support is limited to adding queuing information, so without
ALTQ support in the kernel setting match rules is pointless.
For the avoidance of doubt: this is NOT full support for the match
keyword as found in OpenBSD's pf. That could potentially be built on top
of this, but this commit is NOT that.
MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31115
Rather than allocating however much memory userspace asks for we only
allocate enough for a handful of states, and copy to userspace for each
completed row.
We start out with enough space for 16 states (per row), but grow that as
required. In most configurations we expect at most a handful of states
per row (more than that would have other negative effects on packet
processing performance).
Reviewed by: mjg
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31111
Add a new version of the DIOCGETSTATES call, which extends the struct to
include the original interface information.
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31097
Only print this warning when boot verbose is enabled.
This can get pretty annoying (and useless) in some systems.
Reviewed by: kp
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Happily this wasn't a real bug, because pf_killstates() never fails, but
we should check the return value anyway, in case it does ever start
returning errors.
Reported by: clang --analyze
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
pidx is never NULL, and is used unconditionally later on in the
function.
Add an assertion, as documentation for the requirement to provide an idx
pointer.
Reported by: clang --analyze
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Indicate that this is a kernel-only structure, and make it easier to
distinguish from others used to communicate with userspace.
Reviewed by: mjg
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D31096
Instead serialize against these operations with a dedicated lock.
Prior to the change, When pushing 17 mln pps of traffic, calling
DIOCRGETTSTATS in a loop would restrict throughput to about 7 mln. With
the change there is no slowdown.
Reviewed by: kp (previous version)
Sponsored by: Rubicon Communications, LLC ("Netgate")
Creating tables and zeroing their counters induces excessive IPIs (14
per table), which in turns kills single- and multi-threaded performance.
Work around the problem by extending per-CPU counters with a general
counter populated on "zeroing" requests -- it stores the currently found
sum. Then requests to report the current value are the sum of per-CPU
counters subtracted by the saved value.
Sample timings when loading a config with 100k tables on a 104-way box:
stock:
pfctl -f tables100000.conf 0.39s user 69.37s system 99% cpu 1:09.76 total
pfctl -f tables100000.conf 0.40s user 68.14s system 99% cpu 1:08.54 total
patched:
pfctl -f tables100000.conf 0.35s user 6.41s system 99% cpu 6.771 total
pfctl -f tables100000.conf 0.48s user 6.47s system 99% cpu 6.949 total
Reviewed by: kp (previous version)
Sponsored by: Rubicon Communications, LLC ("Netgate")
This call is particularly slow due to the large amount of data it
returns. Remove all fields pfctl does not use. There is no functional
impact to pfctl, but it somewhat speeds up the call.
It might affect other (i.e. non-FreeBSD) code that uses the new
interface, but this call is very new, so there's unlikely to be any. No
releases contained the previous version, so we choose to live with the
ABI modification.
Reviewed by: donner
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30944
The sysctl nodes which use V_dn_cfg must be marked as CTLFLAG_VNET so
that we use the correct per-vnet offset
PR: 256819
Reviewed by: donner
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30974
stats are not shared and consequently per-CPU counters only waste
memory.
No slowdown was measured when passing over 20M pps.
Reviewed by: kp
Sponsored by: Rubicon Communications, LLC ("Netgate")
ipfw_chk() might call m_pullup() and thus can change the mbuf chain
head. In this case, the new chain head has to be returned to the pfil
hook caller, otherwise the pfil hook caller is left with a dangling
pointer.
Note that this affects only the link-layer hooks installed when the
net.link.ether.ipfw sysctl is set to 1.
PR: 256439, 254015, 255069, 255104
Fixes: f355cb3e6
Reviewed by: ae
MFC after: 3 days
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D30764
Rather than pointers to the headers store full copies. This brings us
slightly closer to what OpenBSD does, and also makes more sense than
storing pointers to stack variable copies of the headers.
Reviewed by: donner, scottl
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30719
copyout() can trigger page faults, so it may potentially sleep.
Reported by: avg
MFC after: 3 days
Sponsored by: Rubicon Communications, LLC ("Netgate")
In the ioctl path use M_WAITOK allocations whereever possible. These are
less sensitive to memory pressure, and ioctl requests have no hard
deadlines.
Reviewed by: donner
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30702
There's no need to check pointers for NULL before free()ing them.
No functional change.
MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30382
These are global (i.e. shared across vnets) structures, so we need
global lock to protect them. However, we look up entries in these lists
(find_aqm_type(), find_sched_type()) and return them. We must ensure
that the returned structures cannot go away while we are using them.
Resolve this by using NET_EPOCH(). The structures can be safely accessed
under it, and we postpone their cleanup until we're sure they're no
longer used.
MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30381
This moves dn_cfg and other parameters into per VNET variables.
The taskqueue and control state remains global.
Reviewed by: kp
Differential Revision: https://reviews.freebsd.org/D29274
There is padding between pfr_astats.pfras_a and pfras_packets that was
not getting initialized.
Reported by: KMSAN
Reviewed by: kp, imp
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D30585
We must also remember to free nvlists added to a parent nvlist with
nvlist_append_nvlist_array().
More importantly, when nvlist_pack() allocates memory for us it does so
in the M_NVLIST zone, so we must free it with free(.., M_NVLIST). Using
free(.., M_TEMP) as we did silently failed to free the memory.
MFC after: 3 days
Reported by: kib@
Tested by: kib@
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30595
This simplifies life a bit, by not requiring us to repease the
declaration for every file where we want static probe points.
It also makes the gcc6 build happy.
Add _opt() variants for the uint* functions. These functions set the
provided default value if the nvlist doesn't contain the relevant value.
This is helpful for optional values (e.g. when the API is extended to
add new fields).
While here simplify the header by also using macros to create the
prototypes for the macro-generated function implementations.
Reviewed by: scottl
MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30510
Separate the conversion functions (between kernel structs and nvlists)
to pf_nv. This reduces the size of pf_ioctl.c, which is already quite
large and complex, a good bit. It also keeps all the fairly
straightforward conversion code together.
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30359
When we create an nvlist and insert it into another nvlist we must
remember to destroy it. The nvlist_add_nvlist() function makes a copy,
just like nvlist_add_string() makes a copy of the string. If we don't
we're leaking memory on every (nvlist-based) ioctl() call.
While here remove two redundant 'break' statements.
PR: 255971
MFC after: 3 days
Sponsored by: Rubicon Communications, LLC ("Netgate")
Floating states get assigned to interface 'all' (V_pfi_all), so when we
try to flush all states for an interface states originally created
through this interface are not flushed. Only if-bound states can be
flushed in this way.
Given that we track the original interface we can check if the state's
interface is 'all', and if so compare to the orig_if instead.
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30246
Track (and display) the interface that created a state, even if it's a
floating state (and thus uses virtual interface 'all').
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30245
We never set 'busy' and never dequeue from the pending mq. Remove this
code.
Reviewed by: ae
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30313
Userspace relies on this pointer to work out if the kif is a group or
not. It can't use it for anything else, because it's a pointer to a
kernel address. Substitute 0xfeedc0de for 'true', so that we don't leak
kernel memory addresses to userspace.
PR: 255852
Reviewed by: donner
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30284
This allows us to kill states created from a rule with route-to/reply-to
set. This is particularly useful in multi-wan setups, where one of the
WAN links goes down.
Submitted by: Steven Brown
Obtained from: https://github.com/pfsense/FreeBSD-src/pull/11/
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30058
Introduce an nvlist based alternative to DIOCKILLSTATES.
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D30054
