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

From: Philipp Rudo <prudo@redhat.com>
To: Mukesh Pilaniya <mpilaniy@redhat.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Huacai Chen <chenhuacai@kernel.org>,
	WANG Xuerui <kernel@xen0n.name>, Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>,
	Andrew Morton <akpm@linux-foundation.org>,
	Baoquan He <baoquan.he@linux.dev>,
	Mike Rapoport <rppt@kernel.org>,
	Pasha Tatashin <pasha.tatashin@soleen.com>,
	Pratyush Yadav <pratyush@kernel.org>, Tao Liu <ltao@redhat.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, loongarch@lists.linux.dev,
	linux-riscv@lists.infradead.org, kexec@lists.infradead.org
Subject: Re: [PATCH v2 2/2] kexec: fix error propagation in kexec_image_probe_default()
Date: Thu, 20 Aug 2026 14:21:14 +0200	[thread overview]
Message-ID: <20260820142114.0a36ac78@rotkaeppchen> (raw)
In-Reply-To: <20260819-mpilaniy-v2-2-95e929ede0e5@redhat.com>

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.

Independent on which implementation you use.

Reviewed-by: Philipp Rudo <prudo@redhat.com>

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)
> 