NetBSD Problem Report #59120

From www@netbsd.org  Mon Mar  3 14:01:25 2025
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 66E8B1A923A
	for <gnats-bugs@gnats.NetBSD.org>; Mon,  3 Mar 2025 14:01:25 +0000 (UTC)
Message-Id: <20250303140124.1C3731A923D@mollari.NetBSD.org>
Date: Mon,  3 Mar 2025 14:01:24 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: x86 efiboot: uninitialized/null pointer deref with efi partitions
X-Send-Pr-Version: www-1.0

>Number:         59120
>Category:       port-amd64
>Synopsis:       x86 efiboot: uninitialized/null pointer deref with efi partitions
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    port-amd64-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Mar 03 14:05:00 +0000 2025
>Originator:     Taylor R Campbell
>Release:        current, 10, 9, ...
>Organization:
The NetGPT Notfoundaguid
>Environment:
>Description:
In the x86 efiboot (sys/arch/i386/stand/efiboot), a bios disk partition can have its size and fstype initialized by biosdisk.c check_gpt without also initializing its guid:

    418 			if (!guid_is_nil(u)) {
    419 				d->part[j].offset = ep[i].ent_lba_start;
    420 				d->part[j].size = ep[i].ent_lba_end -
    421 				    ep[i].ent_lba_start + 1;
    422 				if (guid_is_equal(u, &GET_nbsd_ffs))
    423 					d->part[j].fstype = FS_BSDFFS;
...
    434 				else
    435 					d->part[j].fstype = FS_OTHER;
    436 #ifndef NO_GPT
    437 				for (int k = 0;
    438 				     k < __arraycount(gpt_parts);
    439 				     k++) {
    440 					if (guid_is_equal(u, gpt_parts[k].guid))
    441 						d->part[j].guid = &gpt_parts[k];
    442 				}

https://nxr.netbsd.org/xref/src/sys/arch/i386/stand/lib/biosdisk.c?r=1.61#419

Later, efidisk_get_efi_system_partition may try to dereference the guid:

    392 		if (part[i].size == 0)
    393 			continue;
    394 		if (part[i].fstype == FS_UNUSED)
    395 			continue;
    396 		if (guid_is_equal(part[i].guid->guid, &GET_efi))
    397 			break;

https://nxr.netbsd.org/xref/src/sys/arch/i386/stand/efiboot/efidisk.c?r=1.11#392

In this case, I believe it will always be a null pointer because the whole struct biosdisk object is zeroed on allocation:

    834 void
    835 biosdisk_probe(void)
    836 {
    837 	struct biosdisk *d;
...
    852 		d = alloc(sizeof(*d));
    853 		if (d == NULL) {
    854 			printf("Out of memory\n");
    855 			return;
    856 		}
    857 		memset(d, 0, sizeof(*d));
...

(Same with biosdisk_find_name, biosdisk_find_raid, alloc_biosdisk.)

But this could be done more confidently, and the downstream dereference (whether it is currently uninitialized or null) should be protected by a null check.

[Originally reported by Vladimir `phcoder' Serbinenko: https://mail-index.netbsd.org/tech-kern/2025/03/02/msg030101.html]
>How-To-Repeat:
efiboot an x86 system on a gpt-formatted disk with a partition type not know to the bootloader, i.e., not one of the partition types listed at:

https://nxr.netbsd.org/xref/src/sys/arch/i386/stand/lib/biosdisk.c?r=1.61#160
>Fix:
1. null out d->part[j].guid up front
2. add a null test to efidisk_get_efi_system_partition

Home	PR Database Search