[PATCH] powerpc: Don't print kernel instructions in show_user_instructions()

[Christophe LEROY]

Le 05/10/2018 à 15:21, Michael Ellerman a écrit :
> Recently we implemented show_user_instructions() which dumps the code
> around the NIP when a user space process dies with an unhandled
> signal. This was modelled on the x86 code, and we even went so far as
> to implement the exact same bug, namely that if the user process
> crashed with its NIP pointing into the kernel we will dump kernel text
> to dmesg. eg:
> 
>    bad-bctr[2996]: segfault (11) at c000000000010000 nip c000000000010000 lr 12d0b0894 code 1
>    bad-bctr[2996]: code: fbe10068 7cbe2b78 7c7f1b78 fb610048 38a10028 38810020 fb810050 7f8802a6
>    bad-bctr[2996]: code: 3860001c f8010080 48242371 60000000 <7c7b1b79> 4082002c e8010080 eb610048
> 
> This was discovered on x86 by Jann Horn and fixed in commit
> 342db04ae712 ("x86/dumpstack: Don't dump kernel memory based on usermode RIP").
> 
> Fix it by checking the adjusted NIP value (pc) and number of
> instructions against USER_DS, and bail if we fail the check, eg:
> 
>    bad-bctr[2969]: segfault (11) at c000000000010000 nip c000000000010000 lr 107930894 code 1
>    bad-bctr[2969]: Bad NIP, not dumping instructions.
> 
> Fixes: 88b0fe175735 ("powerpc: Add show_user_instructions()")
> Signed-off-by: Michael Ellerman <mpe at ellerman.id.au>
> ---
>   arch/powerpc/kernel/process.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 913c5725cdb2..bb6ac471a784 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1306,6 +1306,16 @@ void show_user_instructions(struct pt_regs *regs)
>   
>   	pc = regs->nip - (instructions_to_print * 3 / 4 * sizeof(int));
>   
> +	/*
> +	 * Make sure the NIP points at userspace, not kernel text/data or
> +	 * elsewhere.
> +	 */
> +	if (!__access_ok(pc, instructions_to_print * sizeof(int), USER_DS)) {
> +		pr_info("%s[%d]: Bad NIP, not dumping instructions.\n",
> +			current->comm, current->pid);
> +		return;
> +	}
> +

Is there any reason for not using access_ok() here ?

Christophe

>   	pr_info("%s[%d]: code: ", current->comm, current->pid);
>   
>   	for (i = 0; i < instructions_to_print; i++) {
>