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

[Michael Ellerman <mpe@ellerman.id.au>]

Jann Horn <jannh@google.com> writes:
> On Fri, Oct 5, 2018 at 3:21 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>> 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:
>
> This fix looks good to me.

Thanks.

> In the long term, I think it is somewhat awkward to use
> probe_kernel_address(), which uses set_fs(KERNEL_DS), when you
> actually just want to access userspace memory. It might make sense to
> provide a better helper for explicitly accessing memory with USER_DS.

Yes I agree, it's a bit messy. A probe_user_read() that sets USER_DS and
does the access_ok() check would be less error prone I think.

cheers