[PATCHv6 02/13] kexec_file: Move signature validation ahead

Pingfan Liu posted 13 patches 3 weeks ago
[PATCHv6 02/13] kexec_file: Move signature validation ahead
Posted by Pingfan Liu 3 weeks ago
Move the signature validation at the head of the function, so the image
can be unfold and handled later.

Signed-off-by: Pingfan Liu <piliu@redhat.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Philipp Rudo <prudo@redhat.com>
To: kexec@lists.infradead.org
---
 kernel/kexec_file.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index eb62a97942428..0222d17072d40 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -231,18 +231,19 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 	kexec_dprintk("kernel: %p kernel_size: %#lx\n",
 		      image->kernel_buf, image->kernel_buf_len);
 
-	/* Call arch image probe handlers */
-	ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
-					    image->kernel_buf_len);
-	if (ret)
-		goto out;
-
 #ifdef CONFIG_KEXEC_SIG
 	ret = kimage_validate_signature(image);
 
 	if (ret)
 		goto out;
 #endif
+
+	/* Call arch image probe handlers */
+	ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
+					    image->kernel_buf_len);
+	if (ret)
+		goto out;
+
 	/* It is possible that there no initramfs is being loaded */
 	if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
 		ret = kernel_read_file_from_fd(initrd_fd, 0, &image->initrd_buf,
-- 
2.49.0
Re: [PATCHv6 02/13] kexec_file: Move signature validation ahead
Posted by bot+bpf-ci@kernel.org 2 weeks, 6 days ago
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index eb62a9794242..0222d17072d4 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -231,18 +231,19 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
>  	kexec_dprintk("kernel: %p kernel_size: %#lx\n",
>  		      image->kernel_buf, image->kernel_buf_len);
>
> -	/* Call arch image probe handlers */
> -	ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
> -					    image->kernel_buf_len);
> -	if (ret)
> -		goto out;
> -
>  #ifdef CONFIG_KEXEC_SIG
>  	ret = kimage_validate_signature(image);
>
>  	if (ret)
>  		goto out;
>  #endif

Does this reordering break signature verification? The kimage_validate_signature()
function calls kexec_image_verify_sig(), which accesses image->fops->verify_sig:

  static int kexec_image_verify_sig(struct kimage *image, void *buf,
                                    unsigned long buf_len)
  {
      if (!image->fops || !image->fops->verify_sig) {
          pr_debug("kernel loader does not support signature verification.\n");
          return -EKEYREJECTED;
      }
      return image->fops->verify_sig(buf, buf_len);
  }

However, image->fops is set by arch_kexec_kernel_image_probe() via
kexec_image_probe_default(), which now runs after kimage_validate_signature().
Since the kimage struct is zero-initialized by do_kimage_alloc_init(),
image->fops will be NULL when kexec_image_verify_sig() is called.

With this ordering, kexec_image_verify_sig() will always return -EKEYREJECTED.
When CONFIG_KEXEC_SIG_FORCE is enabled (sig_enforce = true), this causes
kimage_validate_signature() to return the error and kexec_file_load() fails.

Is this intentional, or should arch_kexec_kernel_image_probe() remain before
the signature validation call?

> +
> +	/* Call arch image probe handlers */
> +	ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
> +					    image->kernel_buf_len);
> +	if (ret)
> +		goto out;
> +

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/21147860407