Commit Graph

11614 Commits

Author SHA1 Message Date
Richard W.M. Jones
2ecccfce38 daemon: Replace "noalloc" with [@@noalloc] 2021-12-09 13:54:15 +00:00
Richard W.M. Jones
9a5c5ac5bd m4: Remove test for OCaml Bytes module 2021-12-09 13:54:15 +00:00
Richard W.M. Jones
c2c7dfd66b rust: Use distclean to clean cache rather than make clean
Actually cargo caches downloaded libraries.  The previous change
caused cargo to download and rebuild these after make clean which is
overly aggressive.  Use make distclean instead.

Updates: commit 1834f19d20
2021-12-09 13:54:15 +00:00
Richard W.M. Jones
637f193189 rust: Wire up make clean so it runs cargo clean
Before this commit, after make clean:

$ du -sh rust
641M	rust

After:

776K	rust
2021-12-09 13:54:15 +00:00
Laszlo Ersek
ecf4449762 build, docs: spell out minimum version (4.0.0) for the (optional) Yara lib
Commit e597fc5317 ("daemon/yara: fix undefined behavior due to Yara 4.0
API changes", 2021-10-12) prevents the daemon from using such a Yara
version that precedes 4.0.0.

If only yara < 4 is found, treat the library as absent, rather than
attempting and failing to compile the yara module of the daemon. Note the
version requirement in the documentation too.

Suggested-by: Eric Blake <eblake@redhat.com>
Suggested-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20211013133611.21599-4-lersek@redhat.com>
Acked-by: Eric Blake <eblake@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
2021-12-09 13:54:15 +00:00
Laszlo Ersek
56172a193c build: eliminate the AC_CHECK_LIB / AC_CHECK_HEADER tests for Yara
Eliminate the AC_CHECK_LIB / AC_CHECK_HEADER tests for Yara, for the
following reasons:

- Upstream Yara has provided a pkg-config file since 2015, so the
  (now-fixed) pkg-config check should always find it, without the
  AC_CHECK_LIB / AC_CHECK_HEADER fallback branch.

- In a subsequent patch, we'll want to test for the incompatible Yara API
  changes described at
  <https://github.com/VirusTotal/yara/wiki/Backward-incompatible-changes-in-YARA-4.0-API>.

  That's easy to do with pkg-config, but impossible with AC_CHECK_*,
  without a custom test. Namely, both AC_CHECK_DECLS and AC_CHECK_TYPES
  appear unable to check the parameter list of a function pointer typedef
  (namely YR_CALLBACK_FUNC and YR_COMPILER_CALLBACK_FUNC). And writing a
  dedicated test for this is overkill.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20211013133611.21599-3-lersek@redhat.com>
Acked-by: Eric Blake <eblake@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
2021-12-09 13:54:15 +00:00
Laszlo Ersek
12414ef705 build: fix the pkg-config identifier of the (optional) Yara library
The upstream Yara project has always provided its "libyara/yara.pc.in"
file with "Name: yara" -- ever since its introduction in commit
334bd1a671ca ("Add support for pkg-config", 2015-01-08).

In spite of this, when the (optional) Yara dependency was added to
libguestfs in commit 2e24129da3 ("appliance: add yara dependency",
2017-05-02), PKG_CHECK_MODULES was invoked with [libyara] as
list-of-modules.

(That was clearly a bug. I'm unsure what Debian calls their Yara
pkg-config module, but calling it anything else than "yara" would be a
distribution bug: upstream projects provide pkg-config files specifically
so that dependent projects can find their dependencies *regardless of
distribution*.)

As a consequence, on Fedora today, the PKG_CHECK_MODULES macro always
fails, and only the AC_CHECK_LIB / AC_CHECK_HEADER branch finds Yara.

In a subsequent patch, we'll want to add a version requirement to the
PKG_CHECK_MODULES macro invocation, so at first, fix the pkg-config
identifier of Yara.

Fixes: 2e24129da3
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20211013133611.21599-2-lersek@redhat.com>
Acked-by: Eric Blake <eblake@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
2021-12-09 13:54:15 +00:00
Laszlo Ersek
4113eca69c daemon/yara: fix undefined behavior due to Yara 4.0 API changes
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"]
2021-12-09 13:54:15 +00:00
Laszlo Ersek
421ab66c8c lib/proto: suppress "may be used uninitialized" in send_file_complete()
gcc emits the following warning:

> proto.c: In function ‘send_file_complete’:
> proto.c:437:10: error: ‘buf’ may be used uninitialized
> [-Werror=maybe-uninitialized]
>   437 |   return send_file_chunk (g, 0, buf, 0);
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In theory, passing the 1-byte array "buf", with indeterminate contents, to
xdr_bytes() ultimately, could be fine -- assuming xdr_bytes() never reads
the contents of the buffer, due to the buffer size being zero. However,
the xdr_bytes() manual does not seem to guarantee this (it also does not
explicitly permit passing a NULL buffer alongside size=0, which would be
even simpler for the caller).

In order to shut up the compiler, just zero-initialize the buffer --
that's simpler than adding diagnostics pragmas. The "maybe-uninitialized"
warning is otherwise very useful, so keep it globally enabled (per
WARN_CFLAGS / WERROR_CFLAGS).

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20211011223627.20856-3-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
2021-12-09 13:54:15 +00:00
Laszlo Ersek
fc921c6515 build: fix typo in "--enable-werror" help string
While <https://libguestfs.org/guestfs-building.1.html> correctly documents
the "--enable-werror" option, the "./configure" help text itself doesn't.
Replace "--enable-error" with "--enable-werror" now.

Fixes: 0f54df53d2
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20211011223627.20856-2-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
2021-12-09 13:54:15 +00:00
Richard W.M. Jones
aa78dc8ece m4/guestfs-ocaml.m4: Fix deprecated warning format
In OCaml 4.13:

Alert ocaml_deprecated_cli: Setting a warning with a sequence of lowercase or uppercase letters,
like 'CDEFLMPSUVYZX', is deprecated.
Use the equivalent signed form: +C+D+E+F+L+M+P+S+U+V+Y+Z+X+52-3.

(cherry picked from
guestfs-tools commit fa4f59e1d99c08d7e0bae2a7cb54f254a6506d67)
2021-12-09 13:54:15 +00:00
Laszlo Ersek
e2f8db27d0 Go bindings: fix "C array of strings" -- char** -- allocation
The current "arg_string_list" and "free_string_list" implementations go
back to commit b6f01f3260 ("Add Go (language) bindings.", 2013-07-03).
There are two problems with them:

- "free_string_list" doesn't actually free anything,

- at the *first* such g.Internal_test() call site that passes an
  Ostringlist member inside the Optargs argument, namely:

> g.Internal_test ("abc",
>                  string_addr ("def"),
>                  []string{},
>                  false,
>                  0,
>                  0,
>                  "123",
>                  "456",
>                  []byte{'a', 'b', 'c', '\000', 'a', 'b', 'c'},
>                  &guestfs.OptargsInternal_test{Ostringlist_is_set: true,
>                                                Ostringlist: []string{}
>                                               }
>                 )

  the "golang/run-bindtests" case crashes:

> panic: runtime error: cgo argument has Go pointer to Go pointer
>
> goroutine 1 [running]:
> libguestfs.org/guestfs.(*Guestfs).Internal_test.func7(0xc000018180,
> 0xadfb60, 0xadfb80, 0xc000010048, 0x0, 0x0, 0x0, 0xae3e10, 0xae3e30,
> 0xade3a0, ...)
>         golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0xa9
> libguestfs.org/guestfs.(*Guestfs).Internal_test(0xc000018180, 0x4ee3a5,
> 0x3, 0xc000061be8, 0xc000061af8, 0x0, 0x0, 0xc000061a00, 0x0, 0x0, ...)
>         golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0x3c9
> main.main()
>         golang/bindtests/bindtests.go:77 +0x151e
> exit status 2
> FAIL run-bindtests (exit status: 1)

In Daniel P. Berrangé's words [1],

> You're allowed to pass a Go pointer to C via CGo, but the memory that
> points to is not allowed to contained further Go pointers. So the struct
> fields must strictly use a C pointer.

One pattern to solve the problem has been shown on stackoverflow [2].
Thus, rewrite the "arg_string_list" and "free_string_list" functions
almost entirely in C, following that example.

While this approach is not the most idiomatic Go, as a solution exists
without C helper functions [3], it should still be acceptable, at least as
an incremental improvement, for letting "golang/run-bindtests" pass.

[1] https://listman.redhat.com/archives/libguestfs/2021-September/msg00118.html
[2] https://stackoverflow.com/questions/35924545/golang-cgo-panic-runtime-error-cgo-argument-has-go-pointer-to-go-pointer
[3] https://listman.redhat.com/archives/libguestfs/2021-September/msg00106.html

Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210921192939.32468-1-lersek@redhat.com>
Tested-by: "Richard W.M. Jones" <rjones@redhat.com>
Acked-by: "Richard W.M. Jones" <rjones@redhat.com>
2021-09-27 09:57:34 +02:00
Richard W.M. Jones
5c68392175 Version 1.46.0. v1.46.0 2021-09-23 14:52:46 +01:00
Richard W.M. Jones
5b432781e2 build: Remove BUGS file
Recent changes to Red Hat Bugzilla broke our ability to generate this
file.  In any case it's not very useful since a simple Bugzilla query
can be used instead:

https://bugzilla.redhat.com/buglist.cgi?component=libguestfs

Remove the file.  The generator used this file to ensure it was being
run from the correct directory and to take a lock.  Use RELEASES file
instead.

See also: https://github.com/python-bugzilla/python-bugzilla/issues/149
2021-09-23 14:52:46 +01:00
Richard W.M. Jones
ab2d624f46 docs: Finalize release notes for 1.46 release today 2021-09-23 09:44:08 +01:00
Laszlo Ersek
627f808e4b tests: xfs: remove lazy-counter disablement test
According to xfs_admin(8):

>        -c 0|1 Enable (1) or disable (0) lazy-counters in the  filesys‐
>               tem.
>
>               Lazy-counters  may  not  be  disabled  on  Version 5 su‐
>               perblock filesystems (i.e. those with metadata CRCs  en‐
>               abled).
>
>               [...]

According to mkfs.xfs(1):

>        -m global_metadata_options
>        Section Name: [metadata]
>               These  options  specify metadata format options that ei‐
>               ther apply to the entire  filesystem  or  aren't  easily
>               characterised  by  a  specific  functionality group. The
>               valid global_metadata_options are:
>
>                   [...]
>
>                    crc=value
>                           This  is  used  to create a filesystem which
>                           maintains and checks CRC information in  all
>                           metadata  objects  on disk. The value is ei‐
>                           ther 0 to disable the feature, or 1  to  en‐
>                           able the use of CRCs.
>
>                           [...]
>
>                           By  default,  mkfs.xfs  will enable metadata
>                           CRCs.

Consistently with the above, the first "xfs_admin" test case in
"generator/actions_core.ml", which attempts to disable lazy counters,
always fails:

> 534/550 test_xfs_admin_0
> libguestfs: error: xfs_admin: /dev/sda1: Cannot disable lazy-counters on V5 fs

We can resolve this test failure in three ways:

(1) Extend do_mkfs() [daemon/mkfs.c], possibly even introduce
    do_mkfs_xfs(), and permit the caller to specify "-m crc=0" for
    mkfs.xfs. Then use this option when the temporary filesystem is
    created in the XFS test that disables lazy counters.

(2) Extend the "guestfs_int_xfsinfo" structure in the libguestfs-common
    project, with an "xfs_crc" field. Extend parse_xfs_info()
    [daemon/xfs.c] to populate the field from "meta-data=...crc=[01]".
    Modify the test case to check the following post-condition:

      xfs_crc || xfs_lazycount == 0

    instead of the current

      xfs_lazycount == 0

    effectively ignoring "xfs_lazycount" when "xfs_crc" is set.

(3) Remove the test altogether that attempts to disable lazy counters
    after filesystem creation.

Given that new XFS filesystems are created with metadata CRCs enabled by
default, and several XFS features depend on metadata CRCs being enabled,
this patch implements option (3).

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210920052335.3358-4-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
2021-09-21 19:01:18 +02:00
Laszlo Ersek
24f98939ae test-md-and-lvm-devices: work around RAID0 regression in Linux v3.14/v5.4
The "test-md-and-lvm-devices" test case creates, among other things, a
RAID0 array (md127) that spans two *differently sized* block devices
(sda1: 20MB, lv0: 16MB).

In Linux v3.14, the layout of such arrays was changed incompatibly and
undetectably. If an array were created with a pre-v3.14 kernel and
assembled on a v3.14+ kernel, or vice versa, data could be corrupted.

In Linux v5.4, a mitigation was added, requiring the user to specify the
layout version of such RAID0 arrays explicitly, as a module parameter. If
the user fails to specify a layout version, the v5.4+ kernel refuses to
assemble such arrays. This is why "test-md-and-lvm-devices" currently
fails, with any v5.4+ appliance kernel.

Until we implement a more general solution (see the bugzilla link below),
work around the issue by sizing sda1 and lv0 identically. For this,
increase the size of sdb1 to 24MB: when one 4MB extent is spent on LVM
metadata, the resultant lv0 size (20MB) will precisely match the size of
sda1.

This workaround only affects sizes, and does not interfere with the
original purpose of this test case, which is to test various *stackings*
between disk partitions, software RAID (md), and LVM logical volumes.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=2005485
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210920052335.3358-3-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
[lersek@redhat.com: remove stray empty line]
2021-09-21 19:00:59 +02:00
Laszlo Ersek
8156c16520 test-9p: fix the base directory that's exported to the guest
In commit 6d32773e81 ("tests: Run the tests in parallel.", 2021-03-18),
the "abs_srcdir" macro value that the 9p test would see changed from
".../tests/9p" to just ".../tests" -- the last component got dropped.

(Said commit updated some "abs_srcdir"-based references accordingly, for
example under "tests/disks", but "tests/9p/test-9p.sh" was missed.)

Therefore, the guest-visible location of the "/test-9p.sh" file changed to
"/9p/test-9p.sh", and a non-recursive listing of the guest-visible root
directory would not return the file. Thus, the test fails now.

Restore the host-side base directory to ".../tests/9p".

Fixes: 6d32773e81
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210920052335.3358-2-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
2021-09-21 19:00:11 +02:00
Olaf Hering
f47e0bb672 appliance: reorder mounting of special filesystems in init
Make sure proc and dev are available early.
No change in behavior intended.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
2021-09-15 12:37:08 +01:00
Olaf Hering
9db0c98c99 appliance: enable bash's Process Substitution feature
bash can read input from a spawned process, and even provide input to
such process. This feature relies on /dev/fd/ being present. In the
past udev silently created this symlink, so this bash feature worked
more or less by accident. With recent systemd versions, such as 246
which is included in Leap 15.3, the symlink is not created anymore. As
a result scripts, such as /sbin/dhclient-script, fail to work
properly.

This symlink should have been created in version 1 of this variant of /init.

https://bugzilla.opensuse.org/show_bug.cgi?id=1190501

Signed-off-by: Olaf Hering <olaf@aepfle.de>
2021-09-15 12:37:08 +01:00
Olaf Hering
c0de4de902 appliance: add reboot and netconfig for SUSE
systemd-sysvinit contains the reboot command, which is used to
properly stop the VM. This was required by other packages, and as a
result always available. Since Leap 15.3 it will not be installed, and
as a result the VM will just panic because /init died.

If the appliance is started with --network, dhclient will run
/usr/sbin/dhclient-script, which in turn may call /sbin/netconfig to
update /etc/resolv.conf. Install sysconfig-netconfig to make sure DNS
resolving actually works.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
2021-09-14 20:49:02 +01:00
Richard W.M. Jones
46ab3dbbc0 docs: Prepare draft release notes for 1.46 2021-09-13 19:29:17 +01:00
Richard W.M. Jones
ae8ff236ab bugs-in-changelog.sh: Various fixes to cope with new changelog format 2021-09-13 19:29:17 +01:00
Richard W.M. Jones
e14ff93742 lib: direct: Remove use of sga
sga (or "sgabios" or "Serial Graphics Adapter") is an option ROM for
seabios which directs output to the serial adapter.  This is very
useful for debugging BIOS problems during boot.

RHEL wants to deprecate this feature (in fact, they just deprecated it
without telling us).  However there is an equivalent feature in
seabios (seabios >= 1.11 / qemu >= 2.11.0) which can be enabled using
either -nographic or -machine graphics=off

This commit removes sga and enables -machine graphics=off in the
direct backend.

References (for RHEL 9 qemu change):
https://bugzilla.redhat.com/show_bug.cgi?id=2002325
https://bugzilla.redhat.com/show_bug.cgi?id=2000845
https://lists.nongnu.org/archive/html/qemu-devel/2021-09/msg02417.html
https://listman.redhat.com/archives/libvir-list/2021-September/msg00205.html

For the libvirt backend we will continue to use <bios useserial=yes>.
This currently breaks when sga is not available, but I talked to Dan
and the plan there is to adapt libvirt so the same XML will enable
-machine graphics=off.  IOW libguestfs does not need to make any
change.

References (for libvirt change):
https://bugzilla.redhat.com/show_bug.cgi?id=2003092
https://listman.redhat.com/archives/libvir-list/2021-September/msg00193.html

Thanks: Gerd Hoffman, Daniel Berrangé
2021-09-13 10:40:50 +01:00
Laszlo Ersek
523b0180d8 daemon_utils_tests: generalize ocaml-hivex[-devel] lookup
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>
2021-09-09 14:52:03 +02:00
Laszlo Ersek
53e03eefae daemon: generalize ocaml-hivex[-devel] lookup
"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>
2021-09-09 14:52:02 +02:00
Richard W.M. Jones
ceb034c92c python, java: Avoid bogus -fanalyzer warnings
This is essentially the same as the previous OCaml commit.  It does
not fix a real bug.
2021-09-07 16:50:38 +01:00
Richard W.M. Jones
ea04d6b878 ocaml/guestfs-c.c: Avoid bogus -fanalyzer warning
This warning is bogus, caused by the analyzer cannot track that len ==
0 if roots == NULL.  I just changed the code to make it easier to
analyze, this doesn't fix any real bug.

guestfs-c.c: In function 'guestfs_finalize':
guestfs-c.c:85:9: error: dereference of NULL '0B' [CWE-476] [-Werror=analyzer-null-dereference]
   85 |         caml_remove_generational_global_root (roots[i]);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2021-09-07 16:45:23 +01:00
Richard W.M. Jones
a7245fba7a daemon/utils.c: Fix potential unbounded stack usage
utils.c: In function 'prog_exists':
utils.c:650:1: error: stack usage might be unbounded [-Werror=stack-usage=]
  650 | prog_exists (const char *prog)
      | ^
2021-09-07 15:55:48 +01:00
Richard W.M. Jones
58599031f8 daemon/xattr.c: Increase size of temporary buffer for %zu
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,
      |          ^
2021-09-07 15:55:43 +01:00
Richard W.M. Jones
d00c36410b daemon/luks.c: Ignore bogus GCC -fanalyzer double-free warning
As far as I can tell the analysis is completely bogus.  We don't
double-free the tempfile string in do_luks_add_key.  Therefore add a
GCC suppression around the remove_temp function.

luks.c: In function 'do_luks_add_key':
luks.c:84:3: error: double-'free' of 'tempfile_14' [CWE-415] [-Werror=analyzer-double-free]
   84 |   free (tempfile);
      |   ^~~~~~~~~~~~~~~
  'do_luks_add_key': events 1-2
    |
    |  143 | do_luks_add_key (const char *device, const char *key, const char *newkey,
    |      | ^~~~~~~~~~~~~~~
    |      | |
    |      | (1) entry to 'do_luks_add_key'
    |......
    |  146 |   char *keyfile = write_key_to_temp (key);
    |      |                   ~~~~~~~~~~~~~~~~~~~~~~~
    |      |                   |
    |      |                   (2) calling 'write_key_to_temp' from 'do_luks_add_key'
    |
    +--> 'write_key_to_temp': events 3-12
           |
           |   41 | write_key_to_temp (const char *key)
           |      | ^~~~~~~~~~~~~~~~~
           |      | |
           |      | (3) entry to 'write_key_to_temp'
           |......
           |   47 |   tempfile = strdup ("/tmp/luksXXXXXX");
           |      |              ~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |              |
           |      |              (4) allocated here
           |   48 |   if (!tempfile) {
           |      |      ~
           |      |      |
           |      |      (5) assuming 'tempfile_14' is non-NULL
           |      |      (6) following 'false' branch (when 'tempfile_14' is non-NULL)...
           |......
           |   53 |   fd = mkstemp (tempfile);
           |      |   ~~
           |      |   |
           |      |   (7) ...to here
           |   54 |   if (fd == -1) {
           |      |      ~
           |      |      |
           |      |      (8) following 'false' branch...
           |......
           |   59 |   len = strlen (key);
           |      |   ~~~
           |      |   |
           |      |   (9) ...to here
           |   60 |   if (xwrite (fd, key, len) == -1) {
           |      |      ~
           |      |      |
           |      |      (10) following 'false' branch...
           |......
           |   66 |   if (close (fd) == -1) {
           |      |   ~~ ~
           |      |   |  |
           |      |   |  (12) following 'false' branch...
           |      |   (11) ...to here
           |
         'write_key_to_temp': event 13
           |
           |cc1:
           | (13): ...to here
           |
    <------+
    |
  'do_luks_add_key': events 14-17
    |
    |  146 |   char *keyfile = write_key_to_temp (key);
    |      |                   ^~~~~~~~~~~~~~~~~~~~~~~
    |      |                   |
    |      |                   (14) returning to 'do_luks_add_key' from 'write_key_to_temp'
    |  147 |   if (!keyfile)
    |      |      ~
    |      |      |
    |      |      (15) following 'false' branch...
    |......
    |  150 |   char *newkeyfile = write_key_to_temp (newkey);
    |      |   ~~~~               ~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |   |                  |
    |      |   |                  (17) calling 'write_key_to_temp' from 'do_luks_add_key'
    |      |   (16) ...to here
    |
    +--> 'write_key_to_temp': events 18-26
           |
           |   41 | write_key_to_temp (const char *key)
           |      | ^~~~~~~~~~~~~~~~~
           |      | |
           |      | (18) entry to 'write_key_to_temp'
           |......
           |   47 |   tempfile = strdup ("/tmp/luksXXXXXX");
           |      |              ~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |              |
           |      |              (19) allocated here
           |   48 |   if (!tempfile) {
           |      |      ~
           |      |      |
           |      |      (20) following 'false' branch (when 'tempfile_14' is non-NULL)...
           |......
           |   53 |   fd = mkstemp (tempfile);
           |      |   ~~
           |      |   |
           |      |   (21) ...to here
           |   54 |   if (fd == -1) {
           |      |      ~
           |      |      |
           |      |      (22) following 'false' branch...
           |......
           |   59 |   len = strlen (key);
           |      |   ~~~
           |      |   |
           |      |   (23) ...to here
           |   60 |   if (xwrite (fd, key, len) == -1) {
           |      |      ~
           |      |      |
           |      |      (24) following 'false' branch...
           |......
           |   66 |   if (close (fd) == -1) {
           |      |   ~~ ~
           |      |   |  |
           |      |   |  (26) following 'false' branch...
           |      |   (25) ...to here
           |
         'write_key_to_temp': event 27
           |
           |cc1:
           | (27): ...to here
           |
    <------+
    |
  'do_luks_add_key': events 28-32
    |
    |   84 |   free (tempfile);
    |      |   ~~~~~~~~~~~~~~~
    |      |   |
    |      |   (31) first 'free' here
    |      |   (32) second 'free' here; first 'free' was at (31)
    |......
    |  150 |   char *newkeyfile = write_key_to_temp (newkey);
    |      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                      |
    |      |                      (28) returning to 'do_luks_add_key' from 'write_key_to_temp'
    |  151 |   if (!newkeyfile) {
    |      |      ~
    |      |      |
    |      |      (29) following 'false' branch...
    |......
    |  156 |   const char *argv[MAX_ARGS];
    |      |   ~~~~~
    |      |   |
    |      |   (30) ...to here
    |
cc1: all warnings being treated as errors
2021-09-07 14:53:06 +01:00
Richard W.M. Jones
8dd09a16f5 daemon/inotify.c: Clean up error handling
In particular avoid calling fclose on a popen'd handle.

Error identified by GCC -fanalyzer.
2021-09-07 14:48:03 +01:00
Laszlo Ersek
81ad309d47 Update common submodule to commit 19302b64c5f1
Short log:

Laszlo Ersek (2):
      Makefile.am: supply missing $(LIBGUESTFS_CFLAGS)
      Makefile.am: use $(LIBGUESTFS_LIBS) for linking OCaml test programs

Richard W.M. Jones (4):
      tools: Refactor create_standard_options
      tools: Add optional --program-name flag.
      mltools/test-getopt.sh: Allow test to work for tools version 2
      mltools: Add JSON_parser.object_get_bool

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
2021-09-06 18:16:11 +02:00
Richard W.M. Jones
4e02c13941 m4/guestfs-appliance.m4: Add support for Alma and Cloud Linux
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2001636
Thanks: sasha121196@mail.ru
2021-09-06 17:02:19 +01:00
Richard W.M. Jones
ae7187af94 appliance: Add mount package for Debian
https://listman.redhat.com/archives/libguestfs/2021-September/msg00013.html

Reported-by: Joerg Schiermeier
2021-09-06 15:23:03 +01:00
Laszlo Ersek
0a2f7621a0 tests/mount-local: fix relative pathname of FUSE client executable
In commit 6d32773e81 ("tests: Run the tests in parallel.", 2021-03-18),
the working directory relative to which "test-parallel-mount-local" would
be launched (by the test machinery) changed from "tests/mount-local" to
just "tests".

While the relative pathname of the "guestunmount" executable was updated
inside "test-parallel-mount-local" accordingly, the relative pathname of
the FUSE client ("test-parallel-mount-local" itself, just invoked with
"--test") was not. This issue guarantees that the exec call fails in the
child, and so the test case always hangs.

Because we had removed "mount-local" from the end of the working
directory, prepend it now to the relative pathname of the FUSE client
executable.

Fixes: 6d32773e81
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210902135124.15191-3-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
2021-09-03 12:06:54 +02:00
Laszlo Ersek
79d7fc8674 tests/mount-local: exit child immediately when exec fails
Each worker thread of "test-parallel-mount-local" performs the following
steps (among others):

(1) it starts an appliance dedicated to that thread, using a private
    scratch disk image,

(2) exports a dedicated FUSE mount point on the host, exposing the file
    system on the appliance's disk,

(3) launches a child process for manipulating the particular FUSE mount
    point on the host,

(4) enters a FUSE request processing loop, translating requests between
    the host kernel (coming in via the FUSE mount point) and the
    appliance.

Items to note:

- The child process from step (3) consists of a single thread of execution
  (see fork() in POSIX): a duplicate of the parent process's respective
  worker thread.

- The child process from step (3) blocks on any FUSE mount point access on
  the host until the worker thread in the parent process starts processing
  FUSE requests, in step (4).

- The FUSE request processing in step (4), in the worker thread living in
  the parent process, terminates if and only if the child process unmounts
  the FUSE mount point originating from (2).

Should the exec call in step (3) fail for any reason, the child currently
jumps to the "error" label. This is wrong: under the error label, we call
guestfs_close() on the appliance -- but the appliance is owned by the
parent process's worker thread, not the child. What happens is that the
child kills off the appliance while the parent's worker thread is in the
FUSE request processing loop (4).

The "error" label was never meant to be reached by the child process -- if
exec fails for any reason, exit the child immediately. The parent will
remain in the FUSE request processing loop (4) forever, but no state will
be corrupted. For example, using another (interactive) session on the
host, the FUSE mount points can be interacted with, and if all of them are
manually unmounted, the FUSE request processing (4) completes in every
worker thread.

This patch does not fix the primary issue with
"test-parallel-mount-local", but removes "chaos" from the symptoms. The
next patch will fix the actual regression in this test case.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210902135124.15191-2-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
2021-09-03 12:06:45 +02:00
Richard W.M. Jones
13a1ae6da6 Version 1.45.7. v1.45.7 2021-08-31 16:32:02 +01:00
Hilko Bengen
39f514b28d appliance: Fix searching for shared libraries on usr-merged Debian systems
If /lib is a symlink to usr/lib, paths to shared libraries as
determined by ld.so may differ from dpkg's file lists.

We turn the filename search pattern into a glob expression by
prefixing it with a '*', so the required packages are found again:

$ dpkg -S /lib/x86_64-linux-gnu/libpcre2-8.so.0
dpkg-query: no path found matching pattern /lib/x86_64-linux-gnu/libpcre2-8.so.0
$ dpkg -S */lib/x86_64-linux-gnu/libpcre2-8.so.0
libpcre2-8-0:amd64: /usr/lib/x86_64-linux-gnu/libpcre2-8.so.0
2021-08-31 15:49:55 +01:00
Richard W.M. Jones
45de287447 lib: Autodetect backing format for qemu-img create -b
qemu 6.1 has decided to change qemu-img create so that a backing
format (-F) is required if a backing file (-b) is specified.  Since we
don't want to change the libguestfs API to force callers to specify
this because that would be an API break, autodetect it.

This is similar to commit c8c181e8d9 ("launch: libvirt: Autodetect
backing format for readonly drive overlays").

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1998820
2021-08-31 08:35:52 +01:00
Alexandre Iooss
210959cc34 build: Define HAVE_RPM, HAVE_DPKG and HAVE_PACMAN
When using option `--with-distro`, `HAVE_RPM`, `HAVE_DPKG` and
`HAVE_PACMAN` are not defined and make the configure phase fail.
This makes sure that these conditionals are always defined.
2021-08-27 20:25:14 +01:00
Richard W.M. Jones
73cd0a0c8d lib: Add osinfo information for Windows Server 2022 Datacenter
Windows Server 2022 preview is identified as <osinfo>win2k16</osinfo>.
Although current osinfo-db does not have an entry "win2k22", return
this instead.

osinfo-db issue to add win2k22:
https://gitlab.com/libosinfo/osinfo-db/-/issues/82

Inspection information for the guest:

    type: windows
    distro: windows
    product_name: Windows Server 2022 Datacenter
    product_variant: Server
    version: 10.0
    arch: x86_64

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1997446
Reported-by: Yongkui Guo
2021-08-25 11:09:37 +01:00
Heinrich Schuchardt
efb3d01992 launch: board model for RISC-V
On RISC-V there is no default machine type. Invoking QEMU requires to
specify a board model with the -M option. So let's define MACHINE_TYPE
as virt.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
2021-08-20 08:13:10 +01:00
Richard W.M. Jones
90a076fe19 appliance: Add IBM850 iconv converter for syslinux
$ guestfish -N fs:vfat:2G syslinux /dev/sda1
libguestfs: error: syslinux: Error converting to codepage 850 Invalid argument
...

This happens because of the default codepage requested by syslinux
(code page 850) combined with the appliance missing the iconv
converter for this codepage.

Reported-by: Yongkui Guo
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1990720
2021-08-06 08:26:51 +01:00
Richard W.M. Jones
e84c63a2ca python: Don't leak fields when creating Python structs
When creating and returning a Python struct we were adding fields from
the C struct, but did not reduce the ref count on the temporary value
after it had been moved to the struct, resulting in a memory leak.

Reported-by: 朱丹 <zhudan24@huawei.com>
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1985912
2021-07-27 08:58:28 +01:00
Martin Kletzander
e68a844eb4 build: Don't use non-POSIX tests
The `test` builtin/binary usually accepts `==` for string comparison, it is
mostly accepted for typos and people being used to double equals, but is not
documented and not always accepted either.  Since autoconf uses the default
shell, it might just fail in some cases with:

    ./configure: 29986: test: xrustc: unexpected operator
    ./configure: 29990: test: xcargo: unexpected operator

Just change it to single equals as it is done everywhere else.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2021-06-01 10:34:58 +01:00
Richard W.M. Jones
6410c99844 Version 1.45.6. v1.45.6 2021-05-27 17:32:10 +01:00
Richard W.M. Jones
0b223a2877 test-data: Replace deprecated luks_open with cryptsetup_open.
The two calls are identical, so this simply avoids a deprecation
warning.
2021-05-27 17:21:16 +01:00
Daniel P. Berrangé
5e98999b1f point users to Libera Chat rather than FreeNode
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-05-27 13:00:36 +01:00
Richard W.M. Jones
047cf7dcd2 daemon/link.c: Fix out of memory error when reading symlinks
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
2021-05-13 12:04:41 +01:00