r343407 accidentally introduced a typo (folling -> following). While
reading the change out loud, I realized that the original sentence was
wordy. almost sounding like a run-on sentence.
Improve the flow by splitting up the two thoughts into two distinct sentence
fragments.
PR: 194547, 208497
Reviewed by: emaste
Approved by: emaste (mentor)
MFC after: 1 month
MFC with: r343407
Differential Revision: https://reviews.freebsd.org/D18947
While these warnings are false positives, the use of strdup() instead of
malloc() and strcpy() simplifies and clarifies the code.
While checking the remaining uses of strcpy and strcat I noticed an
assignment of a strlen() to a variable "s", whose value needs to be
preserved for use in later output routines (where it is used to allocate
a buffer). I do not think that the value of "s" will come out lower than
its correct value and thus there is no risk of a buffer overflow, in the
general case, but a specially crafter argument might lead to an overflow.
The bogus assignment to "s" is removed since this value was only used a
single time in the following malloc() call, which has been removed.
MFC after: 2 weeks
Add a bunch of examples on how to use ZFS features like:
- listing available space,
- setting and displaying a userquota,
- displaying pool I/O statistics and pool history,
- displaying the compression ratio for a dataset,
- various list options (sorting, removing headers),
- performing a dry-run of a snapshot delete,
- removing a range of snapshots,
- setting a custom property,
- preventing removal of a snapshot with ZFS holds,
- permission sets for zfs send/receive.
Additionally, clarify the existing examples a bit when
it comes to displaying space by mentioning UFS explicitly.
Other examples include displaying I/O in top(1), querying
sysctl(8) for active CPUs and available RAM. Mention systat(1)
and its options, too.
While here, reformat the example to upload a dmesg(8) a bit
to wrap properly.
Thanks to Allan Jude for his help with some of the ZFS examples.
Reviewed by: dru,allanjude
Approved by: allanjude (earlier version)
MFC after: 3 days
Relnotes: yes (ZFS examples in freebsd-tips)
Differential Revision: https://reviews.freebsd.org/D18541
connectivity.
Looking at past changes in this area like r337866, some refcounting
bugs have been introduced, one by one. For example like calling
in6m_disconnect() and in6m_rele_locked() in mld_v1_process_group_timer()
where previously no disconnect nor refcount decrement was done.
Calling in6m_disconnect() when it shouldn't causes IPv6 solitation to no
longer work, because all the multicast addresses receiving the solitation
messages are now deleted from the network interface.
This patch reverts some recent changes while improving the MLD
refcounting and concurrency model after the MLD code was converted
to using EPOCH(9).
List changes:
- All CK_STAILQ_FOREACH() macros are now properly enclosed into
EPOCH(9) sections. This simplifies assertion of locking inside
in6m_ifmultiaddr_get_inm().
- Corrected bad use of in6m_disconnect() leading to loss of IPv6
connectivity for MLD v1.
- Factored out checks for valid inm structure into
in6m_ifmultiaddr_get_inm().
PR: 233535
Differential Revision: https://reviews.freebsd.org/D18887
Reviewed by: bz (net)
Tested by: ae
MFC after: 1 week
Sponsored by: Mellanox Technologies
because the destructor will access the if_ioctl() callback in the ifnet
pointer which is about to be freed. This prevents use-after-free.
PR: 233535
Differential Revision: https://reviews.freebsd.org/D18887
Reviewed by: bz (net)
Tested by: ae
MFC after: 1 week
Sponsored by: Mellanox Technologies
tinybsd offers two choices when prompting user for MFSROOT: 'YES'
and 'NO'. Script logic only handles 'yes'. Change offered values
to lower case.
PR: 131059
Submitted by: Brock Williams <brock@cotcomsol.com>
MFC after: 1 week
* This hopefully avoids some firmware panics, I was occasionally seeing,
when iwm disconnects upon losing signal to an access point at some point.
* This is synchronizing the if_iwm_time_event.c file a bit more from the
corresponding Linux iwlwifi/mvm/time-event.c.
Taken-From: Linux iwlwifi
Submitted by: Augustin Cavalier <waddlesplash@gmail.com> (Haiku)
Obtained from: DragonFlyBSD (e8cb71584a6a72232c13151d60e57f7f229220eb)
* This is a mix of the OpenBSD Git 7fd9664469d1b717a307eebd74aeececbd3c41cc
change, and syncing with the Linux iwlwifi code.
Taken-From: Linux iwlwifi, and OpenBSD
Submitted by: Augustin Cavalier <waddlesplash@gmail.com> (Haiku)
Obtained from: DragonFlyBSD (706a3044afd27c3fecfdf57bec1695310e53e228)
* 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.
This is inspired by the firmware state maintaining change in OpenBSD's iwm,
by stsp@openbsd.org (OpenBSD Git 0ddb056fb7370664b1d4b84392697cb17d1a414a).
Submitted by: Augustin Cavalier <waddlesplash@gmail.com> (Haiku)
Obtained from: DragonFlyBSD (8a41b10ac639d0609878696808387a6799d39b57)
* While there remove unused IWM_UCODE_TLV_CAPA_LMAC_UPLOAD definition,
which isn't defined in iwlwifi.
Taken-From: Linux iwlwifi
Submitted by: Augustin Cavalier <waddlesplash@gmail.com> (Haiku)
Obtained from: DragonFlyBSD (fd4f9de8bc72ea961e50829b45b59d0549040b7d)
* Remove outdated notifications IWM_SCAN_ABORT_CMD,
IWM_SCAN_START_NOTIFICATION and IWM_SCAN_RESULTS_NOTIFICATION.
* Remove unused enum iwm_scan_complete_status.
* Use the updated FW Api version 3 of struct iwm_scan_results_notif.
* No functional change, since struct iwm_scan_results_notif is never
accessed in iwm at the moment.
Taken-From: Linux iwlwifi commits 1083fd7391e989be52022f0f338e9dadc048b063
and 75118fdb63496e4611ab50380499ddd62b9de69f.
Submitted by: Augustin Cavalier <waddlesplash@gmail.com> (Haiku)
Obtained from: DragonFlyBSD (c947b0b8dc96dabefd63f7b70d53695e36c7b64f)
* Rename some structs and struct members for firmware handling.
Submitted by: Augustin Cavalier <waddlesplash@gmail.com> (Haiku)
Obtained from: DragonFlyBSD (4b1006a6e4d0f61d48c67b46e1f791e30837db67)
* 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().
Submitted by: Augustin Cavalier <waddlesplash@gmail.com> (Haiku)
Obtained from: DragonFlyBSD (67b5e090efb225654815fed91020db6cfc16bb19)
* 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.
Submitted by: Augustin Cavalier <waddlesplash@gmail.com> (Haiku)
Obtained from: DragonFlyBSD (0104ee1f4cb6a2313c00c2526c6ae98d42e5041d)
* Doing the iwm_prepare_card_hw() call in iwm_attach() only on Family 8000
hardware matches the code in Linux iwlwifi.
* While there remove DEFAULT_MAX_TX_POWER definition which is unused, and
has a value different from IWL_DEFAULT_MAX_TX_POWER in iwlwifi.
Submitted by: Augustin Cavalier <waddlesplash@gmail.com> (Haiku)
Obtained from: DragonFlyBSD (e8560f8dc58df12a7c79a6bb4e6ccb156e001085)
* 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.
Submitted by: Augustin Cavalier <waddlesplash@gmail.com> (Haiku)
Obtained from: DragonFlyBSD (5d6b465e288ac5b52d7115688d4e6516acbbea1c)
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
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.
Submitted by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed by: shurd@
Sponsored by: Intel Corporation
Differential Revision: https://reviews.freebsd.org/D18545
I should have only changed the format qualifier with the `size_t` value,
`length`, not the other [`off_t`] value, `dest_file_size`.
MFC after: 1 month
MFC with: r343362, r343365, r343367
Approved by: emaste (mentor; implicit)
Reported by: gcc 8.x
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*`.
MFC after: 1 month
MFC with: r343362
Approved by: emaste (mentor; implicit)
Reviewed by: asomers
Differential Revision: https://reviews.freebsd.org/D18934
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
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.
MFC after: 1 month
Reviewed by: asomers
Approved by: emaste (mentor)
PR: 232210
Sponsored by: Netflix, Inc
Differential Revision: https://reviews.freebsd.org/D18625
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
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
Note that the affected interface is available only to root.
admbugs: 765
Reported by: Vlad Tsyrklevich <vlad@tsyrklevich.net>
Reviewed by: emaste, ram
MFC after: 1 day
Security: Kernel memory disclosure
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D18914
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.
No functional change.
Sponsored by: Dell EMC Isilon
