[CRIU] [RFC] compel: mprotect() parasite's .text from writes
Dmitry Safonov
dsafonov at virtuozzo.com
Mon Jan 30 03:08:04 PST 2017
On 01/30/2017 01:41 PM, Laurent Dufour wrote:
> On 27/01/2017 17:35, Dmitry Safonov wrote:
>> Make parasite's .text section rx and data section rw.
>> That'll make parasite more resistant to corruptions.
>>
>> For RFC sending version that always protects parasite,
>> the overhead is added by mprotect() syscall.
>> I can make a new infect-flag if this is too much for
>> the default case.
>
> I do like the idea but test fails on ppc64le :
>
> ======================= Run zdtm/static/dumpable01 in h
> ========================
> Start test
> ./dumpable01 --pidfile=dumpable01.pid --outfile=dumpable01.out
> Run criu dump
> =[log]=> dump/zdtm/static/dumpable01/15/1/dump.log
> ------------------------ grep Error ------------------------
> (00.006646) Error (criu/parasite-syscall.c:93): si_code=4 si_pid=15
> si_status=11
> (00.006650) Error (criu/parasite-syscall.c:101): 15 was stopped by 11
> unexpectedly
> ------------------------ ERROR OVER ------------------------
> ################ Test zdtm/static/dumpable01 FAIL at CRIU dump
> #################
>
> I didn't take much time looking deeper why it fails.
> Unfortunately I don't have free couples at the moment to look at this :(
Ok, no problem.
Will set up ppc VM and test to the moment of real patch, not-rfc
submission. At this moment also lack of time to do this.
Thanks for testing!
>
> Cheers,
> Laurent.
>
>
>>
>> Cc: Cyrill Gorcunov <gorcunov at openvz.org>
>> Cc: Laurent Dufour <ldufour at linux.vnet.ibm.com>
>> Signed-off-by: Dmitry Safonov <dsafonov at virtuozzo.com>
>> ---
>> compel/arch/aarch64/plugins/std/parasite-head.S | 6 ++++--
>> compel/arch/aarch64/scripts/compel-pack.lds.S | 23 +++++++++++++-------
>> compel/arch/arm/plugins/std/parasite-head.S | 7 ++++---
>> compel/arch/arm/scripts/compel-pack.lds.S | 23 +++++++++++++-------
>> compel/arch/ppc64/plugins/std/parasite-head.S | 7 ++++---
>> compel/arch/ppc64/scripts/compel-pack.lds.S | 21 +++++++++++--------
>> compel/arch/x86/plugins/std/parasite-head.S | 1 +
>> compel/arch/x86/scripts/compel-pack.lds.S | 22 +++++++++++--------
>> compel/src/lib/handle-elf.c | 28 ++++++++++++++++++-------
>> compel/src/lib/infect.c | 28 +++++++++++++++++++++++--
>> 10 files changed, 114 insertions(+), 52 deletions(-)
>>
>> diff --git a/compel/arch/aarch64/plugins/std/parasite-head.S b/compel/arch/aarch64/plugins/std/parasite-head.S
>> index 5e7067f6ba9e..09ec6db264bc 100644
>> --- a/compel/arch/aarch64/plugins/std/parasite-head.S
>> +++ b/compel/arch/aarch64/plugins/std/parasite-head.S
>> @@ -15,6 +15,8 @@ ENTRY(__export_parasite_head_start)
>> parasite_args_ptr:
>> .quad __export_parasite_args
>>
>> -__export_parasite_cmd:
>> - .quad 0
>> END(__export_parasite_head_start)
>> +
>> + .section .compel.data, "aw"
>> +GLOBAL(__export_parasite_cmd)
>> + .quad 0
>> diff --git a/compel/arch/aarch64/scripts/compel-pack.lds.S b/compel/arch/aarch64/scripts/compel-pack.lds.S
>> index b07e68c1d905..ba5d9a75c674 100644
>> --- a/compel/arch/aarch64/scripts/compel-pack.lds.S
>> +++ b/compel/arch/aarch64/scripts/compel-pack.lds.S
>> @@ -5,18 +5,29 @@ SECTIONS
>> *(.head.text)
>> *(.text*)
>> . = ALIGN(32);
>> - *(.data*)
>> - . = ALIGN(32);
>> *(.rodata*)
>> . = ALIGN(32);
>> - *(.bss*)
>> - . = ALIGN(32);
>> *(.got*)
>> . = ALIGN(32);
>> *(.toc*)
>> . = ALIGN(32);
>> } =0x00000000,
>>
>> + /* RW data starts on parasite_cmd, check compel_protect_parasite() */
>> + . = ALIGN(0x1000);
>> +
>> + .data : {
>> + *(.compel.data) /* it's parasite_cmd */
>> + . = ALIGN(32);
>> + *(.data*)
>> + . = ALIGN(32);
>> + *(.bss*)
>> + }
>> +
>> + /* Parasite args should have 4 bytes align, as we have futex inside. */
>> + . = ALIGN(4);
>> + __export_parasite_args = .;
>> +
>> /DISCARD/ : {
>> *(.debug*)
>> *(.comment*)
>> @@ -25,8 +36,4 @@ SECTIONS
>> *(.eh_frame*)
>> *(*)
>> }
>> -
>> -/* Parasite args should have 4 bytes align, as we have futex inside. */
>> -. = ALIGN(4);
>> -__export_parasite_args = .;
>> }
>> diff --git a/compel/arch/arm/plugins/std/parasite-head.S b/compel/arch/arm/plugins/std/parasite-head.S
>> index e72646b50804..12c6239afd27 100644
>> --- a/compel/arch/arm/plugins/std/parasite-head.S
>> +++ b/compel/arch/arm/plugins/std/parasite-head.S
>> @@ -4,7 +4,7 @@
>> ENTRY(__export_parasite_head_start)
>> sub r2, pc, #8 @ get the address of this instruction
>>
>> - adr r0, __export_parasite_cmd
>> + ldr r0, =__export_parasite_cmd
>> ldr r0, [r0]
>>
>> adr r1, parasite_args_ptr
>> @@ -16,7 +16,8 @@ ENTRY(__export_parasite_head_start)
>>
>> parasite_args_ptr:
>> .long __export_parasite_args
>> +END(__export_parasite_head_start)
>>
>> -__export_parasite_cmd:
>> + .section .compel.data, "aw"
>> +GLOBAL(__export_parasite_cmd)
>> .long 0
>> -END(__export_parasite_head_start)
>> diff --git a/compel/arch/arm/scripts/compel-pack.lds.S b/compel/arch/arm/scripts/compel-pack.lds.S
>> index fe402787f6fc..14ba5cb0afc3 100644
>> --- a/compel/arch/arm/scripts/compel-pack.lds.S
>> +++ b/compel/arch/arm/scripts/compel-pack.lds.S
>> @@ -5,18 +5,29 @@ SECTIONS
>> *(.head.text)
>> *(.text*)
>> . = ALIGN(32);
>> - *(.data*)
>> - . = ALIGN(32);
>> *(.rodata*)
>> . = ALIGN(32);
>> - *(.bss*)
>> - . = ALIGN(32);
>> *(.got*)
>> . = ALIGN(32);
>> *(.toc*)
>> . = ALIGN(32);
>> } =0x00000000,
>>
>> + /* RW data starts on parasite_cmd, check compel_protect_parasite() */
>> + . = ALIGN(0x1000);
>> +
>> + .data : {
>> + *(.compel.data) /* it's parasite_cmd */
>> + . = ALIGN(32);
>> + *(.data*)
>> + . = ALIGN(32);
>> + *(.bss*)
>> + }
>> +
>> + /* Parasite args should have 4 bytes align, as we have futex inside. */
>> + . = ALIGN(4);
>> + __export_parasite_args = .;
>> +
>> /DISCARD/ : {
>> *(.debug*)
>> *(.comment*)
>> @@ -25,8 +36,4 @@ SECTIONS
>> *(.eh_frame*)
>> *(*)
>> }
>> -
>> -/* Parasite args should have 4 bytes align, as we have futex inside. */
>> -. = ALIGN(4);
>> -__export_parasite_args = .;
>> }
>> diff --git a/compel/arch/ppc64/plugins/std/parasite-head.S b/compel/arch/ppc64/plugins/std/parasite-head.S
>> index c870efdc209d..23d16f0350d9 100644
>> --- a/compel/arch/ppc64/plugins/std/parasite-head.S
>> +++ b/compel/arch/ppc64/plugins/std/parasite-head.S
>> @@ -39,7 +39,8 @@ parasite_service_ptr:
>> // FIXME: There should be a way to specify the global entry here.
>> .quad parasite_service - 8
>>
>> -__export_parasite_cmd:
>> - .long 0
>> -
>> END(__export_parasite_head_start)
>> +
>> + .section .compel.data, "aw"
>> +GLOBAL(__export_parasite_cmd)
>> + .long 0
>> diff --git a/compel/arch/ppc64/scripts/compel-pack.lds.S b/compel/arch/ppc64/scripts/compel-pack.lds.S
>> index 5da9a08460c6..f1abcec4f33e 100644
>> --- a/compel/arch/ppc64/scripts/compel-pack.lds.S
>> +++ b/compel/arch/ppc64/scripts/compel-pack.lds.S
>> @@ -8,11 +8,6 @@ SECTIONS
>> *(.compel.init)
>> }
>>
>> - .data : {
>> - *(.data*)
>> - *(.bss*)
>> - }
>> -
>> .rodata : {
>> *(.rodata*)
>> *(.got*)
>> @@ -22,6 +17,18 @@ SECTIONS
>> *(.toc*)
>> }
>>
>> + /* RW data starts on parasite_cmd, check compel_protect_parasite() */
>> + . = ALIGN(0x1000);
>> + .data : {
>> + *(.compel.data) /* it's parasite_cmd */
>> + *(.data*)
>> + *(.bss*)
>> + }
>> +
>> + /* Parasite args should have 4 bytes align, as we have futex inside. */
>> + . = ALIGN(4);
>> + __export_parasite_args = .;
>> +
>> /DISCARD/ : {
>> *(.debug*)
>> *(.comment*)
>> @@ -29,8 +36,4 @@ SECTIONS
>> *(.group*)
>> *(.eh_frame*)
>> }
>> -
>> -/* Parasite args should have 4 bytes align, as we have futex inside. */
>> -. = ALIGN(4);
>> -__export_parasite_args = .;
>> }
>> diff --git a/compel/arch/x86/plugins/std/parasite-head.S b/compel/arch/x86/plugins/std/parasite-head.S
>> index a988de9d4218..df63831d7f02 100644
>> --- a/compel/arch/x86/plugins/std/parasite-head.S
>> +++ b/compel/arch/x86/plugins/std/parasite-head.S
>> @@ -47,6 +47,7 @@ ENTRY(__export_parasite_head_start)
>> int $0x03
>> END(__export_parasite_head_start)
>>
>> + .section .compel.data, "aw"
>> .align 8
>> GLOBAL(__export_parasite_cmd)
>> .long 0
>> diff --git a/compel/arch/x86/scripts/compel-pack.lds.S b/compel/arch/x86/scripts/compel-pack.lds.S
>> index cd6584ffe710..da82e595afdc 100644
>> --- a/compel/arch/x86/scripts/compel-pack.lds.S
>> +++ b/compel/arch/x86/scripts/compel-pack.lds.S
>> @@ -9,11 +9,6 @@ SECTIONS
>> *(.compel.init)
>> }
>>
>> - .data : {
>> - *(.data*)
>> - *(.bss*)
>> - }
>> -
>> .rodata : {
>> *(.rodata*)
>> *(.got*)
>> @@ -23,6 +18,19 @@ SECTIONS
>> *(.toc*)
>> }
>>
>> + /* RW data starts on parasite_cmd, check compel_protect_parasite() */
>> + . = ALIGN(0x1000);
>> +
>> + .data : {
>> + *(.compel.data) /* it's parasite_cmd */
>> + *(.data*)
>> + *(.bss*)
>> + }
>> +
>> + /* Parasite args should have 4 bytes align, as we have futex inside. */
>> + . = ALIGN(4);
>> + __export_parasite_args = .;
>> +
>> /DISCARD/ : {
>> *(.debug*)
>> *(.comment*)
>> @@ -30,8 +38,4 @@ SECTIONS
>> *(.group*)
>> *(.eh_frame*)
>> }
>> -
>> -/* Parasite args should have 4 bytes align, as we have futex inside. */
>> -. = ALIGN(4);
>> -__export_parasite_args = .;
>> }
>> diff --git a/compel/src/lib/handle-elf.c b/compel/src/lib/handle-elf.c
>> index 4b8b73dfacb4..e4692f71e5d0 100644
>> --- a/compel/src/lib/handle-elf.c
>> +++ b/compel/src/lib/handle-elf.c
>> @@ -459,29 +459,41 @@ int __handle_elf(void *mem, size_t size)
>> goto err;
>> break;
>>
>> - case R_PPC64_REL16_HA:
>> + case R_PPC64_REL16_HA: {
>> + uint16_t inst = (*(uint32_t*)where) >> 16;
>> +
>> value64 += addend64 - place;
>> pr_debug("\t\t\tR_PPC64_REL16_HA at 0x%-4lx val 0x%lx\n",
>> place, value64);
>> - /* check that we are dealing with the addis 2,12 instruction */
>> - if (((*(uint32_t*)where) & 0xffff0000) != 0x3c4c0000) {
>> - pr_err("Unexpected instruction for R_PPC64_REL16_HA\n");
>> + /*
>> + * Check that we are dealing with the addis 2,12 instruction
>> + * or `addis r3,r2`
>> + */
>> + if (inst != 0x3c4c && inst != 0x3c62) {
>> + pr_err("Unexpected instruction %#x for R_PPC64_REL16_HA\n", (unsigned)inst);
>> goto err;
>> }
>> *(uint16_t *)where = ((value64 + 0x8000) >> 16) & 0xffff;
>> break;
>> + }
>> +
>> + case R_PPC64_REL16_LO: {
>> + uint16_t inst = (*(uint32_t*)where) >> 16;
>>
>> - case R_PPC64_REL16_LO:
>> value64 += addend64 - place;
>> pr_debug("\t\t\tR_PPC64_REL16_LO at 0x%-4lx val 0x%lx\n",
>> place, value64);
>> - /* check that we are dealing with the addi 2,2 instruction */
>> - if (((*(uint32_t*)where) & 0xffff0000) != 0x38420000) {
>> - pr_err("Unexpected instruction for R_PPC64_REL16_LO\n");
>> + /*
>> + * Check that we are dealing with the addi 2,2 instruction
>> + * or `addi r3,r2`
>> + */
>> + if (inst != 0x3842 && inst != 0x3862) {
>> + pr_err("Unexpected instruction %#x for R_PPC64_REL16_LO\n", (unsigned)inst);
>> goto err;
>> }
>> *(uint16_t *)where = value64 & 0xffff;
>> break;
>> + }
>>
>> #endif /* ELF_PPC64 */
>>
>> diff --git a/compel/src/lib/infect.c b/compel/src/lib/infect.c
>> index 599c37274c6b..cd8d88231121 100644
>> --- a/compel/src/lib/infect.c
>> +++ b/compel/src/lib/infect.c
>> @@ -703,7 +703,7 @@ static int parasite_mmap_exchange(struct parasite_ctl *ctl, unsigned long size)
>> int fd;
>>
>> ctl->remote_map = remote_mmap(ctl, NULL, size,
>> - PROT_READ | PROT_WRITE | PROT_EXEC,
>> + PROT_READ | PROT_WRITE,
>> MAP_ANONYMOUS | MAP_SHARED, -1, 0);
>> if (!ctl->remote_map) {
>> pr_err("Can't allocate memory for parasite blob (pid: %d)\n", ctl->rpid);
>> @@ -781,7 +781,7 @@ static int parasite_memfd_exchange(struct parasite_ctl *ctl, unsigned long size)
>> }
>>
>> ctl->remote_map = remote_mmap(ctl, NULL, size,
>> - PROT_READ | PROT_WRITE | PROT_EXEC,
>> + PROT_READ | PROT_WRITE,
>> MAP_FILE | MAP_SHARED, fd, 0);
>> if (!ctl->remote_map) {
>> pr_err("Can't rmap memfd for parasite blob\n");
>> @@ -854,6 +854,25 @@ static inline unsigned long total_pie_size(size_t blob_size, size_t nr_gp)
>> return round_up(blob_size + nr_gp * sizeof(long), page_size());
>> }
>>
>> +static int compel_protect_parasite(struct parasite_ctl *ctl)
>> +{
>> + bool __maybe_unused compat_task = !compel_mode_native(ctl);
>> + /* RW parasite area starts from parasite_cmd */
>> + uintptr_t ro_size = ctl->pblob.hdr.addr_cmd_off;
>> + long ret;
>> +
>> + compel_syscall(ctl, __NR(mprotect, compat_task), &ret,
>> + (unsigned long)ctl->remote_map, ro_size,
>> + PROT_READ | PROT_EXEC, 0, 0, 0);
>> + if (ret) {
>> + pr_err("mprotect() parasites [%#lx, %#lx) failed with %ld\n",
>> + (unsigned long)ctl->remote_map,
>> + (unsigned long)ctl->remote_map + ro_size, ret);
>> + return -1;
>> + }
>> + return 0;
>> +}
>> +
>> int compel_infect(struct parasite_ctl *ctl, unsigned long nr_threads, unsigned long args_size)
>> {
>> int ret;
>> @@ -914,6 +933,11 @@ int compel_infect(struct parasite_ctl *ctl, unsigned long nr_threads, unsigned l
>> ctl->r_thread_stack = ctl->remote_map + p;
>> }
>>
>> + pr_info("Protecting parasite's text from writes\n");
>> + if (compel_protect_parasite(ctl))
>> + goto err;
>> +
>> + pr_info("Starting parasite daemon\n");
>> if (parasite_start_daemon(ctl))
>> goto err;
>>
>
--
Dmitry
More information about the CRIU
mailing list