pefile_digest_pe_contents() hashes each PE section with

	crypto_shash_update(desc, pebuf + ctx->secs[i].data_addr,
			    ctx->secs[i].raw_data_size);

where both data_addr and raw_data_size come straight from the untrusted PE
section table. pefile_parse_binary() bounds the section table's location
but not the sections' data ranges, and nothing else on the path checks
them. A section with data_addr and/or raw_data_size pointing outside the
image therefore reads past the pelen-byte buffer (CWE-125); a large
raw_data_size makes it read far past the end and fault.

Commit f7dd32c5179d ("crypto: asymmetric_keys - fix OOB read in
pefile_digest_pe_contents") recently fixed a sibling underflow in this same
function (the hashed_bytes + certs_size trailer computation); the
per-section read was left unchecked.

The digest loop runs only after verify_pkcs7_signature() succeeds, so it
requires a validly signed image -- but not an attacker signing key. The
PKCS#7 signature covers the SpcIndirectDataContent (a self-contained digest
in the certificate table), not the live section bytes, so tampering
data_addr in a publicly available signed image leaves the signature valid
at that step while the out-of-bounds read fires when the digest is
recomputed. It is thus reachable by a local CAP_SYS_BOOT user via
kexec_file_load() with a tampered signed image; the access is an
out-of-bounds read only, but a large raw_data_size faults, which is a
denial of service when panic_on_oops is set.

Validate each section's data range against the image when the section
table is parsed, reusing the existing chkaddr() bounds macro. A malformed
range is rejected with -ELIBBAD at parse time -- before signature
verification and before any hashing -- so a bad image is never digested.

Fixes: af316fc442ef ("pefile: Digest the PE binary and compare to the PKCS#7 data")
Assisted-by: copilot-cli:claude-opus-4-6 frama-c
Signed-off-by: Fabrice Derepas <fabrice.derepas@canonical.com>
---
v2:
  - move the check into pefile_parse_binary() and reuse chkaddr(), per
    Ignat Korchagin's review [1], so a malformed image is rejected at parse
    time -- before signature verification and before hashing. v1 put the
    check in the pefile_digest_pe_contents() section loop.
  - because chkaddr() runs on every section, v2 also validates zero-size
    (raw_data_size == 0) sections, which v1 and the shipping digest loop
    skip. This only rejects a zero-size section whose data_addr points past
    the image -- malformed input that real PEs never produce (.bss uses
    PointerToRawData = 0, which passes) -- and matches the "don't start on
    malformed data" intent.

[1] https://lore.kernel.org/all/CAOs+rJXZ++7_o4L6LuT567FjdYsa=6NEki3qcFSRX+ehJxpoVg@mail.gmail.com/

Tested under KASAN (CONFIG_KASAN_GENERIC, x86-64) with a KUnit case that
calls verify_pefile_signature() on real images: a signed-then-tampered
image (a section's data_addr set to pelen) is now rejected with -ELIBBAD in
pefile_parse_binary(), before verify_pkcs7_signature() and with no KASAN
splat, while an untampered signed image passes parse and reaches the
signature check (-ENOKEY, as the test key is not trusted in this build).
v1's KASAN repro -- a crafted context taken straight into the digest loop --
showed the slab-out-of-bounds this prevents.

 crypto/asymmetric_keys/verify_pefile.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
index cec99db14..c447597f7 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -30,6 +30,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
 	const struct data_dirent *dde;
 	const struct section_header *sec;
 	size_t cursor, datalen = pelen;
+	unsigned int loop;
 
 	kenter("");
 
@@ -112,6 +113,14 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
 		return -ELIBBAD;
 	ctx->secs = pebuf + cursor;
 
+	/* pefile_digest_pe_contents() hashes each section's raw data using
+	 * these fields directly; reject a section whose data range falls
+	 * outside the image so hashing never reads past the buffer.
+	 */
+	for (loop = 0; loop < ctx->n_sections; loop++)
+		chkaddr(0, ctx->secs[loop].data_addr,
+			ctx->secs[loop].raw_data_size);
+
 	return 0;
 }
 

base-commit: d58772d8520c7ef247c4b95c9bd76d3a25da9ff5
-- 
2.53.0

