vm: fix cdev pager object lifecycle and allocation race
cdev_pager_allocate() previously called cdev_pg_ctor before looking up
the object. Repeated allocations therefore called a non-idempotent
constructor multiple times for one allocated vm_object, while the
destructor ran only once.
In addition, cdev_pager_allocate() could race with other threads
allocating an object.
This commit fixes both problems. Reserve a newly allocated object in
the pager list with a NULL ops pointer while its constructor runs
outside dev_pager_mtx. Concurrent lookup and allocation callers wait
for construction to finish. Publish the ops pointer and wake them on
success; remove and mark the object dead before waking them on failure.
Use ops rather than dev as the construction sentinel because DragonFly
has valid NULL-handle pager users.
Derived-from: FreeBSD (commit e93404065177d6c909cd64bf7d74fe0d8df35edf)
GitHub-PR: https://github.com/DragonFlyBSD/DragonFlyBSD/pull/49
vm: Clean up cdev_pager_allocate() a bit
* Introduce 'pindex' variable to clean up the code.
* Add KASSERT() to ensure the object type matches (obtained from FreeBSD)
tcp: Accept a RST whose sequence number is exactly RCV.NXT
An incoming RST is validated against a window anchored on last_ack_sent:
[last_ack_sent, last_ack_sent + rcv_wnd]. When the receiver has
delayed-ACKed data (rcv_nxt > last_ack_sent) and its receive window has
shrunk below that gap (rcv_nxt - last_ack_sent > rcv_wnd), a RST at
rcv_nxt, sits beyond the right edge last_ack_sent + rcv_wnd and is
silently dropped. The caused the connection to stay ESTABLISHED
(half-open), leaving the application to time out and close.
Accept a RST whose sequence number is exactly rcv_nxt, matching the
long-deployed OpenBSD [1] and NetBSD [2] behaviour.
This does not weaken existing RST acceptance: DragonFly already accepts
any in-window RST (RFC 793, no challenge ACK), so an off-path attacker
gains nothing -- the change only stops legitimate RSTs at rcv_nxt from
being dropped. The new behavior also conforms to RFC 793 and RFC 5961
(section 3.2).
[6 lines not shown]
vm: fix cdev pager object lifecycle and allocation race
cdev_pager_allocate() previously called cdev_pg_ctor before looking up
the object. Repeated allocations therefore called a non-idempotent
constructor multiple times for one allocated vm_object, while the
destructor ran only once.
In addition, cdev_pager_allocate() could race with other threads
allocating an object.
This commit fixes both problems. Reserve a newly allocated object in
the pager list with a NULL ops pointer while its constructor runs
outside dev_pager_mtx. Concurrent lookup and allocation callers wait
for construction to finish. Publish the ops pointer and wake them on
success; remove and mark the object dead before waking them on failure.
Use ops rather than dev as the construction sentinel because DragonFly
has valid NULL-handle pager users.
Derived-from: FreeBSD (commit e93404065177d6c909cd64bf7d74fe0d8df35edf)
GitHub-PR: https://github.com/DragonFlyBSD/DragonFlyBSD/pull/49
regression: Add TCP RST receive window test
Note: root is required it it uses BPF to observe the client's final data
segment and a raw IPv4 socket to inject the RST at exactly RCV.NXT.
Before the fix in the last commit:
```
$ ./rst_rcvnxt_window_drop
net.inet.tcp.rfc1323: 1 -> 0
net.inet.tcp.delayed_ack: 1 -> 1
net.inet.tcp.recvbuf_auto: 1 -> 0
FAIL: connection still ESTABLISHED (RST at rcv_nxt was dropped), expected ECONNRESET
net.inet.tcp.rfc1323: 0 -> 1
net.inet.tcp.delayed_ack: 1 -> 1
net.inet.tcp.recvbuf_auto: 0 -> 1
```
After the fix:
```
[12 lines not shown]
tcp: Accept a RST whose sequence number is exactly RCV.NXT
An incoming RST is validated against a window anchored on last_ack_sent:
[last_ack_sent, last_ack_sent + rcv_wnd]. When the receiver has
delayed-ACKed data (rcv_nxt > last_ack_sent) and its receive window has
shrunk below that gap (rcv_nxt - last_ack_sent > rcv_wnd), a RST at
rcv_nxt, sits beyond the right edge last_ack_sent + rcv_wnd and is
silently dropped. The caused the connection to stay ESTABLISHED
(half-open), leaving the application to time out and close.
Accept a RST whose sequence number is exactly rcv_nxt, matching the
long-deployed OpenBSD [1] and NetBSD [2] behaviour.
This does not weaken existing RST acceptance: DragonFly already accepts
any in-window RST (RFC 793, no challenge ACK), so an off-path attacker
gains nothing -- the change only stops legitimate RSTs at rcv_nxt from
being dropped. The new behavior also conforms to RFC 793 and RFC 5961
(section 3.2).
[6 lines not shown]
libc - Remove optimization that breaks malloc_usable_size() use cases
* The malloc implementation had an optimization meant to aid realloc()s
whereby unused portions of large memory blocks could be munmap()d.
However, lots of code uses the space returned by malloc_usable_size()
without calling realloc() to notify libc that additional space is being
used. Leading to segmentation faults.
* Original proposed solution to make malloc_usable_size() aware of a
prior munmap optimization does not completely fix the problem as excess
space may be unmapped after such calls as well as before.
* Removing the optimization and letting the pager deal with any actually-dead
excess space is the only option. So that is what we do.
Reported-by: Several people
bsd.crunchgen.mk: Add CRUNCH_HOSTPROGS_${P} and fix awk in rescue
The usr.bin/awk consists of two subdirs: (1) 'awk' the actual program;
(2) 'maketab' a host program to help generate additional source.
Previously, the initrd/rescue/Makefile pointed the awk source to
'usr.bin/awk', and that caused the following warning:
```
crunchgen: rescue.conf: awk: warning: could not find any .o files
```
And pointing the source to 'usr.bin/awk/awk' couldn't fix it because the
required 'maketab' program would be unavailable.
To fix the problem as well as to extend the crunchgen framework, add the
CRUNCH_HOSTPROGS_${P} variable to specify the host programs required by
the build. Implement the rules and specify the dependencies to build
the host programs.
[2 lines not shown]
initrd: Merge rescue.libcrypto into rescue
After the last commit fixed the 'crc32' symbol conflict between
libhammer and libz, the merge of rescue.libcrypto and rescue only had
one conflict symbol: tilde_expand() from libprivate_ssh and
libprivate_edit. Work around this symbol conflict by setting
libprivate_ssh an internal library for ssh/scp.
This merge reduces the rescue binaries size by ~1.1MB, from 12.1MB
(rescue 5.3MB + rescue.libcrypto 6.8MB) to 11MB.
libhammer(3): Exclude crc32.c and icrc32.c from sys/libkern
The user hammer(8) already includes these sources from sys/libkern, so
don't need to provide them in this library. More importantly, this
avoids the 'crc32' symbol conflict with libz.
crunchgen(1): Improve the handling of internal libraries
Previously, the `libs_int` and `special lib_int` commands specified the
internal libraries by their full paths, following the way of handling of
the shared libraries by `libs_so` and `special lib_so`. However, this
was actually a mistake because the internal libraries require building.
So it was lucky/hacky that rescue/rescue.libcrypto specified the
internal libraries with a combination of source directory and the
library name, e.g.,
```
CRUNCH_INTLIB_grep= ${CRUNCH_PATH_grep}/grep/libgreputils/libgreputils.a
CRUNCH_INTLIB_telnet= ${.CURDIR}/../../lib/libtelnet/libtelnet.a
```
To actually properly support internal libraries, we must explicitly the
source directories, as well as support build options. To this end,
change the `libs_int` and `special lib_int` commands to only specify the
libraries names, and then extend the `special srcdir` to explicitly the
[10 lines not shown]
bsd.crunchgen.mk: Add CRUNCH_HOSTPROGS_${P} and fix awk in rescue
The usr.bin/awk consists of two subdirs: (1) 'awk' the actual program;
(2) 'maketab' a host program to help generate additional source.
Previously, the initrd/rescue/Makefile pointed the awk source to
'usr.bin/awk', and that caused the following warning:
```
crunchgen: rescue.conf: awk: warning: could not find any .o files
```
And pointing the source to 'usr.bin/awk/awk' couldn't fix it because the
required 'maketab' program would be unavailable.
To fix the problem as well as to extend the crunchgen framework, add the
CRUNCH_HOSTPROGS_${P} variable to specify the host programs required by
the build. Implement the rules and specify the dependencies to build
the host programs.
[2 lines not shown]
crunchgen(1): Remove the unneeded '-dc' linker flag
crunchide(1) does not hide symbols by making them local for many years.
We've also been using '-fno-common' compiler flag for a long time. So
the '-dc' linker flag is obsolete. Just remove it.
Obtained-from: FreeBSD (https://reviews.freebsd.org/D34215)
crunchgen(1): Enhance the '-q' (quiet) option to control warnings
Only suppress the warning messages about the component program when the
'-q' option is specified twice.
For example, now the following suppressed warning in crunching
initrd/rescue is shown:
```
crunchgen: rescue.conf: awk: warning: could not find any .o files
```
This also reveals a real problem in the 'awk' component, and I've
created a bug to track it:
https://bugs.dragonflybsd.org/issues/3414
crunchgen(1): Update usage() text and sort options
* Add the missing '-l' option to the usage text.
* Sort the options in the usage text and in the man page.
atomic(9): Add relaxed load/store variants from FreeBSD
Obtained from FreeBSD. However, the original atomic_store_ptr() macro
was causing '-Wcast-qual' warnings, so I changed it based on the
NetBSD's atomic_store_relaxed().
Actually, I found NetBSD's version more clean, but we import the FreeBSD
version to help import code/drivers in the future.
Discussed-with: dillon
hammer2: Disable debug v/f-chain dumps on unmounting
The two dumps were added to track down the HAMMER2-msg leak reported by
malloc_uninit() and would always print the following logs at shutdown:
```
v-chain 0xfffff8008e6204c0 volume.0 00000000000000100000000000000000/0 mir=00000000000
00140
[00002000] (?) refs=1
f-chain 0xfffff8008e620640 freemap.0 00000000000000100000000000000000/0 mir=0000000000
000140
[00002000] (?) refs=1
```
Given that the memory leak has been fixed in commit
bfcedfb468d712f29cadb491bec0928ad4279bad, disable these two debug dumps
now.
ok by dillon.
hammer2: Disable debug v/f-chain dumps on unmounting
The two dumps were added to track down the HAMMER2-msg leak reported by
malloc_uninit() and would always print the following logs at shutdown:
```
v-chain 0xfffff8008e6204c0 volume.0 00000000000000100000000000000000/0 mir=00000000000
00140
[00002000] (?) refs=1
f-chain 0xfffff8008e620640 freemap.0 00000000000000100000000000000000/0 mir=0000000000
000140
[00002000] (?) refs=1
```
Given that the memory leak has been fixed in commit
bfcedfb468d712f29cadb491bec0928ad4279bad, disable these two debug dumps
now.
ok by dillon.
kern: Fix memory leak in kdmsg_iocom_uninit()
When kdmsg_iocom_uninit() is called but no connection was ever
established (no reader/writer threads), a PING message was allocated and
queued but never freed. This caused a 120-byte leak from the HAMMER2-msg
allocator on shutdown.
Fix this by calling kdmsg_drain_msgq() to clean up any queued messages.
This fixes the 'malloc_uninit: 120 bytes of HAMMER2-msg still allocated'
warning. Note that this warning already disappeared after fixing the
hammer2_ioctl_recluster() bug in commit
c5cc81fefa166cc45657541c155797f0cff88c62.
Bug: https://bugs.dragonflybsd.org/issues/3385
kern: Clean up malloc_uninit()
* Correct the diagnostic log message by omitting the CPU id, which
doesn't apply here. The unfreed bytes must be summed from all CPUs.
* Clean up excess INVARIANTS ifdef's by using '__debugvar' instead.
