Page MenuHomeFreeBSD

kern_linker: Hide incompletely loaded modules from userspace
AbandonedPublic

Authored by cem on Oct 6 2016, 7:49 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 28 2024, 4:40 AM
Unknown Object (File)
Feb 16 2024, 11:51 AM
Unknown Object (File)
Dec 28 2023, 12:59 PM
Unknown Object (File)
Dec 20 2023, 1:35 AM
Unknown Object (File)
Nov 30 2023, 11:18 PM
Unknown Object (File)
Nov 16 2023, 2:54 AM
Unknown Object (File)
Oct 15 2023, 1:50 AM
Unknown Object (File)
Oct 11 2023, 3:20 PM
Subscribers
None

Details

Reviewers
kib
emaste
jhb
imp
Summary

For consistency with linker_find_file_by_id, used by kldunload(), add
the option to linker_find_file_by_name (used by kldfind()) to ignore
incompletely linked modules.

It doesn't make sense to find a module before it can be unloaded.

Internal kernel consumers of linker_find_file_by_name continue to find
partially linked modules, as their goal is to avoid loading the same
module twice.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5486
Build 5706: arc lint + arc unit

Event Timeline

cem retitled this revision from to kern_linker: Hide incompletely loaded modules from userspace.
cem updated this object.
cem edited the test plan for this revision. (Show Details)
cem added reviewers: kib, emaste, jhb, imp.

How is that possible at all ? Did you observed this in real-life scenario ?

It seems that the flag is either set for the file, or the file is removed from the list, under the single kld_sx-protected region. Then, I do not see how could kldnext consumer, protected by the same lock, see a file without the flag.

In D8167#169639, @kib wrote:

How is that possible at all ? Did you observed this in real-life scenario ?

Don't know if it is possible. No, not observed.

It seems inconsistent with linker_find_file_by_id and sys_kldnext, which is also only used under the same sx lock.

It seems that the flag is either set for the file, or the file is removed from the list, under the single kld_sx-protected region. Then, I do not see how could kldnext consumer, protected by the same lock, see a file without the flag.

If this is true, I don't see why we need to even check for the flag in kldnext or linker_find_file_by_id.

In D8167#169675, @cem wrote:

If this is true, I don't see why we need to even check for the flag in kldnext or linker_find_file_by_id.

The flag should be removed where not needed. I suspect that it is result of the code first being locked by Giant which is dropped during sleeps.

I did not traced all paths in the code to confirm either validity of the patch in review, nor the removal of flags.

I'm still annoyed at the inconsistency but not interested in exhaustively looking at callers to verify no such path exists for linker_find_file_by_id.

Run-time loaded linker files are present on the global list but missing LINKER_FILE_LINKED during linker_file_sysinit() in linker_file_load(), where kld_sx is dropped.

In D8167#170196, @cem wrote:

Run-time loaded linker files are present on the global list but missing LINKER_FILE_LINKED during linker_file_sysinit() in linker_file_load(), where kld_sx is dropped.

In this case, I think that LINKER_FILE_LINKED might be reasonably set before sysinit sets are invoked. There is no possible failure in linker_file_sysinit(), so the state is de-facto finalized.

Then unset the flag if linker_file_unload/ENOEXEC is returned.

Ok, please commit this as is. I do think that implementing my last note would make the change better, but anyway, just go ahead.