aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrooks Davis <brooks@FreeBSD.org>2026-08-20 10:37:44 +0000
committerBrooks Davis <brooks@FreeBSD.org>2026-08-20 10:38:19 +0000
commit2f024a7cfddd5ce92df8437d89ba5bc3d55dacaf (patch)
treea1add229633783ed1de98d1b0d1cd9af680a12ba
parentd58ca841529c29fec4df90fc0cb93cc427f4f65d (diff)
exterr: relax format restrictions
Rather than passing the format string to printf and forcing the arguments to be (u)intmax_t, partially parse format strings and cast p1 and p2 to the correct type before running the individual format though printf. This restructure has a couple motivatations: - We can skip formats that make no sense (floating point, %n, etc.). - It is possible to special case the printing of pointers in the CHERI case. The first case is motivated by a suggestion from the audiance at one of Kirk's BSDCan talks on exterr to allow userspace to set exterr status. Allowing arbitrary format strings including %n creates a write-what-where gadget so we need to not do that. The second case is motivated by our experinces with CHERI and debugging mmap issues using a different textual error reporting framework. With CHERI, pointers are more than integer addresses and it's useful to include more details. Doing so will follow in a future commit. When the new code encounters an inappropriate format it includes a diagnostic and in most cases prints the format untouched. Reviewed by: kib Effort: CHERI upstreaming Sponsored by: Innovate UK Differential Revision: https://reviews.freebsd.org/D58058
-rw-r--r--lib/libc/gen/uexterr_format.c167
-rw-r--r--share/man/man9/exterror.917
2 files changed, 175 insertions, 9 deletions
diff --git a/lib/libc/gen/uexterr_format.c b/lib/libc/gen/uexterr_format.c
index 99ae85715f43..e32776c728bd 100644
--- a/lib/libc/gen/uexterr_format.c
+++ b/lib/libc/gen/uexterr_format.c
@@ -131,6 +131,170 @@ exterr_verbose_init(void)
}
}
+static void
+uexterr_format_msg(const struct uexterror *ue, char *buf, size_t bufsz)
+{
+ char fmt[32]; /* XXX: how big? */
+ const char *msg = ue->msg;
+ int nextarg = 1, psz;
+ size_t cindex, mindex;
+
+#define PCHAR(c) if (bufsz > 1) { *buf++ = c; bufsz--; } /* reserve last byte */
+#define PFMT(f, a) ({ \
+ psz = snprintf(buf, bufsz, f, a); \
+ if (psz > bufsz) \
+ return; /* Out of space */ \
+ buf += psz; \
+ bufsz -= psz; \
+ })
+#define ARG(_n) ({ \
+ int n = (_n); \
+ n == 1 ? ue->p1 : (n == 2 ? ue->p2 : (uint64_t)-1); \
+ })
+
+ while (*msg != '\0') {
+ if (*msg != '%') {
+ PCHAR(*msg++);
+ continue;
+ }
+
+ msg++;
+ /*
+ * Find the conversion, reject unsound or nonsensical
+ * ones, and then call snprintf to format the result
+ * using the correct argument type cast (potentially
+ * determined by the length modifier).
+ */
+ /* Conversion list ordred by printf(3). */
+ cindex = strcspn(msg, "bBdiouxXDOUeEfFgGaACcSspnm%");
+
+ /* Format too large, just complain */
+ if (cindex >= sizeof(fmt)) {
+ PFMT("%s", "<format-too-large>");
+ goto format_handled;
+ }
+
+ /*
+ * Note: msg points to one past the initial '%' and
+ * cindex is an index to the conversion in msg.
+ */
+ memcpy(fmt, msg - 1, cindex + 2);
+ fmt[cindex + 2] = '\0';
+
+ switch (msg[cindex]) {
+ case 'b':
+ case 'B':
+ case 'd':
+ case 'i':
+ case 'o':
+ case 'u':
+ case 'x':
+ case 'X':
+ /*
+ * Treat longs as 64-bit in 32-bit ABIs because
+ * that's what the kernel will do (unless we're
+ * in some 32-bit only code).
+ *
+ * This isn't quite right for signed values
+ * from 32-bit kernels unless the programmer
+ * took care to sign extend them, but 32-bit
+ * kernels aren't long for the world...
+ */
+
+ /* Find the first length modifier */
+ mindex = strcspn(fmt, "hjltwz");
+
+ switch (fmt[mindex]) {
+ case '\0': /* No length modifier */
+ case 'h': /* h or hh modifier */
+ PFMT(fmt, (unsigned)ARG(nextarg));
+ break;
+
+ case 'l':
+#ifdef __ILP32__
+ if (fmt[mindex + 1] != 'l')
+ fmt[mindex] = 'j';
+#endif
+ PFMT(fmt, (uintmax_t)ARG(nextarg));
+ break;;
+
+ case 't':
+ case 'z':
+#ifdef __ILP32__
+ fmt[mindex] = 'j';
+ /* FALLTHROUGH */
+#endif
+ case 'j':
+ PFMT(fmt, (uintmax_t)ARG(nextarg));
+ break;;
+
+ case 'w':
+ if (fmt[mindex + 1] == 'f')
+ mindex++;
+ if (fmt[mindex + 1] == '6' && fmt[mindex + 2] == '4')
+ PFMT(fmt, (uint64_t)ARG(nextarg));
+ else
+ PFMT(fmt, (unsigned)ARG(nextarg));
+ break;
+ }
+ break;
+
+ case 'C':
+ case 'c':
+ PFMT(fmt, (unsigned)ARG(nextarg));
+ break;
+
+ case 'p':
+ PFMT(fmt, (void *)ARG(nextarg));
+ break;
+
+ case '%':
+ PCHAR('%');
+ break;
+
+ /*
+ * %n is a write-what-where gadget
+ */
+ case 'n':
+ PFMT("<illegal-format>:%s", fmt);
+ break;
+
+ /*
+ * Things we don't support
+ */
+ /* Incomplete expression */
+ case '\0':
+ /* Obsolete formats */
+ case 'D':
+ case 'O':
+ case 'U':
+ /* Floating point */
+ case 'f':
+ case 'F':
+ case 'g':
+ case 'G':
+ case 'a':
+ case 'A':
+ /* String */
+ case 'S':
+ case 's':
+ /* errno */
+ case 'm':
+ /* strcspn list out of sync with this switch. */
+ default:
+ PFMT("<unsupported-format>:%s", fmt);
+ break;
+ }
+format_handled:
+ nextarg++;
+ msg += cindex + 1;
+ }
+ *buf = '\0';
+#undef PCHAR
+#undef PFMT
+#undef ARG
+}
+
int
__uexterr_format(const struct uexterror *ue, char *buf, size_t bufsz)
{
@@ -146,8 +310,7 @@ __uexterr_format(const struct uexterror *ue, char *buf, size_t bufsz)
has_msg = ue->msg[0] != '\0';
if (has_msg) {
- snprintf(buf, bufsz, ue->msg, (uintmax_t)ue->p1,
- (uintmax_t)ue->p2);
+ uexterr_format_msg(ue, buf, bufsz);
} else {
strlcpy(buf, "", bufsz);
}
diff --git a/share/man/man9/exterror.9 b/share/man/man9/exterror.9
index a7e63abab577..ff4033c6fc1d 100644
--- a/share/man/man9/exterror.9
+++ b/share/man/man9/exterror.9
@@ -6,7 +6,7 @@
.\" Konstantin Belousov <kib@FreeBSD.org> under sponsorship
.\" from the FreeBSD Foundation.
.\"
-.Dd August 3, 2026
+.Dd August 20, 2026
.Dt EXTERROR 9
.Os
.Sh NAME
@@ -101,12 +101,15 @@ The format string may include up to two printf-like format
specifiers to insert the optional argument values in the
user output, which is done in userspace.
.Pp
-The format specifier must be for an integer type, and include the
-.Dq j
-format modifier to accept only the types
-.Vt intmax_t
-or
-.Vt uintmax_t .
+The format specifier must be for an character, integer, or pointer type.
+Note that userspace printing assumes all
+.Dt long Ns -derived
+types such as
+.Dt size_t
+are 64-bit and prints them accordingly.
+Signed integer types should thus be cast to
+.Dt int64_t
+or similar to insure proper sign extension.
.El
.Pp
The strings passed as the second argument are only retained