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.
This commit deprecates luks-open/luks-open-ro/luks-close for the more
generic sounding names cryptsetup-open/cryptsetup-close, which also
correspond directly to the cryptsetup commands.
The optional cryptsetup-open readonly flag is used to replace the
functionality of luks-open-ro.
The optional cryptsetup-open crypttype parameter can be used to select
the type (corresponding to cryptsetup open --type), which allows us to
open BitLocker-encrypted disks with no extra effort. As a convenience
the crypttype parameter may be omitted, and libguestfs will use a
heuristic (based on vfs-type output) to try to determine the correct
type to use.
The deprecated functions and the new functions are all (re-)written in
OCaml.
There is no new test here, unfortunately. It would be nice to test
Windows BitLocker support in this new API, however the Linux tools do
not support creating BitLocker disks, and while it is possible to
create one under Windows, the smallest compressed disk I could create
is 37M because of a mixture of the minimum support size for BitLocker
disks and the fact that encrypted parts of NTFS cannot be compressed.
Also synchronise with common module.
I couldn't get GCC 10.1 to ignore this warning any longer, possibly
because I am using LTO. In any case dereferencing a pointer is
undefined behaviour, so let's use GCC's __builtin_trap() function
instead (also supported by clang).
debug.c: In function 'debug_segv':
debug.c:1002:8: error: null pointer dereference [-Werror=null-dereference]
1002 | *ptr = 1;
| ^
The kernel returns xattr names in a slightly peculiar format. We
parsed this format several times in the code. Refactor this parsing
so we only do it in one place.
Linux from around 5.6 now enumerates individual disks in any order
(whereas previously it enumerated only drivers in parallel). This
means that /dev/sdX ordering is no longer stable - in particular we
cannot be sure that /dev/sda inside the guest is the first disk that
was attached to the appliance, /dev/sdb the second disk and so on.
However we can still use SCSI PCI device numbering as found in
/dev/disk/by-path. Use this to translate device names in and out of
the appliance.
Thanks: Vitaly Kuznetsov, Paolo Bonzini, Dan Berrangé.
This function doesn't work reliably with the proposed change to device
name translation. The reason is that strings returned by
Devsparts.list_devices contained translated names, so their indexes
did not correspond to the untranslated names used outside the
appliance..
We can avoid this and make the function much simpler and faster by
implementing it on the library side instead.
The current code was broken, as the field 1 of the exception value is
the error code (int), not an error string, and thus it would have
crashed. This did not happen in practice, as all the usage of
ocaml-augeas were only in the inspection code with ad-hoc exception
catching blocks.
Other than fixing the aforementioned issue, enhance the error reporting
to be as close as possible to what the current AUGEAS_ERROR() macro
does: error message, error minor message (if available), error details
(if available).
Move the interal static libraries as the last items in the list of
libraries of guestfsd, to make sure their symbols are used for all the
other libraries. This is because GCC resolves the symbols looking at
the arguments from the beginning to the end of the command line.
This currently does not cause failures, however it "just works" because
of the tricky situation set up.
The situation is the following:
1) common/utils contains few utility sources: one of them is utils.c,
which contains various functions -- for example
guestfs_int_free_string_list and guestfs_int_drive_name --, it is built
as utils.o, and bundled in the static library libutils.a
2) common/mlutils builds a OCaml library with bindings for some utility
functions in libutils.a, in particular guestfs_int_drive_name (but not
guestfs_int_free_string_list); there are two versions of this library,
one OCaml library (dllmlcutils.so) that links with libutils.a, and one
static library (libmlcutils.a), which cannot specify the libraries it
links to (as it is static)
3) when the daemon is linked, the command line was the following
(simplified):
$ gcc [...] -o guestfsd guestfsd-9p.o other_daemon_object.o [...] \
../common/utils/.libs/libutils.a [...] -lmlcutils [...]
Some of the objects of the daemon itself use
guestfs_int_free_string_list, and thus the compiler opens libutils.a
(it is after the objects in the command line) and picks utils.o, which
contains also guestfs_int_drive_name (not used directly in the daemon);
when linking later on with libmlcutils.a, the symbols for this static
library (like guestfs_int_drive_name) are already resolved, and thus
all the symbols are resolved, and the linking succeeds
This fragile situation can be easily broken by moving e.g.
guestfs_int_drive_name out of common/utils/utils.c to a new source (say
utils2.c) still built as part of libutils.a: since nothing before
-lmlcutils actually needs to pick utils2.o from libutils.a for symbols,
then GCC will not be able to resolve all the symbols in libmlcutils.a.
As solution, move libutils.a (and other internal static libraries) as
last libraries to link guestfsd to: this way, GCC knows where to find
all the symbols needed by all the objects and libraries specified in
the command line.
Remove gnulib modules that provide stuff clearly not used within
libguestfs (library, daemon, and C tools). Among directly and
indirectly modules used previous (and now no more), they are:
cycle-check
d-ino
dev-ino
dup3
dup3-tests
fcntl-safer
fcntl-safer-tests
fdopendir
fdopendir-tests
filevercmp
filevercmp-tests
ftell
ftell-tests
ftello
ftello-tests
fts
getaddrinfo
getaddrinfo-tests
getcwd
getcwd-tests
gnu-make
hostent
i-ring
i-ring-tests
iconv
iconv-tests
inet_ntop
inet_ntop-tests
isatty
isatty-tests
openat-safer
openat-safer-tests
opendirat
ptsname_r
ptsname_r-tests
read-file
read-file-tests
rewinddir
servent
ttyname_r
ttyname_r-tests
xgetcwd
Some of the removed modules are still used pulled indirectly as
dependency of other modules. There should be no behaviour change on
recent Linux distros, although older distros were not tested (adding
a module back is easy, anyway).
Remove accordingly unused automake variables, and ignored files.
Move the read_whole_file function to the common utilities of the daemon,
so other parts can use it. For this purpose, add an out parameter to
get the amount of bytes read.
Except from the parameter addition, this should be just refactoring.
