partedit: Check allocated strings for UFS auto partitioning

[asiciliano]

Summary

After this commit "bsdinstall/partedit: Fix UFS auto partitioning" https://cgit.freebsd.org/src/commit/?id=9b4c606b96ce8a8b011dc50295c71c38741a0f4f

+	} else { /* auto partitioning */
+		items[0].value = strdup(items[0].init);
+		items[1].value = strdup(items[1].init);
+		items[2].value = strdup(items[2].init);
+		if (nitems > 3)
+			items[3].value = strdup(items[3].init);
 	}

a report suggests to check the output of strdup() https://lists.freebsd.org/archives/dev-commits-src-main/2022-April/006059.html

Maybe it could be a bit superfluous in this context, I opened this review to discuss and to get tips.
Briefly, a possible error could be an extreme case: memory full with useful data in a critical memory environment, no some naughty or huge string.

Rationale:
Currently partedit does not check the output of strdup(), probably because the size of the strings is fixed and the TUI libraries guarantee strings with limited size, usually max 16 characters. The user is not allowed to input a string of arbitrary length.

In the reported commit, the allocated strings are not "indefinite", they are almost-constants: "freebsd-[ufs|swap]", "nnn GB", "/|none", "", so the allocated memory is less than 30 bytes. It is much less than the memory required by the dialog form in the interactive case (the flow goes into the if instead of } else { /* auto partitioning */).

Anyway, let' s say we add a check (pseudocode):

// really it is (forms.value = strdup("freebsd-ufs") == NULL)
if ((items[0].value = strdup(items[0].init))== NULL)
	return;
/* ... etc ... */

If we have not 30 bytes available the partitioner will fail an instant later the "return" probably building a new dialog, each dialog/widget requires much more allocated memory than 30 bytes. Surely it will be impossible to rebuild [*] the main menu for the partition table.

Even it would not be possible to build a simple dialog msgbox to warn the user:

if (form.value = strdup("freebsd-ufs") == NULL) {
	bsddialog_msgbox(&conf, Error: Impossible to allocate 30 bytes, 0, 0);
	return;
}

\[*] Probably in this critical memory environment partedit and bsdinstall are not able to start.

However, I have not a strong opinion, if others confirm the necessity I' ll add the check. Of course, everybody can open a review to propose a better solution/idea.

[jrtc27]

Did you simulate the UX of this happening? This looks like it's going to just silently report success and cause the next installer step to fail (or not, if there's already a partition table there?).

[asiciliano]

I believe this review is quite old and can likely be closed at this point.

That said, to preserve the rationale before doing so: after a previous commit, I was asked to verify the return value of strdup/malloc for security reasons, which led me to open this review. However, I think this should really be addressed with a PR. In several places (likely throughout the entire partitioner), the return value of dynamic string allocations is not checked. It would be better to open a PR that verifies malloc return values across the whole partitioner, ensuring the issue is properly and consistently fixed rather than addressed in just a single location.