On Fri, Aug 14, 2026 at 11:06 PM Fabrice Derepas
<fabrice.derepas@canonical.com> wrote:
>
> pefile_digest_pe_contents() hashes each PE section with
>
>         crypto_shash_update(desc, pebuf + ctx->secs[i].data_addr,
>                             ctx->secs[i].raw_data_size);
>
> where both data_addr and raw_data_size come straight from the untrusted PE
> section table. pefile_parse_binary() bounds the section table's location
> but not the sections' data ranges, and nothing else on the path checks
> them. A section with data_addr and/or raw_data_size pointing outside the
> image therefore reads past the pelen-byte buffer (CWE-125); a large
> raw_data_size makes it read far past the end and fault.
>
> Commit f7dd32c5179d ("crypto: asymmetric_keys - fix OOB read in
> pefile_digest_pe_contents") recently fixed a sibling underflow in this same
> function (the hashed_bytes + certs_size trailer computation); the
> per-section read was left unchecked.
>
> The digest loop runs only after verify_pkcs7_signature() succeeds, so it
> requires a validly signed image -- but not an attacker signing key. The
> PKCS#7 signature covers the SpcIndirectDataContent (a self-contained digest
> in the certificate table), not the live section bytes, so tampering
> data_addr in a publicly available signed image leaves the signature valid
> at that step while the out-of-bounds read fires when the digest is
> recomputed. It is thus reachable by a local CAP_SYS_BOOT user via
> kexec_file_load() with a tampered signed image; the access is an
> out-of-bounds read only, but a large raw_data_size faults, which is a
> denial of service when panic_on_oops is set.
>
> Validate each section's data range against the image when the section
> table is parsed, reusing the existing chkaddr() bounds macro. A malformed
> range is rejected with -ELIBBAD at parse time -- before signature
> verification and before any hashing -- so a bad image is never digested.
>
> Fixes: af316fc442ef ("pefile: Digest the PE binary and compare to the PKCS#7 data")
> Assisted-by: copilot-cli:claude-opus-4-6 frama-c
> Signed-off-by: Fabrice Derepas <fabrice.derepas@canonical.com>
> ---
> v2:
>   - move the check into pefile_parse_binary() and reuse chkaddr(), per
>     Ignat Korchagin's review [1], so a malformed image is rejected at parse
>     time -- before signature verification and before hashing. v1 put the
>     check in the pefile_digest_pe_contents() section loop.
>   - because chkaddr() runs on every section, v2 also validates zero-size
>     (raw_data_size == 0) sections, which v1 and the shipping digest loop
>     skip. This only rejects a zero-size section whose data_addr points past
>     the image -- malformed input that real PEs never produce (.bss uses
>     PointerToRawData = 0, which passes) -- and matches the "don't start on
>     malformed data" intent.
>
> [1] https://lore.kernel.org/all/CAOs+rJXZ++7_o4L6LuT567FjdYsa=6NEki3qcFSRX+ehJxpoVg@mail.gmail.com/
>
> Tested under KASAN (CONFIG_KASAN_GENERIC, x86-64) with a KUnit case that
> calls verify_pefile_signature() on real images: a signed-then-tampered

Same as your other patch - any reason not to include this KUnit case?

> image (a section's data_addr set to pelen) is now rejected with -ELIBBAD in
> pefile_parse_binary(), before verify_pkcs7_signature() and with no KASAN
> splat, while an untampered signed image passes parse and reaches the
> signature check (-ENOKEY, as the test key is not trusted in this build).
> v1's KASAN repro -- a crafted context taken straight into the digest loop --
> showed the slab-out-of-bounds this prevents.
>
>  crypto/asymmetric_keys/verify_pefile.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
> index cec99db14..c447597f7 100644
> --- a/crypto/asymmetric_keys/verify_pefile.c
> +++ b/crypto/asymmetric_keys/verify_pefile.c
> @@ -30,6 +30,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
>         const struct data_dirent *dde;
>         const struct section_header *sec;
>         size_t cursor, datalen = pelen;
> +       unsigned int loop;
>
>         kenter("");
>
> @@ -112,6 +113,14 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
>                 return -ELIBBAD;
>         ctx->secs = pebuf + cursor;
>
> +       /* pefile_digest_pe_contents() hashes each section's raw data using
> +        * these fields directly; reject a section whose data range falls
> +        * outside the image so hashing never reads past the buffer.
> +        */
> +       for (loop = 0; loop < ctx->n_sections; loop++)

Looking at the code again: seems ctx->n_sections is user-controlled.
What happens if it is 0?

> +               chkaddr(0, ctx->secs[loop].data_addr,
> +                       ctx->secs[loop].raw_data_size);
> +
>         return 0;
>  }
>
>
> base-commit: d58772d8520c7ef247c4b95c9bd76d3a25da9ff5
> --
> 2.53.0
>
>

