Re: [Libguestfs] [PATCH libguestfs] daemon: rpm: Check return values from librpm calls

We previously didn't bother to check the return values from any librpm calls. In some cases where possibly the RPM database is faulty, this caused us to return a zero-length list of installed applications (but no error indication). One way to reproduce this is given below. Note this reproducer will only work when run on a RHEL 8 host (or more specifically, with rpm <= 4.16): $ virt-builder fedora-28 $ guestfish -a fedora-28.img -i rm /var/lib/rpm/Packages $ guestfish --ro -a fedora-28.img -i inspect-list-applications /dev/sda4 -vx ... chroot: /sysroot: running 'librpm' error: cannot open Packages index using db5 - Read-only file system (30) error: cannot open Packages database in error: cannot open Packages index using db5 - Read-only file system (30) error: cannot open Packages database in librpm returned 0 installed packages ... With this commit we get an error instead: ... chroot: /sysroot: running 'librpm' error: cannot open Packages index using db5 - Read-only file system (30) error: cannot open Packages database in ocaml_exn: 'internal_list_rpm_applications' raised 'Failure' exception guestfsd: error: rpmtsInitIterator guestfsd: => internal_list_rpm_applications (0x1fe) took 0.01 secs libguestfs: trace: internal_list_rpm_applications = NULL (error) libguestfs: error: internal_list_rpm_applications: rpmtsInitIterator libguestfs: trace: inspect_list_applications2 = NULL (error) libguestfs: trace: inspect_list_applications = NULL (error) ... Not in this case, but in some cases of corrupt RPM databases it is possible to recover them by running "rpmdb --rebuilddb" as a guest command (ie. with guestfs_sh). See-also: https://bugzilla.redhat.com/show_bug.cgi?id=2089623#c12 Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2089623 Fixes: commit c9ee831affed55abe0f928134cbbd2ed83b2f510 Reported-by: Xiaodai Wang Reported-by: Ming Xie --- daemon/rpm-c.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/daemon/rpm-c.c b/daemon/rpm-c.c index 74d325a17b..2cb50a62d2 100644 --- a/daemon/rpm-c.c +++ b/daemon/rpm-c.c @@ -24,6 +24,7 @@ #include <inttypes.h> #include <string.h> #include <unistd.h> +#include <errno.h> #include <caml/alloc.h> #include <caml/fail.h> @@ -79,7 +80,14 @@ value guestfs_int_daemon_rpm_init (value unitv) { CAMLparam1 (unitv); - rpmReadConfigFiles (NULL, NULL); + + /* Nothing in actual RPM C code bothers to check if this call + * succeeds, so using that as an example, just print a debug message + * if it failed, but continue. (The librpm Python bindings do check) + */ + if (rpmReadConfigFiles (NULL, NULL) == -1) + fprintf (stderr, "rpmReadConfigFiles: failed, errno=%d\n", errno); + CAMLreturn (Val_unit); } @@ -92,6 +100,8 @@ guestfs_int_daemon_rpm_start_iterator (value unitv) CAMLparam1 (unitv); ts = rpmtsCreate (); + if (ts == NULL) + caml_failwith ("rpmtsCreate"); #ifdef RPMVSF_MASK_NOSIGNATURES /* Disable signature checking (RHBZ#2064182). */ @@ -99,6 +109,14 @@ guestfs_int_daemon_rpm_start_iterator (value unitv) #endif iter = rpmtsInitIterator (ts, RPMDBI_PACKAGES, NULL, 0); + /* This could return NULL in theory if there are no packages, but + * that could not happen in a real guest. However it also returns + * NULL when unable to open the database (RHBZ#2089623) which is + * something we do need to detect. + */ + if (iter == NULL) + caml_failwith ("rpmtsInitIterator"); + CAMLreturn (Val_unit); }