ngie [Fri, 10 Jun 2016 18:22:21 +0000 (18:22 +0000)]
MFstable/10 r301809:
MFC r299513,r299515:
r299513 (by cem):
rtadvd(8): Don't use-after-free
This whole block of code as committed fully formed in r224144. I'm not really
sure what the intent was, but it seems plausible that !persist ifis could need
other member cleanup. Don't free the object until after we've finished
cleaning its members.
ngie [Fri, 10 Jun 2016 18:12:11 +0000 (18:12 +0000)]
MFstable/10 r301806:
MFC r299510:
r299510 (by cem):
libmp: Fix trivial buffer overrun
fgetln yields a non-NUL-terminated buffer and its length. This routine
attempted to NUL-terminate it, but did not allocate space for the NUL. So,
allocate space for the NUL.
ngie [Fri, 10 Jun 2016 18:04:54 +0000 (18:04 +0000)]
MFstable/10 r301803:
MFC r299507:
r299507 (by cem):
rtadvd(8): Fix a typo in full msg receive logic
Check against the size of the struct, not the pointer. Previously, a message
with a cm_len between 9 and 23 (inclusive) could cause int msglen to underflow
and read(2) to be invoked with msglen size (implicitly cast to signed),
overrunning the caller-provided buffer.
All users of cm_recv() supply a stack buffer.
On the other hand, the rtadvd control socket appears to only be writable by the
owner, who is probably root.
While here, correct some types to be size_t or ssize_t.
CID: 1008477
Security: unix socket remotes may overflow stack in rtadvd
ngie [Fri, 10 Jun 2016 17:59:30 +0000 (17:59 +0000)]
MFstable/10 r301801:
MFC r300836:
Quell false positives in svc_vc_create and svc_vc_create_conn with cd and xprt
Both cd and xprt will be non-NULL after their respective malloc(9) wrappers are
called (mem_alloc and svc_xprt_alloc, which calls mem_alloc) as mem_alloc
always gets called with M_WAITOK|M_ZERO today. Thus, testing for them being
non-NULL is incorrect -- it misleads Coverity and it misleads the reader.
Remove some unnecessary NULL initializations as a follow up to help solidify
the fact that these pointers will be initialized properly in sys/rpc/.. with
the interfaces the way they are currently.
ngie [Fri, 10 Jun 2016 14:42:36 +0000 (14:42 +0000)]
MFstable/10 r301789:
MFC r299491:
r299491 (by cem):
route6d(8): Fix potential double-free
In the case that the subsequent sysctl(3) call failed, 'buf' could be free(3)ed
repeatedly. It isn't clear to me that that case is possible, but be clear and
do the right thing in case it is.
ngie [Fri, 10 Jun 2016 14:10:51 +0000 (14:10 +0000)]
MFstable/10 r301782:
MFC r299460:
r299460 (by cem):
fsck_ffs: Don't overrun mount device buffer
Maybe this case is impossible. Either way, when attempting to "/dev/"-prefix a
non-global device name, check that we do not overrun the f_mntfromname buffer.
In this case, truncating (with strlcpy or similar) would not be useful, since
the f_mntfromname result of getmntpt() is passed directly to open(2) later.
ngie [Wed, 8 Jun 2016 19:31:12 +0000 (19:31 +0000)]
MFstable/10 r301655:
MFC r299766:
Fix logically dead code pointed out by clang/Coverity
parse_context, parse_user_security: test for validity of results from
parse_ascii(..) with by casting to int32_t and comparing to -1; comparing
unsigned types to negative values will always be false.
ngie [Wed, 8 Jun 2016 18:23:33 +0000 (18:23 +0000)]
MFstable/10 r301680:
MFC r300625:
Remove unnecessary memset(.., 0, ..)'s
The mem_alloc macro calls calloc (userspace) / malloc(.., M_WAITOK|M_ZERO)
under the covers, so zeroing out memory is already handled by the underlying
calls
ngie [Wed, 8 Jun 2016 17:50:51 +0000 (17:50 +0000)]
MFstable/10 r301663:
MFC r294507,r294567,r299466:
r294507 (by harti):
Fill the ifAlias leaf of the ifXTable with the interface description
if there is one available and it fits into the maximum size (64 characters).
r294567 (by bz):
Change the variable to a #define in order to make gcc happy which
otherwise will complain about "variably modified 'alias' at file scope".
Unbreaks the build on gcc platforms.
r299466 (by cem):
bsnmpd: Fix size of trapsink::comm to match other community arrays
This fixes a number of possible strcpy() buffer overruns between the various
community strings in trap.c.
ngie [Wed, 8 Jun 2016 17:44:22 +0000 (17:44 +0000)]
MFstable/10 r301661:
MFC r256678,r256680,r260986,r272878,r286402:
r256678 (by syrinx):
Fix SNMP Error response PDUs and properly encode them when using v3 auth/encryption.
r256680 (by syrinx):
Fix the -Wconversion warnings produced when compiling the SNMP agent.
r260986 (by harti):
Fix a problem with OBJECT IDENTIFIER encoding: need to check the
second subid to be less than 40, not the first when the first
subid is 0 or 1.
r272878 (by syrinx):
Fix a bug in decoding string indexes in snmp_target(3), thus causing
bsnmpd(1) to not send v3 notifications properly; while here add two
missing return statements which could lead to abort() in case of a
rollback
r286402 (by araujo):
Fix variable 'old' is used uninitialized whenever '&&' condition is false.
Spotted by clang.
ngie [Wed, 8 Jun 2016 17:35:59 +0000 (17:35 +0000)]
MFstable/10 r301657:
MFC r299701:
Move _bsnmptools_debug extern from bsnmpmap.c to bsnmptools.h
It was used in bsnmpmap.c but was stored in bsnmptools.c; moving the extern
to the header allows us to cover all of our bases for the variable, and allows
_bsnmptools_debug to be used in the future elsewhere -- not just bsnmpmap.c.
ngie [Wed, 8 Jun 2016 17:24:57 +0000 (17:24 +0000)]
MFstable/10 r300475:
MFC r299710,r299711,r299763,r299783,r299811:
r299710:
Staticize global variables only used in bsnmpimport.c to fix
-Wmissing-variable-declarations warnings
r299711:
Fold two malloc + memset(.., 0, ..) calls into equivalent calloc calls
r299763:
Mute -Wstrlcpy-strlcat-size warning by using nitems with the size of the buffer
This is a no-op as the malloc above set the size of the buffer to the size used
below, but this keeps things consistent in case the malloc call changes somehow.
r299783:
Convert tok from enum tok to int32_t in function calls
get_token(..) returns int32_t, not enum tok, and in many cases tests for items
not in enum tok (e.g. '('). Make the typing consistent with get_token, which
includes a domino effect of changing enum tok to int32_t.
- getaddrinfo() sets res = NULL on failure and freeaddrinfo() always
dereferences its argument, so we should only free the address list after
a successful call.
- Address a second potential leak caused by getaddrinfo(AF_INET6)
overwriting the address list returned by getaddrinfo(AF_INET).
X-MFC-With: r300941
r300973:
Follow up to r300932
In the event MK_INET6 != no in userspace, but is disabled in the
kernel, or if there aren't any IPv6 addresses configured in userspace
(for lo0 and all physical interfaces), rpcbind would terminate
immediately instead of silently failing on
Skip over the IPv6 block to its respective cleanup with freeifaddrs if
creating the socket failed instead of terminating rpcbind immediately
- getaddrinfo() sets res = NULL on failure and freeaddrinfo() always
dereferences its argument, so we should only free the address list after
a successful call.
- Address a second potential leak caused by getaddrinfo(AF_INET6)
overwriting the address list returned by getaddrinfo(AF_INET).
X-MFC-With: r300941
r300973:
Follow up to r300932
In the event MK_INET6 != no in userspace, but is disabled in the
kernel, or if there aren't any IPv6 addresses configured in userspace
(for lo0 and all physical interfaces), rpcbind would terminate
immediately instead of silently failing on
Skip over the IPv6 block to its respective cleanup with freeifaddrs if
creating the socket failed instead of terminating rpcbind immediately
rpcbind/check_bound.c
Fix CID1347798, a memory leak in mergeaddr.
rpcbind/tests/addrmerge_test.c
Fix CID1347800 through CID1347803, memory leaks in ATF tests. They
are harmless because each ATF test case runs in its own process, but
they are trivial to fix. Fix a few other leaks that Coverity didn't
detect, too.
Fix a bug in rpcbind for multihomed hosts. If the server had interfaces on
two separate subnets, and a client on the first subnet contacted rpcbind at
the address on the second subnet, rpcbind would advertise addresses on the
first subnet. This is a bug, because it should prefer to advertise the
address where it was contacted. The requested service might be firewalled
off from the address on the first subnet, for example.
usr.sbin/rpcbind/check_bound.c
If the address on which a request was received is known, pass that
to addrmerge as the clnt_uaddr parameter. That is what addrmerge's
comment indicates the parameter is supposed to mean. The previous
behavior is that clnt_uaddr would contain the address from which the
client sent the request.
usr.sbin/rpcbind/util.c
Modify addrmerge to prefer to use an IP that is equal to clnt_uaddr,
if one is found. Refactor the relevant portion of the function for
clarity, and to reduce the number of ifdefs.
etc/mtree/BSD.tests.dist
usr.sbin/rpcbind/tests/Makefile
usr.sbin/rpcbind/tests/addrmerge_test.c
Add unit tests for usr.sbin/rpcbind/util.c:addrmerge.
usr.sbin/rpcbind/check_bound.c
usr.sbin/rpcbind/rpcbind.h
usr.sbin/rpcbind/util.c
Constify some function arguments
ngie [Wed, 8 Jun 2016 13:50:48 +0000 (13:50 +0000)]
MFstable/10 r301619:
MFC r300430:
Wrap EXPAND(..) macro with a do-while(0) loop and put a single statement on each line
As a positive side-effect, this eliminates the double semicolons reported by Coverity:
the macro contained a trailing semicolon, in addition to the semicolon placed on
each line where EXPAND(..) was called.
ngie [Wed, 8 Jun 2016 13:41:51 +0000 (13:41 +0000)]
MFstable/10 r301615:
MFC r299844,r300931:
r299844:
Make hostid_save depend on hostid
r300931:
Make netif REQUIRE hostid
As noted in the PR, if etc/rc.d/zvol is removed, netif will be run before
hostid, and the MAC address generated for any bridge devices will be
non-deterministic. Make the MAC address generated be deterministic for
bridge devices by explicitly REQUIRE'ing hostid.
This fixes up the rest of the PR, inadvertently committed in r299844
pfg [Sat, 4 Jun 2016 04:02:03 +0000 (04:02 +0000)]
MFC r300376:
ndis(4): Better mimic the behavior of rand() on Windows.
In ndis(4) we expose a rand() function that was constantly reseeding
with a time depending function every time it was called. This
essentially broke the reasoning behind seeding, and rendered srand()
a no-op.
Keep it simple, just use random() and srandom() as it's meant to work.
It would have been tempting to just go for arc4random() but we
want to mimic Microsoft, and we don't need crypto-grade randomness
here.
hselasky [Fri, 3 Jun 2016 09:17:22 +0000 (09:17 +0000)]
MFC r300489:
Use DELAY() instead of _sleep() when SCHEDULER_STOPPED() is set inside
pause_sbt(). This allows pause() to continue working during a panic()
which is not invoking KDB. This is useful when debugging graphics
drivers using the LinuxKPI.
hselasky [Fri, 3 Jun 2016 08:56:54 +0000 (08:56 +0000)]
MFC r300667:
Check for signals when locking the USB enumeration thread from
userspace, so that USB applications can be killed if an enumeration
thread should be stuck for various reasons.
hselasky [Fri, 3 Jun 2016 08:50:44 +0000 (08:50 +0000)]
MFC r299060:
Extend the UQ_NO_STRINGS quirk to also cover the USB language string
descriptor. This fixes enumeration of some older Samsung Galaxy S3
phones.
pfg [Sun, 29 May 2016 16:32:56 +0000 (16:32 +0000)]
MFC r300378:
libc/regex: fix two buffer underruns.
Fix some rather complex regex issues found on OpenBSD as part of some
ongoing work to fix a sed(1) bug.
Curiously the OpenBSD tests don't trigger segfaults on FreeBSD but the
bugs were confirmed by running a port of FreeBSD's regex under OpenBSD's
malloc. Huge thanks to Ingo for confirming the behavior.
jhb [Tue, 24 May 2016 23:04:16 +0000 (23:04 +0000)]
MFC 299310:
Don't store generated firmware object files in the source directory.
Trim the leading directory of a firmware source file from the resulting
target object file name so the object file is stored in the object
directory. Previously, using 'FIRMWS= /path/to/fw.bin:fw.bin' would
store the generated 'fw.bin.fwo' file in the /path/to directory. Now
it stores it in the object directory of the kernel module being built.
Use the size of the destination buffer, not the source buffer.
Technically this is a no-op, but mute the clang warning in case the malloc call
above for fstring ever changes in the future
r299765:
Fix theoretical buffer overflow issues in snmp_oid2asn_oid
Increase the size of `string` by 1 to account for the '\0' terminator. In the event
that `str` doesn't contain any non-alpha chars, i would be set to MAXSTR, and
the subsequent strlcpy call would overflow by a character.
Remove unnecessary `string[i] = '\0'` -- this is already handled by strlcpy.
r299769:
Use the size of the destination buffer instead of the malloc size, repeated, in order
to mute a -Wstrlcpy-strlcat-size warning
r299770:
Fix up r299764
I meant to use nitems, not sizeof(..) with the destination buffer. Using sizeof(..)
on a pointer will always truncate the output in the destination buffer incorrectly
Pointyhat to: ngie
r299774:
Do minimal work necessary to cure a -Wunused-but-set-variable warning from gcc
How errno is saved before and restored after strtoul calls needs a rethink
r299802:
Fix up both r299764 and r299770
nitems was wrong too, as it was being tested against a pointer instead of a buffer on
the stack.
Since the old code was just doing malloc, then strlcpy'ing the contents of the source
buffer into the destination buffer, replace it all with a call to strdup..
Supersized Duncecap to: ngie
r299803:
Replace malloc + memset(.., 0, ..) with calloc calls
r299805:
Fix up r299769
Similar to r299802, it was noted that using nitems on scalar pointers is
invalid.
Use strdup instead of malloc + strlcpy (which is what the old code was doing
anyhow).
Pointyhat to: ngie
r299814:
Replace malloc + memset(.., 0, ..) with calloc calls
ngie [Tue, 24 May 2016 04:18:15 +0000 (04:18 +0000)]
MFstable/10 r300471:
MFC r299712,r299759,r299760,r299761,r299762:
r299712:
Fix some trivial clang/gcc warnings in bsnmptc.c
- By definition, `enum snmp_tc` can't be false (the implied starting sequence
index for the enum is 0). Don't test for it being < 0.
- Staticize `struct snmp_text_conv` to mute a -Wmissing-variable-declarations
warning from clang.
- Remove set but unused variable, ptr, in parse_bridge_id(..) and
parse_bport_id(..) to mute warning from gcc 4.9+.
- Mark value and string unused in snmp_inetaddr2oct(..) and parse_inetaddr(..)
as they're just stub functions.
r299759:
Use calloc instead of memset(.., 0, ..) + malloc
r299760:
Sort variables in parse_ascii(..) per style(9)
r299761:
parse_ascii: make count size_t to mute a -Wsign-compare issue
count is always unsigned.
r299762:
Mark snmptoolctx unused in parse_authentication(..), parse_privacy(..),
parse_context(..), and parse_user_security(..).
rmacklem [Fri, 20 May 2016 01:25:14 +0000 (01:25 +0000)]
MFC: r299201
Give mountd -S priority over outstanding RPC requests when suspending the nfsd.
It was reported via email that under certain heavy RPC loads
long delays before the exports would be updated was observed
when using "mountd -S". This patch reverses the priority between
the exclusive lock request to suspend the nfsd threads and the
shared lock request for performing RPCs.
As such, when mountd attempts to suspend the nfsd threads, it
gets priority over outstanding RPC requests to do this.
I suspect that the case reported was an artificial test load,
but this patch did fix the problem for the reporter.
cy [Sun, 15 May 2016 15:26:19 +0000 (15:26 +0000)]
Fix:
IP Filter bug 1835705: It is impossible to delete a state using
SIOCDELST ioctl.
This is a direct commit to the stable/9 branch because this patch was
already included in the ipfilter 5.1.2 import in r254562. It was
ultimately included in ipfilter 4.1.29 (whereas stable/9 uses 4.1.28).
ngie [Fri, 13 May 2016 09:54:15 +0000 (09:54 +0000)]
MFstable/10 r299651:
MFC r298839:
Fix memory allocation edgecases in kvm_argv(..)
- Don't leak nbufp on realloc failure in kvm_argv
- Catch malloc errors with bufp
- Set buflen last in the "buflen == 0" case to ensure that
bufp/nbufp is properly reallocated on the next go around
ngie [Fri, 13 May 2016 09:16:01 +0000 (09:16 +0000)]
MFstable/10 r299638:
MFC r298462:
Return `ret` in op_ifentry(..) to mute a -Wunused-but-set-variable warning
This will also now detect error conditions with
value->var.subs[sub - 1] == LEAF_ifPhysAddress where `string_get(..)`
could fail if iifp->physaddr and/or iifp->physaddrlen were deemed
invalid.
ngie [Fri, 13 May 2016 09:07:42 +0000 (09:07 +0000)]
MFstable/10 r299635:
MFC r298665:
r298665 (by cem):
aacraid(4): Fix some mostly trivial buffer overruns
strcpy(3) emits a trailing nul byte, trampling fields after the intended
destination. Instead, use strncpy(3), intentionally leaving these fields
not nul-terminated.
ngie [Fri, 13 May 2016 08:42:21 +0000 (08:42 +0000)]
MFstable/10 r299623:
MFC r298337:
r298337 (by cem):
pty(4): Use strlcpy to guarantee destination buffer isn't overrun
The devtoname() name is strcpyed into a small stack buffer. Sure, we always
expect the name to be ttyXX (or ptyXX). If that's the case, strlcpy() doesn't
hurt.
ngie [Fri, 13 May 2016 08:25:17 +0000 (08:25 +0000)]
MFstable/10 r299616:
MFC r298333:
r298333 (by cem):
Make Racct macro slightly more gracious given RACCT_UNDEFINED
rctl_string_to_rule could previously index below the zeroth element of
racct_types via the macro. Maybe it shouldn't use the macro on
RACCT_UNDEFINED. But given every other RACCT_ definition is non-negative, it
seems pretty easy to foot-shoot this one without the check.
The facility_initialized and facility arrays are the same size and were
intended to be indexed the same. I believe this mismatch was just a
typo/braino in r208731.
davidcs [Tue, 10 May 2016 02:44:23 +0000 (02:44 +0000)]
MFC r298294
1. modify fwdump (a.k.a grcdump) so that grcdump memory is allocated
and freed on as needed basis.
2. grcdump can be taken at failure points by invoking bxe_grc_dump()
when trigger_grcdump sysctl flag is set. When grcdump is taken
grcdump_done sysctl flag is set.
3. grcdump_done can be monitored by the user to retrieve the grcdump