Re: [PATCH v2] crypto: asymmetric_keys - bound PE section ranges in pefile_parse_binary

On Fri, Aug 14, 2026 at 11:29 PM Ignat Korchagin <[email protected]> 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