Re: [PATCH 23/25] powerpc: Deliver SEGV signal on pkey violation

[Michael Ellerman]

Ram Pai <linux...@us.ibm.com> writes:

> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index ec74e20..f2a310d 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -265,6 +266,15 @@ void user_single_step_siginfo(struct task_struct *tsk,
>       info->si_addr = (void __user *)regs->nip;
>  }
>  
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> +static void fill_sig_info_pkey(int si_code, siginfo_t *info, unsigned long 
> addr)
> +{
> +     if (info->si_signo != SIGSEGV || si_code != SEGV_PKUERR)

Just checking si_code is sufficient there I think.

> +             return;
> +     info->si_pkey = get_paca()->paca_pkey;
> +}
> +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */

This should define an empty version in the #else case, so we don't need
the ifdef below.

> @@ -292,6 +302,18 @@ void _exception(int signr, struct pt_regs *regs, int 
> code, unsigned long addr)
>       info.si_signo = signr;
>       info.si_code = code;
>       info.si_addr = (void __user *) addr;
> +
> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> +     /*
> +      * update the thread's pkey related fields.
> +      * core-dump handlers and other sub-systems
> +      * depend on those values.
> +      */
> +     thread_pkey_regs_save(&current->thread);

You shouldn't need to do this.

We're not putting any of the pkey regs in the signal frame, so you don't
need to save before we do that. [And if you did the right place to do it
would be in setup_sigcontext() (or the TM version).]

For ptrace and coredumps it should happen in pkey_get(), see eg.
fpr_get() which does flush_fp_to_thread() as an example.

> +     /* update the violated-key value */
> +     fill_sig_info_pkey(code, &info, addr);
> +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */

> +
>       force_sig_info(signr, &info, current);
>  }

cheers