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

[Laszlo Ersek]

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>