From 0315a5ee7d4541fc69464c7cf09633ff14df5b01 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Sat, 18 Apr 2020 21:12:23 +0800 Subject: [PATCH] platform: msm: gsi: Fix symbol versioning failure for gsi_write_channel_scratch When building the kernel, we'd see the following warning: WARNING: EXPORT symbol "gsi_write_channel_scratch" [vmlinux] version generation failed, symbol will not be versioned. When upgrading the kernel toolchain, the following related linkage failure started occurring: ld.lld: error: relocation R_AARCH64_ABS32 cannot be used against symbol __crc_gsi_write_channel_scratch; recompile with -fPIC This device config enables CONFIG_MODVERSIONS, a way of doing symbol versioning, which can protect from loading kernel modules for which the expected kernel interfaces (ABI) have changed. CONFIG_MODVERSIONS uses scripts/genksyms/genksyms to create a file, Module.symvers, that is a simple mapping of crc's of various symbols' types to the symbol names. It's produces these crc's by using the C preprocessor, then passing this into genksyms. genksyms has a lex/yacc based C parser to parse the preprocessed sources of kernel modules. It turns out that it's incomplete, copied from an upstream project that ceased development in 2013, and was slated to be removed around the 4.9 kernel release. Because genksyms's C parser is incomplete, it doesn't understand compiler __attribute__'s on structs/enums/unions. This has dire implications for MODULE_EXPORT'ed function symbols that expect __attribute__((packed)) structs/enums/unions, such as gsi_write_channel_scratch. The crc failure can be observed: $ grep 0x00000000 out/android-msm-floral-4.14/private/msm-google/Module.symvers 0x00000000 gsi_write_channel_scratch vmlinux EXPORT_SYMBOL $ echo 'union __attribute__((packed)) foo { char bar; };' | \ ./scripts/genksyms/genksyms -d Hash table occupancy 0/4096 = 0 $ echo 'union foo { char bar; };' | ./scripts/genksyms/genksyms -d Defn for union foo == Hash table occupancy 1/4096 = 0.000244141 This results in incorrect relocations being produced to the crc's. Some possible solutions: * Update the kernel's version of genksyms. There's a comment that the kernel's sources were copied from "modutils." It seems that modutils' last release was v2.4.27 in 2004, and that development on it has stopped. Upstream modutils also has the same parsing bug. * Don't rely on MODVERSIONS. While the current work being done on libabigail is meant to help, it's a build time only check and doesn't solve the problem of loading a potentially harmful kernel module at runtime. Further, Android has VTS tests for CONFIG_MODVERSIONS, which will take time to undo. * Fix the parsing bug in genksysms. While the discussion about removing CONFIG_MODVERSIONS has started again upstream due to my bugreport, this would be the optimal solution, if I could just figure out how to rewrite the parser correctly. * Adjust the definition of gsi_channel_scratch when __GENKSYMS__ is defined, such that the type appears to be something that genksyms can at least version. This is suboptimal, but a quick enough fix to unblock the toolchain upgrade. The final option is rather simple: __attribute__((packed)) only needs to be applied to the declaration of a struct/enum/union, not everywhere its used as a formal parameter or variable. For example: struct __attribute__((packed)) foo { short z; int y; }; size_t bar(struct foo my_foo) { return sizeof(my_foo); } would return 6 on an LP64 machine (and would return 8 if foo wasn't packed). Simply remove the __attribute__((packed)) from the declaration of gsi_channel_scratch when __GENKSYMS__ is defined, and remove __packed from all references to the type `union gsi_channel_scratch` since it's not necessary (and the code was already inconsistent in its usage). With gsi_write_channel_scratch being crc'ed correctly, we can now link the kernel, and move on with the toolchain update. A better long term solution would be to replace genksyms's modutils/lex/yacc based incomplete and dead C parser with a libclang based one, but such work is beyond the scope of a toolchain update. For future travelers that would like to take a crack at fixing the existing parser, I found the develop/build/test/debug cycle to be: $ rm scripts/genksyms/genksyms $ make scripts/genksyms/ $ ./scripts/genksyms/genksyms -d < test_case.i $ ./scripts/genksyms/genksyms -d -d < test_case.i Best of luck on that endeavor. Signed-off-by: Andy Chang Signed-off-by: CryllicBuster273 Change-Id: I00b630fc2b4528f626b260eeb156029abea4cce6 --- drivers/platform/msm/gsi/gsi.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/platform/msm/gsi/gsi.c b/drivers/platform/msm/gsi/gsi.c index e94244ef6fe3..c685adebbc30 100755 --- a/drivers/platform/msm/gsi/gsi.c +++ b/drivers/platform/msm/gsi/gsi.c @@ -2508,7 +2508,7 @@ static int gsi_alloc_ap_channel(unsigned int chan_hdl) } static void __gsi_write_channel_scratch(unsigned long chan_hdl, - union __packed gsi_channel_scratch val) + union gsi_channel_scratch val) { gsi_writel(val.data.word1, gsi_ctx->base + GSI_EE_n_GSI_CH_k_SCRATCH_0_OFFS(chan_hdl, @@ -2598,7 +2598,7 @@ int gsi_write_channel_scratch2_reg(unsigned long chan_hdl, EXPORT_SYMBOL(gsi_write_channel_scratch2_reg); static void __gsi_read_channel_scratch(unsigned long chan_hdl, - union __packed gsi_channel_scratch * val) + union gsi_channel_scratch * val) { val->data.word1 = gsi_readl(gsi_ctx->base + GSI_EE_n_GSI_CH_k_SCRATCH_0_OFFS(chan_hdl, @@ -2628,10 +2628,10 @@ static void __gsi_read_wdi3_channel_scratch2_reg(unsigned long chan_hdl, } -static union __packed gsi_channel_scratch __gsi_update_mhi_channel_scratch( +static union gsi_channel_scratch __gsi_update_mhi_channel_scratch( unsigned long chan_hdl, struct __packed gsi_mhi_channel_scratch mscr) { - union __packed gsi_channel_scratch scr; + union gsi_channel_scratch scr; /* below sequence is not atomic. assumption is sequencer specific fields * will remain unchanged across this sequence @@ -2683,7 +2683,7 @@ static union __packed gsi_channel_scratch __gsi_update_mhi_channel_scratch( } int gsi_write_channel_scratch(unsigned long chan_hdl, - union __packed gsi_channel_scratch val) + union gsi_channel_scratch val) { struct gsi_chan_ctx *ctx; @@ -2751,7 +2751,7 @@ EXPORT_SYMBOL(gsi_write_wdi3_channel_scratch2_reg); int gsi_read_channel_scratch(unsigned long chan_hdl, - union __packed gsi_channel_scratch *val) + union gsi_channel_scratch *val) { struct gsi_chan_ctx *ctx;