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).
MFC after: 1 week
Pointyhat to: ngie
Sponsored by: EMC / Isilon Storage Division
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..
Reported by: bde
MFC after: 1 week
X-MFC with: r299764, r299770
Supersized Duncecap to: ngie
Sponsored by: EMC / Isilon Storage Division
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.
MFC after: 2 weeks
Sponsored by: EMC / Isilon Storage Division
- Save errno
- Set errno to 0
- Call strtoul
- Test errno (optional, but many calls to strtoul did this afterwards)
Some of the code was setting errno = 0 after calling strtoul, not setting
errno = 0, or setting errno to saved_errno after the call, but before the
test. These all have unwanted behavioral side-effects, depending on the
initial value of errno and whether or not the input to strtoul was correct
or incorrect.
MFC after: 3 weeks
Sponsored by: EMC / Isilon Storage Division
How errno is saved before and restored after strtoul calls needs a rethink
MFC after: 1 week
Reported by: gcc 5.x
Sponsored by: EMC / Isilon Storage Division
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
MFC after: 1 week
X-MFC with: r299764
Sponsored by: EMC / Isilon Storage Division
rc cannot be negative -- that was already tested for earlier on in
the function
MFC after: 1 week
Reported by: clang, gcc
Sponsored by: EMC / Isilon Storage Division
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.
Reported by: clang, Coverity
CID: 1011432, 1011433
MFC after: 3 weeks
Sponsored by: EMC / Isilon Storage Division
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.
MFC after: 1 week
Reported by: clang
Sponsored by: EMC / Isilon Storage Division
Technically this is a no-op, but mute the clang warning in case the malloc call
above for fstring ever changes in the future
Reported by: clang
MFC after: 1 week
Sponsored by: EMC / Isilon Storage Division
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.
MFC after: 1 week
Reported by: clang
Sponsored by: EMC / Isilon Storage Division
The "- 1" is there specifically to enable checking for NUL termination.
I should also admit the rest change was mostly cosmetic and the
overruns can't occur in practice: still I leave them to pacify
static analyzers.
Pointed out by: bde
- 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.
MFC after: 1 week
Sponsored by: EMC / Isilon Storage Division
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.
MFC after: 3 weeks
Sponsored by: EMC / Isilon Storage Division
may make Coverity think that other negative values of cmd (used
as an index) are possible. Testing < 0 is a more common idiom
in any case.
Reported by: Coverity
CID: 1305629
NUL terminated. The source and destination buffers are the same
size and the source *should* be NUL terminated, but be paranoid.
Reported by: Coverity
CID: 1011274
MFC after: 1 week
terminated. Don't bother checking for truncation since the subsequent
quota_read() should detect that and fail.
Reported by: Coverity
CID: 1009980
MFC after: 1 week
ensure that the latter is NUL terminated since it is passed
as an argument to *printf().
Warn about NIS domains that are too long.
Reported by: Coverity
CID: 1009620, 1009621
MFH: 1 week
cm_send() closes 'fd' on error. In that case, bail out early without trying to
recv from or close 'fd' again.
Reported by: Coverity
CID: 1006078
Sponsored by: EMC / Isilon Storage Division
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.
Reported by: Coverity
CID: 1006079
Sponsored by: EMC / Isilon Storage Division
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.
Reported by: Coverity
CID: 1008477
Security: unix socket remotes may overflow stack in rtadvd
Sponsored by: EMC / Isilon Storage Division
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.
Reported by: Coverity
CID: 272537
Sponsored by: EMC / Isilon Storage Division
Coverity really should have figured this out from the exit(3) call at the end
of the routine, but just make it explicit.
No functional change.
Reported by: Coverity
CID: 1304866 (false positive double-close of 'baz')
Sponsored by: EMC / Isilon Storage Division