Ignat

On Fri, Aug 14, 2026 at 11:29 PM Ignat Korchagin <ignat@linux.win> wrote:
> > Tested under KASAN (CONFIG_KASAN_GENERIC, x86-64) with a KUnit case that
> > calls verify_pefile_signature() on real images: a signed-then-tampered
>
> Same as your other patch - any reason not to include this KUnit case?

The reason I am not including my tests is that I use real signed EFI
binaries (a valid one and a tampered one), ~88 KB each via xxd -i, which is
a lot of binary to carry in-tree. For upstream I'll write a self-contained
case that constructs a minimal PE in the test itself and asserts that
pefile_parse_binary() rejects an out-of-range section, so there are no
embedded blobs. I'll send it as a second patch in a v3 series, so the fix
is not gated on the test.

> > +       for (loop = 0; loop < ctx->n_sections; loop++)
>
> Looking at the code again: seems ctx->n_sections is user-controlled.
> What happens if it is 0?

For this loop, nothing: n_sections is unsigned, so "loop < 0" is false on
the first test and the body never runs -- no chkaddr() call and no
ctx->secs[] dereference. So the patch itself adds no hazard for
n_sections == 0.

But you have pointed at a real pre-existing problem one step further on.
n_sections == 0 is not rejected anywhere -- pefile_parse_binary() only
bounds it from above, against header_size -- and pefile_digest_pe_contents()
then does:

	canon = kcalloc(ctx->n_sections, sizeof(unsigned), GFP_KERNEL);
	if (!canon)
		return -ENOMEM;
	...
	canon[0] = 0;

kcalloc(0, ...) returns ZERO_SIZE_PTR, which is non-NULL, so the !canon
check passes and the unconditional "canon[0] = 0" writes through
ZERO_SIZE_PTR. Because the digest runs only after verify_pkcs7_signature()
succeeds, this has the same reachability as the read this patch fixes: a
validly signed image with n_sections tampered to 0 (the PKCS#7 is
unaffected) reaches it via kexec_file_load().

