[CRIU] [RFC] compel: mprotect() parasite's .text from writes
Laurent Dufour
ldufour at linux.vnet.ibm.com
Mon Jan 30 04:54:17 PST 2017
On 30/01/2017 12:08, Dmitry Safonov wrote:
> 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!
You're welcome.
Feel free to ping me if you need any help later.
Cheers,
Laurent.
>
>>
>> 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;
>>>
>>
>
>
More information about the CRIU
mailing list