Page MenuHomeFreeBSD

exterr: relax format restrictions
Needs ReviewPublic

Authored by brooks on Mon, Jul 6, 4:06 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jul 9, 12:04 AM
Unknown Object (File)
Wed, Jul 8, 10:41 PM
Unknown Object (File)
Wed, Jul 8, 11:42 AM
Unknown Object (File)
Wed, Jul 8, 5:04 AM
Subscribers

Details

Reviewers
kib
mckusick
Group Reviewers
cheri
Summary

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 simply prints
the format code as is. A diagnostic could be emitted in line (similar
to printf(9) printing "(null)" for a NULL string pointer rather than
crashing, but for now we stick with passing the format through
unprocessed.

Effort: CHERI upstreaming
Sponsored by: Innovate UK

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 74551
Build 71434: arc lint + arc unit

Event Timeline

brooks requested review of this revision.Mon, Jul 6, 4:06 PM

Having had overnight to think through how to implement AT_RENAME_EXCHANGE in UFS, it will require a journal. If the operation is single threaded or at least kept to a bounded number in parallel, the log can be kept in an area that fits in the disk block holding the superblock. The journal entry simply needs to keep entries for the two names being exchanged. Specifically, the inode number of their resident directory, their name, and the inode number that they should reference. A flag in the superblock can indicate that the journal needs to be checked. If so, fsck can look up each name and verify that it references the correct inode number correcting it if it is wrong.

Does this operation allow one or both of the names to reference a directory? If so implementing it is going to be a lot more complicated since it will need to deal with '..' resolution if the names are in different directories. The code implementing it is going to start looking like rename which is no small source of maintenance headaches over the years.

Oops, sorry, posted to the wrong phabricator review.

include/exterr.h
18

Why the declaration is in this review, and not in D58059?

lib/libc/gen/uexterr_format.c
65

The function is static, it does not need to be in private namespace.

227