I confirmed it under KASAN. With a trusted key embedded, a signed image
whose NumberOfSections I set to 0 (leaving the certificate table untouched,
so verify_pkcs7_signature() still returns 0) faults in the digest step,
while the untampered image verifies:

  control (trusted, 7 sections): verify_pefile_signature() = 0
  BUG: kernel NULL pointer dereference, address: 0000000000000010
  #PF: supervisor write access in kernel mode
  Oops: 0002 [#1] SMP KASAN NOPTI
  RIP: 0010:verify_pefile_signature+0x5c3   (pefile_digest_pe_contents inlined)

Address 0x10 is ZERO_SIZE_PTR and error_code 0x2 is a write -- the
canon[0] = 0 store -- so it is a write fault, reachable the same way as the
out-of-bounds read this patch already addresses.

Since we are now centralising section-table sanity in pefile_parse_binary(),
the natural fix is to reject n_sections == 0 there as well, e.g. by folding
it into the existing bound:

	if (ctx->n_sections == 0 ||
	    ctx->n_sections > (ctx->header_size - cursor) / sizeof(*sec))
		return -ELIBBAD;

A PE with zero sections is malformed (a signed kernel/EFI image always has
at least one), so rejecting it at parse is safe and closes the
ZERO_SIZE_PTR write too. I will include that in v3 and cover it in the KUnit
case.

Do you prefer it folded into this patch, or kept as a separate fix for the
canon[0] write? Either way I will respin as v3 with the KUnit case once you
let me know.

Thanks for the careful review, Ignat.

Fabrice

Sorry for delay on this,

On Sat, Aug 15, 2026 at 12:45 PM Fabrice Derepas
<fabrice.derepas@canonical.com> wrote:
>
> On Fri, Aug 14, 2026 at 11:29 PM Ignat Korchagin <ignat@linux.win> wrote:
> > > Tested under KASAN (CONFIG_KASAN_GENERIC, x86-64) with a KUnit case that
> > > calls verify_pefile_signature() on real images: a signed-then-tampered
> >
> > Same as your other patch - any reason not to include this KUnit case?
>
> The reason I am not including my tests is that I use real signed EFI
> binaries (a valid one and a tampered one), ~88 KB each via xxd -i, which is
> a lot of binary to carry in-tree. For upstream I'll write a self-contained
> case that constructs a minimal PE in the test itself and asserts that
> pefile_parse_binary() rejects an out-of-range section, so there are no
> embedded blobs. I'll send it as a second patch in a v3 series, so the fix
> is not gated on the test.
>
> > > +       for (loop = 0; loop < ctx->n_sections; loop++)
> >
> > Looking at the code again: seems ctx->n_sections is user-controlled.
> > What happens if it is 0?
>
> For this loop, nothing: n_sections is unsigned, so "loop < 0" is false on
> the first test and the body never runs -- no chkaddr() call and no
> ctx->secs[] dereference. So the patch itself adds no hazard for
> n_sections == 0.
>
> But you have pointed at a real pre-existing problem one step further on.
> n_sections == 0 is not rejected anywhere -- pefile_parse_binary() only
> bounds it from above, against header_size -- and pefile_digest_pe_contents()
> then does:
>
>         canon = kcalloc(ctx->n_sections, sizeof(unsigned), GFP_KERNEL);
>         if (!canon)
>                 return -ENOMEM;
>         ...
>         canon[0] = 0;
>
> kcalloc(0, ...) returns ZERO_SIZE_PTR, which is non-NULL, so the !canon
> check passes and the unconditional "canon[0] = 0" writes through
> ZERO_SIZE_PTR. Because the digest runs only after verify_pkcs7_signature()
> succeeds, this has the same reachability as the read this patch fixes: a
> validly signed image with n_sections tampered to 0 (the PKCS#7 is
> unaffected) reaches it via kexec_file_load().
>
> I confirmed it under KASAN. With a trusted key embedded, a signed image
> whose NumberOfSections I set to 0 (leaving the certificate table untouched,
> so verify_pkcs7_signature() still returns 0) faults in the digest step,
> while the untampered image verifies:
>
>   control (trusted, 7 sections): verify_pefile_signature() = 0
>   BUG: kernel NULL pointer dereference, address: 0000000000000010
>   #PF: supervisor write access in kernel mode
>   Oops: 0002 [#1] SMP KASAN NOPTI
>   RIP: 0010:verify_pefile_signature+0x5c3   (pefile_digest_pe_contents inlined)
>
> Address 0x10 is ZERO_SIZE_PTR and error_code 0x2 is a write -- the
> canon[0] = 0 store -- so it is a write fault, reachable the same way as the
> out-of-bounds read this patch already addresses.
>
> Since we are now centralising section-table sanity in pefile_parse_binary(),
> the natural fix is to reject n_sections == 0 there as well, e.g. by folding
> it into the existing bound:
>
>         if (ctx->n_sections == 0 ||
>             ctx->n_sections > (ctx->header_size - cursor) / sizeof(*sec))
>                 return -ELIBBAD;
>
> A PE with zero sections is malformed (a signed kernel/EFI image always has
> at least one), so rejecting it at parse is safe and closes the
> ZERO_SIZE_PTR write too. I will include that in v3 and cover it in the KUnit
> case.
>
> Do you prefer it folded into this patch, or kept as a separate fix for the

Let's fold into this patch, since it is one place now.

> canon[0] write? Either way I will respin as v3 with the KUnit case once you
> let me know.

Thank you.

>
> Thanks for the careful review, Ignat.
>
> Fabrice
>