The packet_limit can fall to 0, leading to a divide by zero abort in
the "packets % packet_limit".
An possible solution would be to apply a lower limit of 1 after the
calculation of packet_limit, but since any number modulo 1 gives 0,
the more efficient solution is to skip the modulo operation for
packet_limit <= 1.
Since this is a fix for a panic observed in stable/12, merging this
fix to stable/12 and stable/13 before expiry of the 3 day waiting
period might be justified, if it works for the reporter of the issue.
Reported by: Karl Denninger <karl@denninger.net>
MFC after: 3 days
The kernel part of ipfw(8) does initialize LibAlias uncondistionally
with an zeroized port range (allowed ports from 0 to 0). During
restucturing of libalias, port ranges are used everytime and are
therefor initialized with different values than zero. The secondary
initialization from ipfw (and probably others) overrides the new
default values and leave the instance in an unfunctional state. The
obvious solution is to detect such reinitializations and use the new
default value instead.
MFC after: 3 days
The expiration time of direct address mappings is explicitly
uninitialized. Expire times are always compared during housekeeping.
Despite the uninitialized value does not harm, it's simpler to just
set it to a reasonable default. This was detected during valgrinding
the test suite.
MFC after: 3 days
Comparing elements in a tree requires transitiviy. If a < b and b < c
then a must be smaller than c. This way the tree elements are always
pairwise comparable.
Tristate comparsion functions returning values lower, equal, or
greater than zero, are usually implemented by a simple subtraction of
the operands. If the size of the operands are equal to the size of
the result, integer modular arithmetics kick in and violates the
transitivity.
Example:
Working on byte with 0, 120, and 240. Now computing the differences:
120 - 0 = 120
240 - 120 = 120
240 - 0 = -16
MFC after: 3 days
Compiling libalias results in warnings about unused functions.
Those warnings are caused by clang's heuristic to consider an inline
function as in use, iff the declaration is in a *.c file.
Declarations in *.h files do not emit those warnings.
Hence the declarations must be moved to an extra *.h file.
MFC after: 3 days
Differential Revision: https://reviews.freebsd.org/D30844
Current data structure is using a hash of unordered lists. Those
unordered lists are quite efficient, because the least recently
inserted entries are most likely to be used again. In order to avoid
long search times in other cases, the lists are hashed into many
buckets. Unfortunatly a search for a miss needs an exhaustive
inspection and a careful definition of the hash.
Splay trees offer a similar feature: Almost O(1) for access of the
least recently used entries, and amortized O(ln(n)) for almost all
other cases. Get rid of the hash.
Now the data structure should able to quickly react to external
packets without eating CPU cycles for breakfast, preventing a DoS.
PR: 192888
Discussed with: Dimitry Luhtionov
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30536
Current data structure is using a hash of unordered lists. Those
unordered lists are quite efficient, because the least recently
inserted entries are most likely to be used again. In order to avoid
long search times in other cases, the lists are hashed into many
buckets. Unfortunatly a search for a miss needs an exhaustive
inspection and a careful definition of the hash.
Splay trees offer a similar feature - almost O(1) for access of the
least recently used entries), and amortized O(ln(n) - for almost all
other cases. Get rid of the hash.
Discussed with: Dimitry Luhtionov
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30516
The entry deleteAllLinks in the struct libalias is only used to signal
a state between internal calls. It's not used between API calls.
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30604
Get rid of PORT_BASE, replace by AliasRange. Simplify code.
Factor out the search for a new port. Improves the perfomance a bit.
Discussed with: Dimitry Luhtionov
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30581
Let PPTP use its own data structure.
Regroup and rename other lists, which are not PPTP.
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30580
Reorder incoming links by grouping of common search terms.
Significant performance improvement for incoming (missing) flows.
Remove LSNAT from outgoing search.
Slight speedup due to less comparsions in the loop.
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30574
Factor out the outgoing search function.
Preparation for a new data structure.
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30572
Separate the partially specified links into a separate data structure.
This would causes a major parformance impact, if there are many of
them. Use a (smaller) hash table to speed up the partially link
access.
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30570
Search fully specified links first. Some performance loss due to need
to revisit the db twice, if not found.
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30569
Factor out the common Out and In filter
Slightly better performance due to eager skip of search loop
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30568
Summary:
- Use LibAliasTime as a real global variable for central timekeeping.
- Reduce number of syscalls in user space considerably.
- Dynamically adjust the packet counters to match the second resolution.
- Only check the first few packets after a time increase for expiry.
Discussed with: hselasky
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30566
Stats counters are used as unsigned valued (i.e. printf("%u")) but are
defined as signed int. This causes trouble later, so fix it early.
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30587
Replace current expensive, but sparsly called housekeeping
by a single, repetive action.
This is part of a larger restructure of libalias in order to switch to
more efficient data structures. The whole restructure process is
split into 15 reviews to ease reviewing. All those steps will be
squashed into a single commit for MFC in order to hide the
intermediate states from production systems.
Reviewed by: hselasky
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30277
The functionality to detect a newly created link after processing a
single packet is decoupled from the packet processing. Every new
packet is processed asynchronously and will reset the indicator, hence
the function is unusable. I made a Google search for third party code,
which uses the function, and failed to find one.
That's why the function should be removed: It unusable and unused.
A much simplified API/ABI will remain in anything below 14.
Discussed with: kp
Reviewed by: manpages (bcr)
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30275
The commit 189f8eea contains a refactorisation of a constant. During
later review D30283 the naming of the constant was improved and the
initialization became explicit. Put this into the tree, in order to
MFC the correct naming.
At some places the ASSERT was inserted before variable declarations are
finished. This is fixed now.
Reported by: kib
Reviewed by: kib
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D30282
The field nullAddress in struct libalias is never set and never used.
It exists as a placeholder for an unused argument only.
Reviewed by: hselasky
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D30253
libalias is a convolut of various coding styles modified by a series
of different editors enforcing interesting convetions on spacing and
comments.
This patch is a baseline to start with a perfomance rework of
libalias. Upcoming patches should be focus on the code, not on the
style. That's why most annoying style errors should be fixed
beforehand.
Reviewed by: hselasky
Discussed by: emaste
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D30259
terminating a TCP connection.
If a TCP packet must be retransmitted and the data length has changed in the
retransmitted packet, due to the internal workings of TCP, typically when ACK
packets are lost, then there is a 30% chance that the logic in GetDeltaSeqOut()
will find the correct length, which is the last length received.
This can be explained as follows:
If a "227 Entering Passive Mode" packet must be retransmittet and the length
changes from 51 to 50 bytes, for example, then we have three cases for the
list scan in GetDeltaSeqOut(), depending on how many prior packets were
received modulus N_LINK_TCP_DATA=3:
case 1: index 0: original packet 51
index 1: retransmitted packet 50
index 2: not relevant
case 2: index 0: not relevant
index 1: original packet 51
index 2: retransmitted packet 50
case 3: index 0: retransmitted packet 50
index 1: not relevant
index 2: original packet 51
This patch simply changes the searching order for TCP packets, always starting
at the last received packet instead of any received packet, in
GetDeltaAckIn() and GetDeltaSeqOut().
Else no functional changes.
Discussed with: rscheff@
Submitted by: Andreas Longwitz <longwitz@incore.de>
PR: 230755
MFC after: 1 week
Sponsored by: Mellanox Technologies // NVIDIA Networking
to be a true RFC 6598 NAT444 setup, where each network segment (e.g. user,
subnet) can have their own dedicated port aliasing ranges.
Reviewed by: donner, kp
Approved by: 0mp (mentor), donner, kp
Differential Revision: https://reviews.freebsd.org/D23450
This fixes -Wcast-align warnings caused by the underaligned `struct ip`.
This also silences them in the public functions by changing the function
signature from char * to void *. This is source and binary compatible and
avoids the -Wcast-align warning.
Reviewed By: ae, gbe (manpages)
Differential Revision: https://reviews.freebsd.org/D27882
The CU-SeeMe videoconferencing client and associated protocol is at this
point a historical artifact; there is no need to retain support for this
protocol today.
Reviewed by: philip, markj, allanjude
Relnotes: Yes
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D24790
admbugs: 956
Submitted by: markj
Reported by: Vishnu Dev TJ working with Trend Micro Zero Day Initiative
Security: FreeBSD-SA-20:13.libalias
Security: CVE-2020-7455
Security: ZDI-CAN-10849
admbugs: 956
Submitted by: ae
Reported by: Lucas Leong (@_wmliang_) of Trend Micro Zero Day Initiative
Reported by: Vishnu working with Trend Micro Zero Day Initiative
Security: FreeBSD-SA-20:12.libalias
r357614 added CTLFLAG_NEEDGIANT to make it easier to find nodes that are
still not MPSAFE (or already are but aren’t properly marked).
Use it in preparation for a general review of all nodes.
This is non-functional change that adds annotations to SYSCTL_NODE and
SYSCTL_PROC nodes using one of the soon-to-be-required flags.
Mark all obvious cases as MPSAFE. All entries that haven't been marked
as MPSAFE before are by default marked as NEEDGIANT
Approved by: kib (mentor, blanket)
Commented by: kib, gallatin, melifaro
Differential Revision: https://reviews.freebsd.org/D23718
r357614 added CTLFLAG_NEEDGIANT to make it easier to find nodes that are
still not MPSAFE (or already are but aren’t properly marked).
Use it in preparation for a general review of all nodes.
This is non-functional change that adds annotations to SYSCTL_NODE and
SYSCTL_PROC nodes using one of the soon-to-be-required flags.
Approved by: kib (mentor, blanket)
Differential Revision: https://reviews.freebsd.org/D23639
In libalias, a new flag PKT_ALIAS_UNREGISTERED_RFC6598 is added.
This is like PKT_ALIAS_UNREGISTERED_ONLY, but also is RFC 6598 aware.
Also, we add a new NAT option to ipfw called unreg_cgn, which is like
unreg_only, but also is RFC 6598-aware. The reason for the new
flags/options is to avoid breaking existing networks, especially those
which rely on RFC 6598 as an external address.
Submitted by: Neel Chauhan <neel AT neelc DOT org>
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D22877
rename the source to gsb_crc32.c.
This is a prerequisite of unifying kernel zlib instances.
PR: 229763
Submitted by: Yoshihiro Ota <ota at j.email.ne.jp>
Differential Revision: https://reviews.freebsd.org/D20193
It is better to try allocate a big mbuf, than just silently drop a big
packet. A better solution could be reworking of libalias modules to be
able use m_copydata()/m_copyback() instead of requiring the single
contiguous buffer.
PR: 229006
MFC after: 1 week
Plenty of allocation sites pass M_ZERO and sizes which are small and known
at compilation time. Handling them internally in malloc loses this information
and results in avoidable calls to memset.
Instead, let the compiler take the advantage of it whenever possible.
Discussed with: jeff
Make the calloc wrappers behave as expected by using mallocarray.
It is rather weird that the malloc wrappers also zeroes the memory: update
a comment to reflect at least two cases where it is expected.
Reviewed by: tuexen
