The branch main has been updated by brooks: URL: https://cgit.FreeBSD.org/src/commit/?id=2f024a7cfddd5ce92df8437d89ba5bc3d55dacaf
commit 2f024a7cfddd5ce92df8437d89ba5bc3d55dacaf Author: Brooks Davis <[email protected]> AuthorDate: 2026-08-20 10:37:44 +0000 Commit: Brooks Davis <[email protected]> CommitDate: 2026-08-20 10:38:19 +0000 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 --- lib/libc/gen/uexterr_format.c | 167 +++++++++++++++++++++++++++++++++++++++++- share/man/man9/exterror.9 | 17 +++-- 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 <[email protected]> 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
