Re: [PATCH v5] livepatch: Clear relocation targets on a module removal
On Thu, 2022-09-01 at 08:42 -0400, Joe Lawrence wrote: > On Thu, Sep 01, 2022 at 01:39:02PM +1000, Michael Ellerman wrote: > > Joe Lawrence writes: > > > On Thu, Sep 01, 2022 at 08:30:44AM +1000, Michael Ellerman wrote: > > > > Joe Lawrence writes: > > ... > > > > > > Hi Michael, > > > > > > While we're on the topic of klp-relocations and Power, I saw a > > > similar > > > access problem when writing (late) relocations into > > > .data..ro_after_init. I'm not entirely convinced this should be > > > allowed > > > (ie, is it really read-only after .init or ???), but it seems > > > that other > > > arches currently allow it ... > > > > I guess that's because we didn't properly fix apply_relocate_add() > > in > > https://github.com/linuxppc/issues/issues/375 ? > > > > If other arches allow it then we don't want to be the odd one out > > :) > > > > So I guess we need to implement it. > > > > FWIW, I think it this particular relocation is pretty rare, we > haven't > seen it in real patches nor do we have a kpatch test that generates > it. > I only hit a crash as I was trying to write a more exhaustive test > for > the klp-convert implementation. I'll revive my proper fix. I stopped working on it since my previous version was hitting endian bugs with some relocations & it didn't seem necessary at the time. Shouldn't take too much to get it going again. > > > > = TEST: klp-convert data relocations (late module patching) > > > = > > > % modprobe test_klp_convert_data > > > livepatch: enabling patch 'test_klp_convert_data' > > > livepatch: 'test_klp_convert_data': starting patching transition > > > livepatch: 'test_klp_convert_data': patching complete > > > % modprobe test_klp_convert_mod > > > ... > > > module_64: Applying ADD relocate section 54 to 20 > > > module_64: RELOC at 8482d02a: 38-type as > > > .klp.sym.test_klp_convert_mod.static_ro_after_init,0 > > > (0xc008016d0084) + 0 > > > BUG: Unable to handle kernel data access on write at > > > 0xc008021d > > > Faulting instruction address: 0xc0055f14 > > > Oops: Kernel access of bad area, sig: 11 [#1] > > > LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries > > > Modules linked in: test_klp_convert_mod(+) > > > test_klp_convert_data(K) bonding rfkill tls pseries_rng drm fuse > > > drm_panel_orientation_quirks xfs libcrc32c sd_mod t10_pi sg > > > ibmvscsi ibmveth scsi_transport_srp vmx_crypto dm_mirror > > > dm_region_hash dm_log dm_mod [last unloaded: > > > test_klp_convert_mod] > > > CPU: 0 PID: 17089 Comm: modprobe Kdump: loaded Tainted: > > > G K 5.19.0+ #1 > > > NIP: c0055f14 LR: c021ef28 CTR: c0055f14 > > > REGS: c000387af5a0 TRAP: 0300 Tainted: G K > > > (5.19.0+) > > > MSR: 82009033 CR: 88228444 > > > XER: > > > CFAR: c0055e04 DAR: c008021d DSISR: 4200 > > > IRQMASK: 0 > > > GPR00: c021ef28 c000387af840 c2a68a00 > > > c88b3000 > > > GPR04: c00802230084 0026 0036 > > > c008021e0480 > > > GPR08: 7c426214 c0055f14 c0055e08 > > > 0d80 > > > GPR12: c021d9b0 c2d9 c88b3000 > > > c008021f0810 > > > GPR16: c008021c0638 c88b3d80 > > > c1181e38 > > > GPR20: c29dc088 c008021e0480 c008021f0870 > > > aaab > > > GPR24: c88b3c40 c008021d c008021f > > > > > > GPR28: c008021d c008021c0638 > > > 0810 > > > NIP [c0055f14] apply_relocate_add+0x474/0x9e0 > > > LR [c021ef28] klp_apply_section_relocs+0x208/0x2d0 > > > Call Trace: > > > [c000387af840] [c000387af920] 0xc000387af920 > > > (unreliable) > > > [c000387af940] [c021ef28] > > > klp_apply_section_relocs+0x208/0x2d0 > > > [c000387afa30] [c021f080] > > > klp_init_object_loaded+0x90/0x1e0 > > > [c000387afac0] [c02200ac] > > > klp_module_coming+0x3dc/0x5c0 > > > [c000387afb70] [c0231414] load_module+0xf64/0x13a0 > > > [c000387afc90] [c0231b8c] > > > __do_sys_finit_module+0xdc/0x180 > > > [c000387afdb0] [c002f004] > > > system_call_exception+0x164/0x340 > > > [c000387afe10] [c000be68] > > > system_call_vectored_common+0xe8/0x278 > > > --- interrupt: 3000 at 0x7fffb6af4710 > > > NIP: 7fffb6af4710 LR: CTR: > > > REGS: c000387afe80 TRAP: 3000 Tainted: G K > > > (5.19.0+) > > > MSR: 8000f033 CR: > > > 48224244 XER: > > > IRQMASK: 0 > > > GPR00: 0161 7fffe06f5550 7fffb6bf7200 > > > 0005 > > > GPR04: 000105f36ca0 0005 > > > > > > GPR08: > > > > > > GPR12:
Re: [PATCH v5] livepatch: Clear relocation targets on a module removal
On Wed, Aug 31, 2022 at 7:05 PM Joe Lawrence wrote: > > On Wed, Aug 31, 2022 at 03:48:26PM -0700, Song Liu wrote: > > On Wed, Aug 31, 2022 at 3:30 PM Michael Ellerman > > wrote: > > > > > > Joe Lawrence writes: > > > > On Tue, Aug 30, 2022 at 11:53:13AM -0700, Song Liu wrote: > > > >> From: Miroslav Benes > > > >> > > > >> Josh reported a bug: > > > >> > > > >> When the object to be patched is a module, and that module is > > > >> rmmod'ed and reloaded, it fails to load with: > > > >> > > > >> module: x86/modules: Skipping invalid relocation target, existing > > > >> value is nonzero for type 2, loc ba0302e9, val a03e293c > > > >> livepatch: failed to initialize patch 'livepatch_nfsd' for module > > > >> 'nfsd' (-8) > > > >> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing > > > >> to load module 'nfsd' > > > >> > > > >> The livepatch module has a relocation which references a symbol > > > >> in the _previous_ loading of nfsd. When apply_relocate_add() > > > >> tries to replace the old relocation with a new one, it sees that > > > >> the previous one is nonzero and it errors out. > > > >> > > > >> On ppc64le, we have a similar issue: > > > >> > > > >> module_64: livepatch_nfsd: Expected nop after call, got e8410018 at > > > >> e_show+0x60/0x548 [livepatch_nfsd] > > > >> livepatch: failed to initialize patch 'livepatch_nfsd' for module > > > >> 'nfsd' (-8) > > > >> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing > > > >> to load module 'nfsd' > > > ... > > > >> > > > > > > > > Hi Song, > > > > > > > > Applying your patch on top of my latest klp-convert-tree branch [1], I > > > > modified a few of its late module patching tests > > > > (tools/testing/selftests/livepatch/test-song.sh) such that: > > > > > > > > 1 - A livepatch module is loaded > > > >- this module contains klp-relocations to objects in (2) > > > > 2 - A target test module is loaded > > > > 3 - Unload the test target module > > > >- Clear klp-relocations in (1) > > > > 4 - Repeat target module load (2) / unload (3) a few times > > > > 5 - Unload livepatch module > > > > > > If you push that test code somewhere I could test it on ppc64le. > > > > > > > The results: > > > > > > > > x86_64 : pass > > > > s390x : pass > > > > ppc64le : crash > > > > > > > > I suspect Power 32-bit would suffer the same fate, but I don't have > > > > hardware to verify. See the kernel log from the crash below... > > > > > > > > > > > > = TEST: klp-convert symbols (late module patching) = > > > > % modprobe test_klp_convert1 > > > > test_klp_convert1: tainting kernel with TAINT_LIVEPATCH > > > > livepatch: enabling patch 'test_klp_convert1' > > > > livepatch: 'test_klp_convert1': starting patching transition > > > > livepatch: 'test_klp_convert1': patching complete > > > > % modprobe test_klp_convert_mod > > > > livepatch: applying patch 'test_klp_convert1' to loading module > > > > 'test_klp_convert_mod' > > > > test_klp_convert1: saved_command_line, 0: > > > > BOOT_IMAGE=(ieee1275//vdevice/v-scsi@3003/disk@8100,msdos2)/vmlinuz-5.19.0+ > > > > root=/dev/mapper/rhel_ibm--p9z--18--lp7-root ro > > > > crashkernel=2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G > > > > rd.lvm.lv=rhel_ibm-p9z-18-lp7/root rd.lvm.lv=rhel_ibm-p9z-18-lp7/swap > > > > test_klp_convert1: driver_name, 0: test_klp_convert_mod > > > > test_klp_convert1: test_klp_get_driver_name(), 0: test_klp_convert_mod > > > > test_klp_convert1: homonym_string, 1: homonym string A > > > > test_klp_convert1: get_homonym_string(), 1: homonym string A > > > > test_klp_convert1: klp_string.12345 = > > > > lib/livepatch/test_klp_convert_mod_a.c static string > > > > test_klp_convert1: klp_string.67890 = > > > > lib/livepatch/test_klp_convert_mod_b.c static string > > > > % rmmod test_klp_convert_mod > > > > livepatch: reverting patch 'test_klp_convert1' on unloading module > > > > 'test_klp_convert_mod' > > > > module_64: Clearing ADD relocate section 48 to 6 > > > > BUG: Unable to handle kernel data access on write at 0xc00802140150 > > > > Faulting instruction address: 0xc005659c > > > > Oops: Kernel access of bad area, sig: 11 [#1] > > > > LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries > > > > Modules linked in: test_klp_convert_mod(-) test_klp_convert1(K) bonding > > > > tls rfkill pseries_rng drm fuse drm_panel_orientation_quirks xfs > > > > libcrc32c sd_mod t10_pi sg ibmvscsi ibmveth scsi_transport_srp > > > > vmx_crypto dm_mirror dm_region_hash dm_log dm_mod > > > > CPU: 6 PID: 4766 Comm: rmmod Kdump: loaded Tainted: G K > > > > 5.19.0+ #1 > > > > NIP: c005659c LR: c0056590 CTR: 0024 > > > > REGS: c7223840 TRAP: 0300 Tainted: G K > > > > (5.19.0+) > > > > MSR: 80009033 CR: 48008282 XER: > > > > 000a > > > > CFAR: c00a87e0 DAR:
Re: [PATCH v5] livepatch: Clear relocation targets on a module removal
On Thu, Sep 01, 2022 at 01:39:02PM +1000, Michael Ellerman wrote: > Joe Lawrence writes: > > On Thu, Sep 01, 2022 at 08:30:44AM +1000, Michael Ellerman wrote: > >> Joe Lawrence writes: > ... > > > > Hi Michael, > > > > While we're on the topic of klp-relocations and Power, I saw a similar > > access problem when writing (late) relocations into > > .data..ro_after_init. I'm not entirely convinced this should be allowed > > (ie, is it really read-only after .init or ???), but it seems that other > > arches currently allow it ... > > I guess that's because we didn't properly fix apply_relocate_add() in > https://github.com/linuxppc/issues/issues/375 ? > > If other arches allow it then we don't want to be the odd one out :) > > So I guess we need to implement it. > FWIW, I think it this particular relocation is pretty rare, we haven't seen it in real patches nor do we have a kpatch test that generates it. I only hit a crash as I was trying to write a more exhaustive test for the klp-convert implementation. > > = TEST: klp-convert data relocations (late module patching) = > > % modprobe test_klp_convert_data > > livepatch: enabling patch 'test_klp_convert_data' > > livepatch: 'test_klp_convert_data': starting patching transition > > livepatch: 'test_klp_convert_data': patching complete > > % modprobe test_klp_convert_mod > > ... > > module_64: Applying ADD relocate section 54 to 20 > > module_64: RELOC at 8482d02a: 38-type as > > .klp.sym.test_klp_convert_mod.static_ro_after_init,0 (0xc008016d0084) + > > 0 > > BUG: Unable to handle kernel data access on write at 0xc008021d > > Faulting instruction address: 0xc0055f14 > > Oops: Kernel access of bad area, sig: 11 [#1] > > LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries > > Modules linked in: test_klp_convert_mod(+) test_klp_convert_data(K) bonding > > rfkill tls pseries_rng drm fuse drm_panel_orientation_quirks xfs libcrc32c > > sd_mod t10_pi sg ibmvscsi ibmveth scsi_transport_srp vmx_crypto dm_mirror > > dm_region_hash dm_log dm_mod [last unloaded: test_klp_convert_mod] > > CPU: 0 PID: 17089 Comm: modprobe Kdump: loaded Tainted: G K > > 5.19.0+ #1 > > NIP: c0055f14 LR: c021ef28 CTR: c0055f14 > > REGS: c000387af5a0 TRAP: 0300 Tainted: G K(5.19.0+) > > MSR: 82009033 CR: 88228444 XER: > > > > CFAR: c0055e04 DAR: c008021d DSISR: 4200 IRQMASK: 0 > > GPR00: c021ef28 c000387af840 c2a68a00 c88b3000 > > GPR04: c00802230084 0026 0036 c008021e0480 > > GPR08: 7c426214 c0055f14 c0055e08 0d80 > > GPR12: c021d9b0 c2d9 c88b3000 c008021f0810 > > GPR16: c008021c0638 c88b3d80 c1181e38 > > GPR20: c29dc088 c008021e0480 c008021f0870 aaab > > GPR24: c88b3c40 c008021d c008021f > > GPR28: c008021d c008021c0638 0810 > > NIP [c0055f14] apply_relocate_add+0x474/0x9e0 > > LR [c021ef28] klp_apply_section_relocs+0x208/0x2d0 > > Call Trace: > > [c000387af840] [c000387af920] 0xc000387af920 (unreliable) > > [c000387af940] [c021ef28] klp_apply_section_relocs+0x208/0x2d0 > > [c000387afa30] [c021f080] klp_init_object_loaded+0x90/0x1e0 > > [c000387afac0] [c02200ac] klp_module_coming+0x3dc/0x5c0 > > [c000387afb70] [c0231414] load_module+0xf64/0x13a0 > > [c000387afc90] [c0231b8c] __do_sys_finit_module+0xdc/0x180 > > [c000387afdb0] [c002f004] system_call_exception+0x164/0x340 > > [c000387afe10] [c000be68] system_call_vectored_common+0xe8/0x278 > > --- interrupt: 3000 at 0x7fffb6af4710 > > NIP: 7fffb6af4710 LR: CTR: > > REGS: c000387afe80 TRAP: 3000 Tainted: G K(5.19.0+) > > MSR: 8000f033 CR: 48224244 XER: > > > > IRQMASK: 0 > > GPR00: 0161 7fffe06f5550 7fffb6bf7200 0005 > > GPR04: 000105f36ca0 0005 > > GPR08: > > GPR12: 7fffb738c540 0020 > > GPR16: 010024d31de0 000105f37d50 010024d302f8 > > GPR20: 0001 0908 010024d32020 010024d319b0 > > GPR24: 010024d32040 010024d303f0 > > GPR28: 010024d31e00 000105f36ca0 0004 010024d319b0 > > NIP [7fffb6af4710] 0x7fffb6af4710 > > LR [] 0x0 > > --- interrupt: 3000 > > Instruction dump: > > 061c 061c 061c 061c 061c 061c 061c 061c > > 0288 0248 6000 7c992050
Re: [PATCH v5] livepatch: Clear relocation targets on a module removal
Joe Lawrence writes: > On Thu, Sep 01, 2022 at 08:30:44AM +1000, Michael Ellerman wrote: >> Joe Lawrence writes: ... > > Hi Michael, > > While we're on the topic of klp-relocations and Power, I saw a similar > access problem when writing (late) relocations into > .data..ro_after_init. I'm not entirely convinced this should be allowed > (ie, is it really read-only after .init or ???), but it seems that other > arches currently allow it ... I guess that's because we didn't properly fix apply_relocate_add() in https://github.com/linuxppc/issues/issues/375 ? If other arches allow it then we don't want to be the odd one out :) So I guess we need to implement it. > $ ./tools/testing/selftests/livepatch/test-shadow-vars.sh > > = TEST: klp-convert data relocations (late module patching) = > % modprobe test_klp_convert_data > livepatch: enabling patch 'test_klp_convert_data' > livepatch: 'test_klp_convert_data': starting patching transition > livepatch: 'test_klp_convert_data': patching complete > % modprobe test_klp_convert_mod > ... > module_64: Applying ADD relocate section 54 to 20 > module_64: RELOC at 8482d02a: 38-type as > .klp.sym.test_klp_convert_mod.static_ro_after_init,0 (0xc008016d0084) + 0 > BUG: Unable to handle kernel data access on write at 0xc008021d > Faulting instruction address: 0xc0055f14 > Oops: Kernel access of bad area, sig: 11 [#1] > LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries > Modules linked in: test_klp_convert_mod(+) test_klp_convert_data(K) bonding > rfkill tls pseries_rng drm fuse drm_panel_orientation_quirks xfs libcrc32c > sd_mod t10_pi sg ibmvscsi ibmveth scsi_transport_srp vmx_crypto dm_mirror > dm_region_hash dm_log dm_mod [last unloaded: test_klp_convert_mod] > CPU: 0 PID: 17089 Comm: modprobe Kdump: loaded Tainted: G K > 5.19.0+ #1 > NIP: c0055f14 LR: c021ef28 CTR: c0055f14 > REGS: c000387af5a0 TRAP: 0300 Tainted: G K(5.19.0+) > MSR: 82009033 CR: 88228444 XER: > CFAR: c0055e04 DAR: c008021d DSISR: 4200 IRQMASK: 0 > GPR00: c021ef28 c000387af840 c2a68a00 c88b3000 > GPR04: c00802230084 0026 0036 c008021e0480 > GPR08: 7c426214 c0055f14 c0055e08 0d80 > GPR12: c021d9b0 c2d9 c88b3000 c008021f0810 > GPR16: c008021c0638 c88b3d80 c1181e38 > GPR20: c29dc088 c008021e0480 c008021f0870 aaab > GPR24: c88b3c40 c008021d c008021f > GPR28: c008021d c008021c0638 0810 > NIP [c0055f14] apply_relocate_add+0x474/0x9e0 > LR [c021ef28] klp_apply_section_relocs+0x208/0x2d0 > Call Trace: > [c000387af840] [c000387af920] 0xc000387af920 (unreliable) > [c000387af940] [c021ef28] klp_apply_section_relocs+0x208/0x2d0 > [c000387afa30] [c021f080] klp_init_object_loaded+0x90/0x1e0 > [c000387afac0] [c02200ac] klp_module_coming+0x3dc/0x5c0 > [c000387afb70] [c0231414] load_module+0xf64/0x13a0 > [c000387afc90] [c0231b8c] __do_sys_finit_module+0xdc/0x180 > [c000387afdb0] [c002f004] system_call_exception+0x164/0x340 > [c000387afe10] [c000be68] system_call_vectored_common+0xe8/0x278 > --- interrupt: 3000 at 0x7fffb6af4710 > NIP: 7fffb6af4710 LR: CTR: > REGS: c000387afe80 TRAP: 3000 Tainted: G K(5.19.0+) > MSR: 8000f033 CR: 48224244 XER: > > IRQMASK: 0 > GPR00: 0161 7fffe06f5550 7fffb6bf7200 0005 > GPR04: 000105f36ca0 0005 > GPR08: > GPR12: 7fffb738c540 0020 > GPR16: 010024d31de0 000105f37d50 010024d302f8 > GPR20: 0001 0908 010024d32020 010024d319b0 > GPR24: 010024d32040 010024d303f0 > GPR28: 010024d31e00 000105f36ca0 0004 010024d319b0 > NIP [7fffb6af4710] 0x7fffb6af4710 > LR [] 0x0 > --- interrupt: 3000 > Instruction dump: > 061c 061c 061c 061c 061c 061c 061c 061c > 0288 0248 6000 7c992050 <7c9ce92a> 6000 6000 e9310020 > ---[ end trace ]--- > > $ readelf --wide --sections lib/livepatch/test_klp_convert_data.ko | grep -e > '\[20\]' -e '\[54\]' > [20] .data..ro_after_init PROGBITS > 001a58 08 00 WA 0 0 8 > [54] .klp.rela.test_klp_convert_mod..data..ro_after_init RELA > 0426e8 18 18 Ao 49 20 8
Re: [PATCH v5] livepatch: Clear relocation targets on a module removal
On Thu, Sep 01, 2022 at 08:30:44AM +1000, Michael Ellerman wrote: > Joe Lawrence writes: > > On Tue, Aug 30, 2022 at 11:53:13AM -0700, Song Liu wrote: > >> From: Miroslav Benes > >> > >> Josh reported a bug: > >> > >> When the object to be patched is a module, and that module is > >> rmmod'ed and reloaded, it fails to load with: > >> > >> module: x86/modules: Skipping invalid relocation target, existing value > >> is nonzero for type 2, loc ba0302e9, val a03e293c > >> livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd' > >> (-8) > >> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to > >> load module 'nfsd' > >> > >> The livepatch module has a relocation which references a symbol > >> in the _previous_ loading of nfsd. When apply_relocate_add() > >> tries to replace the old relocation with a new one, it sees that > >> the previous one is nonzero and it errors out. > >> > >> On ppc64le, we have a similar issue: > >> > >> module_64: livepatch_nfsd: Expected nop after call, got e8410018 at > >> e_show+0x60/0x548 [livepatch_nfsd] > >> livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd' > >> (-8) > >> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to > >> load module 'nfsd' > ... > >> > > > > Hi Song, > > > > Applying your patch on top of my latest klp-convert-tree branch [1], I > > modified a few of its late module patching tests > > (tools/testing/selftests/livepatch/test-song.sh) such that: > > > > 1 - A livepatch module is loaded > >- this module contains klp-relocations to objects in (2) > > 2 - A target test module is loaded > > 3 - Unload the test target module > >- Clear klp-relocations in (1) > > 4 - Repeat target module load (2) / unload (3) a few times > > 5 - Unload livepatch module > > If you push that test code somewhere I could test it on ppc64le. > > > The results: > > > > x86_64 : pass > > s390x : pass > > ppc64le : crash > > > > I suspect Power 32-bit would suffer the same fate, but I don't have > > hardware to verify. See the kernel log from the crash below... > > > > > > = TEST: klp-convert symbols (late module patching) = > > % modprobe test_klp_convert1 > > test_klp_convert1: tainting kernel with TAINT_LIVEPATCH > > livepatch: enabling patch 'test_klp_convert1' > > livepatch: 'test_klp_convert1': starting patching transition > > livepatch: 'test_klp_convert1': patching complete > > % modprobe test_klp_convert_mod > > livepatch: applying patch 'test_klp_convert1' to loading module > > 'test_klp_convert_mod' > > test_klp_convert1: saved_command_line, 0: > > BOOT_IMAGE=(ieee1275//vdevice/v-scsi@3003/disk@8100,msdos2)/vmlinuz-5.19.0+ > > root=/dev/mapper/rhel_ibm--p9z--18--lp7-root ro > > crashkernel=2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G > > rd.lvm.lv=rhel_ibm-p9z-18-lp7/root rd.lvm.lv=rhel_ibm-p9z-18-lp7/swap > > test_klp_convert1: driver_name, 0: test_klp_convert_mod > > test_klp_convert1: test_klp_get_driver_name(), 0: test_klp_convert_mod > > test_klp_convert1: homonym_string, 1: homonym string A > > test_klp_convert1: get_homonym_string(), 1: homonym string A > > test_klp_convert1: klp_string.12345 = > > lib/livepatch/test_klp_convert_mod_a.c static string > > test_klp_convert1: klp_string.67890 = > > lib/livepatch/test_klp_convert_mod_b.c static string > > % rmmod test_klp_convert_mod > > livepatch: reverting patch 'test_klp_convert1' on unloading module > > 'test_klp_convert_mod' > > module_64: Clearing ADD relocate section 48 to 6 > > BUG: Unable to handle kernel data access on write at 0xc00802140150 > > Faulting instruction address: 0xc005659c > > Oops: Kernel access of bad area, sig: 11 [#1] > > LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries > > Modules linked in: test_klp_convert_mod(-) test_klp_convert1(K) bonding tls > > rfkill pseries_rng drm fuse drm_panel_orientation_quirks xfs libcrc32c > > sd_mod t10_pi sg ibmvscsi ibmveth scsi_transport_srp vmx_crypto dm_mirror > > dm_region_hash dm_log dm_mod > > CPU: 6 PID: 4766 Comm: rmmod Kdump: loaded Tainted: G K > > 5.19.0+ #1 > > NIP: c005659c LR: c0056590 CTR: 0024 > > REGS: c7223840 TRAP: 0300 Tainted: G K(5.19.0+) > > MSR: 80009033 CR: 48008282 XER: 000a > > CFAR: c00a87e0 DAR: c00802140150 DSISR: 0a00 IRQMASK: 0 > > This is saying you don't have permissions to write at that address. > > > GPR00: c0056568 c7223ae0 c2a68a00 0001 > > GPR04: c008021706f0 002d > > GPR08: 0066 00120010 8000 > > GPR12: cffca080 > > GPR16: 010005bf1810 00010c0f7370 c11b7e50 c11b7e68 > > GPR20:
Re: [PATCH v5] livepatch: Clear relocation targets on a module removal
On Wed, Aug 31, 2022 at 03:48:26PM -0700, Song Liu wrote: > On Wed, Aug 31, 2022 at 3:30 PM Michael Ellerman wrote: > > > > Joe Lawrence writes: > > > On Tue, Aug 30, 2022 at 11:53:13AM -0700, Song Liu wrote: > > >> From: Miroslav Benes > > >> > > >> Josh reported a bug: > > >> > > >> When the object to be patched is a module, and that module is > > >> rmmod'ed and reloaded, it fails to load with: > > >> > > >> module: x86/modules: Skipping invalid relocation target, existing > > >> value is nonzero for type 2, loc ba0302e9, val a03e293c > > >> livepatch: failed to initialize patch 'livepatch_nfsd' for module > > >> 'nfsd' (-8) > > >> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing > > >> to load module 'nfsd' > > >> > > >> The livepatch module has a relocation which references a symbol > > >> in the _previous_ loading of nfsd. When apply_relocate_add() > > >> tries to replace the old relocation with a new one, it sees that > > >> the previous one is nonzero and it errors out. > > >> > > >> On ppc64le, we have a similar issue: > > >> > > >> module_64: livepatch_nfsd: Expected nop after call, got e8410018 at > > >> e_show+0x60/0x548 [livepatch_nfsd] > > >> livepatch: failed to initialize patch 'livepatch_nfsd' for module > > >> 'nfsd' (-8) > > >> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing > > >> to load module 'nfsd' > > ... > > >> > > > > > > Hi Song, > > > > > > Applying your patch on top of my latest klp-convert-tree branch [1], I > > > modified a few of its late module patching tests > > > (tools/testing/selftests/livepatch/test-song.sh) such that: > > > > > > 1 - A livepatch module is loaded > > >- this module contains klp-relocations to objects in (2) > > > 2 - A target test module is loaded > > > 3 - Unload the test target module > > >- Clear klp-relocations in (1) > > > 4 - Repeat target module load (2) / unload (3) a few times > > > 5 - Unload livepatch module > > > > If you push that test code somewhere I could test it on ppc64le. > > > > > The results: > > > > > > x86_64 : pass > > > s390x : pass > > > ppc64le : crash > > > > > > I suspect Power 32-bit would suffer the same fate, but I don't have > > > hardware to verify. See the kernel log from the crash below... > > > > > > > > > = TEST: klp-convert symbols (late module patching) = > > > % modprobe test_klp_convert1 > > > test_klp_convert1: tainting kernel with TAINT_LIVEPATCH > > > livepatch: enabling patch 'test_klp_convert1' > > > livepatch: 'test_klp_convert1': starting patching transition > > > livepatch: 'test_klp_convert1': patching complete > > > % modprobe test_klp_convert_mod > > > livepatch: applying patch 'test_klp_convert1' to loading module > > > 'test_klp_convert_mod' > > > test_klp_convert1: saved_command_line, 0: > > > BOOT_IMAGE=(ieee1275//vdevice/v-scsi@3003/disk@8100,msdos2)/vmlinuz-5.19.0+ > > > root=/dev/mapper/rhel_ibm--p9z--18--lp7-root ro > > > crashkernel=2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G > > > rd.lvm.lv=rhel_ibm-p9z-18-lp7/root rd.lvm.lv=rhel_ibm-p9z-18-lp7/swap > > > test_klp_convert1: driver_name, 0: test_klp_convert_mod > > > test_klp_convert1: test_klp_get_driver_name(), 0: test_klp_convert_mod > > > test_klp_convert1: homonym_string, 1: homonym string A > > > test_klp_convert1: get_homonym_string(), 1: homonym string A > > > test_klp_convert1: klp_string.12345 = > > > lib/livepatch/test_klp_convert_mod_a.c static string > > > test_klp_convert1: klp_string.67890 = > > > lib/livepatch/test_klp_convert_mod_b.c static string > > > % rmmod test_klp_convert_mod > > > livepatch: reverting patch 'test_klp_convert1' on unloading module > > > 'test_klp_convert_mod' > > > module_64: Clearing ADD relocate section 48 to 6 > > > BUG: Unable to handle kernel data access on write at 0xc00802140150 > > > Faulting instruction address: 0xc005659c > > > Oops: Kernel access of bad area, sig: 11 [#1] > > > LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries > > > Modules linked in: test_klp_convert_mod(-) test_klp_convert1(K) bonding > > > tls rfkill pseries_rng drm fuse drm_panel_orientation_quirks xfs > > > libcrc32c sd_mod t10_pi sg ibmvscsi ibmveth scsi_transport_srp vmx_crypto > > > dm_mirror dm_region_hash dm_log dm_mod > > > CPU: 6 PID: 4766 Comm: rmmod Kdump: loaded Tainted: G K > > > 5.19.0+ #1 > > > NIP: c005659c LR: c0056590 CTR: 0024 > > > REGS: c7223840 TRAP: 0300 Tainted: G K(5.19.0+) > > > MSR: 80009033 CR: 48008282 XER: 000a > > > CFAR: c00a87e0 DAR: c00802140150 DSISR: 0a00 IRQMASK: 0 > > > > This is saying you don't have permissions to write at that address. > > > > > GPR00: c0056568 c7223ae0 c2a68a00 0001 > > > GPR04: c008021706f0 002d > > >
Re: [PATCH v5] livepatch: Clear relocation targets on a module removal
On Wed, Aug 31, 2022 at 3:30 PM Michael Ellerman wrote: > > Joe Lawrence writes: > > On Tue, Aug 30, 2022 at 11:53:13AM -0700, Song Liu wrote: > >> From: Miroslav Benes > >> > >> Josh reported a bug: > >> > >> When the object to be patched is a module, and that module is > >> rmmod'ed and reloaded, it fails to load with: > >> > >> module: x86/modules: Skipping invalid relocation target, existing value > >> is nonzero for type 2, loc ba0302e9, val a03e293c > >> livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd' > >> (-8) > >> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to > >> load module 'nfsd' > >> > >> The livepatch module has a relocation which references a symbol > >> in the _previous_ loading of nfsd. When apply_relocate_add() > >> tries to replace the old relocation with a new one, it sees that > >> the previous one is nonzero and it errors out. > >> > >> On ppc64le, we have a similar issue: > >> > >> module_64: livepatch_nfsd: Expected nop after call, got e8410018 at > >> e_show+0x60/0x548 [livepatch_nfsd] > >> livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd' > >> (-8) > >> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to > >> load module 'nfsd' > ... > >> > > > > Hi Song, > > > > Applying your patch on top of my latest klp-convert-tree branch [1], I > > modified a few of its late module patching tests > > (tools/testing/selftests/livepatch/test-song.sh) such that: > > > > 1 - A livepatch module is loaded > >- this module contains klp-relocations to objects in (2) > > 2 - A target test module is loaded > > 3 - Unload the test target module > >- Clear klp-relocations in (1) > > 4 - Repeat target module load (2) / unload (3) a few times > > 5 - Unload livepatch module > > If you push that test code somewhere I could test it on ppc64le. > > > The results: > > > > x86_64 : pass > > s390x : pass > > ppc64le : crash > > > > I suspect Power 32-bit would suffer the same fate, but I don't have > > hardware to verify. See the kernel log from the crash below... > > > > > > = TEST: klp-convert symbols (late module patching) = > > % modprobe test_klp_convert1 > > test_klp_convert1: tainting kernel with TAINT_LIVEPATCH > > livepatch: enabling patch 'test_klp_convert1' > > livepatch: 'test_klp_convert1': starting patching transition > > livepatch: 'test_klp_convert1': patching complete > > % modprobe test_klp_convert_mod > > livepatch: applying patch 'test_klp_convert1' to loading module > > 'test_klp_convert_mod' > > test_klp_convert1: saved_command_line, 0: > > BOOT_IMAGE=(ieee1275//vdevice/v-scsi@3003/disk@8100,msdos2)/vmlinuz-5.19.0+ > > root=/dev/mapper/rhel_ibm--p9z--18--lp7-root ro > > crashkernel=2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G > > rd.lvm.lv=rhel_ibm-p9z-18-lp7/root rd.lvm.lv=rhel_ibm-p9z-18-lp7/swap > > test_klp_convert1: driver_name, 0: test_klp_convert_mod > > test_klp_convert1: test_klp_get_driver_name(), 0: test_klp_convert_mod > > test_klp_convert1: homonym_string, 1: homonym string A > > test_klp_convert1: get_homonym_string(), 1: homonym string A > > test_klp_convert1: klp_string.12345 = > > lib/livepatch/test_klp_convert_mod_a.c static string > > test_klp_convert1: klp_string.67890 = > > lib/livepatch/test_klp_convert_mod_b.c static string > > % rmmod test_klp_convert_mod > > livepatch: reverting patch 'test_klp_convert1' on unloading module > > 'test_klp_convert_mod' > > module_64: Clearing ADD relocate section 48 to 6 > > BUG: Unable to handle kernel data access on write at 0xc00802140150 > > Faulting instruction address: 0xc005659c > > Oops: Kernel access of bad area, sig: 11 [#1] > > LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries > > Modules linked in: test_klp_convert_mod(-) test_klp_convert1(K) bonding tls > > rfkill pseries_rng drm fuse drm_panel_orientation_quirks xfs libcrc32c > > sd_mod t10_pi sg ibmvscsi ibmveth scsi_transport_srp vmx_crypto dm_mirror > > dm_region_hash dm_log dm_mod > > CPU: 6 PID: 4766 Comm: rmmod Kdump: loaded Tainted: G K > > 5.19.0+ #1 > > NIP: c005659c LR: c0056590 CTR: 0024 > > REGS: c7223840 TRAP: 0300 Tainted: G K(5.19.0+) > > MSR: 80009033 CR: 48008282 XER: 000a > > CFAR: c00a87e0 DAR: c00802140150 DSISR: 0a00 IRQMASK: 0 > > This is saying you don't have permissions to write at that address. > > > GPR00: c0056568 c7223ae0 c2a68a00 0001 > > GPR04: c008021706f0 002d > > GPR08: 0066 00120010 8000 > > GPR12: cffca080 > > GPR16: 010005bf1810 00010c0f7370 c11b7e50 c11b7e68 > > GPR20: c008021501c8
Re: [PATCH v5] livepatch: Clear relocation targets on a module removal
Joe Lawrence writes: > On Tue, Aug 30, 2022 at 11:53:13AM -0700, Song Liu wrote: >> From: Miroslav Benes >> >> Josh reported a bug: >> >> When the object to be patched is a module, and that module is >> rmmod'ed and reloaded, it fails to load with: >> >> module: x86/modules: Skipping invalid relocation target, existing value is >> nonzero for type 2, loc ba0302e9, val a03e293c >> livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd' >> (-8) >> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to >> load module 'nfsd' >> >> The livepatch module has a relocation which references a symbol >> in the _previous_ loading of nfsd. When apply_relocate_add() >> tries to replace the old relocation with a new one, it sees that >> the previous one is nonzero and it errors out. >> >> On ppc64le, we have a similar issue: >> >> module_64: livepatch_nfsd: Expected nop after call, got e8410018 at >> e_show+0x60/0x548 [livepatch_nfsd] >> livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd' >> (-8) >> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to >> load module 'nfsd' ... >> > > Hi Song, > > Applying your patch on top of my latest klp-convert-tree branch [1], I > modified a few of its late module patching tests > (tools/testing/selftests/livepatch/test-song.sh) such that: > > 1 - A livepatch module is loaded >- this module contains klp-relocations to objects in (2) > 2 - A target test module is loaded > 3 - Unload the test target module >- Clear klp-relocations in (1) > 4 - Repeat target module load (2) / unload (3) a few times > 5 - Unload livepatch module If you push that test code somewhere I could test it on ppc64le. > The results: > > x86_64 : pass > s390x : pass > ppc64le : crash > > I suspect Power 32-bit would suffer the same fate, but I don't have > hardware to verify. See the kernel log from the crash below... > > > = TEST: klp-convert symbols (late module patching) = > % modprobe test_klp_convert1 > test_klp_convert1: tainting kernel with TAINT_LIVEPATCH > livepatch: enabling patch 'test_klp_convert1' > livepatch: 'test_klp_convert1': starting patching transition > livepatch: 'test_klp_convert1': patching complete > % modprobe test_klp_convert_mod > livepatch: applying patch 'test_klp_convert1' to loading module > 'test_klp_convert_mod' > test_klp_convert1: saved_command_line, 0: > BOOT_IMAGE=(ieee1275//vdevice/v-scsi@3003/disk@8100,msdos2)/vmlinuz-5.19.0+ > root=/dev/mapper/rhel_ibm--p9z--18--lp7-root ro > crashkernel=2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G > rd.lvm.lv=rhel_ibm-p9z-18-lp7/root rd.lvm.lv=rhel_ibm-p9z-18-lp7/swap > test_klp_convert1: driver_name, 0: test_klp_convert_mod > test_klp_convert1: test_klp_get_driver_name(), 0: test_klp_convert_mod > test_klp_convert1: homonym_string, 1: homonym string A > test_klp_convert1: get_homonym_string(), 1: homonym string A > test_klp_convert1: klp_string.12345 = lib/livepatch/test_klp_convert_mod_a.c > static string > test_klp_convert1: klp_string.67890 = lib/livepatch/test_klp_convert_mod_b.c > static string > % rmmod test_klp_convert_mod > livepatch: reverting patch 'test_klp_convert1' on unloading module > 'test_klp_convert_mod' > module_64: Clearing ADD relocate section 48 to 6 > BUG: Unable to handle kernel data access on write at 0xc00802140150 > Faulting instruction address: 0xc005659c > Oops: Kernel access of bad area, sig: 11 [#1] > LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries > Modules linked in: test_klp_convert_mod(-) test_klp_convert1(K) bonding tls > rfkill pseries_rng drm fuse drm_panel_orientation_quirks xfs libcrc32c sd_mod > t10_pi sg ibmvscsi ibmveth scsi_transport_srp vmx_crypto dm_mirror > dm_region_hash dm_log dm_mod > CPU: 6 PID: 4766 Comm: rmmod Kdump: loaded Tainted: G K > 5.19.0+ #1 > NIP: c005659c LR: c0056590 CTR: 0024 > REGS: c7223840 TRAP: 0300 Tainted: G K(5.19.0+) > MSR: 80009033 CR: 48008282 XER: 000a > CFAR: c00a87e0 DAR: c00802140150 DSISR: 0a00 IRQMASK: 0 This is saying you don't have permissions to write at that address. > GPR00: c0056568 c7223ae0 c2a68a00 0001 > GPR04: c008021706f0 002d > GPR08: 0066 00120010 8000 > GPR12: cffca080 > GPR16: 010005bf1810 00010c0f7370 c11b7e50 c11b7e68 > GPR20: c008021501c8 c00802150228 0030 6000 > GPR24: c00802160380 c00056b43000 ff20 c00056b43c00 > GPR28: aaab c00056b43b40 c0080214014c > NIP [c005659c] clear_relocate_add+0x11c/0x1c0 >
Re: [PATCH v5] livepatch: Clear relocation targets on a module removal
On Wed, Aug 31, 2022 at 10:05:43AM -0700, Song Liu wrote:
> On Wed, Aug 31, 2022 at 1:01 AM Christophe Leroy
> wrote:
> >
> >
> >
> > Le 30/08/2022 à 20:53, Song Liu a écrit :
> > > From: Miroslav Benes
> > >
> > > Josh reported a bug:
> > >
> > >When the object to be patched is a module, and that module is
> > >rmmod'ed and reloaded, it fails to load with:
> > >
> > >module: x86/modules: Skipping invalid relocation target, existing
> > > value is nonzero for type 2, loc ba0302e9, val a03e293c
> > >livepatch: failed to initialize patch 'livepatch_nfsd' for module
> > > 'nfsd' (-8)
> > >livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing
> > > to load module 'nfsd'
> > >
> > >The livepatch module has a relocation which references a symbol
> > >in the _previous_ loading of nfsd. When apply_relocate_add()
> > >tries to replace the old relocation with a new one, it sees that
> > >the previous one is nonzero and it errors out.
> > >
> > >On ppc64le, we have a similar issue:
> > >
> > >module_64: livepatch_nfsd: Expected nop after call, got e8410018 at
> > > e_show+0x60/0x548 [livepatch_nfsd]
> > >livepatch: failed to initialize patch 'livepatch_nfsd' for module
> > > 'nfsd' (-8)
> > >livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing
> > > to load module 'nfsd'
> > >
> > > He also proposed three different solutions. We could remove the error
> > > check in apply_relocate_add() introduced by commit eda9cec4c9a1
> > > ("x86/module: Detect and skip invalid relocations"). However the check
> > > is useful for detecting corrupted modules.
> > >
> > > We could also deny the patched modules to be removed. If it proved to be
> > > a major drawback for users, we could still implement a different
> > > approach. The solution would also complicate the existing code a lot.
> > >
> > > We thus decided to reverse the relocation patching (clear all relocation
> > > targets on x86_64). The solution is not
> > > universal and is too much arch-specific, but it may prove to be simpler
> > > in the end.
> > >
> > > Reported-by: Josh Poimboeuf
> > > Signed-off-by: Miroslav Benes
> > > Signed-off-by: Song Liu
> > >
> > > ---
> > >
> > > NOTE: powerpc code has not be tested.
> > >
> > > Changes v4 = v5:
> > > 1. Fix compile with powerpc.
> >
> > Not completely it seems.
> >
> >CC kernel/livepatch/core.o
> > kernel/livepatch/core.c: In function 'klp_clear_object_relocations':
> > kernel/livepatch/core.c:352:50: error: passing argument 1 of
> > 'clear_relocate_add' from incompatible pointer type
> > [-Werror=incompatible-pointer-types]
> >352 | clear_relocate_add(pmod->klp_info->sechdrs,
> >|~~^
> >| |
> >| Elf32_Shdr *
> > {aka struct elf32_shdr *}
> > In file included from kernel/livepatch/core.c:19:
> > ./include/linux/moduleloader.h:76:37: note: expected 'Elf64_Shdr *' {aka
> > 'struct elf64_shdr *'} but argument is of type 'Elf32_Shdr *' {aka
> > 'struct elf32_shdr *'}
> > 76 | void clear_relocate_add(Elf64_Shdr *sechdrs,
> >| ^~~
> > cc1: some warnings being treated as errors
> >
> >
> > Fixup:
> >
> > diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
> > index d22b36b84b4b..958e6da7f475 100644
> > --- a/include/linux/moduleloader.h
> > +++ b/include/linux/moduleloader.h
> > @@ -73,7 +73,7 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
> >unsigned int relsec,
> >struct module *mod);
> > #ifdef CONFIG_LIVEPATCH
> > -void clear_relocate_add(Elf64_Shdr *sechdrs,
> > +void clear_relocate_add(Elf_Shdr *sechdrs,
> >const char *strtab,
> >unsigned int symindex,
> >unsigned int relsec,
> >
> >
> > But then the link fails.
> >
> >LD .tmp_vmlinux.kallsyms1
> > powerpc64-linux-ld: kernel/livepatch/core.o: in function
> > `klp_cleanup_module_patches_limited':
> > core.c:(.text+0xdb4): undefined reference to `clear_relocate_add'
>
> Hmm.. I am not seeing either error. Could you please share your .config file?
>
> Thanks,
> Song
>
If it's any help, I see the same build error Christophe reported using
the 'cross-dev' script that's in my klp-convert-tree [1].
$ BUILD_ARCHES="ppc32" ./cross-dev config
$ BUILD_ARCHES="ppc32" ./cross-dev build -j$(nproc)
(The kernel will be built in /tmp/klp-convert-ppc32 btw.)
Applying the header file fix results in the same linker error, too.
[1]
https://github.com/joe-lawrence/klp-convert-tree/tree/klp-convert-v7-devel+song
-- Joe
Re: [PATCH v5] livepatch: Clear relocation targets on a module removal
On Tue, Aug 30, 2022 at 11:53:13AM -0700, Song Liu wrote:
> From: Miroslav Benes
>
> Josh reported a bug:
>
> When the object to be patched is a module, and that module is
> rmmod'ed and reloaded, it fails to load with:
>
> module: x86/modules: Skipping invalid relocation target, existing value is
> nonzero for type 2, loc ba0302e9, val a03e293c
> livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd'
> (-8)
> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to
> load module 'nfsd'
>
> The livepatch module has a relocation which references a symbol
> in the _previous_ loading of nfsd. When apply_relocate_add()
> tries to replace the old relocation with a new one, it sees that
> the previous one is nonzero and it errors out.
>
> On ppc64le, we have a similar issue:
>
> module_64: livepatch_nfsd: Expected nop after call, got e8410018 at
> e_show+0x60/0x548 [livepatch_nfsd]
> livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd'
> (-8)
> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to
> load module 'nfsd'
>
> He also proposed three different solutions. We could remove the error
> check in apply_relocate_add() introduced by commit eda9cec4c9a1
> ("x86/module: Detect and skip invalid relocations"). However the check
> is useful for detecting corrupted modules.
>
> We could also deny the patched modules to be removed. If it proved to be
> a major drawback for users, we could still implement a different
> approach. The solution would also complicate the existing code a lot.
>
> We thus decided to reverse the relocation patching (clear all relocation
> targets on x86_64). The solution is not
> universal and is too much arch-specific, but it may prove to be simpler
> in the end.
>
> Reported-by: Josh Poimboeuf
> Signed-off-by: Miroslav Benes
> Signed-off-by: Song Liu
>
> ---
>
> NOTE: powerpc code has not be tested.
>
> Changes v4 = v5:
> 1. Fix compile with powerpc.
>
> Changes v3 = v4:
> 1. Reuse __apply_relocate_add to make it more reliable in long term.
>(Josh Poimboeuf)
> 2. Add back ppc64 logic from v2, with changes to match current code.
>(Josh Poimboeuf)
>
> Changes v2 => v3:
> 1. Rewrite x86 changes to match current code style.
> 2. Remove powerpc changes as there is no test coverage in v3.
> 3. Only keep 1/3 of v2.
>
> v2: https://lore.kernel.org/all/20190905124514.8944-1-mbe...@suse.cz/T/#u
> ---
> arch/powerpc/kernel/module_64.c | 49 +++
> arch/s390/kernel/module.c | 8 +++
> arch/x86/kernel/module.c| 102 +++-
> include/linux/moduleloader.h| 7 +++
> kernel/livepatch/core.c | 41 -
> 5 files changed, 179 insertions(+), 28 deletions(-)
>
> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> index 7e45dc98df8a..6aaf5720070d 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -739,6 +739,55 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
> return 0;
> }
>
> +#ifdef CONFIG_LIVEPATCH
> +void clear_relocate_add(Elf64_Shdr *sechdrs,
> +const char *strtab,
> +unsigned int symindex,
> +unsigned int relsec,
> +struct module *me)
> +{
> + unsigned int i;
> + Elf64_Rela *rela = (void *)sechdrs[relsec].sh_addr;
> + Elf64_Sym *sym;
> + unsigned long *location;
> + const char *symname;
> + u32 *instruction;
> +
> + pr_debug("Clearing ADD relocate section %u to %u\n", relsec,
> + sechdrs[relsec].sh_info);
> +
> + for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rela); i++) {
> + location = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
> + + rela[i].r_offset;
> + sym = (Elf64_Sym *)sechdrs[symindex].sh_addr
> + + ELF64_R_SYM(rela[i].r_info);
> + symname = me->core_kallsyms.strtab
> + + sym->st_name;
> +
> + if (ELF64_R_TYPE(rela[i].r_info) != R_PPC_REL24)
> + continue;
> + /*
> + * reverse the operations in apply_relocate_add() for case
> + * R_PPC_REL24.
> + */
> + if (sym->st_shndx != SHN_UNDEF &&
> + sym->st_shndx != SHN_LIVEPATCH)
> + continue;
> +
> + instruction = (u32 *)location;
> + if (is_mprofile_ftrace_call(symname))
> + continue;
> +
> + if (!instr_is_relative_link_branch(ppc_inst(*instruction)))
> + continue;
> +
> + instruction += 1;
> + *instruction = PPC_RAW_NOP();
> + }
> +
> +}
> +#endif
> +
> #ifdef CONFIG_DYNAMIC_FTRACE
> int module_trampoline_target(struct module *mod, unsigned long addr,
>unsigned
Re: [PATCH v5] livepatch: Clear relocation targets on a module removal
Le 31/08/2022 à 19:05, Song Liu a écrit :
> On Wed, Aug 31, 2022 at 1:01 AM Christophe Leroy
> wrote:
>>
>>
>>
>> Le 30/08/2022 à 20:53, Song Liu a écrit :
>>> From: Miroslav Benes
>>>
>>> Josh reported a bug:
>>>
>>> When the object to be patched is a module, and that module is
>>> rmmod'ed and reloaded, it fails to load with:
>>>
>>> module: x86/modules: Skipping invalid relocation target, existing value
>>> is nonzero for type 2, loc ba0302e9, val a03e293c
>>> livepatch: failed to initialize patch 'livepatch_nfsd' for module
>>> 'nfsd' (-8)
>>> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to
>>> load module 'nfsd'
>>>
>>> The livepatch module has a relocation which references a symbol
>>> in the _previous_ loading of nfsd. When apply_relocate_add()
>>> tries to replace the old relocation with a new one, it sees that
>>> the previous one is nonzero and it errors out.
>>>
>>> On ppc64le, we have a similar issue:
>>>
>>> module_64: livepatch_nfsd: Expected nop after call, got e8410018 at
>>> e_show+0x60/0x548 [livepatch_nfsd]
>>> livepatch: failed to initialize patch 'livepatch_nfsd' for module
>>> 'nfsd' (-8)
>>> livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to
>>> load module 'nfsd'
>>>
>>> He also proposed three different solutions. We could remove the error
>>> check in apply_relocate_add() introduced by commit eda9cec4c9a1
>>> ("x86/module: Detect and skip invalid relocations"). However the check
>>> is useful for detecting corrupted modules.
>>>
>>> We could also deny the patched modules to be removed. If it proved to be
>>> a major drawback for users, we could still implement a different
>>> approach. The solution would also complicate the existing code a lot.
>>>
>>> We thus decided to reverse the relocation patching (clear all relocation
>>> targets on x86_64). The solution is not
>>> universal and is too much arch-specific, but it may prove to be simpler
>>> in the end.
>>>
>>> Reported-by: Josh Poimboeuf
>>> Signed-off-by: Miroslav Benes
>>> Signed-off-by: Song Liu
>>>
>>> ---
>>>
>>> NOTE: powerpc code has not be tested.
>>>
>>> Changes v4 = v5:
>>> 1. Fix compile with powerpc.
>>
>> Not completely it seems.
>>
>> CC kernel/livepatch/core.o
>> kernel/livepatch/core.c: In function 'klp_clear_object_relocations':
>> kernel/livepatch/core.c:352:50: error: passing argument 1 of
>> 'clear_relocate_add' from incompatible pointer type
>> [-Werror=incompatible-pointer-types]
>> 352 | clear_relocate_add(pmod->klp_info->sechdrs,
>> |~~^
>> | |
>> | Elf32_Shdr *
>> {aka struct elf32_shdr *}
>> In file included from kernel/livepatch/core.c:19:
>> ./include/linux/moduleloader.h:76:37: note: expected 'Elf64_Shdr *' {aka
>> 'struct elf64_shdr *'} but argument is of type 'Elf32_Shdr *' {aka
>> 'struct elf32_shdr *'}
>> 76 | void clear_relocate_add(Elf64_Shdr *sechdrs,
>> | ^~~
>> cc1: some warnings being treated as errors
>>
>>
>> Fixup:
>>
>> diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
>> index d22b36b84b4b..958e6da7f475 100644
>> --- a/include/linux/moduleloader.h
>> +++ b/include/linux/moduleloader.h
>> @@ -73,7 +73,7 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
>> unsigned int relsec,
>> struct module *mod);
>>#ifdef CONFIG_LIVEPATCH
>> -void clear_relocate_add(Elf64_Shdr *sechdrs,
>> +void clear_relocate_add(Elf_Shdr *sechdrs,
>> const char *strtab,
>> unsigned int symindex,
>> unsigned int relsec,
>>
>>
>> But then the link fails.
>>
>> LD .tmp_vmlinux.kallsyms1
>> powerpc64-linux-ld: kernel/livepatch/core.o: in function
>> `klp_cleanup_module_patches_limited':
>> core.c:(.text+0xdb4): undefined reference to `clear_relocate_add'
>
> Hmm.. I am not seeing either error. Could you please share your .config file?
>
defconfig follows:
# CONFIG_LOCALVERSION_AUTO is not set
CONFIG_SYSVIPC=y
CONFIG_POSIX_MQUEUE=y
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_IKCONFIG=y
CONFIG_IKCONFIG_PROC=y
CONFIG_LOG_BUF_SHIFT=14
CONFIG_BLK_DEV_INITRD=y
CONFIG_KALLSYMS_ALL=y
CONFIG_PROFILING=y
CONFIG_ALTIVEC=y
# CONFIG_PPC_CHRP is not set
CONFIG_CPU_FREQ=y
CONFIG_CPU_FREQ_GOV_POWERSAVE=y
CONFIG_CPU_FREQ_GOV_USERSPACE=y
CONFIG_CPU_FREQ_PMAC=y
CONFIG_GEN_RTC=y
CONFIG_HIGHMEM=y
CONFIG_HIBERNATION=y
CONFIG_PM_DEBUG=y
CONFIG_APM_EMULATION=y
CONFIG_LIVEPATCH=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
CONFIG_MODULE_FORCE_UNLOAD=y
CONFIG_PARTITION_ADVANCED=y
CONFIG_BINFMT_MISC=m
# CONFIG_COMPAT_BRK is not set
CONFIG_NET=y
CONFIG_PACKET=y
CONFIG_UNIX=y
CONFIG_XFRM_USER=y
CONFIG_NET_KEY=y
CONFIG_INET=y
Re: [PATCH v5] livepatch: Clear relocation targets on a module removal
On Wed, Aug 31, 2022 at 1:01 AM Christophe Leroy
wrote:
>
>
>
> Le 30/08/2022 à 20:53, Song Liu a écrit :
> > From: Miroslav Benes
> >
> > Josh reported a bug:
> >
> >When the object to be patched is a module, and that module is
> >rmmod'ed and reloaded, it fails to load with:
> >
> >module: x86/modules: Skipping invalid relocation target, existing value
> > is nonzero for type 2, loc ba0302e9, val a03e293c
> >livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd'
> > (-8)
> >livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to
> > load module 'nfsd'
> >
> >The livepatch module has a relocation which references a symbol
> >in the _previous_ loading of nfsd. When apply_relocate_add()
> >tries to replace the old relocation with a new one, it sees that
> >the previous one is nonzero and it errors out.
> >
> >On ppc64le, we have a similar issue:
> >
> >module_64: livepatch_nfsd: Expected nop after call, got e8410018 at
> > e_show+0x60/0x548 [livepatch_nfsd]
> >livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd'
> > (-8)
> >livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to
> > load module 'nfsd'
> >
> > He also proposed three different solutions. We could remove the error
> > check in apply_relocate_add() introduced by commit eda9cec4c9a1
> > ("x86/module: Detect and skip invalid relocations"). However the check
> > is useful for detecting corrupted modules.
> >
> > We could also deny the patched modules to be removed. If it proved to be
> > a major drawback for users, we could still implement a different
> > approach. The solution would also complicate the existing code a lot.
> >
> > We thus decided to reverse the relocation patching (clear all relocation
> > targets on x86_64). The solution is not
> > universal and is too much arch-specific, but it may prove to be simpler
> > in the end.
> >
> > Reported-by: Josh Poimboeuf
> > Signed-off-by: Miroslav Benes
> > Signed-off-by: Song Liu
> >
> > ---
> >
> > NOTE: powerpc code has not be tested.
> >
> > Changes v4 = v5:
> > 1. Fix compile with powerpc.
>
> Not completely it seems.
>
>CC kernel/livepatch/core.o
> kernel/livepatch/core.c: In function 'klp_clear_object_relocations':
> kernel/livepatch/core.c:352:50: error: passing argument 1 of
> 'clear_relocate_add' from incompatible pointer type
> [-Werror=incompatible-pointer-types]
>352 | clear_relocate_add(pmod->klp_info->sechdrs,
>|~~^
>| |
>| Elf32_Shdr *
> {aka struct elf32_shdr *}
> In file included from kernel/livepatch/core.c:19:
> ./include/linux/moduleloader.h:76:37: note: expected 'Elf64_Shdr *' {aka
> 'struct elf64_shdr *'} but argument is of type 'Elf32_Shdr *' {aka
> 'struct elf32_shdr *'}
> 76 | void clear_relocate_add(Elf64_Shdr *sechdrs,
>| ^~~
> cc1: some warnings being treated as errors
>
>
> Fixup:
>
> diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
> index d22b36b84b4b..958e6da7f475 100644
> --- a/include/linux/moduleloader.h
> +++ b/include/linux/moduleloader.h
> @@ -73,7 +73,7 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
>unsigned int relsec,
>struct module *mod);
> #ifdef CONFIG_LIVEPATCH
> -void clear_relocate_add(Elf64_Shdr *sechdrs,
> +void clear_relocate_add(Elf_Shdr *sechdrs,
>const char *strtab,
>unsigned int symindex,
>unsigned int relsec,
>
>
> But then the link fails.
>
>LD .tmp_vmlinux.kallsyms1
> powerpc64-linux-ld: kernel/livepatch/core.o: in function
> `klp_cleanup_module_patches_limited':
> core.c:(.text+0xdb4): undefined reference to `clear_relocate_add'
Hmm.. I am not seeing either error. Could you please share your .config file?
Thanks,
Song
Re: [PATCH v5] livepatch: Clear relocation targets on a module removal
Le 30/08/2022 à 20:53, Song Liu a écrit :
> From: Miroslav Benes
>
> Josh reported a bug:
>
>When the object to be patched is a module, and that module is
>rmmod'ed and reloaded, it fails to load with:
>
>module: x86/modules: Skipping invalid relocation target, existing value is
> nonzero for type 2, loc ba0302e9, val a03e293c
>livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd'
> (-8)
>livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to
> load module 'nfsd'
>
>The livepatch module has a relocation which references a symbol
>in the _previous_ loading of nfsd. When apply_relocate_add()
>tries to replace the old relocation with a new one, it sees that
>the previous one is nonzero and it errors out.
>
>On ppc64le, we have a similar issue:
>
>module_64: livepatch_nfsd: Expected nop after call, got e8410018 at
> e_show+0x60/0x548 [livepatch_nfsd]
>livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd'
> (-8)
>livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to
> load module 'nfsd'
>
> He also proposed three different solutions. We could remove the error
> check in apply_relocate_add() introduced by commit eda9cec4c9a1
> ("x86/module: Detect and skip invalid relocations"). However the check
> is useful for detecting corrupted modules.
>
> We could also deny the patched modules to be removed. If it proved to be
> a major drawback for users, we could still implement a different
> approach. The solution would also complicate the existing code a lot.
>
> We thus decided to reverse the relocation patching (clear all relocation
> targets on x86_64). The solution is not
> universal and is too much arch-specific, but it may prove to be simpler
> in the end.
>
> Reported-by: Josh Poimboeuf
> Signed-off-by: Miroslav Benes
> Signed-off-by: Song Liu
>
> ---
>
> NOTE: powerpc code has not be tested.
>
> Changes v4 = v5:
> 1. Fix compile with powerpc.
Not completely it seems.
CC kernel/livepatch/core.o
kernel/livepatch/core.c: In function 'klp_clear_object_relocations':
kernel/livepatch/core.c:352:50: error: passing argument 1 of
'clear_relocate_add' from incompatible pointer type
[-Werror=incompatible-pointer-types]
352 | clear_relocate_add(pmod->klp_info->sechdrs,
|~~^
| |
| Elf32_Shdr *
{aka struct elf32_shdr *}
In file included from kernel/livepatch/core.c:19:
./include/linux/moduleloader.h:76:37: note: expected 'Elf64_Shdr *' {aka
'struct elf64_shdr *'} but argument is of type 'Elf32_Shdr *' {aka
'struct elf32_shdr *'}
76 | void clear_relocate_add(Elf64_Shdr *sechdrs,
| ^~~
cc1: some warnings being treated as errors
Fixup:
diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
index d22b36b84b4b..958e6da7f475 100644
--- a/include/linux/moduleloader.h
+++ b/include/linux/moduleloader.h
@@ -73,7 +73,7 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
unsigned int relsec,
struct module *mod);
#ifdef CONFIG_LIVEPATCH
-void clear_relocate_add(Elf64_Shdr *sechdrs,
+void clear_relocate_add(Elf_Shdr *sechdrs,
const char *strtab,
unsigned int symindex,
unsigned int relsec,
But then the link fails.
LD .tmp_vmlinux.kallsyms1
powerpc64-linux-ld: kernel/livepatch/core.o: in function
`klp_cleanup_module_patches_limited':
core.c:(.text+0xdb4): undefined reference to `clear_relocate_add'
Christophe
>
> Changes v3 = v4:
> 1. Reuse __apply_relocate_add to make it more reliable in long term.
> (Josh Poimboeuf)
> 2. Add back ppc64 logic from v2, with changes to match current code.
> (Josh Poimboeuf)
>
> Changes v2 => v3:
> 1. Rewrite x86 changes to match current code style.
> 2. Remove powerpc changes as there is no test coverage in v3.
> 3. Only keep 1/3 of v2.
>
> v2: https://lore.kernel.org/all/20190905124514.8944-1-mbe...@suse.cz/T/#u
> ---
> arch/powerpc/kernel/module_64.c | 49 +++
> arch/s390/kernel/module.c | 8 +++
> arch/x86/kernel/module.c| 102 +++-
> include/linux/moduleloader.h| 7 +++
> kernel/livepatch/core.c | 41 -
> 5 files changed, 179 insertions(+), 28 deletions(-)
>
> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> index 7e45dc98df8a..6aaf5720070d 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -739,6 +739,55 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
> return 0;
> }
>
> +#ifdef CONFIG_LIVEPATCH
> +void clear_relocate_add(Elf64_Shdr *sechdrs,
> +
[PATCH v5] livepatch: Clear relocation targets on a module removal
From: Miroslav Benes
Josh reported a bug:
When the object to be patched is a module, and that module is
rmmod'ed and reloaded, it fails to load with:
module: x86/modules: Skipping invalid relocation target, existing value is
nonzero for type 2, loc ba0302e9, val a03e293c
livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd' (-8)
livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to load
module 'nfsd'
The livepatch module has a relocation which references a symbol
in the _previous_ loading of nfsd. When apply_relocate_add()
tries to replace the old relocation with a new one, it sees that
the previous one is nonzero and it errors out.
On ppc64le, we have a similar issue:
module_64: livepatch_nfsd: Expected nop after call, got e8410018 at
e_show+0x60/0x548 [livepatch_nfsd]
livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd' (-8)
livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to load
module 'nfsd'
He also proposed three different solutions. We could remove the error
check in apply_relocate_add() introduced by commit eda9cec4c9a1
("x86/module: Detect and skip invalid relocations"). However the check
is useful for detecting corrupted modules.
We could also deny the patched modules to be removed. If it proved to be
a major drawback for users, we could still implement a different
approach. The solution would also complicate the existing code a lot.
We thus decided to reverse the relocation patching (clear all relocation
targets on x86_64). The solution is not
universal and is too much arch-specific, but it may prove to be simpler
in the end.
Reported-by: Josh Poimboeuf
Signed-off-by: Miroslav Benes
Signed-off-by: Song Liu
---
NOTE: powerpc code has not be tested.
Changes v4 = v5:
1. Fix compile with powerpc.
Changes v3 = v4:
1. Reuse __apply_relocate_add to make it more reliable in long term.
(Josh Poimboeuf)
2. Add back ppc64 logic from v2, with changes to match current code.
(Josh Poimboeuf)
Changes v2 => v3:
1. Rewrite x86 changes to match current code style.
2. Remove powerpc changes as there is no test coverage in v3.
3. Only keep 1/3 of v2.
v2: https://lore.kernel.org/all/20190905124514.8944-1-mbe...@suse.cz/T/#u
---
arch/powerpc/kernel/module_64.c | 49 +++
arch/s390/kernel/module.c | 8 +++
arch/x86/kernel/module.c| 102 +++-
include/linux/moduleloader.h| 7 +++
kernel/livepatch/core.c | 41 -
5 files changed, 179 insertions(+), 28 deletions(-)
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 7e45dc98df8a..6aaf5720070d 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -739,6 +739,55 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
return 0;
}
+#ifdef CONFIG_LIVEPATCH
+void clear_relocate_add(Elf64_Shdr *sechdrs,
+ const char *strtab,
+ unsigned int symindex,
+ unsigned int relsec,
+ struct module *me)
+{
+ unsigned int i;
+ Elf64_Rela *rela = (void *)sechdrs[relsec].sh_addr;
+ Elf64_Sym *sym;
+ unsigned long *location;
+ const char *symname;
+ u32 *instruction;
+
+ pr_debug("Clearing ADD relocate section %u to %u\n", relsec,
+sechdrs[relsec].sh_info);
+
+ for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rela); i++) {
+ location = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
+ + rela[i].r_offset;
+ sym = (Elf64_Sym *)sechdrs[symindex].sh_addr
+ + ELF64_R_SYM(rela[i].r_info);
+ symname = me->core_kallsyms.strtab
+ + sym->st_name;
+
+ if (ELF64_R_TYPE(rela[i].r_info) != R_PPC_REL24)
+ continue;
+ /*
+* reverse the operations in apply_relocate_add() for case
+* R_PPC_REL24.
+*/
+ if (sym->st_shndx != SHN_UNDEF &&
+ sym->st_shndx != SHN_LIVEPATCH)
+ continue;
+
+ instruction = (u32 *)location;
+ if (is_mprofile_ftrace_call(symname))
+ continue;
+
+ if (!instr_is_relative_link_branch(ppc_inst(*instruction)))
+ continue;
+
+ instruction += 1;
+ *instruction = PPC_RAW_NOP();
+ }
+
+}
+#endif
+
#ifdef CONFIG_DYNAMIC_FTRACE
int module_trampoline_target(struct module *mod, unsigned long addr,
unsigned long *target)
diff --git a/arch/s390/kernel/module.c b/arch/s390/kernel/module.c
index 2d159b32885b..cc6784fbc1ac 100644
--- a/arch/s390/kernel/module.c
+++ b/arch/s390/kernel/module.c
@@ -500,6 +500,14 @@ static int
