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

Submitted by alice on Sept. 28, 2017, 11:50 a.m.

Details

Message ID 20170928115012.47967-2-alice@linux.vnet.ibm.com
State New
Series "s390: Add support for GS and RI control blocks"
Headers show

Commit Message

alice Sept. 28, 2017, 11:50 a.m.
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@linux.vnet.ibm.com>
Reviewed-by: Michael Holzheu <holzheu@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(-)

Patch hide | download patch | download mbox

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);
 	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);
 	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

Comments

Dmitry Safonov Sept. 28, 2017, 6:48 p.m.
2017-09-28 12:50 GMT+01:00 Alice Frosi <alice@linux.vnet.ibm.com>:
> 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@linux.vnet.ibm.com>
> Reviewed-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>

Reviewed-by: Dmitry Safonov <0x7f454c46@gmail.com>
Andrey Vagin Sept. 30, 2017, 12:10 a.m.
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@linux.vnet.ibm.com>
> Reviewed-by: Michael Holzheu <holzheu@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?

>  	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@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
alice Oct. 4, 2017, 6:58 a.m.
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@linux.vnet.ibm.com>
>> Reviewed-by: Michael Holzheu <holzheu@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@openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu