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

[Mukesh Pilaniya <mpilaniy@redhat.com>]

Hi Philipp,

On 20/08/26 11:41 pm, Pratyush Yadav 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.
> 
Thanks for the review. I agree the switch-case reads nicely, but since
the current version is functionally equivalent and already
reviewed, I'll keep it as-is to avoid re-spinning for a style change.
Happy to switch if you or Pratyush feel strongly about it.

>>
>> Independent on which implementation you use.
>>
>> Reviewed-by: Philipp Rudo <prudo@redhat.com>
> 
> 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.
> 
>>
>>
>> On Wed, 19 Aug 2026 23:17:23 +0530
>> Mukesh Pilaniya <mpilaniy@redhat.com> 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 <pratyush@kernel.org>
>>> Signed-off-by: Mukesh Pilaniya <mpilaniy@redhat.com>
>>> ---
>>>  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)
>>>
>>
> 


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv