Messages in this thread | | | From | Josh Poimboeuf <> | Subject | [RFC][PATCH 0/5] Improve static call NULL handling | Date | Fri, 10 Mar 2023 12:31:12 -0800 |
| |
Static calling a NULL pointer is a NOP, unless you're one of those poor souls running on an arch (or backported x86 monstrosity) with CONFIG_HAVE_STATIC_CALL=n, then it's a panic.
The "fix" for this undefined behavior is to tell the user to just use static_call_cond() instead, if they want consistent NOP behavior. But forgetting to do that is likely to cause subtle bugs. It actually already did (during RHEL development).
There are two ways to make it consistent:
a) Make static_call(NULL) a NOP for all configs; or
b) Make static_call(NULL) a panic for all configs.
Do (a) because it's consistent with the existing HAVE_STATIC_CALL behavior. Also it seems simpler to implement and use, and based on looking at the existing use cases, it's common to want the "do nothing and return 0" behavior by default.
Then take it a step further and get rid of the distinction between STATIC_CALL_NULL and STATIC_CALL_RET0.
The end result is less confusing semantics and simpler code all around.
EPILOGUE --------
If any users wanted panic-on-NULL by default instead of NOP-on-NULL, that could be added on top of this. They could just initialize the static call with a __static_call_bug() helper.
void __static_call_bug(void) { BUG(); } .. DEFINE_STATIC_CALL(foo, (func_type)__static_call_bug);
We could take that even further:
DEFINE_STATIC_CALL_NOP(foo, func_type); DEFINE_STATIC_CALL_BUG(bar, func_type); ... #define STATIC_CALL_NOP (func_type)__static_call_nop #define STATIC_CALL_BUG (func_type)__static_call_bug ... static_call_update(foo, STATIC_CALL_NOP); // do nothing and return 0 static_call_update(foo, STATIC_CALL_BUG); // panic static_call_update(foo, NULL); // ???
The default behavior for NULL could be a key-specific policy, stored as a flag in the static_call_key struct.
The key-specific policy would be easier to deal with than the call-site-specific policy we have today with static_call_cond().
Josh Poimboeuf (5): static_call: Make NULL static calls consistent static_call: Make NULL static calls return 0 static_call: Remove static_call_cond() and its usages static_call: Remove DEFINE_STATIC_CALL_RET0() and its uses x86/kvm: Simplify static call handling
arch/powerpc/include/asm/static_call.h | 1 - arch/powerpc/kernel/irq.c | 2 +- arch/x86/events/amd/core.c | 2 +- arch/x86/events/core.c | 26 ++--- arch/x86/include/asm/kvm-x86-ops.h | 86 +++++++------- arch/x86/include/asm/kvm-x86-pmu-ops.h | 17 +-- arch/x86/include/asm/kvm_host.h | 6 +- arch/x86/include/asm/static_call.h | 8 -- arch/x86/kvm/irq.c | 2 +- arch/x86/kvm/lapic.c | 22 ++-- arch/x86/kvm/pmu.c | 11 +- arch/x86/kvm/x86.c | 36 +++--- include/linux/static_call.h | 131 +++++----------------- kernel/events/core.c | 8 +- kernel/sched/core.c | 10 +- security/keys/trusted-keys/trusted_core.c | 2 +- 16 files changed, 126 insertions(+), 244 deletions(-)
-- 2.39.2
| |