Libguestfs - Apr 2023 - [libguestfs-common PATCH 2/2] detect_kernels: deal with RHEL's kernel-core / kernel-modules-core split

In "kernel-5.14.0-269.el9", the "kernel-modules-core"
subpackage got split
from the "kernel-core" subpackage. Therefore, a single binary RPM
containing *both* the "/boot/vmlinuz-5.14.0-269.el9.x86_64" file *and*
the
"/lib/modules/5.14.0-269.el9.x86_64" directory no longer exists. The
file
now belongs to "kernel-core", and the directory to
"kernel-modules-core".

As a result, when we investigate the file list of "kernel-core" (based
on
"kernel-core" providing
"/boot/vmlinuz-5.14.0-269.el9.x86_64"), the first
match against "/lib/modules/" is not the actual module root directory
"/lib/modules/5.14.0-269.el9.x86_64", but the nonsensical
"/lib/modules/5.14.0-269.el9.x86_64/.vmlinuz.hmac" regular file. This
latter file is never a directory, therefore we rule out "kernel-core"
as a
kernel package.

We also rule out "kernel-modules-core" (even earlier) because it does
not
contain "/boot/vmlinuz-5.14.0-269.el9.x86_64".

Now, the code already deals with the case if the prospective kernel
package *does not provide a match* for the "/lib/modules/" prefix: in
that
case, we construct the modpath manually, from said prefix, and the version
number found in "/boot/vmlinuz-<version>". This fallback is
good, but it's
unreachable if *there is* a candidate, it's just wrong (i.e., not a
directory).

Perform the "is_dir" check on the candidate modpath earlier, so that
we
can fall back to the manual modpath construction if the modpath candidate
exists, but is wrong.

With this, the original "is_dir" check becomes superfluous
(duplicated)
*except* when the "Not_found" branch is taken. Therefore, hoist the
original "is_dir" check into that branch.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2175703
Reported-by: Vera Wu <vwu at redhat.com>
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
Tested-by: Vera Wu <vwu at redhat.com>
---

Notes:
    context:-U5

 mldrivers/linux_kernels.ml | 24 +++++++++++++++-----
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/mldrivers/linux_kernels.ml b/mldrivers/linux_kernels.ml
index 5b8bdee9923d..23ff76a5504c 100644
--- a/mldrivers/linux_kernels.ml
+++ b/mldrivers/linux_kernels.ml
@@ -125,31 +125,43 @@ let detect_kernels (g : G.guestfs) root bootloader apps   
*)
            let modpath, version               let prefix =
"/lib/modules/" in
              let prefix_len = String.length prefix in
              try
