Bug 17393
| Summary: | kldload syscall allows the same kernel module to be loaded multiple times. | ||
|---|---|---|---|
| Product: | Base System | Reporter: | steved <steved> |
| Component: | kern | Assignee: | freebsd-bugs (Nobody) <bugs> |
| Status: | Closed FIXED | ||
| Severity: | Affects Only Me | ||
| Priority: | Normal | ||
| Version: | 3.2-RELEASE | ||
| Hardware: | Any | ||
| OS: | Any | ||
State Changed From-To: open->closed This doesn't appear to be a problem with the kernel module system. |
When two daemons (using the kldload() syscall) try and dynamically load the same kernel module, at the same time, they both succeed. Which means the same kernel module is loaded twice (which can't be good). Fix: The problem (as I see it) lies in the kernel routine linker_load_file(). The files (or linker_files in 3.3) list is never checked under an exclusive lock. In linker_load_file(), the linker_find_file_by_name() routine is used to check to see if the filename has already been loaded. linker_find_file_by_name() takes out a LK_SHARED lock to check the linker_files list to see of there is a entry for the filename. After this check, the linker_files list is never again check under an exclusive lock. Which means it very easily for two threads to be in the lc->ops->load_file() code at the same time. For example (on a non-SMP machine): thread1 -> calls linker_find_file_by_name() which return a NULL lf. thread1 -> calls malloc and sleeps OR calls lc->ops->load_file and sleeps before the file is added to the linker_files list. thread2 -> calls linker_find_file_by_name() which return a NULL lf (since thread1 has not added the filename to the linker_files list. thread2-> calls lc->ops->load_file. At this point, both thread1 and thread2 will load the filename. I fixed the problem by taking out an exclusive to: 1) recheck to linker_files list (w/ a non-locking version of linker_find_file_by_name) and increment the lf->refs when the filename exists (similar to when linker_find_file_by_name() returns a non-null lf). 2) only allow one thread to be in the lc->ops->load_file code. Basically, the lock is held from after the sprintf(koname, "%s.ko", filename) call to the out: label. I agree this is the big hammer approach, but it works with very little code change and the extra latency caused by this lock will probably never be be noticed (at least not by my apps) If you want the actually kern_linker.c file please let me know. How-To-Repeat: Create two daemons that load the same kernel module and Start them at the same time.