Re: [PATCH] kexec: return -ENOEXEC from image probe functions on mismatch

[Pratyush Yadav <pratyush@kernel.org>]

On Fri, Aug 14 2026, Mukesh Pilaniya wrote:

> Hi Pratyush,
> On 14/08/26 7:43 pm, Pratyush Yadav wrote:
>> On Thu, Aug 13 2026, Mukesh Pilaniya wrote:
>> 
>>> Several kexec_file_load() image probe functions return -EINVAL when
>>> they do not recognize the image format.  A probe function that rejects
>>> an image should return -ENOEXEC to indicate that the image is not a
>>> recognized executable format.  -EINVAL implies a problem with the
>>> syscall parameters, not with image recognition.
>>>
>>> kexec_image_probe_default() iterates through registered loaders and
>>> returns the last probe's error code to the caller.  That error
>>> propagates as the kexec_file_load() return value to userspace.
>>> Returning -EINVAL from a probe when no loader matches is semantically
>>> incorrect and misleads userspace about the nature of the failure.
>>>
>>> Return -ENOEXEC from all probe functions and their helpers when the
>>> image format is not recognized.
>> 
>> Sounds fine in principle but can you please also share what the real
>> problem you face is and how changing these return codes helps? These
>> error codes are uAPI and while we _can_ change them as long as we don't
>> break something, there should be a clear motivation for doing so.
>> 
>> [...]
>> 
> While debugging a misleading error on s390x where kexec -s reported
> "syscall kexec_file_load not available" instead of the actual EINVAL
> from a kernel command line that exceeded the architecture limit, we
> traced the problem to the kexec-tools userspace utility treating
> EINVAL the same as ENOSYS and ENOEXEC -- as a signal to silently fall
> back to kexec_load().
>
> kexec-tools supports two syscalls: kexec_file_load() and the older
> kexec_load(). With -a (the default), it tries kexec_file_load()
> first and falls back to kexec_load() when the syscall is not
> implemented (ENOSYS) or the kernel does not have a loader for the
> image format. With -s, it uses kexec_file_load() only with no
> fallback.
>
> When the kernel returns -EINVAL it means something went wrong while
> loading the image, not that the syscall is missing or the image
> format is unrecognized. kexec-tools should not fall back to the
> older syscall in that case. However, some kernel probe functions
> currently return -EINVAL when the image header does not match,
> instead of returning -ENOEXEC. Keeping EINVAL in the fallback set
> to accommodate these probes has the side effect of also hiding
> genuine loading errors like an oversized command line.
>
> kexec-tools should only fall back when kexec_file_load() is not
> implemented or does not have a matching loader -- not when something
> goes wrong during load.
>
> The fix on the kexec-tools side is to remove EINVAL from the fallback
> set, but that requires the kernel to be clean first -- probe functions
> must return -ENOEXEC when they do not recognize an image format, not -EINVAL.

Okay, the idea makes sense. Though I am curious if in practice there are
images that kexec_file_load() can't load but kexec-tools can.

But then I went and looked at the code. The only caller of the probe
functions I can see is kexec_image_probe_default(). Looking at its code:

  int kexec_image_probe_default(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;
  }

It defaults ret to -ENOEXEC, which makes sense, but then lets it be
over-written by each loader. So in practice, it returns what the _last_
loader returned. _This_ makes no sense. Why should the last loader be
any special?

From reading the code, I think what the author of this wanted to do is
to try all loaders, and return -ENOEXEC if none succeeded. But the code
of course strays from that and ends up returning the last loader's
value.

So how about the below diff instead? (** only compile tested **)

--- 8< ---
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 2bfbb2d144e6..cfb2b8cd5679 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -67,17 +67,16 @@ int kexec_image_probe_default(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) {
+		if (!(*fops)->probe(buf, buf_len)) {
 			image->fops = *fops;
-			return ret;
+			return 0;
 		}
 	}
 
-	return ret;
+	/* No loader found. */
+	return -ENOEXEC;
 }
 
 static void *kexec_image_load_default(struct kimage *image)

-- 
Regards,
Pratyush Yadav