Details

Reviewers

imp
kib
emaste

Commits

rGdb887713de2b: kldload: Improve error handling

Summary

Addresses a failure in linker_load_module (sys/kern/kern_linker.c) to verify that an already-loaded module matches the version requirement, which causes the method to return the error (EEXIST). This is then propagated back up to kldload, which incorrectly prints that the module has already been loaded.

This patch adds a lookup to modlist_lookup2 to distinguish between the two cases at that point in the code:

A module is already loaded that is of the correct version, so the error (EEXIST) should be returned
An already-loaded module is of the incorrect version, so the error (ENOEXEC) is returned (changed from ENOENT)

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

jim.chen.1827_gmail.com created this revision.May 14 2026, 3:42 PM

Herald added subscribers: olce, imp. · View Herald TranscriptMay 14 2026, 3:42 PM

jim.chen.1827_gmail.com requested review of this revision.May 14 2026, 3:42 PM

jim.chen.1827_gmail.com added a reviewer: emaste_freebsdfoundation.org.May 14 2026, 3:47 PM

This looks good to my eye.
However, in the future, can you please use git-arc to upload patches, or generate the raw diffs with -U99999 please? There's no context to this review making it harder to verify that other paths make this same check.

This revision is now accepted and ready to land.May 14 2026, 3:52 PM

Changed ENOENT errors to ENOEXEC for more semantic correctness in the case where the file exists but is unusable due to version incompatibility.

Also updated patch to implement some suggestions from Mr. Maste:

Use EXTERROR to give additional context regarding the errors
Unbreak error/warning strings so future developers are able to "grep" for the source

This revision now requires review to proceed.May 15 2026, 6:22 PM

jim.chen.1827_gmail.com retitled this revision from Bug 295139 - kldload prints wrong error message to terminal upon attempt to load incorrect module version to Bug 295139 - (WIP) kldload prints wrong error message to terminal upon attempt to load incorrect module version.May 15 2026, 6:36 PM

jim.chen.1827_gmail.com updated this revision to Diff 177904.May 15 2026, 8:13 PM

jim.chen.1827_gmail.com retitled this revision from Bug 295139 - (WIP) kldload prints wrong error message to terminal upon attempt to load incorrect module version to Bug 295139 - kldload prints wrong error message to terminal upon attempt to load incorrect module version.

emaste added a reviewer: kib.May 20 2026, 9:23 PM

emaste edited reviewers, added: emaste; removed: emaste_freebsdfoundation.org.May 20 2026, 9:28 PM

exterr_cat_filenames.h should be regenerated.

Overall, the messages are too long and too wordy, I tried to suggest more concise texts.

sys/kern/kern_linker.c
60	Move the definition before the include block, and move the sys/exterrvar.h into the include block.
461	`"security level %d"` should be enough in the error text, there is already 'permission denied' part from EPERM
515	`"no modules loaded"`
550	`"module format error"`
553	`"kld file not found"`
2272	`"root not yet mounted"`
2276	`"module was already loaded"`
2278	See above
2289	`"kld file not found"`
2302
2306	`"incompatible module version already loaded"`
2328	`"incompatible module version"` Perhaps would be nice to report the version
2377	`"module version %d already loaded"`

Shortened error messages
Added entry to exterr_cat_filenames.h
Addressed potential memleak introduced in linker_load_module() line 2315

Added missing diff context

Added suggested edit to comment

jim.chen.1827_gmail.com marked 12 inline comments as done.May 21 2026, 4:29 PM

Rebased onto main

kib accepted this revision.May 22 2026, 10:29 AM

kib added inline comments.

sbin/kldload/kldload.c
185	No need to wrap the line
sys/kern/kern_linker.c
2269	The message is reversed, it should be 'verinfo is NULL', after all.

This revision is now accepted and ready to land.May 22 2026, 10:29 AM

Removed unnecessary line wrap

This revision now requires review to proceed.May 22 2026, 3:00 PM

jim.chen.1827_gmail.com marked 2 inline comments as done.May 22 2026, 3:01 PM

jim.chen.1827_gmail.com added inline comments.

sys/kern/kern_linker.c
2269	I'm not sure I understand. It seems to me that there will be a panic only if `verinfo == NULL` is false, which would make the error message "verinfo is not NULL" correct when a panic does occur. If the message is ambiguous due to what it "is" at time of panic vs. what it "should be," I can edit it to "verinfo should be NULL" or "verinfo is unexpectedly not NULL" It also occurs to me that since the message offers virtually no additional information compared to MPASS, just the information that "Assertion verinfo == NULL failed" could be an equivalent but less ambiguous choice here. Which course of action would you suggest?

emaste added inline comments.May 22 2026, 3:14 PM

sys/kern/kern_linker.c
2269	Yes, this is one of my complaints about `KASSERT`. You could make the message `verinfo must be NULL` to avoid the ambiguity about is, but I'd just go with MPASS; `panic: Assertion verinfo == NULL failed at kern_linker.c:2268` seems like a fine message.