-               List.find_map (
+               let modpath, version = List.find_map (
                  fun filename ->
                    let filename_len = String.length filename in
                    if filename_len > prefix_len &&
                       String.is_prefix filename prefix then (
                      let version = String.sub filename prefix_len
                                               (filename_len - prefix_len) in
                      Some (filename, version)
                    ) else
                      None
-               ) files
+               ) files in
+               (* Fall back to the version in the vmlinuz file name not only if
+                * a candidate pathname couldn't be found under
/lib/modules/,
+                * but also in case the candidate pathname doesn't reference
a
+                * directory. See RHBZ#2175703.
+                *
+                * Note that this "is_dir" check is deliberately kept
outside of
+                * the "find_map"'s mapper function above: we want
the first
+                * candidate *to be* a directory, and not the first candidate
+                * *that is* a directory.
+                *)
+               if not (g#is_dir ~followsymlinks:true modpath) then
+                 raise Not_found;
+               modpath, version
              with Not_found ->
                let version                   String.sub vmlinuz 14
(String.length vmlinuz - 14) in
                let modpath = prefix ^ version in
+               (* Check that the modpath exists. *)
+               if not (g#is_dir ~followsymlinks:true modpath) then
+                 raise Not_found;
                modpath, version in
 
-           (* Check that the modpath exists. *)
-           if not (g#is_dir ~followsymlinks:true modpath) then
-             raise Not_found;
-
            (* Find the initramfs which corresponds to the kernel.
             * Since the initramfs is built at runtime, and doesn't have
             * to be covered by the RPM file list, this is basically
             * guesswork.
             *)

On Mon, Mar 20, 2023 at 12:53:00PM +0100, Laszlo Ersek
wrote:
> In "kernel-5.14.0-269.el9", the "kernel-modules-core"
subpackage got split
> from the "kernel-core" subpackage. Therefore, a single binary RPM
> containing *both* the "/boot/vmlinuz-5.14.0-269.el9.x86_64" file
*and* the
> "/lib/modules/5.14.0-269.el9.x86_64" directory no longer exists.
The file
> now belongs to "kernel-core", and the directory to
"kernel-modules-core".
> 
> As a result, when we investigate the file list of "kernel-core"
(based on
> "kernel-core" providing
"/boot/vmlinuz-5.14.0-269.el9.x86_64"), the first
> match against "/lib/modules/" is not the actual module root
directory
> "/lib/modules/5.14.0-269.el9.x86_64", but the nonsensical
> "/lib/modules/5.14.0-269.el9.x86_64/.vmlinuz.hmac" regular file.
This
> latter file is never a directory, therefore we rule out
"kernel-core" as a
> kernel package.
> 
> We also rule out "kernel-modules-core" (even earlier) because it
does not
> contain "/boot/vmlinuz-5.14.0-269.el9.x86_64".
> 
> Now, the code already deals with the case if the prospective kernel
> package *does not provide a match* for the "/lib/modules/"
prefix: in that
> case, we construct the modpath manually, from said prefix, and the version
> number found in "/boot/vmlinuz-<version>". This fallback is
good, but it's
> unreachable if *there is* a candidate, it's just wrong (i.e., not a
> directory).
> 
> Perform the "is_dir" check on the candidate modpath earlier, so
that we
> can fall back to the manual modpath construction if the modpath candidate
> exists, but is wrong.
> 
> With this, the original "is_dir" check becomes superfluous
(duplicated)
> *except* when the "Not_found" branch is taken. Therefore, hoist
the
> original "is_dir" check into that branch.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2175703
> Reported-by: Vera Wu <vwu at redhat.com>
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> Tested-by: Vera Wu <vwu at redhat.com>
> ---
> 
> Notes:
>     context:-U5
> 
>  mldrivers/linux_kernels.ml | 24 +++++++++++++++-----
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/mldrivers/linux_kernels.ml b/mldrivers/linux_kernels.ml
> index 5b8bdee9923d..23ff76a5504c 100644
> --- a/mldrivers/linux_kernels.ml
> +++ b/mldrivers/linux_kernels.ml
> @@ -125,31 +125,43 @@ let detect_kernels (g : G.guestfs) root bootloader
apps >              *)
>             let modpath, version >               let prefix =
"/lib/modules/" in
>               let prefix_len = String.length prefix in
>               try
> -               List.find_map (
> +               let modpath, version = List.find_map (
>                   fun filename ->
>                     let filename_len = String.length filename in
>                     if filename_len > prefix_len &&
>                        String.is_prefix filename prefix then (
>                       let version = String.sub filename prefix_len
>                                                (filename_len - prefix_len)
in
>                       Some (filename, version)
>                     ) else
>                       None
> -               ) files
> +               ) files in
> +               (* Fall back to the version in the vmlinuz file name not
only if
> +                * a candidate pathname couldn't be found under
/lib/modules/,
> +                * but also in case the candidate pathname doesn't
reference a
> +                * directory. See RHBZ#2175703.
> +                *
> +                * Note that this "is_dir" check is deliberately
kept outside of
> +                * the "find_map"'s mapper function above: we
want the first
> +                * candidate *to be* a directory, and not the first
candidate
> +                * *that is* a directory.
> +                *)
> +               if not (g#is_dir ~followsymlinks:true modpath) then
> +                 raise Not_found;
> +               modpath, version
>               with Not_found ->
>                 let version >                   String.sub vmlinuz 14
(String.length vmlinuz - 14) in
>                 let modpath = prefix ^ version in
> +               (* Check that the modpath exists. *)
> +               if not (g#is_dir ~followsymlinks:true modpath) then
> +                 raise Not_found;
>                 modpath, version in
>  
> -           (* Check that the modpath exists. *)
> -           if not (g#is_dir ~followsymlinks:true modpath) then
> -             raise Not_found;
> -
>             (* Find the initramfs which corresponds to the kernel.
>              * Since the initramfs is built at runtime, and doesn't
have
>              * to be covered by the RPM file list, this is basically
>              * guesswork.
>              *)
I see it's upstream, but anyway a late:

ACK

That detect_kernels function is horribly complicated and hard to
follow.  At some point we need to attack it and split it up into
manageable chunks.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v