[PATCH v2 0/3] Add generic data patching functions
From: Benjamin Gray <hidden>
Date: 2023-10-16 05:04:39
Currently patch_instruction() bases the write length on the value being written. If the value looks like a prefixed instruction it writes 8 bytes, otherwise it writes 4 bytes. This makes it potentially buggy to use for writing arbitrary data, as if you want to write 4 bytes but it decides to write 8 bytes it may clobber the following memory or be unaligned and trigger an oops if it tries to cross a page boundary. To solve this, this series pulls out the size parameter to the 'top' of the text patching logic, and propagates it through the various functions. The two sizes supported are int and long; this allows for patching instructions and pointers on both ppc32 and ppc64. On ppc32 these are the same size, so care is taken to only use the size parameter on static functions, so the compiler can optimise it out entirely. Unfortunately GCC trips over its own feet here and won't optimise in a way that is optimal for strict RWX (mpc85xx_smp_defconfig) and no RWX (pmac32_defconfig). In the first case, patch_memory() is very large and can only be inlined if a single function calls it. While the source only calls it in patch_instruction(), an earlier optimisation pass inlined patch_instruction() into patch_branch(), so now there are 'two' references to patch_memory() and it cannot be inlined into patch_instruction(). Instead patch_instruction() becomes a single branch directly to patch_memory(). We can fix this by marking patch_instruction() as noinline, but this prevents patch_memory() from being directly inlined into patch_branch() when RWX is disabled and patch_memory() is very small. It may be possible to avoid this by merging together patch_instruction() and patch_memory() on ppc32, but the only way I can think to do this without duplicating the implementation involves using the preprocessor to change if is_dword is a parameter or a local variable, which is gross. For now I've removed the noinline, because at least the compiler might get smarter in future and do the inlines correctly. If noinline remains then there is no chance of it working. Changes from v1: * Addressed the v1 review actions * Removed noinline (for now) v1: https://patchwork.ozlabs.org/project/linuxppc-dev/cover/20230207015643.590684-1-bgray@linux.ibm.com/ Benjamin Gray (3): powerpc/code-patching: Add generic memory patching powerpc/64: Convert patch_instruction() to patch_u32() powerpc/32: Convert patch_instruction() to patch_uint() arch/powerpc/include/asm/code-patching.h | 33 ++++++++++++ arch/powerpc/kernel/module_64.c | 5 +- arch/powerpc/kernel/static_call.c | 2 +- arch/powerpc/lib/code-patching.c | 66 ++++++++++++++++++------ arch/powerpc/platforms/powermac/smp.c | 2 +- 5 files changed, 87 insertions(+), 21 deletions(-) -- 2.39.2