When ethtool_nl_get_optics() fails after ethtool_nl_cb() has allocated
oi->raw this memory would leak because only the struct optics* itself
was freed. Fix this by using optics_free() and relying on free(oi->raw)
being a no-op for NULL pointers in combination with using util_zalloc()
to handle errors both before and after oi->raw was allocated.
Fixes: c34adb9cab ("opticsmon: Introduce opticsmon tool")
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Don't ignore the return value of monitor_wait_loop() but instead forward
it to the caller.
Fixes: c34adb9cab ("opticsmon: Introduce opticsmon tool")
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
The epoll fd (epfd) in monitor_wait_loop() is never closed. Close it in
all return paths. Also there is no explicit check for an error return of
epoll_create1() add that too.
Fixes: c34adb9cab ("opticsmon: Introduce opticsmon tool")
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
The "tx_fault" field in the JSON output accidentally contained the
rx_los value instead of the tx_fault one. Change it to the correct
value. Note that this does not impact the data relayed to firmware as
that uses the raw module data.
Fixes: c34adb9cab ("opticsmon: Introduce opticsmon tool")
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
The audit directory ts-shell configuration and also the output of
getpwuid() are considered tainted when running in Perl taint (-T) mode.
Provide untaint checks with validating the content.
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
The updateConfiguration() function updates and checks the ts-shell
configuration for each line in the ts-shell configuration file.
Run the checks just once after the configuration is completely read.
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Introduce a configuration setting to allow administrators to specify
path to the iucvconn binary as part of the ts-shell configuration.
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Perl provides a Taint (-T) mode which tracks data that is obtained
by external means (e.g. arguments, reading file contents, environment
variables, ...)
Introduce a first set of corrections to clear and validate data
obtained from external sources.
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Do not obtain the pager program through the PAGER environment
variable. Instead, introduce a configuration setting to specify
the pager program as part of the ts-shell configuration.
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
When ts-shell creates the authorization for current user, the
user name and group names are interpreted as regex. This might
create additional authorizations which are not permitted for
current user.
Correct this behavior by quoting the user name in the regex.
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Check the return code of fcntl() calls to ensure the close-on-exec
setting succeeded. Otherwise, the PTY and server file descriptors
would become available to the forked client process.
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
iucvtty expects to receive the TERM environment information right
after a connection has been established. Reading this information
can lead to a connection stall (DoS). Introduce a temporary timeout
and fall back to the default TERM environment.
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
The iucvtty_read_msg() now receives the entire message header.
Perform message header checks for specific message types where the
payload length is clearly defined. Also this needs to be done prior
starting the message chunk processing because the message datalen
field will be adjusted based on the read chunks.
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
The inclusion of termio.h leads to the following compilation error:
CC iucvterm/tests/test_functions.o
test_functions.c:16:10: fatal error: termio.h: No such file or directory
16 | #include <termio.h>
| ^~~~~~~~~~
The termio ioctl interface has been obsolete for a very long time and
was replaced by termios. termio.h was still present for compatibility
but was removed with glibc 2.42.
iucvterm uses termios.h correctly in the tool. The test however not only
includes termio.h erroneously, it doesn't even require it. Remove the
inclusion of termio.h.
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
If iucvtty_read_data() receives message data and reads less than
MSG_DATA_OFFSET, out-of-bound reads will happen.
Correct and improve the receiving path by always reading the entire
message header. Also improve the "residual" message data handling by
replacing and processing every message now as chunk.
Acked-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
The PTY slave file descriptor is the controlling terminal
allocated for the child process. When all file descriptors
are being closed, a SIGHUP is triggered for the child process.
Keeping the file descriptor in the parent open would prevent
the SIGHUP to be delivered to the child process.
For proper SIGHUP processing, close the file descriptor in
the parent process.
Acked-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Improve handling of TERM environment processing by validating
the received terminal name. If the terminal name is not valid,
a message will be displayed and the default terminal will be used.
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Explicitly unmount the root partition on unit stopping to prevent still
mounted boot partition on switch root which results in boot being unable
to be mounted after switch root.
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Signed-off-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Since this unit cannot have a dependency on the actual needed
dev-disk-by2xdlabel-boot.device unit because it might not exist if the
label is never set but this unit has to run after it when it does exist
wait for udev-settle which creates the symlink if it does exist.
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Signed-off-by: Finn Callies <fcallies@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Update osasnmpd to support Object ID Sub-IDs with length up-to and
including 10 digits, in line with SNMP specification. While at it, also
replace sprintf() calls with snprintf(). Together, these changes
contribute to better memory safety.
Suggested-by: Alexandra Winter <wintera@linux.ibm.com>
Reviewed-by: Hidayath Khan <hidayath@linux.ibm.com>
Signed-off-by: Aswin Karuvally <aswin@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
These messages indicate verification failures and are therefore reported
as errors rather than debug output. Unfortunately,
X509StoreContext::init(...) expects the callback to return an OpenSSL
ErrorStack, so it's not possible to propagate these failures through our
own error hierarchy.
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Enforce root CA organization name requirements during certificate chain
validation to prevent accepting certificates from unexpected root CAs.
Add RootCaVerification enum with RootCaOrganizationPinning and
SkipPinning variants. Extend verify_chain() to validate root CA
organization. Default to pinning "DigiCert" for standard trust chains,
skip pinning when custom root CA is provided.
Assisted-by: IBM Bob:1.0.6
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
The command line length value is not properly checked and limited to the
documented 1024 bytes, even though a MAX_CMD_LEN is already defined but
not used.
With this, an overflow of the static char mon_record[] buffer is possible
in read_cmdline(), corrupting adjacent .bss data. The data is sanitized to
printable ASCII bytes, but in theory a crash of the mon_procd daemon could
be possible.
Fix it by adding a check and truncation similar to the other restricted
fields like e.g. ruser_len or cmd_len.
Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
Signed-off-by: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
The yes_no() function was leaking memory when returning
early from the loop, as the 'answer' buffer allocated by
getline() was not freed before the return statements.
Restructure the function to use a single exit point, ensuring
free(answer) is always called before returning.
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Volkan Unal <vunal@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
sort_bls_entries() already checks for the keyword "title"
duplication. Add also checks for keywords "linux", "initrd", and
"options" duplication (Rule #5).
Reviewed-by: Stefan Haberland <sth@linux.ibm.com>
Signed-off-by: Eduard Shishkin <edward6@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Check decimal prefixes, representing site values in on-disk
environment block. Reject any values different from {0, ..., 9}
as invalid ones.
Reviewed-by: Stefan Haberland <sth@linux.ibm.com>
Signed-off-by: Eduard Shishkin <edward6@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Before extracting a file trailer, check that the file is big enough;
Check that signature length stored in the extracted trailer doesn't
exceed the size of the file without the trailer. Treat the case of
the failed checks as unsigned file.
Without the checks, memcmp() may read the area before the allocated
@buffer, dumping heap into bootmap or crashing.
Reviewed-by: Stefan Haberland <sth@linux.ibm.com>
Signed-off-by: Eduard Shishkin <edward6@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
When parsing environment block, check each its line for the
compliance with the 'foo=bar\n' pattern. In case of missing '=',
or '\n' abort the parsing procedure with a warning message.
Without this check, stage3 may write a zero byte to the absolute
address 0 (robustness defect)
Reviewed-by: Stefan Haberland <sth@linux.ibm.com>
Signed-off-by: Eduard Shishkin <edward6@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
When parsing environment block, limit the number of the hash table
entries, so that total size of all records doesn't exceed PAGE_SIZE
bytes allocated for that hash table.
Reviewed-by: Stefan Haberland <sth@linux.ibm.com>
Signed-off-by: Eduard Shishkin <edward6@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
By design, before replacement of '${FOO}' with its value, the
procedure checks that the resulted command line doesn't exceed the
maximum one.
The old check used the never updated length of the original command
line, which is incorrect. Instead, use its current length resulting
from the replacement happened at the previous iteration.
Reviewed-by: Stefan Haberland <sth@linux.ibm.com>
Signed-off-by: Eduard Shishkin <edward6@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
mem_init_flex() allocates the page header index array sized for
ADDR_TO_IDX(mem_end) + 1 entries, but never checked that each
page record's address falls within [0, mem_end).
A crafted LKCD page record with addr >= mem_end writes 8 bytes
beyond the allocation.
Fix by rejecting any page record whose address is > mem_end - PAGE_SIZE
with ERR_EXIT(), immediately after the end-of-dump marker check.
Signed-off-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>
Reviewed-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
read_page_buf() uses the file-controlled pg_hdr->size directly as the
byte count for zg_read() into fixed stack buffers of PAGE_SIZE.
A crafted LKCD dump with pg_hdr->size > PAGE_SIZE overflows the buffer
and smashes the stack frame. A size of 0 for a raw page silently
produces uninitialised data.
Enforce page header size constraints following crash-utility's logic:
- Compressed or Raw pages with size > PAGE_SIZE are invalid;
exit with an error.
- Raw pages with size 0 contain no data in the file; fill the
output buffer with zeros without reading.
- Compressed pages with size 0 are caught by the uncompress() return
code check.
- Raw pages with size != PAGE_SIZE are invalid; exit with an error.
Check the return code of uncompress() and exit with an error if
decompression fails, rather than silently proceeding with an incomplete
output buffer.
Signed-off-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>
Reviewed-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Declare binary data symbols as arrays to correctly represent
linker-provided symbols of unknown size. This resolves compiler warnings
about reading 22-24 bytes from a 1-byte region when including stage*
loaders.
DEBUG: In function ‘memcpy’,
DEBUG: inlined from ‘boot_init_eckd_ldl_stage0’ at boot.c:181:2,
DEBUG: inlined from ‘install_svdump_eckd_ldl’ at install.c:891:2,
DEBUG: inlined from ‘install_dump’ at install.c:1221:9,
DEBUG: inlined from ‘main’ at zipl.c:185:9:
DEBUG: /usr/include/bits/string_fortified.h:29:10: warning: ‘__builtin_memcpy’ reading 22 bytes from a region of size 1 [-Wstringop-overread]
DEBUG: 29 | return __builtin___memcpy_chk (__dest, __src, __len,
DEBUG: | ^
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Ensure that the path to a dump image specified in the NGDump meta file
of a dump device points to a valid location within the dump device.
Especially, disallow escaping from a dump device with a dump image path
using references to '..' or symbolic links pointing outside of the dump
device.
Signed-off-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Replace the popen() call to the external tool sha256sum with OpenSSL's
SHA256 digest algorithm to compute the checksum of a dump image
to improve security of NGDump.
The call to popen() is inherently insecure because a malicious user
controlling a dump device could manipulate the latter and let the user
applying zgetdump to the dump device execute arbitrary shell code.
Furthermore, getting rid of process and shell spawning performed by
popen() shall improve performance of calculation of dump image
checksums.
Signed-off-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Reviewed-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
Use the file descriptor opened in zg_open() before to obtain information
about the file. Theoretically, there is a tiny window for a race
condition between opening a file and obtaining its information with
stat(). A malicious user could use it to entice incorrect behavior
from zgetdump. Using the same file descriptor for both operations
prevents such situations.
Suggested-by: Peter Jin <pjin@linux.ibm.com>
Signed-off-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Reviewed-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
The name of a private NGDump function does not require to have a unique
prefix to avoid naming conflicts with public functions of other C modules.
Furthermore, be consistent with private NGDump functions not having
the prefix 'ngdump_'.
Signed-off-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Reviewed-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
dfo_elf_init() allocates the output ELF header buffer based on
HDR_BASE_SIZE, per-CPU note sizes, and per-memory-chunk overhead,
but does not account for the vmcoreinfo note. notes_init() then
writes the full vmcoreinfo string into the buffer via nt_vmcoreinfo(),
overflowing the heap allocation for any dump with a vmcoreinfo blob
larger than what fits in HDR_BASE_SIZE. The following ABORT check
fires after the heap was already corrupted.
Fix by extracting the allocation size calculation into a new
hdr_alloc_size() function that explicitly accounts for all items
written into the header buffer.
Remove the opaque HDR_BASE_SIZE and HDR_PER_MEMC_SIZE macros.
Signed-off-by: Mikhail Zaslonko <zaslonko@linux.ibm.com>
Reviewed-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
