kern/60620: Fatal logic inversion in fdtbus_get_u32prop_with_default() breaks usmsc(4) and other FDT drivers

["jun%soum.co.jp@localhost via gnats" <gnats-admin%NetBSD.org@localhost>]

>Number:         60620
>Category:       kern
>Synopsis:       Fatal logic inversion in fdtbus_get_u32prop_with_default() breaks usmsc(4) and other FDT drivers
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Aug 19 05:05:00 +0000 2026
>Originator:     Jun Ebihara
>Release:        NetBSD 11.99.7
>Organization:
SOUM Corporation
>Environment:
NetBSD rpi 11.99.6 NetBSD 11.99.7 (RPI2) #0: Wed Aug 19 13:29:46 JST 2026  jun%soum.co.jp@localhost:/usr/src/sys/arch/evbarm/compile/GENERIC evbarm

>Description:
A fatal regression was introduced in sys/dev/fdt/fdt_subr.c (revision 1.43)
with the implementation of fdtbus_get_u32prop_with_default().

The internal logic of of_getprop_uint32() return value checking is inverted:

----------------------------------------------------------------------
static int
fdtbus_get_u32prop_with_default(int phandle, const char *prop, int dflt)
{
        uint32_t val;
        if (of_getprop_uint32(phandle, prop, &val)) {
                val = dflt;
        }
        return val;
}
----------------------------------------------------------------------

In NetBSD, of_getprop_uint32() returns 0 on success and non-zero on failure.
As a result of the above implementation, when the property successfully 
exists in the Device Tree, the code incorrectly overwrites `val` with `dflt`.
Conversely, if the property does not exist, it returns an uninitialized 
stack variable `val`.

This completely breaks the parsing of crucial properties like "#address-cells" 
and "#size-cells", causing fdtbus_get_reg64() to decode incorrect I/O base 
addresses for various hardware components. 

For instance, this causes the usmsc(4) driver to fail to talk to the hardware, 
spatting the following errors during boot:
usmsc0: warning: Failed to read register 0x114
usmsc0: warning: MII is busy

Additionally, fdtbus_get_phandle_with_data() suffers from an uninitialized 
variable risk where `cells_num` is left uninitialized if of_getprop_uint32() 
fails, leading to an out-of-bounds pointer advance (`p += reclen`).


>How-To-Repeat:
Boot a NetBSD/evbarm kernel built with sys/dev/fdt/fdt_subr.c (v1.43) on 
a target board utilizing FDT-based attachments (e.g., Raspberry Pi with usmsc).
Observe that the MAC address/MII registers fail to register properly.

or try this sample test code:

% gcc -Wall -o test_fdt test_fdt_subr.c
%./test_fdt

===> test_fdt_subr.c

#include <stdio.h>
#include <stdint.h>
#include <stdbool.h>

typedef uint32_t u_int;

static bool mock_property_exists = false;
static uint32_t mock_property_value = 0;

int
of_getprop_uint32(int phandle, const char *prop, uint32_t *val)
{
	if (mock_property_exists) {
		*val = mock_property_value;
		return 0; 
	}
	return -1; 
}

static int
fdtbus_get_u32prop_with_default_ORIG(int phandle, const char *prop, int dflt)
{
	uint32_t val;
	if (of_getprop_uint32(phandle, prop, &val)) {
		val = dflt;
	}
	return val;
}

static int
fdtbus_get_u32prop_with_default_FIXED(int phandle, const char *prop, int dflt)
{
	uint32_t val;
	if (of_getprop_uint32(phandle, prop, &val) != 0) {
		return dflt;
	}
	return val;
}

int
main(void)
{
	int dflt_input = 100;
	int result_orig, result_fixed;

	printf("=== FDT Regression Test Bench ===\n\n");

	mock_property_exists = true;
	mock_property_value = 200;

	result_orig  = fdtbus_get_u32prop_with_default_ORIG(1, "test-prop", dflt_input);
	result_fixed = fdtbus_get_u32prop_with_default_FIXED(1, "test-prop", dflt_input);

	printf("[Case 1] Property EXISTS (Expected output: 200)\n");
	printf("  - ORIGINAL (Buggy): %d  --> %s\n", result_orig,
	    (result_orig == 200) ? "PASS" : "FAIL (Inverted logic applied dflt or stack garbage)");
	printf("  - FIXED    (Patch): %d  --> %s\n", result_fixed,
	    (result_fixed == 200) ? "PASS" : "FAIL");
	printf("\n");

	mock_property_exists = false;
	
	result_orig  = fdtbus_get_u32prop_with_default_ORIG(1, "test-prop", dflt_input);
	result_fixed = fdtbus_get_u32prop_with_default_FIXED(1, "test-prop", dflt_input);

	printf("[Case 2] Property DOES NOT EXIST (Expected output: 100)\n");
	printf("  - ORIGINAL (Buggy): %d  --> %s\n", result_orig,
	    (result_orig == dflt_input) ? "PASS" : "FAIL (Returned uninitialized variable)");
	printf("  - FIXED    (Patch): %d  --> %s\n", result_fixed,
	    (result_fixed == dflt_input) ? "PASS" : "FAIL");
	printf("\n");

	printf("=== Test finished ===\n");
	return 0;
}

Results:

=== FDT Regression Test Bench ===

[Case 1] Property EXISTS (Expected output: 200)
  - ORIGINAL (Buggy): 100  --> FAIL (Inverted logic applied dflt or stack garbage)
  - FIXED    (Patch): 200  --> PASS

[Case 2] Property DOES NOT EXIST (Expected output: 100)
  - ORIGINAL (Buggy): 200  --> FAIL (Returned uninitialized variable)
  - FIXED    (Patch): 100  --> PASS

=== Test finished ===



>Fix:
Apply the following patch to fix the logic inversion in 
fdtbus_get_u32prop_with_default() and prevent the uninitialized variable 
risk in fdtbus_get_phandle_with_data().

--- sys/dev/fdt/fdt_subr.c.orig	2026-08-07 14:31:36.000000000 +0900
+++ sys/dev/fdt/fdt_subr.c	2026-08-19 06:00:00.000000000 +0900
@@ -58,9 +58,9 @@
 fdtbus_get_u32prop_with_default(int phandle, const char *prop, int dflt)
 {
 	uint32_t val;
-	if (of_getprop_uint32(phandle, prop, &val)) {
-		val = dflt;
-	}
+	if (of_getprop_uint32(phandle, prop, &val) != 0) {
+		return dflt;
+	}
 	return val;
 }
 
@@ -95,11 +95,14 @@
 	for (int i = 0; len > 0; i++) {
 		u_int phandle_ref = be32toh(*p);
 		const u_int iparent = fdtbus_get_phandle_from_native(phandle_ref);
-		uint32_t cells_num;
-		of_getprop_uint32(iparent, cells, &cells_num);
+		uint32_t cells_num = 0;
+		if (of_getprop_uint32(iparent, cells, &cells_num) != 0) {
+			return EINVAL;
+		}
 		if (index == i) {
 			if (data != NULL) {
 				data->phandle = iparent;
 				data->count = cells_num;
 				data->values = p + offset;
 			}
 			goto done;
 		}
 		const u_int reclen = offset + cells_num;
+		if (len < (int)(reclen * sizeof(u_int))) {
+			return EINVAL;
+		}
 		len -= reclen * sizeof(u_int);
 		p += reclen;
 	}