On Mon, Feb 26, 2018 at 12:17:18PM +0100, Philipp Rudo wrote:

Hi AKASHI

On Thu, 22 Feb 2018 20:17:22 +0900
AKASHI Takahiro [off-list ref] wrote:

[...]

quoted

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 990adae52151..a6d14a768b3e 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c

@@ -26,34 +26,83 @@
 #include <linux/vmalloc.h>
 #include "kexec_internal.h"

+const __weak struct kexec_file_ops * const kexec_file_loaders[] = {NULL};
+

Having a weak definition of kexec_file_loaders causes trouble on s390 with 
gcc 4.8 (newer versions seem to work fine). For me it looks like that in this
version gcc doesn't recognize __weak but use the default value for
optimization. This leads to _kexec_kernel_image_probe to always return ENOEXEC
because the for-loop gets optimized out.

I gave it a try to compile with gcc 4.9 (not 4.8) for arm64
and didn't see any errors or warnings, but

Your change is just fine with me, too.
I will incorporate it in my next version.

Thanks,
-Takahiro AKASHI

A nice side effect of this solution is, that a developer who forgets to define
kexec_file_loaders gets a linker error. So he directly knows what's missing
instead of first having to find out where/why an error gets returned.

Otherwise the series is fine for me.

Thanks
Philipp

quoted

 #ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
 static int kexec_calculate_store_digests(struct kimage *image);
 #else
 static int kexec_calculate_store_digests(struct kimage *image) { return 0; };
 #endif

+int _kexec_kernel_image_probe(struct kimage *image, void *buf,
+			     unsigned long buf_len)
+{
+	const struct kexec_file_ops * const *fops;
+	int ret = -ENOEXEC;
+
+	for (fops = &kexec_file_loaders[0]; *fops && (*fops)->probe; ++fops) {
+		ret = (*fops)->probe(buf, buf_len);
+		if (!ret) {
+			image->fops = *fops;
+			return ret;
+		}
+	}
+
+	return ret;
+}
+
 /* Architectures can provide this probe function */
 int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
 					 unsigned long buf_len)
 {
-	return -ENOEXEC;
+	return _kexec_kernel_image_probe(image, buf, buf_len);
+}
+
+void *_kexec_kernel_image_load(struct kimage *image)
+{
+	if (!image->fops || !image->fops->load)
+		return ERR_PTR(-ENOEXEC);
+
+	return image->fops->load(image, image->kernel_buf,
+				 image->kernel_buf_len, image->initrd_buf,
+				 image->initrd_buf_len, image->cmdline_buf,
+				 image->cmdline_buf_len);
 }

 void * __weak arch_kexec_kernel_image_load(struct kimage *image)
 {
-	return ERR_PTR(-ENOEXEC);
+	return _kexec_kernel_image_load(image);
+}
+
+int _kimage_file_post_load_cleanup(struct kimage *image)
+{
+	if (!image->fops || !image->fops->cleanup)
+		return 0;
+
+	return image->fops->cleanup(image->image_loader_data);
 }

 int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
 {
-	return -EINVAL;
+	return _kimage_file_post_load_cleanup(image);
 }

 #ifdef CONFIG_KEXEC_VERIFY_SIG
+int _kexec_kernel_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);
+}
+
 int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
 					unsigned long buf_len)
 {
-	return -EKEYREJECTED;
+	return _kexec_kernel_verify_sig(image, buf, buf_len);
 }
 #endif