From 9bb75659e23d974516279d6613ed39f5dad13458 Mon Sep 17 00:00:00 2001 From: Evgenii Shatokhin Date: Tue, 5 May 2020 15:36:54 +0300 Subject: [PATCH] kpatch-build: Detect R_X86_64_64 dynrelas with large addends Or, to be exact, with addend values which cannot be represented by a signed int variable. This only applies to the old KPatch core. Commit 15067fcd "kmod/core: apply dynrela addend for R_X86_64_64" fixed calculation of the values for R_X86_64_64 dynrelas. This revealed another issue, similar to https://github.com/dynup/kpatch/issues/1064. Dynrelas are stored as 'struct kpatch_patch_dynrela' instances in the patch module but both the patch module and kpatch.ko use 'struct kpatch_dynrela' to work with the dynrelas. 'addend' has type 'long' in kpatch_patch_dynrela but 'int' in kpatch_dynrela, so this value can be truncated when read. R_X86_64_64 dynrela can be created, for example, if a patch for vmlinux refers to something like '(unsigned long)&idt_table+0x80000000' (a global variable which is not exported, with some addend). The addend == +0x80000000, however, effectively becomes 0xffffffff80000000 (== -0x80000000) due to this bug. Unfortunately, 'struct kpatch_dynrela' is a part of the ABI between kpatch.ko and patch modules. Plain changing 'int addend' into 'long addend' there could be problematic. The patch module built using the new 'struct kpatch_dynrela' will either fail to load if kpatch.ko is using the old 'struct kpatch_dynrela' or cause crashes or data corruptions. Unloading and reloading patch modules and kpatch.ko is not always an option either. Luckily, R_X86_64_64 dynrelas seem to be quite rare in the production patch modules and R_X86_64_64 dynrelas with large addends are expected to be even more rare. So, instead of fixing the truncation of addends right away, I propose to detect it, for now, when building a patch. If one never hits such conditions, it is not worth it to fix the issue. If R_X86_64_64 dynrelas with large addends do happen and cannot be avoided, we can try to figure out how to fix this properly, without breaking too much. Signed-off-by: Evgenii Shatokhin --- kpatch-build/create-kpatch-module.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/kpatch-build/create-kpatch-module.c b/kpatch-build/create-kpatch-module.c index 05978ed..ac1bc88 100644 --- a/kpatch-build/create-kpatch-module.c +++ b/kpatch-build/create-kpatch-module.c @@ -47,6 +47,9 @@ static void create_dynamic_rela_sections(struct kpatch_elf *kelf, struct section struct symbol *sym; struct rela *rela; unsigned int index, nr, offset, dest_offset, objname_offset, name_offset; + unsigned int type; + long addend; + char *target_name; ksyms = ksymsec->data->d_buf; krelas = krelasec->data->d_buf; @@ -93,9 +96,17 @@ static void create_dynamic_rela_sections(struct kpatch_elf *kelf, struct section name_offset = (unsigned int)rela->addend; /* Fill in dynrela entry */ + type = krelas[index].type; + addend = krelas[index].addend; + if (type == R_X86_64_64 && (addend > INT_MAX || addend <= INT_MIN)) { + target_name = (char *)strsec->data->d_buf + name_offset; + ERROR("got R_X86_64_64 dynrela for '%s' with addend too large or too small for an int: %lx", + target_name, addend); + } + dynrelas[index].src = ksym->src; - dynrelas[index].addend = krelas[index].addend; - dynrelas[index].type = krelas[index].type; + dynrelas[index].addend = addend; + dynrelas[index].type = type; dynrelas[index].external = krelas[index].external; dynrelas[index].sympos = ksym->sympos;