Add serial-number to hw.fdt sysctl area if found in fdt.

[• karels]

Summary

Add serial-number if that fdt property exists.

Test Plan

tested on Raspberry Pi 3B+ and 4

[imp]

sys/dev/ofw/ofw_fdt.c

194

what ensures this is NUL terminated?

[• karels]

sys/dev/ofw/ofw_fdt.c

194

Good question. I modeled this after the other sysctls in this group. The upper bound is set when registering the sysctl, but it looks like sysctl_handle_string doesn't use it in this case. If it's a bug, it's a bug for the other sysctls here. @manu?

[manu]

sys/dev/ofw/ofw_fdt.c

194

Mhm, I don't remember why I used 80 for model but that seems wrong.
A string in a dtb is NUL terminated (and yes one can probably load a non valid dtb but I expect other problems if that's the case).
IIRC the max string length is 255 so compatible is ok (but I can't find this info in the spec so maybe it's not, that being said a lot of code in FreeBSD assume a 255 max length for a string value).

[manu]

The problem I have with this is that serial-number isn't standard so we don't know what a manufacturer could put in it (string, stringlist, integer etc ...)

[• karels]

Would it be sufficient to check for ASCII with a NUL termination at the end?

We could check the length vs. the buffer size on the others to make sure there is room for the NUL.

[manu]

In D34356#778306, @karels wrote:

Would it be sufficient to check for ASCII with a NUL termination at the end?

I think so.

We could check the length vs. the buffer size on the others to make sure there is room for the NUL.

Yup

[• karels]

Check for ASCII string for serial-number, check len of hw.fdt strings.

[imp]

sys/dev/ofw/ofw_fdt.c

207

So what happens if it is all 'a's, then? I think this fails. I think you want:

for (i = 0; i < len - 1; i++) {
    if (!isascii(fdt_serial[i])) break;
}
if (i < len || fdt_serial[len - 1] != '\0') fdt_serial[0] = 0;

(properly formatted) That is, if there's any non-ascii characters (I'd be tempted even to restrict it to isalnum() instead), break. if we don't get through all the characters, or the last one isn't a NUL, zero the string.

[manu]

sys/dev/ofw/ofw_fdt.c

207

'-' and '.' are likely to be found in serial number I think.

[• karels]

Fix test for missing NUL, switch to isprint().

[• karels]

Oops, you are right; I tried to combine tests and it failed to check for a final NUL properly. Fixed, and I changed to isprint(); that should be liberal enough.

[• karels]

Where do we stand on this? Is the change acceptable?

[imp]

It looks good to me now.