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: 00
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: c00802140
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 <7c9ce9
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: c008
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 > > > G
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 c00802
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 > L
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 long
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 CO
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, > +const