diff options
| author | Brooks Davis <brooks@FreeBSD.org> | 2026-08-20 10:37:44 +0000 |
|---|---|---|
| committer | Brooks Davis <brooks@FreeBSD.org> | 2026-08-20 10:38:19 +0000 |
| commit | 2f024a7cfddd5ce92df8437d89ba5bc3d55dacaf (patch) | |
| tree | a1add229633783ed1de98d1b0d1cd9af680a12ba | |
| parent | d58ca841529c29fec4df90fc0cb93cc427f4f65d (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.c | 167 | ||||
| -rw-r--r-- | 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 <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 |
