Details
- Reviewers
brooks - Commits
- rGa56fe703c206: Provide user interface to retrieve reported extended errors
rG8e8d3068dcbe: amd64 GENERIC: enable bloating kernel with ext errors strings
rG98af94cae78a: sysconf(3): add _SC_UEXTERR_LEN, the max length of the extended error string
rGb9c8a07d4dd9: C runtime: enable extended error reporting from kernel
rG7212b3734593: kinfo_proc: report address of extended kernel error structure
rGd995dc937a3c: vm/vm_mmap.c: add two examples of using exterrors
rG92b393c0d287: libsys: export exterrctl symbol
rG09dfe066f00c: kernel: copyout extended errors to userspace and add exterrctl(2) to control it
rG2761de0832c1: kern: add extended errors support
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Not tested, but at least it compiles.
The formatting of the errors to display them to user is not implemented: uexterr_gettext() is a stub.
I only added two places for mmap() to demonstrate use of the kernel interface.
Over all this looks great.
I'm on the fence about extending struct pthread on vs adding avoid additional mallocs. It's certainly simplifying, but people might want it off in some cases (e.g., if the kernel doesn't include messages or the developer isn't using APIs they think would benefit from extra explanations.)
| sys/kern/sys_generic.c | ||
|---|---|---|
| 2239 | Enforce uap->flags == 0? | |
| 2244 | I wonder if it makes more sense to encode the version in the flags and not try to dance around its presence in the structure. | |
| 2257 | IMO this shouldn't exist. If part of the thread's execution doesn't know where the buffer is that should be respected. If we want to leak the pointer for diagnostic purposes let's do it without provenance via kinfo_proc. I'd almost certainly disable this on CheriABI. | |
| sys/kern/syscalls.master | ||
| 3356 | There annotation should probably be _Inout_updates_bytes_opt_(sizeof(struct uexterror)), but you might need to hardcode the value rather than using sizeof. | |
| sys/sys/exterrvar.h | ||
| 28 | I wonder if EXTERR_CATEGORY is the right name. It's effectively the file in this design. | |
Then how would it disabled?
FWIW, I indeed can do 'malloc; exterrctl(); if error {free}' in libthr. But I do not see a good option for the libc.
| sys/kern/sys_generic.c | ||
|---|---|---|
| 2239 | Flags are used in the current version, I added the check for valid bits. | |
| 2244 | IMO signatures are useful. If only for debugging. | |
| sys/kern/syscalls.master | ||
| 3356 | No, I do not think so. I changed it to _In_reads_bytes_(4), this is all what this syscall does. Assuming the direct reading the spec as plain english is correct. And there is similar annotation bug for sigfastblock(2), I suspect. | |
| sys/sys/exterrvar.h | ||
| 28 | I wanted to automatically convert the source path of the compiled files (perhaps with everything before sys/ trimmed) into some identifier, easily mapped back. I was not able to create a scheme that did not require an additional build artifact to be consulted afterward. So I renamed that to category. For instance, I imagined that kern_descrip.c and sys_generic.c fall into same EXTERR_CAT_FILEDESC name. I certainly can rename this to EXTERR_FILEID or use a different scheme of generating something to match source lines. | |
Fix hand-off from libc to libthr.
Generate SIGSEGV on EFAULT from copyout of the exterr.
Remove getptr.
Report exterr address through kinfo_proc.
Still not tested.
Fix fork and exec.
Add sysconf() to get max exterr buffer
Provide real exterr format procedure
Fixed flags handling in syscall
This version works.
I'm still not a big fan of putting the strings in the kernel syscall ABI (as i guarantee someone's gonna end up writing stuff that depends upon the content of the strings!), but I /am/ a fan of how the kernel source is being updated with errors. We can at least grep for the macro use to see what they're doing, and they're a no-op if it's not compiled in.
I think my concerns are all addressed.
I'll eventually want the ability to log CHERI capabilities as arguments (as intptr_t). The current reserved space is sufficient in both size and alignment so not handling that now seems fine.
To @adrian's point, I somewhat agree, but really want to not have to manage a list of errors shipped with userspace and associated namespaces around error numbers. Keeping the list in sync would be annoying enough, but how to manage vendor drivers, experiments, etc? One possible solution would be to add a guid for each error. Then there's no need for a central authority, just run a script for error code allocation. By default I'd keep the strings in the kernel as that's best for developers, but a guid would enable translation and could support a model where you produce the extended errors and don't keep the strings in the kernel if size is a major constraint.
| sys/conf/options | ||
|---|---|---|
| 56 | I suspect that you really mean BLOAT_KERNEL_WITH_EXTERR | |