Richard W.M. Jones
2022-May-25 16:01 UTC
[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);
}
--
2.35.1
Laszlo Ersek
2022-May-26 08:38 UTC
[Libguestfs] [PATCH libguestfs] daemon: rpm: Check return values from librpm calls
On 05/25/22 18:01, Richard W.M. Jones wrote:> 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); > } > >Acked-by: Laszlo Ersek <lersek at redhat.com>