Lutz Donnerhacke [Wed, 10 Feb 2021 10:47:38 +0000 (11:47 +0100)]
netgraph/ng_bridge: Add counters for the first link, too
For broadcast, multicast and unknown unicast, the replication loop
sends a copy of the packet to each link, beside the first one. This
special path is handled later, but the counters are not updated.
Factor out the common send and count actions as a function.
Jessica Clarke [Wed, 10 Feb 2021 16:41:35 +0000 (16:41 +0000)]
localedef: Fix bootstrapping on Ubuntu 16.04
Glibc's stdlib.h defines various prototypes for GNU extensions that take
a locale_t. Newer versions use locale_t directly and include an internal
bits/types/locale_t.h in order to get its definition, but older versions
include xlocale.h for that, for which our bootstrap version is empty.
Moreover it expects to use the glibc-specific __locale_t type. Thus,
provide dummy definitions of both types in order to ensure the
prototypes don't give any errors, and guard against the header being
inadvertently included between the bootstrapping namespace.h and
un-namespace.h, where locale_t is #define'd.
This header is not used when bootstrapping on FreeBSD and exists solely
to stub out glibc's, so this should have no impact on FreeBSD hosts.
Mark Johnston [Wed, 10 Feb 2021 16:10:27 +0000 (11:10 -0500)]
vm: Honour the "noreuse" flag to vm_page_unwire_managed()
This flag indicates that the page should be enqueued near the head of
the inactive queue, skipping the LRU queue. It is used when unwiring
pages from the buffer cache following direct I/O or after I/O when
POSIX_FADV_NOREUSE or _DONTNEED advice was specified, or when
sendfile(SF_NOCACHE) completes. For the direct I/O and sendfile cases
we only enqueue the page if we decide not to free it, typically because
it's mapped.
Pass "noreuse" through to vm_page_release_toq() so that we actually
honour the desired LRU policy for these scenarios.
Alex Richardson [Wed, 10 Feb 2021 15:25:14 +0000 (15:25 +0000)]
Always build the sanitizer runtimes when compiling with clang
This allows instrumenting e.g. test binaries even when compiling with an
external clang (e.g. CROSS_TOOLCHAIN=llvm11). I have some upcoming patches
that allow building the entire base system with ASan/UBSan/etc.
instrumentation and this is required in preparation for this.
57785538c6e0d7e8ca0f161ab95bae10fd304047 change the test for FreeBSD
from __FreeBSD_version to __FreeBSD__. However this test was performed
before sys/param.h was included, therefore __FreeBSD_version was never
defined. As the test was never true opt_random_ip_id.h was never included.
Michael Tuexen [Tue, 9 Feb 2021 22:35:55 +0000 (23:35 +0100)]
libsysdecode: fix decoding of TCP_NOPUSH and TCP_MD5SIG
TCP_FASTOPEN_MIN_COOKIE_LEN was incorrectly registered as a name of
a IPPROTO_TCP level socket option, which overwrote TCP_NOPUSH.
TCP_FASTOPEN_PSK_LEN was incorrectly registered as a name of an
IPPROTO_TCP level socket option, which overwrote TCP_MD5SIG.
Jessica Clarke [Tue, 9 Feb 2021 21:40:24 +0000 (21:40 +0000)]
bsd.compiler.mk: Detect distribution-provided GCC when executed as cc
Clang always prints "clang $VERSION" regardless of the name used to
execute it, whereas GCC prints "$progname $VERSION", meaning if CC is
set to cc and cc is GCC it will print "cc $VERSION". We are able to
detect some of those cases since it then prints "($PKGVERSION)", where
the default is "GCC", but many distributions override that to print
their name and the package version number (e.g. "Debian 10.2.1-6"), so
nothing tells us it's GCC other than the fact that it's not Clang (and
that there's an FSF copyright disclaimer).
However, GCC's -v option will always print "gcc version $VERSION", so
fall back on using that to detect GCC. Whilst Clang also supports this
option, we should never get here, so Clang handling is not added.
netgraph/ng_bridge: Document staleness in multithreaded operation
In the data path of ng_bridge(4), the only value of the host struct,
which needs to be modified, is the staleness, which is reset every
time a frame is received. It's save to leave the code as it is.
This patch is part of a series to make ng_bridge(4) multithreaded.
In a earlier version of ng_bridge(4) the exernal visible host entry
structure was a strict subset of the internal one. So internal view
was a direct annotation of the external structure. This strict
inheritance was lost many versions ago. There is no need to
encapsulate a part of the internal represntation as a separate
structure.
This patch is a preparation to make the internal structure read only
in the data path in order to make ng_bridge(4) multithreaded.
Antonio Russo [Mon, 8 Feb 2021 17:15:05 +0000 (10:15 -0700)]
Set file mode during zfs_write
Apply https://github.com/openzfs/zfs/pull/11576
Direct commit from upstream openzfs. Full commit message below:
Set file mode during zfs_write
3d40b65 refactored zfs_vnops.c, which shared much code verbatim between
Linux and BSD. After a successful write, the suid/sgid bits are reset,
and the mode to be written is stored in newmode. On Linux, this was
propagated to both the in-memory inode and znode, which is then updated
with sa_update.
3d40b65 accidentally removed the initialization of newmode, which
happened to occur on the same line as the inode update (which has been
moved out of the function).
The uninitialized newmode can be saved to disk, leading to a crash on
stat() of that file, in addition to a merely incorrect file mode.
Reviewed-by: Ryan Moeller <ryan@ixsystems.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Antonio Russo <aerusso@aerusso.net>
Closes #11474
Closes #11576
Obtained from: openzfs/zfs@f8ce8aed0
MFC after: 0 days
Sponsored by: iXsystems, Inc.
Alex Richardson [Tue, 19 Jan 2021 11:35:07 +0000 (11:35 +0000)]
usr.bin/grep: Fix Address OOB read error
I found this when compiling all the bootstrap tools with -fsanitize=addres:
==65590==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62d000008400 at pc 0x000000473053 bp 0x7ffc1c7dd910 sp 0x7ffc1c7dd0b8
READ of size 32769 at 0x62d000008400 thread T0
#0 0x473052 in regexec (/local/scratch/alr48/cheri/build/freebsd-amd64-build/local/scratch/alr48/cheri/freebsd/amd64.amd64/tmp/legacy/bin/grep+0x473052)
#1 0x4c9cf3 in procline /local/scratch/alr48/cheri/freebsd/usr.bin/grep/util.c:539:8
#2 0x4c8687 in procfile /local/scratch/alr48/cheri/freebsd/usr.bin/grep/util.c:379:18
#3 0x4c6596 in main /local/scratch/alr48/cheri/freebsd/usr.bin/grep/grep.c:714:8
0x62d000008400 is located 0 bytes to the right of 32768-byte region [0x62d000000400,0x62d000008400)
allocated by thread T0 here:
#0 0x493d5d in malloc (/local/scratch/alr48/cheri/build/freebsd-amd64-build/local/scratch/alr48/cheri/freebsd/amd64.amd64/tmp/legacy/bin/grep+0x493d5d)
#1 0x4cad75 in grep_malloc /local/scratch/alr48/cheri/freebsd/usr.bin/grep/util.c:656:13
#2 0x4c8129 in procfile /local/scratch/alr48/cheri/freebsd/usr.bin/grep/util.c
#3 0x4c6596 in main /local/scratch/alr48/cheri/freebsd/usr.bin/grep/grep.c:714:8
SUMMARY: AddressSanitizer: heap-buffer-overflow (/local/scratch/alr48/cheri/build/freebsd-amd64-build/local/scratch/alr48/cheri/freebsd/amd64.amd64/tmp/legacy/bin/grep+0x473052) in regexec
Toomas Soome [Mon, 8 Feb 2021 18:49:09 +0000 (20:49 +0200)]
loader: remove BORDER_PIXELS
BORDER_PIXELS is left over from picking up the source from illumos
port. Since FreeBSD VT does not use border in terminal size
calculation, there is no reason why should loader use it.
Kyle Evans [Wed, 20 Jan 2021 17:53:05 +0000 (11:53 -0600)]
kern: sonewconn: set so_options before pru_attach()
Protocol attachment has historically been able to observe and modify
so->so_options as needed, and it still can for newly created sockets. 779f106aa169 moved this to after pru_attach() when we re-acquire the
lock on the listening socket.
Restore the historical behavior so that pru_attach implementations can
consistently use it. Note that some pru_attach() do currently rely on
this, though that may change in the future. D28265 contains a change to
remove the use in TCP and IB/SDP bits, as resetting the requested linger
time on incoming connections seems questionable at best.
This does move the assignment out from under the head's listen lock, but
glebius notes that head won't be going away and applications cannot
assume any specific ordering with a race between a connection coming in
and the application changing socket options anyways.
Warner Losh [Mon, 8 Feb 2021 19:29:20 +0000 (12:29 -0700)]
hid: bump HID_ITEM_MAXUSAGES to 8
My YOGA requires a minimum of 7 to parse w/o an error. Since the memory savings
are trivial and the yoga a popular system, bump the default up to 8. There's no
API/ABI issues in doing this. This hid_item struct isn't exported to userland
and the one libusbhid has is different and only shares a name...
MFC After: 3 days
Reviewed by: wulf@
Differential Revision: https://reviews.freebsd.org/D28543
Warner Losh [Mon, 8 Feb 2021 21:43:25 +0000 (14:43 -0700)]
acpi: limit the AMDI0020/AMDI0010 workaround to an option
It appears that production versions of EPYC firmware get the _STA method right
for these nodes. In fact, this workaround breaks on production hardware by
including too many uart nodes. This work around was for pre-release hardware
that wound up not having a large deployment. Move this work around to a kernel
option since the machines that needed it have been powered off and are difficult
to resurrect. Should there be a more significant deployment than is understood,
we can restrict it based on smbios strings.
Discussed with: mmacy@, seanc@, jhb@
MFC After: 3 days
SO_RERROR indicates that receive buffer overflows should be handled as errors.
Historically receive buffer overflows have been ignored and programs
could not tell if they missed messages or messages had been truncated
because of overflows. Since programs historically do not expect to get
receive overflow errors, this behavior is not the default.
This is really really important for programs that use route(4) to keep in sync
with the system. If we loose a message then we need to reload the full system
state, otherwise the behaviour from that point is undefined and can lead
to chasing bogus bug reports.
A BIOS bug may apparently cause the BSP to report that it does not
implement CMCI, with some APs reporting that they do. In this scenario,
avoid a NULL pointer dereference that occurs in cmci_monitor() because
cmc_state was not allocated by the BSP.
Mateusz Guzik [Mon, 8 Feb 2021 17:01:48 +0000 (18:01 +0100)]
amd64: implement strlen in assembly
The C variant in libkern performs excessive branching to find the
non-zero byte instead of using the bsfq instruction. The same code
patched to use it is still slower than the routine implemented here
as the compiler keeps neglecting to perform certain optimizations
(like using leaq).
On top of that the routine can is a starting point for copyinstr
which operates on words instead of bytes.
Kyle Evans [Mon, 8 Feb 2021 18:31:17 +0000 (12:31 -0600)]
grep: fix -A handling in conjunction with -m match limitation
The basic issue here is that grep, when given -m 1, would stop all
line processing once it hit the match count and exit immediately. The
problem with exiting immediately is that -A processing only happens when
subsequent lines are processed and do not match.
The fix here is relatively easy; when bsdgrep matches a line, it resets
the 'tail' of the matching context to the value supplied to -A and
dumps anything that's been queued up for -B. After the current line has
been printed and tail is reset, we check our mcount and do what's
needed. Therefore, at the time that we decide we're doing nothing, we
know that 'tail' of the context is correct and we can simply continue
on if there's still more to pick up.
With this change, we still bail out immediately if there's been no -A
flag. If -A was supplied, we signal that we should continue on. However,
subsequent lines will not even bothere to try and process the line. We
have reached the match count, so even if the next line would match then
we must process it if it hadn't. Thus, the loop in procfile() can
short-circuit and just process the line as a non-match until
procmatches() indicates that it's safe to stop.
A test has been added to reflect both that we should be picking up the
next line and that the next line should be considered a non-match even
if it should have been.
Mark Johnston [Mon, 8 Feb 2021 14:19:27 +0000 (09:19 -0500)]
ktls: Avoid wakeups and locking for synchronous callbacks
When performing encryption in software, the KTLS crypto callback always
locks the session to deliver a wakeup. But, if we're handling the
operation synchronously this is wasted effort and can result in
sleepqueue lock contention on large systems.
Use CRYPTO_SESS_SYNC() to determine whether the operation will be
completed asynchronously or not, and select a callback appropriately.
Avoid locking the session to check for completion if the session handles
requests synchronously.
Mark Johnston [Mon, 8 Feb 2021 14:19:19 +0000 (09:19 -0500)]
opencrypto: Introduce crypto_dispatch_async()
Currently, OpenCrypto consumers can request asynchronous dispatch by
setting a flag in the cryptop. (Currently only IPSec may do this.) I
think this is a bit confusing: we (conditionally) set cryptop flags to
request async dispatch, and then crypto_dispatch() immediately examines
those flags to see if the consumer wants async dispatch. The flag names
are also confusing since they don't specify what "async" applies to:
dispatch or completion.
Add a new KPI, crypto_dispatch_async(), rather than encoding the
requested dispatch type in each cryptop. crypto_dispatch_async() falls
back to crypto_dispatch() if the session's driver provides asynchronous
dispatch. Get rid of CRYPTOP_ASYNC() and CRYPTOP_ASYNC_KEEPORDER().
Similarly, add crypto_dispatch_batch() to request processing of a tailq
of cryptops, rather than encoding the scheduling policy using cryptop
flags. Convert GELI, the only user of this interface (disabled by
default) to use the new interface.
Add CRYPTO_SESS_SYNC(), which can be used by consumers to determine
whether crypto requests will be dispatched synchronously. This is just
a helper macro. Use it instead of looking at cap flags directly.
Fix style in crypto_done(). Also get rid of CRYPTO_RETW_EMPTY() and
just check the relevant queues directly. This could result in some
unnecessary wakeups but I think it's very uncommon to be using more than
one queue per worker in a given workload, so checking all three queues
is a waste of cycles.
Mark Johnston [Mon, 8 Feb 2021 14:19:10 +0000 (09:19 -0500)]
armv8crypto: Extract GCM state into a structure
This makes it easier to refactor the GCM code to operate on
crypto_buffer_cursors rather than plain contiguous buffers, with the aim
of minimizing the amount of copying and zeroing done today.
Mark Johnston [Mon, 8 Feb 2021 14:19:07 +0000 (09:19 -0500)]
armv8crypto: Fix some edge cases in the AES-GCM implementation
- We were only hashing up to the first 16 bytes of the AAD.
- When computing the digest during decryption, handle the case where
len == trailer, i.e., len < AES_BLOCK_LEN, properly.
While here:
- trailer is always smaller than AES_BLOCK_LEN, so remove a pair of
unnecessary modulus operations.
- Replace some byte-by-byte loops with memcpy() and memset() calls.
In particular, zero the full block before copying a partial block into
it since we do that elsewhere and it means that the memset() length is
known at compile time.
Reviewed by: jhb
Sponsored by: Ampere Computing
Submitted by: Klara, Inc.
MFC after: 3 days
Differential Revision: https://reviews.freebsd.org/D28501
kldload(8): Add note about using kld_list in rc.conf(5)
While here, also recommend that loader.conf(5) should only be used in
order to get to mountroot, as rc(8) is less fragile, faster, and is
easier to fix by booting to single-user mode instead of having to
blacklist modules in the loader.
epoll: Store epoll_event udata member in ext member of kevent.
Current epoll implementation stores udata fields of epoll_event
structure in special dynamically-sized table rather than in udata field
of backing kevent structure because of 2 reasons:
1. Kevent's udata size is smaller than epoll's on 32-bit archs.
2. Kevent's udata can be clobbered on execution EPOLL_CTL_ADD as kqueue
modifies existing event while epoll returns error in this case.
After r320043 has introduced four new 64bit user data members (ext[]),
we can store epoll udata in one of them and drop aforementioned table.
According to kqueue_register() source code ext members are not updated
when existing kevent is modified that fixes p.2.
Brandon Bergren [Sun, 7 Feb 2021 22:05:41 +0000 (16:05 -0600)]
powerpc64: Fix boot on virtual-mode OF (PowerMac G5)
In 78599c32efed3247d165302a1fbe8d9203e38974, CFI endproc decoration was
added to locore64.S. However, it missed the subtle detail that
__restartkernel_virtual() falls through to __restartkernel(). This was
causing boot failure on PowerMac G5, as it tried to execute the
epilogue as code.
Fix this by branching to __restartkernel() instead of intentionally
running off the end of the function.
While here, add some additional notes on how the virtual mode restart
works.
Lutz Donnerhacke [Wed, 13 Jan 2021 22:18:55 +0000 (23:18 +0100)]
netgraph/ng_bridge: Make simple internal functions read-only
The data path in netgraph is designed to work on an read only state of
the whole netgraph network. Currently this is achived by convention,
there is no technical enforcment. In the case of NETGRAPH_DEBUG all
nodes can be annotated for debugging purposes, so the strict
enforcment needs to be lifted for this purpose.
This patch is part of a series to make ng_bridge multithreaded, which
is done by rewrite the data path to operate on const.
Lutz Donnerhacke [Wed, 13 Jan 2021 22:18:55 +0000 (23:18 +0100)]
netgraph/ng_bridge: Make simple internal functions read-only
The data path in netgraph is designed to work on an read only state of
the whole netgraph network. Currently this is achived by convetion,
there is no technical enforcment. This patch is part of a series to
make ng_brigde multithreaded, which is done by rewrite the data path
to const handling.
This was announced to happen after the 12 relases.
Remove a depeciated ABI.
The complete removal is for HEAD only. I'll remove the #define in
stable/13 as MFC, so the code will still exist in 13.x, but will not
included by default. Earlier versions will not be affected.
Reviewed by: kp
MFC after: 5 days
Differential Revision: https://reviews.freebsd.org/D28518
linux: fix handling of flags for 32 bit send(2) syscall
Previously the flags were passed as-is, which could resulted
in spurious EAGAIN returned for non-blocking sockets, which
broke some Steam games.
PR: 248065
Reported By: Alex S <iwtcex@gmail.com>
Tested By: Alex S <iwtcex@gmail.com>
Reviewed By: emaste
MFC After: 3 days
Sponsored By: The FreeBSD Foundation
netgraph/ng_bridge: Derive forwarding mode from first attached hook
Handling of unknown MACs on an bridge with incomplete learning
capabilites (aka uplink ports) can be defined in different ways.
The classical approach is to broadcast unicast frames send to an
unknown MAC, because the unknown devices can be everywhere. This mode
is default for ng_bridge(4).
In the case of dedicated uplink ports, which prohibit learning of MAC
addresses in order to save memory and CPU cycles, the broadcast
approach is dangerous. All traffic to the uplink port is broadcasted
to every downlink port, too. In this case, it's better to restrict the
distribution of frames to unknown MAC to the uplink ports only.
In order to keep the chance small and the handling as natural as
possible, the first attached link is used to determine the behaviour
of the bridge: If it is an "uplink" port, then the bridge switch from
classical mode to restricted mode.
netgraph/ng_bridge: Introduce "uplink" ports without MAC learning
The ng_bridge(4) node is designed to work in moderately small
environments. Connecting such a node to a larger network rapidly fills
the MAC table for no reason. It even become complicated to obtain data
from the gettable message, because the result is too large to
transmit.
This patch introduces, two new functionality bits on the hooks:
- Allow or disallow MAC address learning for incoming patckets.
- Allow or disallow sending unknown MACs through this hook.
Uplinks are characterized by denied learing while sending out
unknowns. Normal links are charaterized by allowed learning and
sending out unknowns.
Cy Schubert [Fri, 5 Feb 2021 14:26:04 +0000 (06:26 -0800)]
ipfilter: Use the softn (NAT softc) host map size in ip_nat6 calculation.
The ipfilter NAT table host map size is a tunable that defaults to
a macro value defined at build time. HOSTMAP_SIZE is saved in softn
(the ipnat softc) at initialization. It can be tuned (changed) at runtime
using the ipf -T command. If the hostmap_size tunable is adjusted the
calculation to determine where to put new entries in the table was
incorrect. Use the tunable in the NAT softc instead of the static build
time value.
Piotr Kubaj [Sat, 6 Feb 2021 02:21:55 +0000 (03:21 +0100)]
powerpc64le: readd COMPAT_FREEBSD11 and COMPAT_FREEBSD12
lang/rust needs COMPAT_FREEBSD11 to build, even though powerpc64le itself is supported only since 13.0.
I also corrected a comment, because if we ever have lib32 for powerpc64le, it will be for powerpcle.
Mateusz Guzik [Fri, 5 Feb 2021 23:16:55 +0000 (00:16 +0100)]
cache: remove the largely obsolete general description
Examples of inconsistencies with the current state:
- references LRU of all entries, removed years ago
- references a non-existent lock (neglist)
- claims negative entries have a NULL target
It will be replaced with a more accurate and more informative
description.
In the meantime take it out so it stops misleading.
Glen Barber [Fri, 5 Feb 2021 16:46:49 +0000 (11:46 -0500)]
release: disable the 'reldoc' target after the ASCIIDoctor switch
The 'reldoc' target includes release-related documentation on
installation medium. Since the switch from XML to ASCIIDoctor,
the file locations have moved, and it will take some time to sort
out how this target should work now.
MFC after: 3 days
Sponsored by: Rubicon Communications, LLC ("Netgate")
Mark Johnston [Fri, 5 Feb 2021 14:44:47 +0000 (09:44 -0500)]
tools/git: Add git-arc
This is a handy script for creating and updating Differential revisions
from git commits. It tries to avoid forcing the user to manage their
git tree in any particular way, but makes two major assumptions:
- there is a one-to-one mapping between git commits and Differential
revisions,
- the title of a Differential revision is the same as the summary line
of the corresponding commit.
A verbose description of the script's functionality is provided in its
usage message, which should probably be converted to a man page.
A description of workflows using git-arc is here:
https://lists.freebsd.org/pipermail/freebsd-hackers/2021-January/056979.html
There are some loose ends but this is functional enough to be useful.
Andrew Turner [Fri, 5 Feb 2021 10:50:29 +0000 (10:50 +0000)]
Add support for arm64 nGnRE device memory
On arm64 we can select how strongly we order device memory. Currently
we use the strongest type of non-Gathering, non-Reordering, no Early
write acknowledgement. This is equivalent to VM_MEMATTR_SO in the 32-bit
arm code.
Create a new memory type to remove the no Early write acknowledgement
option to create a memory attribute that is equivalent to the arm
VM_MEMATTR_DEVICE.
Keep the the old nGnRnE memory as what we provide for VM_MEMATTR_DEVICE
until we can test nGnRE on more hardware. A method for dynamically
switching back may be needed as at least one vendor is known to have
broken nGnRE memory.
Kyle Evans [Thu, 4 Feb 2021 21:26:45 +0000 (15:26 -0600)]
grep: fix null pattern and empty pattern file behavior
The null pattern semantics were terrible because I tried to match gnugrep,
but I got it wrong. Let's unwind that:
- The null pattern should match every line if neither -w nor -x.
- The null pattern should match empty lines if -x.
- The null pattern should not match any lines if -w.
The first two will stop processing (shortcut) even if additional patterns
are specified. In any other case, we will continue processing other
patterns. If no other patterns are specified beside a null pattern, then
we match if neither -w nor -x or set and do not match if either of those
are specified.
The justification for -w is that it should match on a whole word, but the
null pattern deos not have a whole word to match on.
Empty pattern files should never match anything, and more importantly, -v
should cause everything to be written.
X700 family of controllers has limited number of available VLAN
HW filters. Driver did not handle properly a case when user
assigned more VLANs to the interface which had all filters
already in use. Fix that by disabling HW filtering when
it is impossible to create filters for all requested VLANs.
Keep track of registered VLANs using bitstring to be able
to re-enable HW filtering when number of requested VLANs
drops below the limit.
Also switch all allocations to use M_IXL malloc type
to ease detecting memory leaks in the driver.
Toomas Soome [Thu, 4 Feb 2021 20:49:02 +0000 (22:49 +0200)]
loader.efi: There are systems without ConOut, also use ConOutDev
Conout does contian the default output device name.
ConOutDev does contain all possible output device names, so we can
use it as fallback, when there is no ConOut.
Ryan Stone [Fri, 29 Jan 2021 20:48:50 +0000 (15:48 -0500)]
Fix mismerge in OFED update
When OFED was upgraded to Linux v4.9, a bunch of Linux-specific
netlink changes were dropped. Unfortunately, there was a mismerge
in this process and as a result ib_sa_cancel_query() would fail to
cancel an outstanding MAD.
This was causing rdma_destroy_id() to hang indefinitely waiting
for the MAD to complete and release the final reference.
Ryan Stone [Thu, 7 Jan 2021 17:25:56 +0000 (12:25 -0500)]
Fix race condition in linuxkpi workqueue
Consider the following scenario:
1. A delayed_work struct in the WORK_ST_TIMER state.
2. Thread A calls mod_delayed_work()
3. Thread B (a callout thread) simultaneously calls
linux_delayed_work_timer_fn()
The following sequence of events is possible:
A: Call linux_cancel_delayed_work()
A: Change state from TIMER TO CANCEL
B: Change state from CANCEL to TASK
B: taskqueue_enqueue() the task
A: taskqueue_cancel() the task
A: Call linux_queue_delayed_work_on(). This is a no-op because the
state is WORK_ST_TASK.
As a result, the delayed_work struct will never be invoked. This is
causing address resolution in ib_addr.c to stop permanently, as it
never tries to reschedule a task that it thinks is already scheduled.
Fix this by introducing locking into the cancel path (which
corresponds with the lock held while the callout runs). This will
prevent the callout from changing the state of the task until the
cancel is complete, preventing the race.