Re: [patches 1/2] ddb recursive panic fix

[christos%astron.com@localhost (Christos Zoulas)]

In article <52D5D1EA.1070202%bbn.com@localhost>, Richard Hansen  
<rhansen%bbn.com@localhost> wrote:
>On 2014-01-09 16:14, Richard Hansen wrote:


>I just noticed that the above changes were committed to current.  Thank
>you Christos!

Thank you for the patches.

>Would it be possible to pull up the following two changes to netbsd-6?
>
>http://mail-index.netbsd.org/source-changes/2014/01/11/msg050708.html
>http://mail-index.netbsd.org/source-changes/2014/01/11/msg050709.html

I asked for them.

>Also, does anyone have any comments about:
>http://mail-index.netbsd.org/source-changes/2014/01/11/msg050710.html
>The change introduced by that commit is repeated here:
>
>> diff --git a/src/sys/arch/x86/x86/vm_machdep.c
>b/src/sys/arch/x86/x86/vm_machdep.c
>> index 3b8dbf4..90e29de 100644
>> --- a/src/sys/arch/x86/x86/vm_machdep.c
>> +++ b/src/sys/arch/x86/x86/vm_machdep.c
>> @@ -228,6 +228,11 @@ cpu_lwp_fork(struct lwp *l1, struct lwp *l2, void
>*stack, size_t stacksize,
>>      pcb2->pcb_rsp = (uint64_t)sf;
>>      pcb2->pcb_rbp = (uint64_t)l2;
>>  #else
>> +    /*
>> +     * XXX Is there a reason sf->sf_edi isn't initialized here?
>> +     * Could this leak potentially sensitive information to new
>> +     * userspace processes?
>> +     */
>>      sf->sf_esi = (int)func;
>>      sf->sf_ebx = (int)arg;
>>      sf->sf_eip = (int)lwp_trampoline;
>
>Am I right that not initializing sf->sf_edi could leak sensitive
>information?  I'm wondering if it's possible for an attacker to fork()
>and immediately read %edi to try and get 4 bytes of a password or
>cryptographic key from a recently terminated process or something.  (An
>attack like that is probably not possible, but it's not immediately
>obvious and I'd rather err on the side of caution.)

I think it is hard to take advantage of, but nothing seems to set or use
sf_edi, so setting it to zero should not cause a problem.

I am still looking at the second patch. Ideally I want to apply something
similar for amd64 if required so that we don't have 2 flag days and ifdefs
in the userland code.

christos