Add checks to see if ftrace subsystem is disabled #386
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If ftrace_bug() is called during a function, ftrace_disabled gets set in the kernel but some of the functions don't return an error. This patch checks the ftrace_disabled symbol in several places and forces the code flow down the proper erorr path. Additionally, the 'active' flag is added to the kpatch_func struct so kpatch_unlink_object() isn't called on symbols that haven't been evaluated yet, resulting in less calls to WARN() when they are unnecessary. Finally, in the event of a disabled ftrace subsystem, the reason why is more apparent.
|
This is to address issue #385 |
|
I updated the change - if you accept this, f26de22 makes the code simpler by just tracking a pointer to ftrace_disabled, rather than a pointer to it and a local variable that we have to update on each check. |
If kpatch is inserted while ftrace is turned off, nothing it does will work, and there will be no errors reported to the user. If ftrace gets turned off while kpatch is loaded, anything that gets loaded while ftrace is off will also have no effect. This patch aims to prevent kpatch or any of its modules from going in when we know that it won't work. We could hook the systl .proc_handler "ftrace_enable_sysctl" so it can't be turned off while kpatch is loaded, but there's no real reason to do that as anything that's currently loaded will start to work correctly again after it gets turned back on. And if things are unloaded while ftrace is turned off, things appear to unregister correctly. Since "ftrace_enabled" is defined in ftrace.h we can't re-use the name here, so append _ptr to it, and rename ftrace_disabled with the _ptr appended as well to stay consistent. This patch also moves the initialization code for these pointers up a bit in kpatch_init so they error out properly, and not cause kpatch_exit to throw an error.
|
7426d53 now also in to check for ftrace_enabled. |
|
As discussed in #385, plan A is to get ftrace to properly report errors. This PR is plan B, so closing for now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If ftrace_bug() is called during a function, ftrace_disabled gets
set in the kernel but some of the functions don't return an error.
This patch checks the ftrace_disabled symbol in several places
and forces the code flow down the proper erorr path. Additionally,
the 'active' flag is added to the kpatch_func struct so
kpatch_unlink_object() isn't called on symbols that haven't been
evaluated yet, resulting in less calls to WARN() when they are
unnecessary.
Finally, in the event of a disabled ftrace subsystem, the reason
why is more apparent.