Use a snprintf to write an environment variable to a fixed-size buffer to
avoid stack overflow.
Reported by: Coverity (CWE-120)
CID: 1238926
Sponsored by: Dell EMC Isilon
The environment variable TMPDIR was copied unchecked into a fixed-size heap
buffer. Use a length-limiting snprintf in place of ordinary sprintf to
prevent the overflow. Long TMPDIR variables can still cause odd truncated
filenames, which may be undesirable.
Reported by: Coverity (CWE-120)
CIDs: 1006706, 1006707
Sponsored by: Dell EMC Isilon
The destination buffer is sized as the sum of program argument lengths, so
it has plenty of room for *argv. Appease Coverity by using strlcpy instead
of strcpy. Similar to a nearby cleanup performed in r316500.
No functional change.
Reported by: Coverity (CWE-120)
CID: 1006703
Sponsored by: Dell EMC Isilon
It may not do the right thing with these obviously wrong inputs, but at
least it won't smash the stack.
Reported by: Coverity (CWE-120)
CIDs: 1006697, 1006698
Sponsored by: Dell EMC Isilon
Right after cross-tools, a compiler-metadata.mk file is created that
stores all of the bsd.compiler.mk metadata. It is then read in
with a fail-safe during installworld time.
The file is explicitly removed when invoking cross-tools to ensure that
a stale file is not left around from odd manual 'make _cross-tools' ->
'make installworld' invocations.
This fixes several issues:
- With WITH_SYSTEM_COMPILER (default yes on head and no on releng/11.0):
If you build on a system where the bootstrap compiler does not
build due to the host compiler matching the in-tree one, but then
installworld on another system where that logic fails (a
bootstrap compiler is needed), the installworld immediately fails
with:
sh: cc: not found
Note that fixing this logic may then hit a case where a rebuild is
attempted in installworld. Normally cc would be ran with
'CFLAGS+=ERROR-tried-to-rebuild-during-make-install' to cause an
error such as:
cc: error: no such file or directory: 'ERROR-tried-to-rebuild-during-make-install'
However, now it will just fail with the 'cc: not found' error.
Inspection of the compile line will show
'ERROR-tried-to-rebuild-during-make-install'; It's not useful to
set CC to anything other than 'cc' during install as it is more
helpful to see the attempted compile rather than some other bogus
error.
- This now avoids running bsd.compiler.mk (cc executions) even more
during installworld. There are compiler-dependent SUBDIR in the
tree which required having a compiler during install.
There is at least 1 case where CC is still executed in the install,
such as from a LOOKUP!= in secure/lib/libcrypto/Makefile.inc checking
for 'vzeroall' support. This is not significant for installworld
as the lookup has a fallback (and hides its error) and only modifies CFLAGS,
thus it's not worth fixing.
PR: 212877
MFC after: 2 weeks
Sponsored by: Dell EMC Isilon
Some NFSv4.1 servers such as AmazonEFS can only support a small fixed number
of open_owner4s. This patch adds a mount option called "oneopenown" that
can be used for NFSv4.1 mounts to make the client do all Opens with the
same open_owner4 string. This option can only be used with NFSv4.1 and
may not work correctly when Delegations are is use.
Reported by: cperciva
Tested by: cperciva
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D8988
A function called svcpool_close() was added to the server side krpc by
r313735, so that a pool could be closed without destroying the data structures.
This little patch adds a call to it for the callback pool (svcpool_nfscbd),
so that the nfscbd daemon can be killed/restarted and continue to work
correctly.
MFC after: 2 weeks
Some dummynet modules used strcpy() to copy from a larger buffer
(dn_aqm->name) to a smaller buffer (dn_extra_parms->name). It happens that
the lengths of the strings in the dn_aqm buffers were always hardcoded to be
smaller than the dn_extra_parms buffer ("CODEL", "PIE").
Use strlcpy() instead, to appease static checkers. No functional change.
Reported by: Coverity
CIDs: 1356163, 1356165
Sponsored by: Dell EMC Isilon
According to the C standard, it is invalid to copy beyond the end of an
object, even if that object is obviously a member of a larger object (a
struct, in this case).
Appease the standard and Coverity by refactoring the copy in a
straightforward way. No functional change.
Reported by: Coverity (CWE-120)
CIDs: 1007819, 1007820, 1007821, 1007822, 1009668, 1009669
Security: no (false positive detection)
Sponsored by: Dell EMC Isilon
When checksums of received IP and UDP header already checked, UDP uses
sbappendaddr_locked() to pass received data to the socket.
sbappendaddr_locked() uses given mbuf as is, and if NIC supports checksum
offloading, mbuf contains csum_data and csum_flags that were calculated
for already stripped headers. Some NICs support only limited checksums
offloading and do not use CSUM_PSEUDO_HDR flag, and csum_data contains
some value that UDP/TCP should use for pseudo header checksum calculation.
When L2TP is used for tunneling with mpd5, ng_ksocket receives mbuf with
filled csum_flags and csum_data, that were calculated for outer headers.
When L2TP header is stripped, a packet that was tunneled goes to the IP
layer and due to presence of csum_flags (without CSUM_PSEUDO_HDR) and
csum_data, the UDP/TCP checksum check fails for this packet.
Reported by: Irina Liakh <spell at itl ua>
Tested by: Irina Liakh <spell at itl ua>
MFC after: 1 week
Demotions preserve PG_NX, so it is enough to set nx bit for initial
lowest-level paging entries.
Suggested and reviewed by: alc
Sponsored by: The FreeBSD Foundation
MFC after: 1 week
The maximum scale is 6 (K, M, G, T, P, E) (B is 0).
Overly large explict scales were checked correctly, but for sufficently
large numbers HN_AUTOSCALE would get to 7 resulting in an out of bounds
read.
Found with humanize_number_test and CHERI bounds checking.
Reviewed by: emaste
Obtained from: CheriBSD
MFC after: 1 week
Sponsored by: DARPA, AFRL
Differential Revision: https://reviews.freebsd.org/D10376
The arm64 binutils only accepts 0 as an offset to the Load-Acquire Register
instructions where llvm will acceps both 0 and 0x0. The thread switching
code uses these with SCHED_ULE to block waiting for a lock to be released.
As the offset of the data to be loaded is zero this is safe, however it is
useful to keep the offset in the instruction to document what is being
loaded.
To work around this issue in binutils only generate the 0x prefix for
non-zero values.
Reported by: kan
Sponsored by: DARPA, AFRL
We need to set the Execute-never bits when mapping device memory as the
hardware may perform speculative instruction fetches.
Set the Privileged Execute-ever bit on userspace memory to stop the kernel
if it is tricked into executing it.
Reviewed by: kib
Sponsored by: DARPA, AFRL
Differential Revision: https://reviews.freebsd.org/D10382
When opening a vdev whose path is unknown, vdev_geom must find a geom
provider with a label whose guids match the desired vdev. However, due to
partitioning, it is possible that two non-synonomous providers will share
some labels. For example, if the first partition starts at the beginning of
the drive, then ada0 and ada0p1 will share the first label. More troubling,
if the last partition runs to the end of the drive, then ada0p3 and ada0
will share the last label. If vdev_geom opens ada0 when it should've opened
ada0p3, then the pool won't be readable. If it opens ada0 when it should've
opened ada0p1, then it will corrupt some other partition when it writes the
3rd and 4th labels.
The easiest way to reproduce this problem is to install a mirrored root pool
with the default partition layout, then swap the positions of the two boot
drives and reboot. Whether the bug manifests depends on the order in which
geom lists its providers, which is arbitrary.
Fix this situation by modifying the search algorithm to prefer geom
providers that have all four labels intact. If no such provider exists, then
open whichever provider has the most.
Reviewed by: mav
MFC after: 3 weeks
Sponsored by: Spectra Logic Corp
Differential Revision: https://reviews.freebsd.org/D10365
When the replay window size is large than UINT8_MAX, add to the request
the SADB_X_EXT_SA_REPLAY extension header that was added in r309144.
Also add support of SADB_X_EXT_NAT_T_TYPE, SADB_X_EXT_NAT_T_SPORT,
SADB_X_EXT_NAT_T_DPORT, SADB_X_EXT_NAT_T_OAI, SADB_X_EXT_NAT_T_OAR,
SADB_X_EXT_SA_REPLAY, SADB_X_EXT_NEW_ADDRESS_SRC, SADB_X_EXT_NEW_ADDRESS_DST
extension headers to the key_debug that is used by `setkey -x`.
Modify kdebug_sockaddr() to use inet_ntop() for IP addresses formatting.
And modify kdebug_sadb_x_policy() to show policy scope and priority.
Reviewed by: gnn, Emeric Poupon
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D10375
from the kernel. Make use of this to restrict accessing userspace to just
the functions that explicitly handle crossing the user kernel boundary.
Reported by: kib
Sponsored by: DARPA, AFRL
Differential Revision: https://reviews.freebsd.org/D10371
The current multiboot loader code doesn't clean the metadata added to the
kernel after the bi_load64 dry run, which breaks accounting of the required
memory for the metadata.
This issue didn't show itself before because all the metadata items where small
(8bytes), but after r316343 there's a big blob in the metadata, which triggers
this. Fix it by cleaning the metadata added to the kernel after the bi_load64
dry run. Also add a comment describing the memory layout when booting using
multiboot (Xen Dom0).
This unbreaks booting a FreeBSD/Xen Dom0 after r316343.
MFC after: 3 weeks
Sponsored by: Citrix Systems R&D
Adds:
-E: Use UEFI mode
-f: path to UEFI firmware image (default: path to uefi-edk2-bhyve package)
-F: UEFI framebuffer size (default: w=1024,h=768)
-L: IP to listen for VNC connections on (default: 127.0.0.1)
-P: Port to listen for VNC connections on (default: 5900)
-T: Enable tablnet device (for VNC)
-v: Wait for VNC client before booting VM
Submitted by: Shawn Webb <shawn.webb@hardenedbsd.org>
MFC after: 2 weeks
Relnotes: yes
Differential Revision: https://reviews.freebsd.org/D10378
When an mmap'd text file is written and then executed immediately
afterwards, it was possible that the modify time would change after the
text file was executing, resulting in the process executing the file
being killed. This was usually only observed when the file system's
times were set to higher resolution, but could have occurred for any
time resolution.
This was reported on a recent email list discussion.
This patch adds a VOP_SET_TEXT() to the NFS client which flushed all
dirty pages to the NFS server and then makes sure that n_mtime is up
to date to avoid this from occurring.
Thanks go to kib@ and pho@ for their help with developing this patch.
Tested by: pho
Reviewed by: kib
MFC after: 2 weeks
for example not in SYN-SENT.
This patch adds code to check the sysctl variable in other states than
LISTEN.
Thanks to ae and gnn for providing comments.
Reviewed by: gnn
MFC after: 1 week
Sponsored by: Netflix, Inc.
Differential Revision: https://reviews.freebsd.org/D9894
the method a lot.
Reduce the AND mask to the complement of the cursor's frame, so that area
inside the frame is not drawn first in black and then in lightwhite. The
AND-OR method is only directly suitable for the text mouse image, since
it doesn't go to the hardware there. Planar mode Mouse cursor drawing
takes 10-20 usec on my Haswell system (approx. 100 graphics accesses
at 130 nsec each), so the transient was not visible.
The method used the fancy read mode 1 and its color compare and color
don't care registers with value 0 in them so that all colors matched.
All that this did was make byte reads of frame buffer memory return 0xff,
so that the x86 case could obfuscate read+write as "and". The read must
be done for its side effect on the graphics controller but is not used,
except it must return 0xff to avoid affecting the write when the write
is obfuscated as a read-modify-write "and". Perhaps that was a good
optimization for 8088 CPUs where each extra instruction byte took as
long as a byte memory access.
Just use read+write after removing the fancy read mode. Remove x86
ifdefs that did the "and". After removing the "and" in the non-x86
part of the ifdefs, fix 4 of 6 cases where the shift was wrong.
- State that the units are kB.
- Be more complete/concise in terms of what is required (in this case
`minfree` must be at least `X`kB)
MFC after: 7 weeks
Sponsored by: Dell EMC Isilon
of the current usermode implementation of the POSIX semaphores.
For NSEMS_MAX, return -1 without changing errno, which indicates that
the variable has no limit. Before, sysconf(3) returned parameters
queried from the ksem(9) legacy implementation, which apparently has
low defaults for NSEMS_MAX.
Reported and tested by: jbeich
Sponsored by: The FreeBSD Foundation
MFC after: 1 week
in unusual cases. Optimize and significantly clean up removal in this
renderer. Optimize removal in the vga direct renderer.
Removal only needs to be done in the border (the part with pixels) in
both cases. The planar renderer used the condition scp->xoff > 0 to
test whether a right border exists. This actually tests for a left
border, and when the total horizontal border is 8 pixels, rounding gives
only a right border. This was the unusual broken case. An example
is easy to configure using something like "vidcontrol -f 8x16 iso-8x16
-g 79x25 MODE_27".
Optimize the planar case a little by only removing 9x13 active pixels
out of 16x16. Optimize it a lot by not doing anything if there is no
overlap with the border. Don't unroll the main loop or hard-code so
many assumptions about font sizes in it. On my Haswell system, graphics
memory and i/o accesses takes about 520 cycles each so optimizations from
unrolling are in the noise.
Optimize the direct case to not do anything if there is no overlap with
the border. Do a sanity check on the saveunder's coordinates. This
requires a previous change to pass non-rounded coordinates.
The previous commit also fixed the coordinates passed to the mouse
removal renderer. The coordinates were rounded down to a character
boundary, and thus essentially unusable. The renderer had to keep
track of the previous position, or clear a larger area. The latter
is only safe in the border, which is all that needs special handling
anyway.
I think no renderer depends on the bug. They have the following
handling:
- gfb sparc64: this seems to assume non-rounded coordinates
- gfb other: does nothing (seems to be missing border handling)
- vga text: does nothing (doesn't need border handling)
- vga planar: clears extras in the border, with some bugs. The fixes
will use the precise coordinates to optimize.
- vga direct: clears at the previous position with no check that it
is active, and clears everything. Checking finds this bug.
- others: are there any?
reduce hard-coded assumptions on font sizes so that the cursor size
can be more independent of the font size. Moving the mouse in the
buggy mode left trails of garbage.
The mouse cursor currently has size 9x13 in all modes. This can occupy
2x3 character cells with 8x8 fonts, but the algorithm was hard-coded
for only 2x2 character cells. Rearrange to hard-code only a maximum
cursor size (now 10x16) and to not hard-code in the logic. The number
of cells needed is now over-estimated in some cases.
2x3 character cells should also be used in text mode with 8x8 fonts
(except with large pixels, the cursor size should be reduced), but
hard-coding for 2x2 in the implementation makes it not very easy to
expand, and it practice it shifts out bits to reduce to 2x2.
In graphics modes, expansion is easier and there is no shifting out
for 9x13 cursors (but 9 is a limit for hard-coding for 2 8-bit VGA
cells wide). A previous commit removed the same buggy hard-coding for
removal at a lower level in planar mode. Another previous commit fixed
the much larger code for lower-level removal in direct mode; this is
independent of the font size so worked for 8x8 fonts. Text mode always
depended on the higher-level removal here, and always worked since
everything was hard-coded consistently for 2x2 character cells.
pagetables. This sets both bits when entering an address we know shouldn't
be executed.
I expect we could mark all userspace pages as Privileged execute-never to
ensure the kernel doesn't branch to one of these addresses.
While here add the ARMv8.1 upper attributes.
Reviewed by: alc, kib (previous version)
MFC after: 1 week
Sponsored by: DARPA, AFRL
Differential Revision: https://reviews.freebsd.org/D10360
scteken_init(). Move the internals of scteken_sync() into a local
function to help do this.
scteken_init() reset or adjusted the default attribute and screen
position at least 3 and 5 times, respectively. Warm init shouldn't
do any more than reset the "input" state.
(scterm-sc.c (which still works after minor editing), only resets
the escape state and the saved cursor position, and then does a
nearly-null sync of the current color.)
This mainly broke mode changes, and was most noticeable when the
background color is not teken's default (usually black). Then the
screen gets cleared in the wrong color. vidcontrol restores the
default normal attribute and tries to restore the default reverse
attribute. vidcontrol doesn't clear the screen again after restoring
the attribute(s), and it is too late to do it there without flicker.
Now the default normal attribute is restored before the change affects
the rendering.
When the foreground color is not teken's default, clearing with the
wrong attributes gave strange cursor colors for some cursor types.
The default reverse attribute is not restored since it is unsupported.
2/3 of the clobbering was from 2 resetting window resizing calls. The
second one is needed to restore the size, but must not reset. Window
resizing also sanitizes the cursor position, and after the main reset
resets the window size, the cursor row would often be adjusted from
24 to 23 if it were not already reset to 0. scteken_sync() is good
for restoring the window size and the cursor position in the correct
order, but was unusable at init time since scp->ts is not always
initialized then. Adjust to use its internals.
I didn't notice any problems from the cursor reset. The cursor should
be reset, and a previous fix was to reset it consistently a little
later.
Doing nothing for warm init works almost as well, if not better. It
is not very useful to reset the escape state for mode changes, since
the reset is especially likely to be null then. The escape state is
most likely to be non-initial and corrupted by its most normal uses
-- sloppy non-atomic output where a context switch or just mixing
stdout with stderr splits up escape sequences.
places possible in the kernel. This forces these functions to fail if
userspace is unable to access a given memory location, even if it is in
the user memory range.
This will simplify adding Privileged Access Never support later.
MFC after: 1 week
Sponsored by: DARPA, AFRL
