[CRIU] [PATCH 1/4] Restore registers not present in the sigreturn signal frame

[Alice Frosi]

On 30.09.2017 02:10, Andrei Vagin wrote:
> On Thu, Sep 28, 2017 at 01:50:09PM +0200, Alice Frosi wrote:
>> Add new function arch_set_task_regs_nosigrt(). It allows to restore
>> architecture-specific registers not present in sigreturn signal frame.
>>
>> Each architecture can overwrite this function.
>>
>> The arch_set_task_regs_nosigrt function restores the registers between the
>> final sigreturn and PTRACE_DETACH. We do this at this point because
>> we are sure that all threads have already been created.
>>
>> Add arch_set_thread_regs in pre-dump because some registers may have
>> been changed by the parasite infection and they are not present in
>> sigreturn signal frame.
>>
>> Signed-off-by: Alice Frosi <alice at linux.vnet.ibm.com>
>> Reviewed-by: Michael Holzheu <holzheu at linux.vnet.ibm.com>
>> ---
>>   criu/cr-dump.c         | 14 ++++++++++++--
>>   criu/cr-restore.c      | 10 ++++++++++
>>   criu/include/dump.h    |  3 +++
>>   criu/include/restore.h |  3 +++
>>   4 files changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
>> index c83f5ee..67284ae 100644
>> --- a/criu/cr-dump.c
>> +++ b/criu/cr-dump.c
>> @@ -87,8 +87,13 @@
>>   /*
>>    * Architectures can overwrite this function to restore register sets that
>>    * are not covered by ptrace_set/get_regs().
>> + *
>> + * with_threads = false: Only the register sets of the tasks are restored
>> + * with_threads = true : The register sets of the tasks with all their threads
>> + *			 are restored
>>    */
>> -int __attribute__((weak)) arch_set_thread_regs(struct pstree_item *item)
>> +int __attribute__((weak)) arch_set_thread_regs(struct pstree_item *item,
>> +					       bool with_threads)
>>   {
>>   	return 0;
>>   }
>> @@ -1560,6 +1565,11 @@ static int cr_pre_dump_finish(int ret)
>>   {
>>   	struct pstree_item *item;
>>   
>> +	/*
>> +	 * Restore registers for tasks only. The threads have not been
>> +	 * infected. Therefore, the thread register sets have not been changed.
>> +	 */
>> +	arch_set_thread_regs(root_item, false);
> arch_set_thread_regs may return an error, why don't we handle it?

You are right, thanks! I will resubmit the patches with the error handling. I will also split the test patch in two.

>
>>   	pstree_switch_state(root_item, TASK_ALIVE);
>>   
>>   	timing_stop(TIME_FROZEN);
>> @@ -1785,7 +1795,7 @@ static int cr_dump_finish(int ret)
>>   	if (!ret && opts.lazy_pages)
>>   		ret = cr_lazy_mem_dump();
>>   
>> -	arch_set_thread_regs(root_item);
>> +	arch_set_thread_regs(root_item, true);
> the same question
>
>>   	pstree_switch_state(root_item,
>>   			    (ret || post_dump_ret) ?
>>   			    TASK_ALIVE : opts.final_state);
>> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
>> index 4d6f6e4..a691b4f 100644
>> --- a/criu/cr-restore.c
>> +++ b/criu/cr-restore.c
>> @@ -122,6 +122,15 @@ static int prepare_rlimits(int pid, struct task_restore_args *, CoreEntry *core)
>>   static int prepare_posix_timers(int pid, struct task_restore_args *ta, CoreEntry *core);
>>   static int prepare_signals(int pid, struct task_restore_args *, CoreEntry *core);
>>   
>> +/*
>> + * Architectures can overwrite this function to restore registers that are not
>> + * present in the sigreturn signal frame.
>> + */
>> +int __attribute__((weak)) arch_set_thread_regs_nosigrt(struct pid *pid)
>> +{
>> +	return 0;
>> +}
>> +
>>   static inline int stage_participants(int next_stage)
>>   {
>>   	switch (next_stage) {
>> @@ -2081,6 +2090,7 @@ static void finalize_restore_detach(int status)
>>   				break;
>>   			}
>>   
>> +			arch_set_thread_regs_nosigrt(item->threads[i]);
>>   			if (ptrace(PTRACE_DETACH, pid, NULL, 0))
>>   				pr_perror("Unable to execute %d", pid);
>>   		}
>> diff --git a/criu/include/dump.h b/criu/include/dump.h
>> index d9a4b61..1c14468 100644
>> --- a/criu/include/dump.h
>> +++ b/criu/include/dump.h
>> @@ -1,4 +1,7 @@
>>   #ifndef __CR_INC_DUMP_H__
>>   #define __CR_INC_DUMP_H__
>>   #include "asm/dump.h"
>> +
>> +extern int arch_set_thread_regs(struct pstree_item *item, bool with_threads);
>> +
>>   #endif
>> diff --git a/criu/include/restore.h b/criu/include/restore.h
>> index 07e8fd8..511f253 100644
>> --- a/criu/include/restore.h
>> +++ b/criu/include/restore.h
>> @@ -1,8 +1,11 @@
>>   #ifndef __CR_INC_RESTORE_H__
>>   #define __CR_INC_RESTORE_H__
>> +
>> +#include "pid.h"
>>   #include "types.h"
>>   #include "asm/restore.h"
>>   
>>   extern int criu_signals_setup(void (*handler)(int, siginfo_t *, void *));
>> +extern int arch_set_thread_regs_nosigrt(struct pid *pid);
>>   
>>   #endif
>> -- 
>> 2.9.3
>>
>> _______________________________________________
>> CRIU mailing list
>> CRIU at openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu