This reverts commit 781221f084.
Revert "pf tests: ensure that neighbour discovery works as expected"
This reverts commit 631d6e5300.
Revert "pf: fully annotated patch of disabling state tracking and issues for ND"
This reverts commit f858272896.
Revert "pf: invert direction for inner icmp state lookups"
This reverts commit c61a3c23fb.
Revert "pf tests: ensure that traceroutes using ICMP work"
This reverts commit 9c53965169.
Revert "pf: fix icmp-in-icmp state lookup"
This reverts commit e854cb4789.
Revert "pf: allow MLD LR to be sent without state"
This reverts commit 9b2e3cf60b.
Revert "pf: split ICMP/ICMPv6 number space in pf_icmp_mapping()"
This reverts commit ee1b7126a9.
Revert "pf: some ICMP types that also have icmp_id, pointed out by markus@"
This reverts commit c21004ce41.
Revert "pf: stricter state checking for ICMP and ICMPv6 packets"
This reverts commit 7f1f57ed78.
PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=280701
Sloppy state tracking renders ICMP direction check useless
and harmful as we might see only half of the connection in
the asymmetric setups but ignore the state match. The bug
was reported and fix was verified by Insan Praja <insan ()
ims-solusi ! com>. Thanks! OK mcbride, henning
MFC after: 1 week
Obtained from: OpenBSD, mikeb <mikeb@openbsd.org>, 538596657140
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 3da3eb6081a2e2f6ea2fed1728d5dd7f9e8786e5)
'ushm_refcnt' is unsigned. Don't leave the impression it isn't.
No functional change (intended).
Reviewed by: kib
Approved by: emaste (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46126
(cherry picked from commit c75a18905e308f69b01f19c3d7d613883a008e79)
(cherry picked from commit 4938f55446)
Approved by: so
This hardens against provoked use-after-free occurences should there be
reference counting leaks in the future (which is currently not the
case).
At the deepest level, umtx_shm_find_reg_unlocked() now returns EOVERFLOW
when it cannot grant an additional reference to the registry object, and
so will umtx_shm_find_reg(). umtx_shm_create_reg() will fail if calling
umtx_shm_find_reg() returns EOVERFLOW (meaning a SHM object for the
passed key already exists, but we can't acquire another reference on
it), avoiding the creation of a duplicate registry entry for a given key
(this wouldn't pose problem for the rest of the code in its current
form, but is expressly avoided for intelligibility and hardening
purposes).
Since umtx_shm_find_reg*(), and consequently the whole _umtx_op() system
call, can only return EOVERFLOW on such a bug manifesting, we don't
document that return value.
Reviewed by: kib, emaste
Approved by: emaste (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46126
(cherry picked from commit c3e6dfe55c0e81d0717b0458bc95128384c3ebe8)
(cherry picked from commit b20ae16087)
Approved by: so
umtx_shm_unref_reg_locked() would unconditionally drop the "registry"
reference, tied to USHMF_LINKED.
This is not a problem for caller umtx_shm_object_terminated(), which
operates under the 'umtx_shm_lock' lock end-to-end, but it is for
indirect caller umtx_shm(), which drops the lock between
umtx_shm_find_reg() and the call to umtx_shm_unref_reg(true) that
deregisters the umtx shared region (from 'umtx_shm_registry';
umtx_shm_find_reg() only finds registered shared mutexes).
Thus, two concurrent user-space callers of _umtx_op() with UMTX_OP_SHM
and flags UMTX_SHM_DESTROY, both progressing past umtx_shm_find_reg()
but before umtx_shm_unref_reg(true), would then decrease twice the
reference count for the single reference standing for the shared mutex's
registration.
Reported by: Synacktiv
Reviewed by: kib
Approved by: emaste (mentor)
Security: FreeBSD-SA-24:14.umtx
Security: CVE-2024-43102
Security: CAP-01
Sponsored by: The Alpha-Omega Project
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46126
(cherry picked from commit 62f40433ab47ad4a9694a22a0313d57661502ca1)
(cherry picked from commit be7dc46139)
Approved by: so
...into the only USHMF_LINKED, as they are always set or unset together.
This is both to stop giving the impression that they can be set/unset
independently, which they can't with the current code, and to make it
clearer that an upcoming reference counting fix is correct.
Reviewed by: kib
Approved by: emaste (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46126
(cherry picked from commit dd83da532c36830a0c0aac624903849262ec6f68)
(cherry picked from commit 2d4511bb81)
Approved by: so
Previously 3 bytes of data from the heap could be leaked to ctl
consumers.
Reported by: Synacktiv
Reviewed by: asomers, mav
Sponsored by: The Alpha-Omega Project
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46091
(cherry picked from commit db87c98168b1605f067d283fa36a710369c3849d)
(cherry picked from commit 131b7dcb2f)
Approved by: so
This vulnerability is directly accessible to a guest VM through the
pci_virtio_scsi bhyve device.
In the function ctl_report_supported_opcodes() accessible from the VM,
the option RSO_OPTIONS_OC_ASA does not check the requested
service_action value before accessing &ctl_cmd_table[].
Reported by: Synacktiv
Reviewed by: asomers
Security: FreeBSD-SA-24:11.ctl
Security: CVE-2024-42416
Security: HYP-06
Sponsored by: The Alpha-Omega Project
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46027
(cherry picked from commit af438acbfde3d25dbdc82b2b3d72380f0191e9d9)
(cherry picked from commit 803e0c2ab2)
Approved by: so
The functions ctl_write_buffer() and ctl_read_buffer() are vulnerable to
a kernel memory disclosure caused by an uninitialized kernel allocation.
If one of these functions is called for the first time for a given LUN, a
kernel allocation is performed without the M_ZERO flag. Then a call to
ctl_read_buffer() returns the content of this allocation, which may
contain kernel data.
Reported by: Synacktiv
Reviewed by: asomers
Reviewed by: jhb
Security: FreeBSD-SA-24:11.ctl
Security: CVE-2024-8178
Security: HYP-05
Sponsored by: The Alpha-Omega Project
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D45952
(cherry picked from commit ea44766b78d639d3a89afd5302ec6feffaade813)
(cherry picked from commit cdfdb3b008)
Approved by: so
The virtio_scsi device allows a guest VM to directly send SCSI commands
to the kernel driver exposed on /dev/cam/ctl. This setup makes the
vulnerability directly accessible from VMs through the pci_virtio_scsi
bhyve device.
The function ctl_write_buffer sets the CTL_FLAG_ALLOCATED flag, causing
the kern_data_ptr to be freed when the command finishes processing.
However, the buffer is still stored in lun->write_buffer, leading to a
Use-After-Free vulnerability.
Since the buffer needs to persist indefinitely, so it can be accessed by
READ BUFFER, do not set CTL_FLAG_ALLOCATED.
Reported by: Synacktiv
Reviewed by: Pierre Pronchery <pierre@freebsdfoundation.org>
Reviewed by: jhb
Security: FreeBSD-SA-24:11.ctl
Security: CVE-2024-45063
Security: HYP-03
Sponsored by: Axcient
Sponsored by: The Alpha-Omega Project
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46424
(cherry picked from commit 670b582db6cb827a8760df942ed8af0020a0b4d0)
(cherry picked from commit 29937d7a1a)
Approved by: so
During unpacking, we ensure that we do not read beyond the
declared size. However, unpack uses a function that copies
null-terminated strings. Prior to this commit, if the last string
was not null-terminated, it could result in copying data into a
buffer smaller than the allocated size.
Security: FreeBSD-24:09.libnv
Security: CVE-2024-45288
Security: CAP-03
Reported by: Synacktiv
Sponsored by: The Alpha-Omega Project
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46138
(cherry picked from commit 3aaaca1b51ad844ef9e9b3d945217ab3dd189bae)
(cherry picked from commit 9c2ef10216)
Approved by: so
Ensure that the calculation of size of array doesn't
overflow.
Security: FreeBSD-24:09.libnv
Security: CVE-2024-45287
Security: CAP-02
Reported by: Synacktiv
Reported by: Taylor R Campbell (NetBSD)
Sponsored by: The Alpha-Omega Project
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46131
(cherry picked from commit 36fa90dbde0060aacb5677d0b113ee168e839071)
(cherry picked from commit 371af89975)
Approved by: so
Some SCTP implementations will abort connections and then later re-use the same
port numbers (i.e. both src and dst) for a new connection, before pf has fully
purged the old connection.
Apply the same hack we already have for similarly misbehaving TCP
implementations and forcibly remove the old state so we can create a new one.
MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 82e021443a76b1f210cfb929a495185179606868)
Unfortunately this will likely resurface CVE-2024-6640 which was what all the
effort was about here in the first place. In this situation, however, the
general stability of ND behaviour is more important than fixing this.
(e.g. traceroute with icmp)
ok henning, jsing
Also extend the test case to cover this scenario.
PR: 280701
Obtained from: OpenBSD
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
In 534ee17e6 pf state checking for ICMP(v6) was made stricter. This change
failed to correctly set the pf_pdesc for ICMP-in-ICMP lookups, resulting in ICMP
error packets potentially being dropped incorrectly.
Specially, it copied the ICMP header into a separate variable, not into the
pf_pdesc.
Populate the required pf_pdesc fields for the embedded ICMP packet's state lookup.
PR: 280701
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
Change PF behavior to allow MLD Listener Report packets to be sent
without needing a previously created state by MLD Listener Query. It
wasn't working because: (1) you might not have a previous MLD Listener
Query and (2) the addresses of the Query and Report don't match.
ok mikeb@, sashan@
Approved by: so
Security: FreeBSD-SA-24:05.pf
Security: CVE-2024-6640
MFC after: 1 day
Obtained From: OpenBSD, rzalamena <rzalamena@openbsd.org>, 5c526dbdb0f2
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 1afe4da75d1d2acd33b25eea942af28aa41c82c2)
(cherry picked from commit 3382c691dc)
In pf_icmp_mapping() the ICMP and ICMPv6 types shared the same
number space. In fact they are independent and must be handled
separately. Fix traceroute via pf by splitting pf_icmp_mapping()
into IPv4 and IPv6 sections.
ok henning@ mcbride@; tested mcbride@; sure deraadt@
Approved by: so
Security: FreeBSD-SA-24:05.pf
Security: CVE-2024-6640
MFC after: 1 day
Obtained From: OpenBSD, bluhm <bluhm@openbsd.org> ef4bccd7509e
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 46755f52247bd34a7f013d6844ed0c673ac0defc)
(cherry picked from commit 7f77305a5b)
Include
the ICMP type in one port of the state key, using the type to determine which
side should be the id, and which should be the type.
Also:
- Handle ICMP6 messages which are typically sent to multicast addresses but
recieve unicast replies, by doing fallthrough lookups against the correct
multicast address.
- Clear up some mistaken assumptions in the PF code:
- Not all ICMP packets have an icmp_id, so simulate one based on other
data if we can, otherwise set it to 0.
- Don't modify the icmp id field in NAT unless it's echo
- Use the full range of possible id's when NATing icmp6 echoy
ok henning marco
testing matthieu todd
Approved by: so
Security: FreeBSD-SA-24:05.pf
Security: CVE-2024-6640
MFC after: 1 day
Obtained From: OpenBSD, mcbride <mcbride@openbsd.org> 70bf7555ef4c
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 534ee17e61ee094ec175703bc50e88ff6587703e)
(cherry picked from commit 2f6b4611b5)
The cloner has the ability to limit the maximum unit. Employ it to do
that rather than roll our own.
No functional change intended.
Reviewed by: kp
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D45767
(cherry picked from commit 07d138afc7e5efee73368459dd047493713056cf)
Some drivers, e.g. if_enc(4), only allow one instance to be created, but
the KPI ifc_attach_cloner() treat zero value of maxunit as not limited,
aka IF_MAXUNIT.
Introduce a new flag IFC_F_LIMITUNIT to indicate that the requested
maxunit is limited and should be respected.
Consumers should use the new flag if there is an intended limit.
Reviewed by: glebius
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D45757
(cherry picked from commit a2cac544a668d2834ed41986aca32b44b9819c89)
In main [1] this warning message is suppressed but no plans to MFC the
change as the message may be still useful for users that upgrade from
older releases to 14.x or 13.x. Well emitting this warning message every
time increasing the fib number is confusing for users not for the feature
`net.add_addr_allfibs`, let's limit it to be printed only once.
1. a48f7a2eb90b fibs: Suppress the WARNING message for setups with multiple fibs
This is a direct commit to stable/14 and stable/13.
PR: 280097
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D46204
These variables are tunables, so in principle they never change at runtime.
That would mean they don't need to be tracked per-vnet.
However, they both can be decreased (back to their default values) if the
memory allocations for their respective tables fail, and these allocations are
per-vnet. That is, it's possible for a few vnets to be started and have the
tuned size for the hash and srchash tables only to have later vnets fail the
initial allocation and fall back to smaller allocations. That would confuse
the previously created vnets (because their actual table size and size/mask
variables would no longer match).
Avoid this by turning these into per-vnet variables.
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 271f146955641857c93705b5b1916b8004e5623c)
Change PF behavior to allow MLD Listener Report packets to be sent
without needing a previously created state by MLD Listener Query. It
wasn't working because: (1) you might not have a previous MLD Listener
Query and (2) the addresses of the Query and Report don't match.
ok mikeb@, sashan@
Approved by: so
Security: FreeBSD-SA-24:05.pf
Security: CVE-2024-6640
MFC after: 1 day
Obtained From: OpenBSD, rzalamena <rzalamena@openbsd.org>, 5c526dbdb0f2
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 1afe4da75d1d2acd33b25eea942af28aa41c82c2)
(cherry picked from commit 3382c691dc)
In pf_icmp_mapping() the ICMP and ICMPv6 types shared the same
number space. In fact they are independent and must be handled
separately. Fix traceroute via pf by splitting pf_icmp_mapping()
into IPv4 and IPv6 sections.
ok henning@ mcbride@; tested mcbride@; sure deraadt@
Approved by: so
Security: FreeBSD-SA-24:05.pf
Security: CVE-2024-6640
MFC after: 1 day
Obtained From: OpenBSD, bluhm <bluhm@openbsd.org> ef4bccd7509e
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 46755f52247bd34a7f013d6844ed0c673ac0defc)
(cherry picked from commit 7f77305a5b)
Include
the ICMP type in one port of the state key, using the type to determine which
side should be the id, and which should be the type.
Also:
- Handle ICMP6 messages which are typically sent to multicast addresses but
recieve unicast replies, by doing fallthrough lookups against the correct
multicast address.
- Clear up some mistaken assumptions in the PF code:
- Not all ICMP packets have an icmp_id, so simulate one based on other
data if we can, otherwise set it to 0.
- Don't modify the icmp id field in NAT unless it's echo
- Use the full range of possible id's when NATing icmp6 echoy
ok henning marco
testing matthieu todd
Approved by: so
Security: FreeBSD-SA-24:05.pf
Security: CVE-2024-6640
MFC after: 1 day
Obtained From: OpenBSD, mcbride <mcbride@openbsd.org> 70bf7555ef4c
Sponsored by: Rubicon Communications, LLC ("Netgate")
(cherry picked from commit 534ee17e61ee094ec175703bc50e88ff6587703e)
(cherry picked from commit 2f6b4611b5)
The NFS RFCs are pretty loose with respect to what characters
can be in a filename returned by a Readdir. However, FreeBSD,
as a POSIX system will not handle imbedded '/' or nul characters
in file names. Also, for NFSv4, the file names "." and ".."
are handcrafted on the client and should not be returned by a
NFSv4 server.
This patch scans for the above in filenames returned by Readdir and
ignores any entry returned by Readdir which has them in it.
Because an imbedded nul would be a string terminator, it was
not possible to code this check efficiently using string(3)
functions.
Approved by: so
Security: FreeBSD-SA-24:07.nfsclient
Security: CVE-2024-6759
Reported by: Apple Security Engineering and Architecture (SEAR)
(cherry picked from commit 026cdaa3b3a92574d9ac3155216e5cc0b0bd4c51)
(cherry picked from commit 9328ded386)
netmap's generic mode tries to improve performance by minimizing mbuf
allocations. In service of this goal, it maintains an extra reference
to the mbuf and polls the counter to see if the driver has released its
reference by calling m_freem(). As a result, the extref destructor is
not called when expected by the netfront driver, and mbufs tags are not
freed.
Modify the tx path to release its mbuf tags promptly when reclaiming tx
descriptors. They are drawn from a fixed-size pool, so otherwise are
quickly exhausted when a netfront interface is in netmap generic mode.
Co-authored by: royger
MFC after: 2 weeks
Fixes: dabb3db7a8 ("xen/netfront: deal with mbuf data crossing a page boundary")
Sponsored by: Cloud Software Group
Sponsored by: Klara, Inc.
Sponsored by: Zenarmor
When we release a multicast address (e.g. on interface shutdown) we may
still have packets queued in inm_scq. We have to free those, or we'll
leak memory.
Reviewed by: glebius
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D43033
This reverts commit fa03d37432caf17d56a931a9e6f5d9b06f102c5b.
This commit caused us to not send IGMP leave messages if the inpcb went
away. In other words: we freed pending packets whenever the socket
closed rather than when the interface (or address) goes away.
Reviewed by: glebius
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D43032
The socket is unused, and not passing it means that there's less to
think about when considering how KTLS is synchronized with the rest of
the socket code. No functional change intended.
Reviewed by: gallatin
MFC after: 2 weeks
Sponsored by: Klara, Inc.
Sponsored by: Stormshield
Differential Revision: https://reviews.freebsd.org/D45675
(cherry picked from commit 5dfca6c375d530908eedb7f103681c2493cf0ca3)
