From: Pratyush Yadav <pratyush@kernel.org>
To: Mukesh Pilaniya <mpilaniy@redhat.com>
Cc: Pratyush Yadav <pratyush@kernel.org>,
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>,
Tao Liu <ltao@redhat.com>, Philipp Rudo <prudo@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] kexec: return -ENOEXEC from image probe functions on mismatch
Date: Wed, 19 Aug 2026 11:10:50 +0200 [thread overview]
Message-ID: <2vxzqzjuzd9h.fsf@kernel.org> (raw)
In-Reply-To: <0d7acb43-2eb4-44ec-9bfb-b89440afc605@redhat.com> (Mukesh Pilaniya's message of "Wed, 19 Aug 2026 11:28:10 +0530")
On Wed, Aug 19 2026, Mukesh Pilaniya wrote:
> Hi Pratyush,
>
> On 18/08/26 3:14 pm, Pratyush Yadav wrote:
>> On Fri, Aug 14 2026, Mukesh Pilaniya wrote:
[...]
>>
>> 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;
>> }
> Nice catch, Pratyush but this discards all non-zero return values from
> probe functions, which means real errors get swallowed. For example,
> kexec_elf_probe() can return -ENOMEM when kzalloc() fails in
> elf_read_phdrs(). With this diff, that -ENOMEM becomes -ENOEXEC, which
> tells userspace "no loader found" when the actual problem was a memory
> allocation failure.
Right, good point.
>
>>
>> static void *kexec_image_load_default(struct kimage *image)
>>
> How about the following instead? It keeps the probe return value and
> distinguishes -ENOEXEC (format not recognized, try next loader) from any
> other error (real failure, propagate immediately):
>
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 59fb9d71e9d8..ceb5c97cedac 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;
> + int ret;
>
> for (fops = &kexec_file_loaders[0]; *fops && (*fops)->probe; ++fops) {
> ret = (*fops)->probe(buf, buf_len);
> - if (!ret) {
> + 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)
>
>
> This requires probe functions to return -ENOEXEC from fixes in patch 1.
> I will add this as a separate patch on top in v2.
>
> Does this looks good to you ?
LGTM. The only thing I'd change is to perhaps move the declaration of
ret inside the loop so it can never be used outside it, since ret is
only useful in the loop in this function.
--
Regards,
Pratyush Yadav
WARNING: multiple messages have this Message-ID (diff)
From: Pratyush Yadav <pratyush@kernel.org>
To: Mukesh Pilaniya <mpilaniy@redhat.com>
Cc: Pratyush Yadav <pratyush@kernel.org>,
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>,
Tao Liu <ltao@redhat.com>, Philipp Rudo <prudo@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] kexec: return -ENOEXEC from image probe functions on mismatch
Date: Wed, 19 Aug 2026 11:10:50 +0200 [thread overview]
Message-ID: <2vxzqzjuzd9h.fsf@kernel.org> (raw)
In-Reply-To: <0d7acb43-2eb4-44ec-9bfb-b89440afc605@redhat.com> (Mukesh Pilaniya's message of "Wed, 19 Aug 2026 11:28:10 +0530")
On Wed, Aug 19 2026, Mukesh Pilaniya wrote:
> Hi Pratyush,
>
> On 18/08/26 3:14 pm, Pratyush Yadav wrote:
>> On Fri, Aug 14 2026, Mukesh Pilaniya wrote:
[...]
>>
>> 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;
>> }
> Nice catch, Pratyush but this discards all non-zero return values from
> probe functions, which means real errors get swallowed. For example,
> kexec_elf_probe() can return -ENOMEM when kzalloc() fails in
> elf_read_phdrs(). With this diff, that -ENOMEM becomes -ENOEXEC, which
> tells userspace "no loader found" when the actual problem was a memory
> allocation failure.
Right, good point.
>
>>
>> static void *kexec_image_load_default(struct kimage *image)
>>
> How about the following instead? It keeps the probe return value and
> distinguishes -ENOEXEC (format not recognized, try next loader) from any
> other error (real failure, propagate immediately):
>
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 59fb9d71e9d8..ceb5c97cedac 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;
> + int ret;
>
> for (fops = &kexec_file_loaders[0]; *fops && (*fops)->probe; ++fops) {
> ret = (*fops)->probe(buf, buf_len);
> - if (!ret) {
> + 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)
>
>
> This requires probe functions to return -ENOEXEC from fixes in patch 1.
> I will add this as a separate patch on top in v2.
>
> Does this looks good to you ?
LGTM. The only thing I'd change is to perhaps move the declaration of
ret inside the loop so it can never be used outside it, since ret is
only useful in the loop in this function.
--
Regards,
Pratyush Yadav
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2026-08-19 9:11 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-13 7:06 [PATCH] kexec: return -ENOEXEC from image probe functions on mismatch Mukesh Pilaniya
2026-08-13 7:06 ` Mukesh Pilaniya
2026-08-13 13:13 ` Philipp Rudo
2026-08-13 13:13 ` Philipp Rudo
2026-08-14 7:59 ` Mukesh Pilaniya
2026-08-14 7:59 ` Mukesh Pilaniya
2026-08-14 14:13 ` Pratyush Yadav
2026-08-14 14:13 ` Pratyush Yadav
2026-08-14 17:27 ` Mukesh Pilaniya
2026-08-14 17:27 ` Mukesh Pilaniya
2026-08-18 9:44 ` Pratyush Yadav
2026-08-18 9:44 ` Pratyush Yadav
2026-08-19 5:58 ` Mukesh Pilaniya
2026-08-19 5:58 ` Mukesh Pilaniya
2026-08-19 9:10 ` Pratyush Yadav [this message]
2026-08-19 9:10 ` Pratyush Yadav
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2vxzqzjuzd9h.fsf@kernel.org \
--to=pratyush@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=alex@ghiti.fr \
--cc=aou@eecs.berkeley.edu \
--cc=baoquan.he@linux.dev \
--cc=catalin.marinas@arm.com \
--cc=chenhuacai@kernel.org \
--cc=kernel@xen0n.name \
--cc=kexec@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=loongarch@lists.linux.dev \
--cc=ltao@redhat.com \
--cc=mark.rutland@arm.com \
--cc=mpilaniy@redhat.com \
--cc=palmer@dabbelt.com \
--cc=pasha.tatashin@soleen.com \
--cc=pjw@kernel.org \
--cc=prudo@redhat.com \
--cc=rppt@kernel.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.