Kyle Evans [Thu, 24 Jan 2019 03:45:55 +0000 (03:45 +0000)]
iwm - Track firmware state better, and improve handling in iwm_newstate().
* This avoids firmware resets in all the cases in iwm_newstate(). Instead
iwm_bring_down_firmware() is called, which tears down all the STA
connection state, according to the sc->sc_firmware_state value.
* Improve the behaviour of the LED blinking a bit, so it only blinks when
there really is a wireless scan going on.
* Print the newstate arg in debug output of iwm_newstate(), to help in
debugging.
Kyle Evans [Thu, 24 Jan 2019 03:42:59 +0000 (03:42 +0000)]
if_iwm - Check sc->sc_attached flag in suspend/resume callbacks.
* There is (almost) nothing to do in suspend/resume if if_iwm has failed
during initialization (e.g. because of firmware load failure) and was
already uninitialized by iwm_detach_local().
Kyle Evans [Thu, 24 Jan 2019 03:42:23 +0000 (03:42 +0000)]
if_iwm - Move iwm_read_firmware() call into iwm_attach().
* We should load the firmware exactly once before the driver really
initializes the hardware the first time, and unload it at detach time.
There is no need to retrieve the firmware during execution of
iwm_mvm_load_ucode_wait_alive(), we should make sure we already have the
firmware data at hand before that.
* The existing sc_preinit_hook code fails to deal with the case where
if_iwm is loaded by the loader (or is statically linked) and the
firmware needs to be loaded from disk. So we can just call
iwm_read_firmware() from iwm_attach() directly.
* A separate solution will have to be added to properly defer the firmware
loading during bootup, until the necessary filesystem is mounted.
Kyle Evans [Thu, 24 Jan 2019 03:41:09 +0000 (03:41 +0000)]
if_iwm - Update firmware rs table, instead of indexing the table in tx cmds.
* Rather than providing a non-zero index into the firmware RS table,
we should always use index 0 and update the firmware RS table whenever
our chosen tx rate for data-frames changes.
* Send IWM_LQ_CMD updates when the tx rate gets updated by the net80211
rate control (which is after we tell the tx status to the net80211
rate-control in iwm_mvm_rx_tx_cmd_single()).
* Disregard frames transferred with a different tx rate than the currently
selected rate for the rate-control calculations. This way we avoid
counting management frames (which are sent at a slow, and fixed rate),
as well as frames we added to the tx queue just before a new IWM_LQ_CMD
update took effect.
Eric Joyner [Thu, 24 Jan 2019 01:08:37 +0000 (01:08 +0000)]
ixl(4): Fix handling data passed with ioctl from NVM update tool
From Krzysztof:
Ensure that the entire data buffer passed from the NVM update tool is copied in
to kernel space and copied back out to user space using copyin() and copyout().
PR: 234104
Submitted by: Krzysztof Galazka <krzysztof.galazka@intel.com>
Reported by: Finn <ixbug@riseup.net>
MFC after: 5 days
Sponsored by: Intel Corporation
Differential Revision: https://reviews.freebsd.org/D18817
Eric Joyner [Thu, 24 Jan 2019 01:03:00 +0000 (01:03 +0000)]
intel iflib drivers: correct initialization of tx_cidx_processed
From Jake:
In r341156 ("Fix first-packet completion", 2018-11-28) a hack to work
around a delta calculation determining how many descriptors were used
was added to ixl_isc_tx_credits_update_dwb.
The same fix was also applied to the em and igb drivers in r340310, and
to ix in r341156.
The hack checked the case where prev and cur were equal, and then added
one. This works, because by the time we do the delta check, we already
know there is at least one packet available, so the delta should be at
least one.
However, it's not a complete fix, and as indicated by the comment is
really a hack to work around the real bug.
The real problem is that the first time that we transmit a packet,
tx_cidx_processed will be set to point to the start of the ring.
Ultimately, the credits_update function expects it to point to the
*last* descriptor that was processed. Since we haven't yet processed any
descriptors, pointing it to 0 results in this incorrect calculation.
Fix the initialization code to have it point to the end of the ring
instead. One way to think about this, is that we are setting the value
to be one prior to the first available descriptor.
Doing so, corrects the delta calculation in all cases. The original fix
only works if the first packet has exactly one descriptor. Otherwise, we
will report 1 less than the correct value.
As part of this fix, also update the MPASS assertions to match the real
expectations. First, ensure that prev is not equal to cur, since this
should never happen. Second, remove the assertion about prev==0 || delta
!= 0. It looks like that originated from when the em driver was
converted to iflib. It seems like it was supposed to ensure that delta
was non-zero. However, because we originally returned 0 delta for the
first calculation, the "prev == 0" was tacked on.
Instead, replace this with a check that delta is greater than zero,
after the correction necessary when the ring pointers wrap around.
This new solution should fix the same bug as r341156 did, but in a more
robust way.
Enji Cooper [Wed, 23 Jan 2019 23:06:39 +0000 (23:06 +0000)]
Unbreak the gcc build with sendfile_test after r343362
gcc 8.x is more pedantic than clang 7.x with format strings and the tests
passed `void*` variables while supplying `%s` (which is technically
incorrect).
Make the affected `void*` variables use `char*` storage instead to address
this issue, as the compiler will upcast the values to `char*`.
Mark Johnston [Wed, 23 Jan 2019 22:18:23 +0000 (22:18 +0000)]
Fix an LLE lookup race.
After the afdata read lock was converted to epoch(9), readers could
observe a linked LLE and block on the LLE while a thread was
unlinking the LLE. The writer would then release the lock and schedule
the LLE for deferred free, allowing readers to continue and potentially
schedule the LLE timer. By the point the timer fires, the structure is
freed, typically resulting in a crash in the callout subsystem.
Fix the problem by modifying the lookup path to check for the LLE_LINKED
flag upon acquiring the LLE lock. If it's not set, the lookup fails.
PR: 234296
Reviewed by: bz
Tested by: sbruno, Victor <chernov_victor@list.ru>,
Mike Andrews <mandrews@bit0.com>
MFC after: 3 days
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D18906
Enji Cooper [Wed, 23 Jan 2019 22:00:17 +0000 (22:00 +0000)]
Add [initial] functional tests for sendfile(2) as lib/libc/sys/sendfile
These testcases exercise a number of functional requirements for sendfile(2).
The testcases use IPv4 and IPv6 domain sockets with TCP, and were confirmed
functional on UFS and ZFS. UDP address family sockets cannot be used per the
sendfile(2) contract, thus using UDP sockets is outside the scope of
testing the syscall in positive cases. As seen in
`:s_negative_udp_socket_test`, UDP is used to test the sendfile(2) contract
to ensure that EINVAL is returned by sendfile(2).
The testcases added explicitly avoid testing out `SF_SYNC` due to the
complexity of verifying that support. However, this is a good next logical
item to verify.
The `hdtr_positive*` testcases work to a certain degree (the header
testcases pass), but the trailer testcases do not work (it is an expected
failure). In particular, the value received by the mock server doesn't match
the expected value, and instead looks something like the following (using
python array notation):
`trailer[:]message[1:]`
instead of:
`message[:]trailer[:]`
This makes me think there's a buffer overrun issue or problem with the
offset somewhere in the sendfile(2) system call, but I need to do some
other testing first to verify that the code is indeed sane, and my
assumptions/code isn't buggy.
The `sbytes_negative` testcases that check `sbytes` being set to an
invalid value resulting in `EFAULT` fails today as the other change
(which checks `copyout(9)`) has not been committed [1]. Thus, it
should remain an expected failure (see bug 232210 for more details
on this item).
Next steps for testing sendfile(2):
1. Fix the header/trailer testcases so that they pass.
2. Setup if_tap interface and test with it, instead of using "localhost", per
@asomers's suggestion.
3. Handle short recv(2)'s in `server_cat(..)`.
4. Add `SF_SYNC` support.
5. Add some more negative tests outside the scope of the functional contract.
Mark Johnston [Wed, 23 Jan 2019 20:02:17 +0000 (20:02 +0000)]
Remove extraneous setutxent() calls in write(1).
We already call setutxent() once during initialization. Furthermore,
the subsequent calls occur after the process has entered capability
mode, so they fail, and attempts to fetch database entries fail as
a result.
PR: 235096
Submitted by: fullermd@over-yonder.net
MFC after: 3 days
Mark Johnston [Wed, 23 Jan 2019 18:58:15 +0000 (18:58 +0000)]
Correct uma_prealloc()'s use of domainset iterators after r339925.
The iterator should be reinitialized after every successful slab
allocation. A request to advance the iterator is interpreted as
an allocation failure, so a sufficiently large preallocation would
cause the iterator to believe that all domains were exhausted,
resulting in a sleep with the keg lock held. [1]
Also, keg_alloc_slab() should pass the unmodified wait flag to the
item initialization routine, which may use it to perform allocations
from other zones.
Reported and tested by: slavah
Diagnosed by: kib [1]
Reviewed by: kib
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
Conrad Meyer [Wed, 23 Jan 2019 16:44:21 +0000 (16:44 +0000)]
gmirror: Relocate DEVICE_FLAGS to adjacent lines
gmirror's sc_flags is shared between some on-disk state and some runtime
only state. There's no real reason for that and they could probably be
split up. Until they are, locate all of the flags for the same field
nearby each other in the source, for clarity.
netmap: improvements to the netmap kloop (CSB mode)
Changelist:
- Add the proper memory barriers in the kloop ring processing
functions.
- Fix memory barriers usage in the user helpers (nm_sync_kloop_appl_write,
nm_sync_kloop_appl_read).
- Fix nm_kr_txempty() helper to look at rhead rather than rcur. This
is important since the kloop can read a value of rcur which is ahead
of the value of rhead (see explanation in nm_sync_kloop_appl_write)
- Remove obsolete ptnetmap_guest_write_kring_csb() and
ptnet_guest_read_kring_csb(), and update if_ptnet(4) to use those.
- Prepare in advance the arguments for netmap_sync_kloop_[tr]x_ring(),
to make the kloop faster.
- Provide kernel and user implementation for nm_ldld_barrier() and
nm_ldst_barrier()
netmap: fix knote() argument to match the mutex state
The nm_os_selwakeup function needs to call knote() to wake up kqueue(9)
users. However, this function can be called from different code paths,
with different lock requirements.
This patch fixes the knote() call argument to match the relavant lock state.
Also, comments have been updated to reflect current code.
net80211: fix channel list construction for non-auto operating mode.
Change the way how channel list mode <-> desired mode match is done:
- Match channel list mode for next non-auto desired modes:
* 11b: 11g, 11ng, 11acg;
* 11a: 11na, 11ac
- Add pre-defined channels only when one of the next conditions met:
* the desired channel mode is 'auto' or
* the desired channel and selected channel list modes are exactly
the same or
* the previous rule (11g / 11n / 11ac promotion) applies.
Before r275875 construction work properly for all except
11ng / 11na / 11acg / 11ac modes - these were broken at all
(i.e., the scan list was empty); after r275875 all checks were removed,
so scan table was populated by all device-compatible channels
(desired mode was ignored).
For example, if I will set 'ifconfig wlan0 mode 11ng' for RTL8821AU:
- pre-r275875: nothing, scan will not work;
- after r275875: both 11ng and 11na bands were scanned; also, since 11b
channel list was used, 14th channel was scanned too.
- after this change: only 11ng - 1-13 channels - are used for scanning.
Tested with:
* RTL8188EE, STA mode.
* RTL8821AU, STA mode.
Stefan Eßer [Wed, 23 Jan 2019 10:05:27 +0000 (10:05 +0000)]
Silence Clang Scan warning about use of unitialized variable.
While the warning is a false positive, it is possible to clarify the code by
always initializing the variable. This does also allow to make the sending
of the "beep" control sequence depend on the validity of its parameters.
I have left the redundant assignment of 0 to the now initialized variables
in place since this makes the code simpler to understand and does not add
any run-time overhead (the compiler completely removes the "else if" test
and the assignments).
There was an embedded literal escape character in a string, which messes up
diplaying the source code on a terminal that interprets ANSI sequences. The
literal escape has been replaced by \e (non-standard, but supported by all
relevant compilers, and already used in other source files in base).
Fix systat's :only command parser for the multiple arguments case
According to systat(1) :only option is supposed to accept multiple drives
but the parser for its arguments stops after first entry. Fix the parser
logic to accept multiple drives.
PR: 59220
Reported by: Andy Farkas <andyf@speednet.com.au>
MFC after: 1 week
Previously, we directly used libzfs_core's lzc_receive to import to a
temporary snapshot, then cloned the snapshot and setup the properties. This
failed when attempting to import replication streams with questionable
error.
libzfs's zfs_receive is a much better fit here, so we now use it instead
with the destination dataset and let libzfs take care of the dirty details.
be_import is greatly simplified as a result.
Reported by: Marie Helene Kvello-Aune <freebsd@mhka.no>
MFC after: 1 week
Stefan Eßer [Tue, 22 Jan 2019 13:11:15 +0000 (13:11 +0000)]
Silence a CI warning regarding the use of strcpy().
While this is a false positive (a sufficiently large buffer has been
allocated in the line above), the use of strdup() simplifies and clarifies
the code.
[ndis] Fix unregistered use of FPU by NDIS in kernel on amd64
amd64 miniport drivers are allowed to use FPU which triggers "Unregistered use
of FPU in kernel" panic.
Wrap all variants of MSCALL with fpu_kern_enter/fpu_kern_leave. To reduce
amount of allocations/deallocations done via
fpu_kern_alloc_ctx/fpu_kern_free_ctx maintain cache of fpu_kern_ctx elements.
Kristof Provost [Tue, 22 Jan 2019 02:13:33 +0000 (02:13 +0000)]
pf: Validate psn_len in DIOCGETSRCNODES
psn_len is controlled by user space, but we allocated memory based on it.
Check how much memory we might need at most (i.e. how many source nodes we
have) and limit the allocation to that.
Patrick Kelsey [Tue, 22 Jan 2019 00:50:24 +0000 (00:50 +0000)]
Reduce pf.conf parsing cost for configs that define N queues from O(N^2) to O(N)
The number of syscalls made during parsing of any config that
defines tables is also reduced, and incorrect warnings that HFSC
parent queue bandwidths were smaller than the sum of their child
bandwidths have been fixed.
Mark Johnston [Mon, 21 Jan 2019 19:38:53 +0000 (19:38 +0000)]
Optimize RISC-V copyin(9)/copyout(9) routines.
The existing copyin(9) and copyout(9) routines on RISC-V perform only a
simple byte-by-byte copy. Improve their performance by performing
word-sized copies where possible.
Navdeep Parhar [Mon, 21 Jan 2019 18:42:16 +0000 (18:42 +0000)]
cxgbe(4): Allow negative values in hw.cxgbe.fw_install and take them to
mean that the driver should taste the firmware in the KLD and use that
firmware's version for all its fw_install checks.
The driver gets firmware version information from compiled-in values by
default and this change allows custom (or older/newer) firmware modules
to be used with the stock driver.
Ed Maste [Mon, 21 Jan 2019 17:12:16 +0000 (17:12 +0000)]
linuxulator: fix stack memory disclosure in linux_sigaltstack
Most siginfo_to_lsiginfo callers already zeroed the l_siginfo_t before
callit it, but linux_waitid did not. Instead of zeroing in the called
function to address linux_waitid (as in commit 2e6ebe70), just do it in
linux_waitid.
admbugs: 765
Reported by: Vlad Tsyrklevich <vlad@tsyrklevich.net>
Reviewed by: Andrew
MFC after: 1 day
Security: Kernel stack memory disclosure
Sponsored by: The FreeBSD Foundation
Kyle Evans [Mon, 21 Jan 2019 14:35:36 +0000 (14:35 +0000)]
awg: fix soft reset failure with no link
U-Boot will leave the ephy reset de-asserted and the MAC soft reset will
fail on these boards with internal PHY and no link established. Toggle reset
again before proceeding to attach/init.
Fix duplicate wpa_supplicant(8) / hostapd(8) startup with devd(8)
Do not invoke 'wlan_up' function from devd(8) on interface
creation event (an example to create such event:
'ifconfig wlan0 create wlandev rtwn0');
they're typically produced during 'service netif (re)start'
and result in duplicate interface initialization.
From the user side if WPA option is used, this result in messages like:
- /etc/rc.d/wpa_supplicant: WARNING: failed to start wpa_supplicant
or
- wpa_supplicant already running? (pid=xxxx).
(for HOSTAP interfaces this race may result in startup failure).
As a side effect, wpa_supplicant(8) / hostapd(8) will not be
invoked when new wlan(4) interface is created manually and
corresponding configuration for it is present in rc.conf(5).
This change does not affect device attach / removal events.
- Check if buffer can contain Rx descriptor before accessing it.
- Verify upper / lower bounds for frame length.
- Do not pass too short frames into ieee80211_find_rxnode().
While here:
- Move cleanup to the function end.
- Reuse IEEE80211_IS_DATA() macro.
- Discard frames that are bigger than MCLBYTES (to prevent buffer overrun).
- Check buffer length before accessing its contents.
- Fix len <-> dmalen check - the last includes Rx Wireless information
structure size.
- Fix out-of-bounds read during Rx node search for ACK / CTS frames
(monitor mode only).
While here:
- Mark few suspicious places with comments.
- Move common cleanup to the function end.
Navdeep Parhar [Sun, 20 Jan 2019 23:30:16 +0000 (23:30 +0000)]
cxgbe(4): Clear the reply-pending status of a hashfilter when the reply
indicates an error. Also, do not remove it twice from the hf list in
this case.
Don't mess with BLOCKSIZE in shell startup files - it's set by login.conf(5);
there's no need to even mention it in shell rc files. Not that it's wrong;
just pointless and somewhat misleading.
Jilles Tjoelker [Sun, 20 Jan 2019 14:25:25 +0000 (14:25 +0000)]
sh: Send libedit "ferr" output to fd 2
The libedit "fout" output must be sent to fd 2 since it contains prompts
that POSIX says must be sent to fd 2. However, the libedit "ferr" output
receives error messages such as from "bind" that make no sense to send to fd
1.
net80211: resolve ioctl <-> detach race for ieee80211com structure
Since r287197 ieee80211com is a part of drivers softc; as a result,
after detach all pointers to it (iv_ic, ni_ic) are invalid. Most
possible users (tasks, interrupt handlers) are blocked / removed
when device is stopped; however, ioctl handlers were not tracked
and may crash if ieee80211com structure is accessed.
Since ieee80211com pointer access from ieee80211vap structure is not
protected by lock (constant after interface creation) and used in
many other places just use reference counting for ioctl handlers;
on detach set 'detached' flag and wait until reference counter goes to 0.
For HEAD ieee80211vap size was changed (__FreeBSD_version bumped);
however, in stable branches I'm going to split / reuse the last
iv_spare field for KBI stability.
Toomas Soome [Sat, 19 Jan 2019 20:45:41 +0000 (20:45 +0000)]
ixgbe: this statement may fall through warnings with gcc
The recent gcc versions (7 and 8 at least) can check for switch case
statements for fall through (implicit-fallthrough). When fall through
is intentional, the default method for warning suppression is to place
comment /* FALLTHROUGH */ exactly before next case statement.
net80211: drop m_pullup call from ieee80211_crypto_decap.
For most wireless drivers Rx mbuf is allocated as one
contiguous chunk; only few are using chains for allocations -
but even then at least MCLBYTES (minus Rx descriptor size) is
available in the first mbuf.
In addition to the above, m_pullup was never called here - otherwise,
reallocation will break post-crypto_decap logic (ieee80211_decap,
ieee80211_deliver_data...), so just remove it; length check is left
in case if some truncated frame appears here.
Justin Hibbits [Sat, 19 Jan 2019 05:20:31 +0000 (05:20 +0000)]
dpaa: fix 32-bit build
Book-E powerpc uses 64-bit vm_paddr_t, and 32-bit powerpc has 32-bit pointers,
so gcc errors with cast to pointer from integer of different size. As this will
not actually be used in reality anyway, simply quiet the warning by casting
through uintptr_t.
Justin Hibbits [Sat, 19 Jan 2019 04:54:12 +0000 (04:54 +0000)]
powerpc/dpaa: Handle DMAP virtual addresses in DPAA
sendfile(2) appears to now use DMAP wherever possible. These addresses are not
managed by pmap, so pmap_kextract() returns a 0 physical address, causing
failure.
Justin Hibbits [Sat, 19 Jan 2019 04:47:19 +0000 (04:47 +0000)]
powerpc: Fix opaque irq data initialization
The powerpc_intr structure is not zero-initialized, so on an invariants
build would panic in the xics driver with an invalid pointer. Also fix the
xics driver to share the private data setup code between xics_enable() and
xics_bind().
Conrad Meyer [Fri, 18 Jan 2019 23:54:51 +0000 (23:54 +0000)]
vmm(4): Mask Spectre feature bits on AMD hosts
For parity with Intel hosts, which already mask out the CPUID feature
bits that indicate the presence of the SPEC_CTRL MSR, do the same on
AMD.
Eventually we may want to have a better support story for guests, but
for now, limit the damage of incorrectly indicating an MSR we do not yet
support.
Eventually, we may want a generic CPUID override system for
administrators, or for minimum supported feature set in heterogenous
environments with failover. That is a much larger scope effort than
this bug fix.
Justin Hibbits [Fri, 18 Jan 2019 23:26:31 +0000 (23:26 +0000)]
Fix top(1) long options handling
getopt_long(3) requires the long options be terminated by a NULL block.
Without the terminator, an invalid long option results in a segmentation
fault.