RepoMirrors/kpatch
1
0
Fork 0
mirror of https://github.com/dynup/kpatch synced 2025-02-07 04:31:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1318 from jpoimboe/static-call-patch-author-guide

Browse Source 
patch-author-guide: update jump label / static call descriptions
This commit is contained in:
Josh Poimboeuf 2022-12-02 11:47:41 -06:00 committed by GitHub
commit 059467dbc5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 126 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

96
doc/patch-author-guide.md
View File

@ -24,7 +24,7 @@ Table of contents
- [Code removal](#code-removal)
- [Once macros](#once-macros)
- [inline implies notrace](#inline-implies-notrace)
- [Jump labels](#jump-labels)
- [Jump labels and static calls](#jump-labels-and-static-calls)
- [Sibling calls](#sibling-calls)
- [Exported symbol versioning](#exported-symbol-versioning)
- [System calls](#system-calls)
@ -747,41 +747,81 @@ changes to all of `__tcp_mtu_to_mss()` callers (ie, it was inlined as
requested). In this case, a simple workaround is to specify
`__tcp_mtu_to_mss()` as `__always_inline` to force the compiler to do so.
Jump labels
-----------
Jump labels and static calls
----------------------------
When modifying a function that contains a jump label, kpatch-build may
return an error like: `ERROR: oom_kill.o: kpatch_regenerate_special_section: 2109: Found a jump label at out_of_memory()+0x10a, using key cpusets_enabled_key. Jump labels aren't currently supported. Use static_key_enabled() instead.`
### Late module patching vs special section relocations
This is due to a limitation in the kernel to process static key
livepatch relocations (resolved by late-module patching). Older
versions of kpatch-build may have reported successfully building
kpatch module, but issue
[#931](https://github.com/dynup/kpatch/issues/931) revealed potentially
dangerous behavior if the static key value had been modified from its
compiled default.
Jump labels and static calls can be problematic due to "late module patching",
which is a feature (design flaw?) in upstream livepatch. When a livepatch
module patches another module, unfortunately the livepatch module doesn't have
an official module dependency on the patched module. That means the patched
module doesn't even have to be loaded when the livepatch module gets loaded.
In that case the patched module gets patched on demand whenever it might get
loaded in the future. It also gets unpatched on demand whenever it gets
unloaded.
The current workaround is to remove the jump label by explictly checking
the static key:
Loading (and patching) the module at some point after loading the livepatch
module is called "late module patching". In order to support this
(mis?)feature, all relocations in the livepatch module which reference module
symbols must be converted to "klp relocations", which get resolved at patching
time.
```c
DEFINE_STATIC_KEY_TRUE(true_key);
DEFINE_STATIC_KEY_FALSE(false_key);
In all modules (livepatch and otherwise), jump labels and static calls rely on
special sections which trigger jump-label/static-call code patching when a
module gets loaded. But unfortunately those special sections have relocations
which need to get resolved, so there's an ordering issue.
/* unsupported */
if (static_key_true(&true_key))
if (static_key_false(&false_key))
if (static_branch_likely(&key))
When a (livepatch) module gets loaded, first its relocations are resolved, then
its special section handling (and code patching) is done. The problem is, for
klp relocations, if they reference another module's symbols, and that module
isn't loaded, they're not yet defined. So if a `.static_call_sites` entry
tries to reference its corresponding `struct static_call_key`, but that key
lives in another module which is not yet loaded, the key reference won't be
resolved, and so `mod->static_call_sites` will be corrupted when
`static_call_module_notify()` runs when the livepatch module first loads.
/* supported */
if (static_key_enabled(&true_key))
if (static_key_enabled(&false_key))
if (likely(static_key_enabled(&key)))
### Jump labels
With pre-5.8 kernels, kpatch-build will error out if it encounters any jump
labels:
```
oom_kill.o: Found a jump label at out_of_memory()+0x10a, using key cpusets_enabled_key. Jump labels aren't supported with this kernel. Use static_key_enabled() instead.
```
Note that with Linux 5.8+, jump labels in patched functions are now supported
when the static key was originally defined in the kernel proper (vmlinux). The
above error will not be seen unless the static key lives in a module.
With Linux 5.8+, klp relocation handling is integrated with the module relocation
code, so jump labels in patched functions are supported when the static key was
originally defined in the kernel proper (vmlinux).
However, if the static key lives in a module, jump labels are _not_ supported
in patched code, due to the ordering issue described above. If the jump label
is a tracepoint, kpatch-build will silently remove the tracepoint. Otherwise,
there will be an error:
```
vmx.o: Found a jump label at vmx_hardware_enable.cold()+0x23, using key enable_evmcs, which is defined in a module. Use static_key_enabled() instead.
```
When you get one of the above errors, the fix is to remove the jump label usage
in the patched function, replacing it with a regular C conditional.
This can be done by replacing any usages of `static_branch_likely()`,
`static_branch_unlikely()`, `static_key_true()`, and `static_key_false()` with
`static_key_enabled()` in the patch file.
### Static calls
Similarly, static calls are not supported when the corresponding static call
key was originally defined in a module. If such a static call is part of a
tracepoint, kpatch-build will silently remove it. Otherwise, there will be an
error:
```
cpuid.o: Found a static call at kvm_set_cpuid.cold()+0x32c, using key __SCK__kvm_x86_vcpu_after_set_cpuid, which is defined in a module. Use KPATCH_STATIC_CALL() instead.
```
To fix this error, simply replace such static calls with regular indirect
branches (or retpolines, if applicable) by adding `#include "kpatch-macros.h"`
to the patch source and replacing usages of `static_call()` with
`KPATCH_STATIC_CALL()`.
Sibling calls
-------------

95
kpatch-build/create-diff-object.c
View File

@ -1759,14 +1759,14 @@ static void kpatch_include_symbol(struct symbol *sym)
/*
* The symbol gets included even if its section isn't needed, as it
* might be needed: either permanently for a rela, or temporarily for
* the later creation of a dynrela.
* the later creation of a klp relocation.
*/
sym->include = 1;
/*
* For a function/object symbol, if it has a section, we only need to
* include the section if it has changed. Otherwise the symbol will be
* used by relas/dynrelas to link to the real symbol externally.
* used by relas/klp_relocs to link to the real symbol externally.
*
* For section symbols, we always include the section because
* references to them can't otherwise be resolved externally.
@ -1800,8 +1800,8 @@ static void kpatch_include_standard_elements(struct kpatch_elf *kelf)
*
* Note that if any of these sections have rela sections, they
* will also be included in their entirety. That may result in
* some extra (unused) dynrelas getting created, which should
* be harmless.
* some extra (unused) klp relocations getting created, which
* should be harmless.
*/
if (!strcmp(sec->name, ".shstrtab") ||
!strcmp(sec->name, ".strtab") ||
@ -2284,8 +2284,8 @@ static bool jump_table_group_filter(struct lookup_table *lookup,
* (in the klp module load) as normal relas, before jump label init.
* On the other hand, jump labels based on static keys which are
* defined in modules aren't supported, because late module patching
* can result in the klp relas getting applied *after* the klp module's
* jump label init.
* can result in the klp relocations getting applied *after* the klp
* module's jump label init.
*/
if (lookup_symbol(lookup, key->sym, &symbol) &&
@ -3204,8 +3204,8 @@ static int function_ptr_rela(const struct rela *rela)
rela->type == R_PPC64_TOC16_LO_DS));
}
static bool need_dynrela(struct kpatch_elf *kelf, struct lookup_table *table,
struct section *relasec, const struct rela *rela)
static bool need_klp_reloc(struct kpatch_elf *kelf, struct lookup_table *table,
struct section *relasec, const struct rela *rela)
{
struct lookup_result symbol;
@ -3214,7 +3214,7 @@ static bool need_dynrela(struct kpatch_elf *kelf, struct lookup_table *table,
/*
* These references are treated specially by the module loader and
* should never be converted to dynrelas.
* should never be converted to klp relocations.
*/
if (rela->type == R_PPC64_REL16_HA || rela->type == R_PPC64_REL16_LO ||
rela->type == R_PPC64_ENTRY)
@ -3259,16 +3259,16 @@ static bool need_dynrela(struct kpatch_elf *kelf, struct lookup_table *table,
if (rela->sym->sec) {
/*
* Internal symbols usually don't need dynrelas, because they
* live in the patch module and can be relocated normally.
* Internal symbols usually don't need klp relocations, because
* they live in the patch module and can be relocated normally.
*
* There's one exception: function pointers.
*
* If the rela references a function pointer, we convert it to
* a dynrela, so that the function pointer will refer to the
* original function rather than the patched function. This
* can prevent crashes in cases where the function pointer is
* called asynchronously after the patch module has been
* a klp relocation, so that the function pointer will refer to
* the original function rather than the patched function.
* This can prevent crashes in cases where the function pointer
* is called asynchronously after the patch module has been
* unloaded.
*/
if (!function_ptr_rela(rela))
@ -3279,13 +3279,13 @@ static bool need_dynrela(struct kpatch_elf *kelf, struct lookup_table *table,
* special case. They are not supposed to be visible outside
* of the function that defines them. Their names may differ
* in the original and the patched kernels which makes it
* difficult to use dynrelas. Fortunately, nested functions
* are rare and are unlikely to be used as asynchronous
* callbacks, so the patched code can refer to them directly.
* It seems, one can only distinguish such functions by their
* names containing a dot. Other kinds of functions with such
* names (e.g. optimized copies of functions) are unlikely to
* be used as callbacks.
* difficult to use klp relocations. Fortunately, nested
* functions are rare and are unlikely to be used as
* asynchronous callbacks, so the patched code can refer to
* them directly. It seems, one can only distinguish such
* functions by their names containing a dot. Other kinds of
* functions with such names (e.g. optimized copies of
* functions) are unlikely to be used as callbacks.
*
* Function pointers to *new* functions don't have this issue,
* just use a normal rela for them.
@ -3309,8 +3309,9 @@ static bool need_dynrela(struct kpatch_elf *kelf, struct lookup_table *table,
rela->sym->name);
/*
* The symbol is (formerly) local. Use a dynrela to access the
* original version of the symbol in the patched object.
* The symbol is (formerly) local. Use a klp relocation to
* access the original version of the symbol in the patched
* object.
*/
return true;
}
@ -3322,9 +3323,9 @@ static bool need_dynrela(struct kpatch_elf *kelf, struct lookup_table *table,
* On powerpc, the symbol is global and exported, but
* it was also in the changed object file. In this
* case the rela refers to the 'localentry' point, so a
* normal rela wouldn't work. Force a dynrela so it
* can be handled correctly by the livepatch relocation
* code.
* normal rela wouldn't work. Force a klp relocation
* so it can be handled correctly by the livepatch
* relocation code.
*/
return true;
}
@ -3340,8 +3341,9 @@ static bool need_dynrela(struct kpatch_elf *kelf, struct lookup_table *table,
/*
* The symbol is exported by the to-be-patched module, or by
* another module which the patched module depends on. Use a
* dynrela because of late module loading: the patch module may
* be loaded before the to-be-patched (or other) module.
* klp relocation because of late module loading: the patch
* module may be loaded before the to-be-patched (or other)
* module.
*/
return true;
}
@ -3349,8 +3351,8 @@ static bool need_dynrela(struct kpatch_elf *kelf, struct lookup_table *table,
if (symbol.global) {
/*
* The symbol is global in the to-be-patched object, but not
* exported. Use a dynrela to work around the fact that it's
* an unexported sybmbol.
* exported. Use a klp relocation to work around the fact that
* it's an unexported sybmbol.
*/
return true;
}
@ -3366,9 +3368,8 @@ static bool need_dynrela(struct kpatch_elf *kelf, struct lookup_table *table,
/*
* kpatch_create_intermediate_sections()
*
* The primary purpose of this function is to convert some relas (also known as
* relocations) to dynrelas (also known as dynamic relocations or livepatch
* relocations or klp relas).
* The primary purpose of this function is to convert some relocations to klp
* relocations.
*
* If the patched code refers to a symbol, for example, if it calls a function
* or stores a pointer to a function somewhere or accesses some global data,
@ -3377,7 +3378,7 @@ static bool need_dynrela(struct kpatch_elf *kelf, struct lookup_table *table,
*
* If the symbol lives outside the patch module, and if it's not exported by
* vmlinux (e.g., with EXPORT_SYMBOL) then the rela needs to be converted to a
* dynrela so the livepatch code can resolve it at runtime.
* klp relocation so the livepatch code can resolve it at runtime.
*/
static void kpatch_create_intermediate_sections(struct kpatch_elf *kelf,
struct lookup_table *table,
@ -3408,18 +3409,18 @@ static void kpatch_create_intermediate_sections(struct kpatch_elf *kelf,
nr++;
/*
* We set 'need_dynrela' here in the first pass because
* the .toc section's 'need_dynrela' values are
* dependent on all the other sections. Otherwise, if
* we did this analysis in the second pass, we'd have
* to convert .toc dynrelas at the very end.
* We set 'need_klp_reloc' here in the first pass
* because the .toc section's 'need_klp_reloc' values
* are dependent on all the other sections. Otherwise,
* if we did this analysis in the second pass, we'd
* have to convert .toc klp relocations at the very end.
*
* Specifically, this is needed for the powerpc
* internal symbol function pointer check which is done
* via .toc indirection in need_dynrela().
* via .toc indirection in need_klp_reloc().
*/
if (need_dynrela(kelf, table, relasec, rela))
toc_rela(rela)->need_dynrela = 1;
if (need_klp_reloc(kelf, table, relasec, rela))
toc_rela(rela)->need_klp_reloc = true;
}
}
@ -3464,7 +3465,7 @@ static void kpatch_create_intermediate_sections(struct kpatch_elf *kelf,
}
list_for_each_entry_safe(rela, safe, &relasec->relas, list) {
if (!rela->need_dynrela) {
if (!rela->need_klp_reloc) {
rela->sym->strip = SYMBOL_USED;
continue;
}
@ -3483,7 +3484,7 @@ static void kpatch_create_intermediate_sections(struct kpatch_elf *kelf,
* runs due to late module patching.
*/
if (!KLP_ARCH && !vmlinux && special)
ERROR("unsupported dynrela reference to symbol '%s' in module-specific special section '%s'",
ERROR("unsupported klp relocation reference to symbol '%s' in module-specific special section '%s'",
rela->sym->name, relasec->base->name);
if (!lookup_symbol(table, rela->sym, &symbol))
@ -3764,7 +3765,7 @@ static void kpatch_create_mcount_sections(struct kpatch_elf *kelf)
/*
* This function strips out symbols that were referenced by changed rela
* sections, but the rela entries that referenced them were converted to
* dynrelas and are no longer needed.
* klp relocations and are no longer needed.
*/
static void kpatch_strip_unneeded_syms(struct kpatch_elf *kelf,
struct lookup_table *table)
@ -4090,7 +4091,7 @@ int main(int argc, char *argv[])
kpatch_no_sibling_calls_ppc64le(kelf_out);
/* create strings, patches, and dynrelas sections */
/* create strings, patches, and klp relocation sections */
kpatch_create_strings_elements(kelf_out);
kpatch_create_patches_sections(kelf_out, lookup, parent_name);
kpatch_create_intermediate_sections(kelf_out, lookup, parent_name, patch_name);

18
kpatch-build/create-klp-module.c
View File

@ -113,9 +113,9 @@ static struct symbol *find_or_add_ksym_to_symbols(struct kpatch_elf *kelf,
}
/*
* Create a klp rela section given the base section and objname
* Create a .klp.rela section given the base section and objname
*
* If a klp rela section matching the base section and objname
* If a .klp.rela section matching the base section and objname
* already exists, return it.
*/
static struct section *find_or_add_klp_relasec(struct kpatch_elf *kelf,
@ -163,8 +163,8 @@ static struct section *find_or_add_klp_relasec(struct kpatch_elf *kelf,
* 1) Allocate a symbol for the corresponding .kpatch.symbols entry if
* it doesn't already exist (find_or_add_ksym_to_symbols())
* This is the symbol that the relocation points to (rela->sym)
* 2) Allocate a rela, and add it to the corresponding .klp.rela. section. If
* the matching .klp.rela. section (given the base section and objname)
* 2) Allocate a rela, and add it to the corresponding .klp.rela section. If
* the matching .klp.rela section (given the base section and objname)
* doesn't exist yet, create it (find_or_add_klp_relasec())
*/
static void create_klp_relasecs_and_syms(struct kpatch_elf *kelf, struct section *krelasec,
@ -212,12 +212,12 @@ static void create_klp_relasecs_and_syms(struct kpatch_elf *kelf, struct section
if (!sym)
ERROR("error finding or adding ksym to symtab");
/* Create (or find) the .klp.rela. section for the dest sec and object */
/* Create (or find) the .klp.rela section for the dest sec and object */
klp_relasec = find_or_add_klp_relasec(kelf, dest->sec, objname);
if (!klp_relasec)
ERROR("error finding or adding klp relasec");
ERROR("error finding or adding .klp.rela section");
/* Add the klp rela to the .klp.rela. section */
/* Add the klp relocation to the .klp.rela section */
ALLOC_LINK(rela, &klp_relasec->relas);
rela->offset = (unsigned int)(dest->sym.st_value + dest_off);
rela->type = krelas[index].type;
@ -471,7 +471,7 @@ int main(int argc, char *argv[])
ERROR("number of krelas and ksyms do not match");
/*
* Create klp rela sections and klp symbols from
* Create .klp.rela sections and klp symbols from
* .kpatch.{relocations,symbols} sections
*/
create_klp_relasecs_and_syms(kelf, krelasec, ksymsec, strings);
@ -488,7 +488,7 @@ int main(int argc, char *argv[])
remove_intermediate_sections(kelf);
kpatch_reindex_elements(kelf);
/* Rebuild rela sections, new klp rela sections will be rebuilt too. */
/* Rebuild rela sections, new .klp.rela sections will be rebuilt too. */
symtab = find_section_by_name(&kelf->sections, ".symtab");
if (!symtab)
ERROR("missing .symtab section");

2
kpatch-build/kpatch-elf.h
View File

@ -101,7 +101,7 @@ struct rela {
unsigned int offset;
long addend;
char *string;
bool need_dynrela;
bool need_klp_reloc;
};
struct string {