[CRIU] [PATCH] restore: fix memory corruption due to the parent_act array

[Pavel Emelyanov]

On 08/07/2014 05:14 PM, Andrey Vagin wrote:
> The size of parent_act array was SIGMAX, but we wrote in
> parent_act[SIGMAX].
> 
> This patch takes into account that the parent_act array doesn't have
> entries for 0, SIGKILL, SIGSTOP, SIGCHLD.
> 
> Signed-off-by: Andrey Vagin <avagin at openvz.org>
> ---
>  cr-restore.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/cr-restore.c b/cr-restore.c
> index 1b538e1..2a3f3e6 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -616,16 +616,17 @@ static int open_vmas(int pid)
>  }
>  
>  static rt_sigaction_t sigchld_act;
> -static rt_sigaction_t parent_act[SIGMAX];
> +#define PARENT_ACT_SIZE SIGMAX - 3 /*0, SIGKILL, SIGSTOP, SIGCHLD*/
> +static rt_sigaction_t parent_act[PARENT_ACT_SIZE];
>  
> -static bool sa_inherited(int sig, rt_sigaction_t *sa)
> +static bool sa_inherited(int idx, rt_sigaction_t *sa)
>  {
>  	rt_sigaction_t *pa;
>  
>  	if (current == root_item)
>  		return false; /* XXX -- inherit from CRIU? */
>  
> -	pa = &parent_act[sig];
> +	pa = &parent_act[idx];
>  	return pa->rt_sa_handler == sa->rt_sa_handler &&
>  		pa->rt_sa_flags == sa->rt_sa_flags &&
>  		pa->rt_sa_restorer == sa->rt_sa_restorer &&
> @@ -682,7 +683,7 @@ static int prepare_sigactions(void)
>  			continue;
>  		}
>  
> -		if (sa_inherited(sig, &act))
> +		if (sa_inherited(rst, &act))

So if we managed to inherit SIGHUP from parent, rst will still be 0 and we will
compare our's SIGHUP's sa with parent's SIGHUP sa again.

>  			continue;
>  
>  		/*
> @@ -695,8 +696,9 @@ static int prepare_sigactions(void)
>  			goto err;
>  		}
>  
> -		parent_act[sig] = act;
> +		parent_act[rst] = act;

For the init's child parent_act[0] may contain sa for e.g. init's SIGUSR1, right?
But this kid's kid will compare it's SIGHUP's sa with it.

>  		rst++;
> +		BUG_ON(rst > PARENT_ACT_SIZE);
>  	}
>  
>  	pr_info("Restored %d/%d sigacts\n", rst,
>