Skip to content

Remove unsafe try_module_get(THIS_MODULE) #328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 6, 2025
Merged

Conversation

jserv
Copy link
Contributor

@jserv jserv commented Aug 5, 2025

This commit removes all instances of try_module_get(THIS_MODULE) and corresponding module_put(THIS_MODULE) calls from example modules. This pattern is considered unsafe because:

  • The kernel automatically manages module reference counting during file operations
  • Manual reference counting within a module's own code can lead to race conditions
  • It is redundant and unnecessary for proper module operation

It also updates documentation to:

  • Remove references to try_module_get/module_put functions
  • Add note explaining why these functions should be avoided
  • Keep only module_refcount() as a safe display function

Close #52

Summary by Bito

This pull request enhances kernel module safety by removing unsafe functions try_module_get(THIS_MODULE) and module_put(THIS_MODULE), which can cause race conditions. It promotes the use of the safe module_refcount() function and updates the documentation accordingly.

@jserv jserv requested review from 0xff07 and linD026 August 5, 2025 09:28
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Aug 5, 2025
@jserv jserv force-pushed the drop-this-module branch from f103e15 to 7a3f63d Compare August 5, 2025 10:07
Copy link
Collaborator

@linD026 linD026 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, it looks good to me.
However, I am outside now, so might not be able to look the code changes deeply.

lkmpg.tex Outdated
It is important to keep the counter accurate; if you ever lose track of the correct usage count, you will never be able to unload the module; it is now reboot time.
This is bound to happen to you sooner or later during a module's development.
Note: The use of \cpp|try_module_get(THIS_MODULE)| and \cpp|module_put(THIS_MODULE)| within a module's own code is considered unsafe and should be avoided.
The kernel automatically manages the reference count when file operations are in progress, so manual reference counting is unnecessary and can lead to race conditions.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to have some external references to this, such as:

https://stackoverflow.com/questions/1741415/linux-kernel-modules-when-to-use-try-module-get-module-put

This commit removes all instances of try_module_get(THIS_MODULE) and
corresponding module_put(THIS_MODULE) calls from example modules. This
pattern is considered unsafe because:
- The kernel automatically manages module reference counting during file
  operations
- Manual reference counting within a module's own code can lead to race
  conditions
- It is redundant and unnecessary for proper module operation

It also updates documentation to:
- Remove references to try_module_get/module_put functions
- Add note explaining why these functions should be avoided
- Keep only module_refcount() as a safe display function

Close #52
@jserv jserv force-pushed the drop-this-module branch from 7a3f63d to b859b8b Compare August 6, 2025 04:53
@jserv jserv merged commit a9caf93 into master Aug 6, 2025
1 check passed
@jserv jserv deleted the drop-this-module branch August 6, 2025 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid using try_module_get(THIS_MODULE)
2 participants