Background:
The fact that unbalanced override/revert can stay in the kernel as a non-issue (in ovl_create_or_link) suggests that maybe we are wrong or just unnecessarily strict in expecting and requiring the balance. The first idea would have been to reset (not decrement) off on revert_creds, however I recall @Adam-pi3 said (in private discussions) that actual nesting was also seen, where it would be too early to reset off on the first revert (would trigger false positives from credentials discrepancies). Another obvious idea would be, instead of having that off flag at all, to update our shadow credentials on overrides and reverts. However, that would make these functions just as usable as commit_creds by exploits, which would then bypass LKRG by simply using override_creds in place of commit_creds. This is also why we don't just hook and update on commit_creds, but instead hook the many other places where credentials are modified.
Proposal:
Finally, moving to the idea that I think is new (wasn't discussed privately before): maybe on override_creds we can store the new credentials not in our main shadow credentials (that we validate current ones against when not off), but in a separate new overridden credentials stack (essentially an array indexed by off depth), which we'd only use on revert_creds to see how many levels up we're reverting to. Then we end up handling unbalanced override/revert in a generic manner. The only drawback I see is it's actually more complex than hooking the different ovl_ functions and doing some hard-coded checks/adjustments (like we chose to do for now in #217).
This is item 3 in https://github.com/lkrg-org/lkrg/issues/215#issuecomment-1195744061
Background: The fact that unbalanced override/revert can stay in the kernel as a non-issue (in
ovl_create_or_link) suggests that maybe we are wrong or just unnecessarily strict in expecting and requiring the balance. The first idea would have been to reset (not decrement)offonrevert_creds, however I recall @Adam-pi3 said (in private discussions) that actual nesting was also seen, where it would be too early to resetoffon the first revert (would trigger false positives from credentials discrepancies). Another obvious idea would be, instead of having thatoffflag at all, to update our shadow credentials on overrides and reverts. However, that would make these functions just as usable ascommit_credsby exploits, which would then bypass LKRG by simply usingoverride_credsin place ofcommit_creds. This is also why we don't just hook and update oncommit_creds, but instead hook the many other places where credentials are modified.Proposal: Finally, moving to the idea that I think is new (wasn't discussed privately before): maybe on
override_credswe can store the new credentials not in our main shadow credentials (that we validate current ones against when notoff), but in a separate new overridden credentials stack (essentially an array indexed byoffdepth), which we'd only use onrevert_credsto see how many levels up we're reverting to. Then we end up handling unbalanced override/revert in a generic manner. The only drawback I see is it's actually more complex than hooking the differentovl_functions and doing some hard-coded checks/adjustments (like we chose to do for now in #217).