Option "-C" of setfiles(8) causes setfiles(8) to exit with status 1 rather
than status 255 if it encounters relabeling errors, but no other (fatal)
error. Pass "-C" to setfiles(8) in "selinux-relabel", because we don't
want the "selinux-relabel" API to fail if setfiles(8) only encounters
relabeling errors.
(NB even without "-C", setfiles(8) continues traversing the directory
tree(s) and relabeling files across relabeling errors, so this change is
specifically about the exit status.)
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1794518
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20220511122345.14208-3-lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
In guestfs_readdir(), the daemon currently sends each XDR-encoded
"guestfs_int_dirent" to the library with a separate send_file_write()
call.
Determine the largest encoded size (from the longest filename that a
"guestfs_int_dirent" could carry, from readdir()'s "struct dirent"), and
batch up the XDR encodings until the next encoding might not fit in
GUESTFS_MAX_CHUNK_SIZE. Call send_file_write() only then.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1674392
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20220502085601.15012-3-lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Currently the guestfs_readdir() API can not list long directories, due to
it sending back the whole directory listing in a single guestfs protocol
response, which is limited to GUESTFS_MESSAGE_MAX (approx. 4MB) in size.
Introduce the "internal_readdir" action, for transferring the directory
listing from the daemon to the library through a FileOut parameter.
Rewrite guestfs_readdir() on top of this new internal function:
- The new "internal_readdir" action is a daemon action. Do not repurpose
the "readdir" proc_nr (138) for "internal_readdir", as some distros ship
the binary appliance to their users, and reusing the proc_nr could
create a mismatch between library & appliance with obscure symptoms.
Replace the old proc_nr (138) with a new proc_nr (511) instead; a
mismatch would then produce a clear error message. Assume the new action
will first be released in libguestfs-1.48.2.
- Turn "readdir" from a daemon action into a non-daemon one. Call the
daemon action guestfs_internal_readdir() manually, receive the FileOut
parameter into a temp file, then deserialize the dirents array from the
temp file.
This patch sneakily fixes an independent bug, too. In the pre-patch
do_readdir() function [daemon/readdir.c], when readdir() returns NULL, we
don't distinguish "end of directory stream" from "readdir() failed". This
rewrite fixes this problem -- I didn't see much value separating out the
fix for the original do_readdir().
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1674392
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20220502085601.15012-2-lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
On RHEL 7 (rpm-devel-4.11.3-45.el7.x86_64):
rpm-c.c: In function ‘guestfs_int_daemon_rpm_start_iterator’:
rpm-c.c:97:44: error: ‘RPMVSF_MASK_NOSIGNATURES’ undeclared (first use in this function)
rpmtsSetVSFlags (ts, rpmtsVSFlags (ts) | RPMVSF_MASK_NOSIGNATURES);
^
rpm-c.c:97:44: note: each undeclared identifier is reported only once for each function it appears in
Fixes: commit aa6f8038f8
Bytes.get_uint8 was added in OCaml 4.08. To support OCaml >= 4.04 (in
particular, RHEL 8 has OCaml 4.07) we have to replace this function
with the equivalent native call. We can remove this commit once the
baseline OCaml moves up.
Updates: commit edfebee404
Older distros (eg CentOS 6) used SHA-1 RPM package signatures which
some newer distros (eg RHEL 9.0) prevent us from verifying.
This resulted in packages with SHA-1 signatures being skipped by
librpm (there is a warning in debug output, but if you're not looking
at that then the package is silently ignored). In some cases
essential packages like the kernel were skipped, which would be
visible as a failure of virt-v2v. In other cases (eg virt-inspector)
you'd just see fewer installed packages in the <applications> list.
Since verifying package signatures is not essential for inspection,
disable this feature in librpm.
Reported-by: Xiaodai Wang
Thanks: Panu Matilainen
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2064182
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
This was a feature that allowed you to add drives to the appliance
after launching it. It was complicated to implement, and only worked
for the libvirt backend (not "direct", which is the default backend).
It also turned out to be a bad idea. The original concept was that
appliance creation was slow, so to examine multiple guests you should
launch the handle once then hot-add the disks from each guest in turn
to manipulate them. However this is terrible from a security point of
view, especially for multi-tenant, because the drives from one guest
might compromise the appliance and thus the filesystems/drives from
subsequent guests.
It also turns out that hotplugging is very slow. Nowadays appliance
creation should be faster than hotplugging.
The main use case for this was virt-df, but virt-df no longer uses it
after we discovered the problems outlined above.
These APIs were an experimental feature for passing through 9p
filesystems from the host to the libguestfs appliance. It was never
possible to use this without hacking the qemu command line of the
appliance to add such drives by hand. It also didn't fit the
libguestfs model very well. And 9p is generally deprecated in
upstream qemu.
Note that for ABI reasons these APIs are not actually removed, they
have been changed so that they always return an error. These APIs
were actually hard-removed from all versions of RHEL.
See-also: https://bugzilla.redhat.com/921710
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2030709
Thanks: label@rockylinux.org
---
RWMJ notes: I fixed the original patch so it compiled. This patch
sets osinfo to "rocky8", which doesn't exist in the osinfo db yet.
Arguably we might want to set this to "centos8", but we can see what
libosinfo decides to do. Here is partial virt-inspector output on a
Rocky Linux disk image:
$ ./run virt-inspector -a disk.img
<?xml version="1.0"?>
<operatingsystems>
<operatingsystem>
<root>/dev/rl/root</root>
<name>linux</name>
<arch>x86_64</arch>
<distro>rocky</distro>
<product_name>Rocky Linux 8.5 (Green Obsidian)</product_name>
<major_version>8</major_version>
<minor_version>5</minor_version>
<package_format>rpm</package_format>
<package_management>dnf</package_management>
<hostname>localhost.localdomain</hostname>
<osinfo>rocky8</osinfo>
<mountpoints>
<mountpoint dev="/dev/rl/root">/</mountpoint>
<mountpoint dev="/dev/sda1">/boot</mountpoint>
</mountpoints>
<filesystems>
<filesystem dev="/dev/rl/root">
<type>xfs</type>
<uuid>fed8331f-9f25-40cd-883e-090cd640559d</uuid>
</filesystem>
<filesystem dev="/dev/rl/swap">
<type>swap</type>
<uuid>6da2c121-ea7d-49ce-98a3-14a37fceaadd</uuid>
</filesystem>
<filesystem dev="/dev/sda1">
<type>xfs</type>
<uuid>4efafe61-2d20-4d93-8055-537e09bfd033</uuid>
</filesystem>
</filesystems>
The "is_partition_can_hold_filesystem" function calls
"Parted.part_get_gpt_type" on the partition if:
- the partition table type is GPT,
- or the partition table type is MBR, and the partition is primary or
logical.
The one entry in the fake MBR partition table described in the previous
patch passes the second branch of this check, therefore
"Parted.part_get_gpt_type" is reached, and it invokes "sgdisk -i 1" on the
disk.
Surprisingly (not), while "sgdisk -i" copes fine with valid MBR partition
tables, it chokes on the fake one. The output does not contain the
"Partition GUID code" line, and so "sgdisk_info_extract_field" throws an
exception.
Prevent calling "Parted.part_get_gpt_type" on a bogus MBR partition table,
similarly to the "extended entry in MBR partition table" case; the
difference is that the bogus primary entry, unlike a valid extended entry,
*can* hold a filesystem.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1931821
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
Message-Id: <20211125094954.9713-6-lersek@redhat.com>
"parted" incorrectly reports "loop" rather than "msdos" for the partition
table type, when the (fake) partition table comes from the "--mbr" option
of "mkfs.fat" (in dosfstools-4.2+), and the FAT variant in question is
FAT16 or FAT32. (See RHBZ#2026224.) Work this around by
- parsing the partition table ourselves, and
- overriding "loop" with "msdos" when appropriate.
Note that when the FAT variant is FAT12, "parted" fails to parse the fake
MBR partition table completely (see RHBZ#2026220), which we cannot work
around. However, FAT12 should be a rare corner case in libguestfs usage --
"mkfs.fat" auto-chooses FAT12 only below 9MB disk size, and even "-F 12"
can only be forced up to and including 255MB disk size.
Add the helper function "has_bogus_mbr" to the Utils module; we'll use it
elsewhere too.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1931821
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
Message-Id: <20211125094954.9713-5-lersek@redhat.com>
[lersek@redhat.com: drop "fun" keyword, and use partial application, in
the definition of "sec0at" [Rich]]
Since commit 994ca1f8eb ("daemon: Reimplement 'part_get_mbr_part_type'
API in OCaml.", 2018-05-02), we've not had any calls to
print_partition_table() that would pass a "false" argument for the
"add_m_option" parameter.
Remove the parameter, and inside part_get_mbr_part_type(), remove the dead
branch.
Relatedly, update the comment on the
"print_partition_table_machine_readable" OCaml function, originally from
commit 32e661f421 ("daemon: Reimplement ‘part_list’ API in OCaml.",
2017-07-27). Because print_partition_table() now passes "-m" to "parted"
unconditionally, and there are no use cases left that would *forbid* "-m",
"print_partition_table_machine_readable" is almost equivalent to
print_partition_table() -- modulo the enforcement of the "BYT;" header.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
Message-Id: <20211125094954.9713-4-lersek@redhat.com>
The directory that readdir() and closedir() work on is BUS_PATH
("/sys/bus/virtio/drivers/9pnet_virtio"), not "/sys/block". Fix the error
messages that are sent when readdir() or closedir() fails.
(The invalid "sys/block" pathname could be a leftover from when the
directory reading logic was (perhaps) copied from "daemon/sync.c".)
Fixes: 5f10c33503
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
Message-Id: <20211125094954.9713-3-lersek@redhat.com>
Search the usage output of "mkfs.fat" for "--mbr[="; cache the result for
further invocations. If the option is supported, pass "--mbr=n" to
"mkfs.fat". This will prevent the creation of a bogus partition table
whose first (and only) entry describes a partition that contains the
partition table.
(Such a bogus partition table breaks "parted", which is a tool used by
libguestfs extensively, both internally and in public libguestfs APIs.)
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1931821
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
Message-Id: <20211125094954.9713-2-lersek@redhat.com>
Currently, the Yara test case ("yara/test-yara-scan.sh") fails, with the
following obscure error message:
> ><fs> yara-scan /text.txt
> libguestfs: error: deserialise_yara_detection_list:
Namely, the Yara rule match list serialization / de-serialization, between
the daemon and the library, is broken. It is caused by the following
incompatible pointer passing (i.e., undefined behavior), in function
do_internal_yara_scan(), file "daemon/yara.c":
> r = yr_rules_scan_fd (rules, fd, 0, yara_rules_callback, (void *) path, 0);
^^^^^^^^^^^^^^^^^^^
The prototype of yara_rules_callback() is:
> static int
> yara_rules_callback (int code, void *message, void *data)
however, in Yara commit 2b121b166d25 ("Track string matches using
YR_SCAN_CONTEXT.", 2020-02-27), which was included in the upstream v4.0.0
release, the rules callback prototype was changed as follows:
> diff --git a/libyara/include/yara/types.h b/libyara/include/yara/types.h
> index cad095cd70c2..f415033c4aa6 100644
> --- a/libyara/include/yara/types.h
> +++ b/libyara/include/yara/types.h
> @@ -661,6 +644,7 @@ struct YR_MEMORY_BLOCK_ITERATOR
>
>
> typedef int (*YR_CALLBACK_FUNC)(
> + YR_SCAN_CONTEXT* context,
> int message,
> void* message_data,
> void* user_data);
Therefore, the yara_rules_callback() function is entered with a mismatched
parameter list in the daemon, and so it passes garbage to
send_detection_info(), for serializing the match list.
This incompatible change was in fact documented by the Yara project:
https://github.com/VirusTotal/yara/wiki/Backward-incompatible-changes-in-YARA-4.0-API#scanning-callback
Gcc too warns about the incompatible pointer type, under
"-Wincompatible-pointer-types". However, libguestfs is built without
"-Werror" by default, so the warning is easy to miss, and the bug only
manifests at runtime.
(The same problem exists for yr_compiler_set_callback() /
compile_error_callback():
https://github.com/VirusTotal/yara/wiki/Backward-incompatible-changes-in-YARA-4.0-API#compiler-callback
except that this instance of the problem is not triggered by the test
case, as the rule list always compiles.)
Rather than simply fixing the parameter lists, consider the following
approach.
If Yara's YR_CALLBACK_FUNC and YR_COMPILER_CALLBACK_FUNC typedefs were not
for *pointer* types but actual *function* prototypes, then we could use
them directly in the declarations of our callback functions. Then any
future changes in the param lists would force a "conflicting types"
*compilation error* (not a warning). Illustration:
/* this is *not* a pointer type */
typedef int HELLO_FUNC (void);
/* function declarations */
static HELLO_FUNC my_hello_good;
static HELLO_FUNC my_hello_bad;
/* function definition, with explicit parameter list */
static int my_hello_good (void) { return 1; }
/* function definition with wrong param list -> compilation error */
static int my_hello_bad (int i) { return i; }
Unfortunately, given that the Yara-provided typedefs are already pointers,
we can't do this, in standard C anyway. Type derivation only allows for
array, structure, union, function, and pointer type derivation; it does
not allow "undoing" previous derivations.
However, using gcc's "typeof" keyword, the idea is possible. Given
YR_CALLBACK_FUNC, the expression
(YR_CALLBACK_FUNC)NULL
is a well-defined null pointer, and the function designator expression
*(YR_CALLBACK_FUNC)NULL
has the desired function type.
Of course, evaluating this expression would be undefined behavior, but in
the GCC extension expression
typeof (*(YR_CALLBACK_FUNC)NULL)
the operand of the "typeof" operator is never evaluated, as it does not
have a variably modified type. We can therefore use this "typeof" in the
same role as HELLO_FUNC had in the above example.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20211011223627.20856-4-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
[lersek@redhat.com: clean up whitespace in "YR_RULE *rule"]
Pass $(HIVEX_LIBS) with -cclib under the "daemon_utils_tests_LINK" target;
otherwise the OCaml compiler does not tell the linker where "-lhivex" can
be found, and the linking step fails if "-lhivex" is not on a system
library path.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210908133542.19002-3-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
"ocamlc -where" is supposed to "print the location of the standard library
and exit". While this directory contains core OCaml C header files, it
does not contain hivex-related C header files. Trim "guestfsd_CPPFLAGS"
accordingly.
Furthermore, the hivex module for OCaml may exist elsewhere than under the
OCaml standard library directory. Invoke "ocamlfind query hivex" to find
this module. This is what AC_CHECK_OCAML_PKG(hivex) does too, in
"m4/guestfs-ocaml.m4" and "m4/ocaml.m4".
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210908133542.19002-2-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
Found by GCC -fanalyzer:
xattr.c:478:32: error: '%zu' directive output may be truncated writing between 1 and 19 bytes into a region of size 16 [-Werror=format-truncation=]
478 | snprintf (num, sizeof num, "%zu", nr_attrs);
| ^
xattr.c:478:32: note: directive argument in the range [0, 2305843009213693950]
/usr/include/bits/stdio2.h:71:10: note: '__builtin___snprintf_chk' output between 2 and 20 bytes into a destination of size 16
71 | return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
| ^
Commit 0f54df53d2 ("build: Remove gnulib") introduced a bug when I
rewrote existing code that used gnulib areadlink().
A missing "continue" statement on the path where fstatat(2) failed
caused fall-through to the case where it tries to use malloc(3) on the
value from the uninitialized stat buf. This caused a huge amount of
memory to be allocated, invoking the oom-killer inside the appliance.
Reported-by: Yongkui Guo
Fixes: commit 0f54df53d2
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1960217
Commit 2f587bbaec ("daemon: Read ISO9660 Primary Volume Descriptor
directly.") changed daemon/isoinfo.ml to read the PVD directly. This
was fine for guestfs_isoinfo_device which opens a device name, but did
not work for ISOs embedded within filesystems opened using
guestfs_isoinfo because we did not chroot into the filesystem first.
Example reproducer (run from the libguestfs source directory):
$ guestfish -N fs -m /dev/sda1 upload ./test-data/test.iso /test.iso
$ guestfish --ro -a test1.img -m /dev/sda1 isoinfo /test.iso
libguestfs: error: isoinfo: open: /test.iso: No such file or directory
After this fix:
$ guestfish --ro -a test1.img -m /dev/sda1 isoinfo /test.iso
iso_system_id:
iso_volume_id: ISOIMAGE
iso_volume_space_size: 2490
[etc.]
Reported-by: Yongkui Guo
Fixes: commit 2f587bbaec
Fixes: https://bugzilla.redhat.com/show_bug.cgi
In RHEL 8+, /usr/etc no longer exists. Since we were looking for this
directory in order to detect a separate /usr partition, those were no
longer detected, so the merging of /usr data into the root was not
being done. The result was incomplete inspection data and failure of
virt-v2v.
All Linux systems since forever have had /usr/src but not /src, so
detect this instead.
Furthermore the merging code didn't work, because we expected that the
root filesystem had a distro assigned, but in this configuration we
may need to look for that information in /usr/lib/os-release (not on
the root filesystem). This change makes the merging work even if we
have incomplete information about the root filesystem, so long as we
have an /etc/fstab entry pointing to the /usr mountpoint.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1949683
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1930133
Fixes: commit 394d11be49
As part of our efforts to clean up and simplify libguestfs, removing
gnulib deletes a large dependency that we mostly no longer use and
causes problems for new users trying to build the library from source.
A few modules from gnulib are still used (under a compatible license)
and these are copied into gnulib/lib/
It turns out we can read the information we need for the isoinfo API
directly from the ISO9660 PVD. We don't need to use either isoinfo or
xorriso. This also has the advantages of reducing by 1 the number of
dependencies in the appliance, and reducing potential vulnerability to
a crafted ISO file.
This also fixes timezone calculation for the datetime fields.
Thanks: Thomas Schmitt
Updates: commit efb8a766ca
Currently the guestfs_isoinfo and guestfs_isoinfo_device APIs run
isoinfo inside the appliance to extract the information.
isoinfo is part of genisoimage which is somewhat dead upstream.
xorriso is supposedly the new thing. (For a summary of the situation
see: https://wiki.debian.org/genisoimage).
This commit rewrites the parsing from C to OCaml to make it easier to
deal with, and allows you to use either isoinfo or xorriso.
Mostly the same fields are available from either tool, but xorriso is
a bit more awkward to parse.
The child (chrooted) process wrote its answer on the pipe and then
exited. Meanwhile the parent waiting for the child to exit before
reading from the pipe. Thus if the output was larger than a Linux
pipebuffer then the whole thing would deadlock.
This was returning "readdir: Invalid argument" which is actually
impossible (readdir(3) cannot fail with EINVAL). It turns out that
the problem is just errno from some other place leaking out.
This was deprecated in btrfs 4.14.1 and recently removed (see
btrfs-progs commit 4bd94dba8a "btrfs-progs: mkfs: remove alloc start
options and docs"). If the option is set simply ignore it.
Current GNU tar does not restore all extended attributes. In
particular only user.* capabilities are restored (although all
are saved in the tarball).
To restore capabilities, SELinux security attributes, and other things
we need to use --xattrs-include=*
For further information on the tar bug, see:
https://bugzilla.redhat.com/show_bug.cgi?id=771927
Previously callers were unable to distinguish a regular error (like an
I/O error) from the case where you call this API on something which is
valid but not a logical volume. Set errno to a known value in this
case.
In case any bare filesystems were decrypted using cryptsetup-open,
they would appear as /dev/mapper/name devices. Since list-filesystems
did not consider those when searching for filesystems, the unencrypted
filesystems would not be returned.
Note that previously this worked for LUKS because the common case
(eg. for Fedora) was that whole devices were encrypted and thoes
devices contained LVs, so luks-open + vgactivate would activate the
LVs which would then be found by list-filesystems. For Windows
BitLocker, the common case seems to be that each separate NTFS
filesystem is contained in a separate BitLocker wrapper.
