Re: [PATCH v2 2/2] kexec: fix error propagation in kexec_image_probe_default()

Hi Pratyush,

On Thu, 20 Aug 2026 20:11:14 +0200
Pratyush Yadav <[email protected]> wrote:

> On Thu, Aug 20 2026, Philipp Rudo wrote:
> 
> > Hi Mukesh,
> > Hi Pratyush,
> >
> > having this patch makes sense. Personally I'd go with having a
> > switch-case rather than individual if-blocks. E.g.
> >
> > 	switch (ret) {
> > 		case 0:
> > 			image->fops = *fops;
> > 			return 0;
> > 		case -ENOEXEC:
> > 			continue;
> > 		default:
> > 			return ret;
> > 	}
> >
> > IMHO that is better readable and easier to extend in case other error
> > codes need special handling as well. But that is only my personal
> > opinion.  
> 
> Well, IMO both look roughly the same so I unless Mukesh prefers your
> version more, we can keep it as-is.

that's why I've marked it as a personal preference. I don't expect
Mukesh to switch to it. Still I thought it is worth mentioning just to
give an other perspective.

> >
> > Independent on which implementation you use.
> >
> > Reviewed-by: Philipp Rudo <[email protected]>  
> 
> Thanks for reviewing!
> 
> One tiny suggestion. Can you please reply in-line the next time around?
> It is a bit easier reading this response below the code that it refers
> to.

Agree, especially with larger, more complex patches. In this case
however, I thought that the patch is so small that it wouldn't make
much of a difference.
Anyway, I'll reply in-line next time in such cases as well.

Thanks for your review!
Philipp

> >
> >
> > On Wed, 19 Aug 2026 23:17:23 +0530
> > Mukesh Pilaniya <[email protected]> wrote:
> >  
> >> kexec_image_probe_default() overwrites ret with each loader's probe
> >> return value and returns whatever the last loader returned when no
> >> probe matches.  The error code reaching userspace depends on whichever
> >> loader happens to be last in kexec_file_loaders[].
> >> 
> >> Only continue to the next loader when a probe returns -ENOEXEC.
> >> Propagate any other error such as -ENOMEM immediately. Return -ENOEXEC
> >> only when all loaders have been tried and none matched.
> >> 
> >> Suggested-by: Pratyush Yadav <[email protected]>
> >> Signed-off-by: Mukesh Pilaniya <[email protected]>
> >> ---
> >>  kernel/kexec_file.c | 12 +++++++-----
> >>  1 file changed, 7 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> >> index 59fb9d71e9d8..b3060e984ef3 100644
> >> --- a/kernel/kexec_file.c
> >> +++ b/kernel/kexec_file.c
> >> @@ -68,17 +68,19 @@ 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) {
> >> +		int ret = (*fops)->probe(buf, buf_len);
> >> +
> >> +		if (ret == 0) {
> >>  			image->fops = *fops;
> >> -			return ret;
> >> +			return 0;
> >>  		}
> >> +		if (ret != -ENOEXEC)
> >> +			return ret;
> >>  	}
> >>  
> >> -	return ret;
> >> +	return -ENOEXEC;
> >>  }
> >>  
> >>  static void *kexec_image_load_default(struct kimage *image)
> >>   
> >  
>