Re: WARNING: kernel stack regs has bad 'bp' value (3)

2018-05-26 Thread Eric Biggers 
On Sat, May 12, 2018 at 10:43:08AM +0200, Dmitry Vyukov wrote:
> On Fri, Feb 2, 2018 at 11:18 PM, Eric Biggers  wrote:
> > On Fri, Feb 02, 2018 at 02:57:32PM +0100, Dmitry Vyukov wrote:
> >> On Fri, Feb 2, 2018 at 2:48 PM, syzbot
> >>  wrote:
> >> > Hello,
> >> >
> >> > syzbot hit the following crash on upstream commit
> >> > 7109a04eae81c41ed529da9f3c48c3655ccea741 (Thu Feb 1 17:37:30 2018 +)
> >> > Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide
> >> >
> >> > So far this crash happened 4 times on net-next, upstream.
> >> > C reproducer is attached.
> >> > syzkaller reproducer is attached.
> >> > Raw console output is attached.
> >> > compiler: gcc (GCC) 7.1.1 20170620
> >> > .config is attached.
> >>
> >>
> >> From suspicious frames I see salsa20_asm_crypt there, so +crypto 
> >> maintainers.
> >>
> >
> > Looks like the x86 implementations of Salsa20 (both i586 and x86_64) need 
> > to be
> > updated to not use %ebp/%rbp.
> 
> Ard,
> 
> This was bisected as introduced by:
> 
> commit 83dee2ce1ae791c3dc0c9d4d3a8d42cb109613f6
> Author: Ard Biesheuvel 
> Date:   Fri Jan 19 12:04:34 2018 +
> 
> crypto: sha3-generic - rewrite KECCAK transform to help the
> compiler optimize
> 
> https://gist.githubusercontent.com/dvyukov/47f93f5a0679170dddf93bc019b42f6d/raw/65beac8ddd30003bbd4e9729236dc8572094abf7/gistfile1.txt

Note that syzbot's original C reproducer (from Feb 1) for this actually
triggered the warning through salsa20-asm, which I've just proposed to "fix" by
https://patchwork.kernel.org/patch/10428863/.  sha3-generic is apparently
another instance of the same bug, where the %rbp register is used for data.

Eric

Re: WARNING: kernel stack regs has bad 'bp' value (3)

2018-05-14 Thread Josh Poimboeuf 
On Sat, May 12, 2018 at 12:11:17PM +0200, Ard Biesheuvel wrote:
> On 12 May 2018 at 11:50, Dmitry Vyukov  wrote:
> > On Sat, May 12, 2018 at 11:09 AM, Ard Biesheuvel
> >  wrote:
> >> (+ Arnd)
> >>
> >> On 12 May 2018 at 10:43, Dmitry Vyukov  wrote:
> >>> On Fri, Feb 2, 2018 at 11:18 PM, Eric Biggers  wrote:
>  On Fri, Feb 02, 2018 at 02:57:32PM +0100, Dmitry Vyukov wrote:
> > On Fri, Feb 2, 2018 at 2:48 PM, syzbot
> >  wrote:
> > > Hello,
> > >
> > > syzbot hit the following crash on upstream commit
> > > 7109a04eae81c41ed529da9f3c48c3655ccea741 (Thu Feb 1 17:37:30 2018 
> > > +)
> > > Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide
> > >
> > > So far this crash happened 4 times on net-next, upstream.
> > > C reproducer is attached.
> > > syzkaller reproducer is attached.
> > > Raw console output is attached.
> > > compiler: gcc (GCC) 7.1.1 20170620
> > > .config is attached.
> >
> >
> > From suspicious frames I see salsa20_asm_crypt there, so +crypto 
> > maintainers.
> >
> 
>  Looks like the x86 implementations of Salsa20 (both i586 and x86_64) 
>  need to be
>  updated to not use %ebp/%rbp.
> >>>
> >>> Ard,
> >>>
> >>> This was bisected as introduced by:
> >>>
> >>> commit 83dee2ce1ae791c3dc0c9d4d3a8d42cb109613f6
> >>> Author: Ard Biesheuvel 
> >>> Date:   Fri Jan 19 12:04:34 2018 +
> >>>
> >>> crypto: sha3-generic - rewrite KECCAK transform to help the
> >>> compiler optimize
> >>>
> >>> https://gist.githubusercontent.com/dvyukov/47f93f5a0679170dddf93bc019b42f6d/raw/65beac8ddd30003bbd4e9729236dc8572094abf7/gistfile1.txt
> >>
> >> Ouch.
> >>
> >> I'm not an expert in x86 assembly. Could someone please check the
> >> generated code to see what's going on? The C code changes are not that
> >> intricate, they basically unroll a loop, replacing accesses to
> >> 'array[indirect_index[i]]' with 'array[constant]'.
> >>
> >> As mentioned in the commit log, the speedup is more than significant
> >> for architectures with lots of GPRs so I'd prefer fixing the patch
> >> over reverting it (if there is anything wrong with the code in the
> >> first place)
> >
> > I suspect the problem is with __attribute__((__optimize__("O3"))). It
> > makes compiler use rbp register, which must not be used.
> 
> IIRC, the additional speedup from adding that was significant but not
> huge. Given that we don't use O3 anywhere else, I guess we should just
> remove it.
> 
> Could you please check whether that makes the issue go away?
> 
> If so,
> 
> Acked-by: Ard Biesheuvel 
> 
> for any patch that removes the O3 attribute override from keccakf()

The issue only affects CONFIG_FRAME_POINTER (which is no longer the
default on x86-64), so maybe -O3 could only be enabled for
CONFIG_FRAME_POINTER=n, in which case you'd still get the speedup with
the default ORC unwinder.

-- 
Josh

Re: WARNING: kernel stack regs has bad 'bp' value (3)

2018-05-12 Thread Ard Biesheuvel 
On 12 May 2018 at 11:50, Dmitry Vyukov  wrote:
> On Sat, May 12, 2018 at 11:09 AM, Ard Biesheuvel
>  wrote:
>> (+ Arnd)
>>
>> On 12 May 2018 at 10:43, Dmitry Vyukov  wrote:
>>> On Fri, Feb 2, 2018 at 11:18 PM, Eric Biggers  wrote:
 On Fri, Feb 02, 2018 at 02:57:32PM +0100, Dmitry Vyukov wrote:
> On Fri, Feb 2, 2018 at 2:48 PM, syzbot
>  wrote:
> > Hello,
> >
> > syzbot hit the following crash on upstream commit
> > 7109a04eae81c41ed529da9f3c48c3655ccea741 (Thu Feb 1 17:37:30 2018 +)
> > Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide
> >
> > So far this crash happened 4 times on net-next, upstream.
> > C reproducer is attached.
> > syzkaller reproducer is attached.
> > Raw console output is attached.
> > compiler: gcc (GCC) 7.1.1 20170620
> > .config is attached.
>
>
> From suspicious frames I see salsa20_asm_crypt there, so +crypto 
> maintainers.
>

 Looks like the x86 implementations of Salsa20 (both i586 and x86_64) need 
 to be
 updated to not use %ebp/%rbp.
>>>
>>> Ard,
>>>
>>> This was bisected as introduced by:
>>>
>>> commit 83dee2ce1ae791c3dc0c9d4d3a8d42cb109613f6
>>> Author: Ard Biesheuvel 
>>> Date:   Fri Jan 19 12:04:34 2018 +
>>>
>>> crypto: sha3-generic - rewrite KECCAK transform to help the
>>> compiler optimize
>>>
>>> https://gist.githubusercontent.com/dvyukov/47f93f5a0679170dddf93bc019b42f6d/raw/65beac8ddd30003bbd4e9729236dc8572094abf7/gistfile1.txt
>>
>> Ouch.
>>
>> I'm not an expert in x86 assembly. Could someone please check the
>> generated code to see what's going on? The C code changes are not that
>> intricate, they basically unroll a loop, replacing accesses to
>> 'array[indirect_index[i]]' with 'array[constant]'.
>>
>> As mentioned in the commit log, the speedup is more than significant
>> for architectures with lots of GPRs so I'd prefer fixing the patch
>> over reverting it (if there is anything wrong with the code in the
>> first place)
>
> I suspect the problem is with __attribute__((__optimize__("O3"))). It
> makes compiler use rbp register, which must not be used.

IIRC, the additional speedup from adding that was significant but not
huge. Given that we don't use O3 anywhere else, I guess we should just
remove it.

Could you please check whether that makes the issue go away?

If so,

Acked-by: Ard Biesheuvel 

for any patch that removes the O3 attribute override from keccakf()

Thanks,
Ard.

Re: WARNING: kernel stack regs has bad 'bp' value (3)

2018-05-12 Thread Dmitry Vyukov 
On Sat, May 12, 2018 at 11:09 AM, Ard Biesheuvel
 wrote:
> (+ Arnd)
>
> On 12 May 2018 at 10:43, Dmitry Vyukov  wrote:
>> On Fri, Feb 2, 2018 at 11:18 PM, Eric Biggers  wrote:
>>> On Fri, Feb 02, 2018 at 02:57:32PM +0100, Dmitry Vyukov wrote:
 On Fri, Feb 2, 2018 at 2:48 PM, syzbot
  wrote:
 > Hello,
 >
 > syzbot hit the following crash on upstream commit
 > 7109a04eae81c41ed529da9f3c48c3655ccea741 (Thu Feb 1 17:37:30 2018 +)
 > Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide
 >
 > So far this crash happened 4 times on net-next, upstream.
 > C reproducer is attached.
 > syzkaller reproducer is attached.
 > Raw console output is attached.
 > compiler: gcc (GCC) 7.1.1 20170620
 > .config is attached.


 From suspicious frames I see salsa20_asm_crypt there, so +crypto 
 maintainers.

>>>
>>> Looks like the x86 implementations of Salsa20 (both i586 and x86_64) need 
>>> to be
>>> updated to not use %ebp/%rbp.
>>
>> Ard,
>>
>> This was bisected as introduced by:
>>
>> commit 83dee2ce1ae791c3dc0c9d4d3a8d42cb109613f6
>> Author: Ard Biesheuvel 
>> Date:   Fri Jan 19 12:04:34 2018 +
>>
>> crypto: sha3-generic - rewrite KECCAK transform to help the
>> compiler optimize
>>
>> https://gist.githubusercontent.com/dvyukov/47f93f5a0679170dddf93bc019b42f6d/raw/65beac8ddd30003bbd4e9729236dc8572094abf7/gistfile1.txt
>
> Ouch.
>
> I'm not an expert in x86 assembly. Could someone please check the
> generated code to see what's going on? The C code changes are not that
> intricate, they basically unroll a loop, replacing accesses to
> 'array[indirect_index[i]]' with 'array[constant]'.
>
> As mentioned in the commit log, the speedup is more than significant
> for architectures with lots of GPRs so I'd prefer fixing the patch
> over reverting it (if there is anything wrong with the code in the
> first place)

I suspect the problem is with __attribute__((__optimize__("O3"))). It
makes compiler use rbp register, which must not be used.

Re: WARNING: kernel stack regs has bad 'bp' value (3)

2018-05-12 Thread Ard Biesheuvel 
(+ Arnd)

On 12 May 2018 at 10:43, Dmitry Vyukov  wrote:
> On Fri, Feb 2, 2018 at 11:18 PM, Eric Biggers  wrote:
>> On Fri, Feb 02, 2018 at 02:57:32PM +0100, Dmitry Vyukov wrote:
>>> On Fri, Feb 2, 2018 at 2:48 PM, syzbot
>>>  wrote:
>>> > Hello,
>>> >
>>> > syzbot hit the following crash on upstream commit
>>> > 7109a04eae81c41ed529da9f3c48c3655ccea741 (Thu Feb 1 17:37:30 2018 +)
>>> > Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide
>>> >
>>> > So far this crash happened 4 times on net-next, upstream.
>>> > C reproducer is attached.
>>> > syzkaller reproducer is attached.
>>> > Raw console output is attached.
>>> > compiler: gcc (GCC) 7.1.1 20170620
>>> > .config is attached.
>>>
>>>
>>> From suspicious frames I see salsa20_asm_crypt there, so +crypto 
>>> maintainers.
>>>
>>
>> Looks like the x86 implementations of Salsa20 (both i586 and x86_64) need to 
>> be
>> updated to not use %ebp/%rbp.
>
> Ard,
>
> This was bisected as introduced by:
>
> commit 83dee2ce1ae791c3dc0c9d4d3a8d42cb109613f6
> Author: Ard Biesheuvel 
> Date:   Fri Jan 19 12:04:34 2018 +
>
> crypto: sha3-generic - rewrite KECCAK transform to help the
> compiler optimize
>
> https://gist.githubusercontent.com/dvyukov/47f93f5a0679170dddf93bc019b42f6d/raw/65beac8ddd30003bbd4e9729236dc8572094abf7/gistfile1.txt

Ouch.

I'm not an expert in x86 assembly. Could someone please check the
generated code to see what's going on? The C code changes are not that
intricate, they basically unroll a loop, replacing accesses to
'array[indirect_index[i]]' with 'array[constant]'.

As mentioned in the commit log, the speedup is more than significant
for architectures with lots of GPRs so I'd prefer fixing the patch
over reverting it (if there is anything wrong with the code in the
first place)

-- 
Ard.

Re: WARNING: kernel stack regs has bad 'bp' value (3)

2018-05-12 Thread Dmitry Vyukov 
On Fri, Feb 2, 2018 at 11:18 PM, Eric Biggers  wrote:
> On Fri, Feb 02, 2018 at 02:57:32PM +0100, Dmitry Vyukov wrote:
>> On Fri, Feb 2, 2018 at 2:48 PM, syzbot
>>  wrote:
>> > Hello,
>> >
>> > syzbot hit the following crash on upstream commit
>> > 7109a04eae81c41ed529da9f3c48c3655ccea741 (Thu Feb 1 17:37:30 2018 +)
>> > Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide
>> >
>> > So far this crash happened 4 times on net-next, upstream.
>> > C reproducer is attached.
>> > syzkaller reproducer is attached.
>> > Raw console output is attached.
>> > compiler: gcc (GCC) 7.1.1 20170620
>> > .config is attached.
>>
>>
>> From suspicious frames I see salsa20_asm_crypt there, so +crypto maintainers.
>>
>
> Looks like the x86 implementations of Salsa20 (both i586 and x86_64) need to 
> be
> updated to not use %ebp/%rbp.

Ard,

This was bisected as introduced by:

commit 83dee2ce1ae791c3dc0c9d4d3a8d42cb109613f6
Author: Ard Biesheuvel 
Date:   Fri Jan 19 12:04:34 2018 +

crypto: sha3-generic - rewrite KECCAK transform to help the
compiler optimize

https://gist.githubusercontent.com/dvyukov/47f93f5a0679170dddf93bc019b42f6d/raw/65beac8ddd30003bbd4e9729236dc8572094abf7/gistfile1.txt

Re: WARNING: kernel stack regs has bad 'bp' value (3)

2018-02-02 Thread Eric Biggers 
On Fri, Feb 02, 2018 at 02:57:32PM +0100, Dmitry Vyukov wrote:
> On Fri, Feb 2, 2018 at 2:48 PM, syzbot
>  wrote:
> > Hello,
> >
> > syzbot hit the following crash on upstream commit
> > 7109a04eae81c41ed529da9f3c48c3655ccea741 (Thu Feb 1 17:37:30 2018 +)
> > Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide
> >
> > So far this crash happened 4 times on net-next, upstream.
> > C reproducer is attached.
> > syzkaller reproducer is attached.
> > Raw console output is attached.
> > compiler: gcc (GCC) 7.1.1 20170620
> > .config is attached.
> 
> 
> From suspicious frames I see salsa20_asm_crypt there, so +crypto maintainers.
> 

Looks like the x86 implementations of Salsa20 (both i586 and x86_64) need to be
updated to not use %ebp/%rbp.

- Eric

Re: WARNING: kernel stack regs has bad 'bp' value (3)

2018-02-02 Thread Dmitry Vyukov 
On Fri, Feb 2, 2018 at 2:48 PM, syzbot
 wrote:
> Hello,
>
> syzbot hit the following crash on upstream commit
> 7109a04eae81c41ed529da9f3c48c3655ccea741 (Thu Feb 1 17:37:30 2018 +)
> Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide
>
> So far this crash happened 4 times on net-next, upstream.
> C reproducer is attached.
> syzkaller reproducer is attached.
> Raw console output is attached.
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached.


>From suspicious frames I see salsa20_asm_crypt there, so +crypto maintainers.


> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+ffa3a158337bbc01f...@syzkaller.appspotmail.com
> It will help syzbot understand when the bug is fixed. See footer for
> details.
> If you forward the report, please keep this part and the footer.
>
> WARNING: kernel stack regs at a8291e69 in syzkaller047086:4677 has
> bad 'bp' value 1077994c
> unwind stack type:0 next_sp:  (null) mask:0x6 graph_idx:0
> 1d3b7fe2: 8801db4075c8 (0x8801db4075c8)
> 83b445d5: 8128e6de (__save_stack_trace+0x6e/0xd0)
> b52ac563:  ...
> 3035eb8b: 8801addd (0x8801addd)
> ee6283c3: 8801addd8000 (0x8801addd8000)
> 331afaf0:  ...
> b93daa43: 0006 (0x6)
> aa09edca: 8801acaca4c0 (0x8801acaca4c0)
> 5388d67c: 0101 (0x101)
> d567f6b6:  ...
> fd90d54b: 8801db407540 (0x8801db407540)
> d598c5fd: 8135b07e (._mainloop+0x187/0x4ca)
> 255a8082: 8801addd7658 (0x8801addd7658)
> 0175a1d9: 0100 (0x100)
> 6420fb62: 8801aca0c780 (0x8801aca0c780)
> ef007705: 8801aca0c7a0 (0x8801aca0c7a0)
> c3a16804: 82213878 (selinux_cred_free+0x48/0x70)
> 35d2f6f8: 8801db4075d8 (0x8801db4075d8)
> d255d236: 8128e75a (save_stack_trace+0x1a/0x20)
> ad4323cc: 8801db407808 (0x8801db407808)
> a76fbd41: 81a8d883 (save_stack+0x43/0xd0)
> 104bc778: 004c (0x4c)
> 920efa26: 8801db407600 (0x8801db407600)
> a66a1c57:  (0x)
> 602c97d2: 81a8d883 (save_stack+0x43/0xd0)
> 11ee1976: 81a8e191 (kasan_slab_free+0x71/0xc0)
> c84a6163: 81a8bf06 (kfree+0xd6/0x260)
> e47cbeab: 82213878 (selinux_cred_free+0x48/0x70)
> 2d741013: 821fddd8 (security_cred_free+0x48/0x80)
> 4adf7771: 814a82b6 (put_cred_rcu+0x106/0x400)
> 30fd4806: 815e6f2c (rcu_process_callbacks+0xd6c/0x17f0)
> 3b1f46f7: 85c002d7 (__do_softirq+0x2d7/0xb85)
> f227b7b3: 8142fbac (irq_exit+0x1cc/0x200)
> 5a82eab3: 85a05adb (smp_apic_timer_interrupt+0x16b/0x700)
> 055afa4e: 85a01d69 (apic_timer_interrupt+0xa9/0xb0)
> a35590a8: 8135b07e (._mainloop+0x187/0x4ca)
> 179a751c: 86b45498 (rcu_sched_state+0x18/0x1520)
> 0ba49ddf: 8801acaca4c0 (0x8801acaca4c0)
> a404c5c9: 41b58ab3 (0x41b58ab3)
> 1d2af172: 867cd428 (regoff.32610+0x28dca8/0x29dc80)
> 584a687b: 81563440 (print_irqtrace_events+0x270/0x270)
> 6c14677d: 11003b680edd (0x11003b680edd)
> 9dfd46e8: 88271400 (obj_hash+0xebe00/0x100020)
> ba757625: 8801aca0d000 (0x8801aca0d000)
> 6f6bffaa: 8801db407708 (0x8801db407708)
> 0e864f6d: 11003b680edd (0x11003b680edd)
> 005451e3: 8801aca0c7a0 (0x8801aca0c7a0)
> 605a674b: 8801aca0c780 (0x8801aca0c780)
> 5f59b991: 000ebe00 (0xebe00)
> c279fdba: fbfff104e280 (0xfbfff104e280)
> 3f76ca22: 8801aca0d000 (0x8801aca0d000)
> a56ab73c: 88271408 (obj_hash+0xebe08/0x100020)
> ac22805a: 8801db407708 (0x8801db407708)
> f9e3111c: 41b58ab3 (0x41b58ab3)
> f1cc5486: 8681d8f0 (K512_4+0x3e6f0/0xd09a0)
> 64d36645: 825e6230 (free_obj_work+0x690/0x690)
> 2f3949a8: 41b58ab3 (0x41b58ab3)
> 37d72c89:  ...
> bec2a1d3: 81563440 (print_irqtrace_events+0x270/0x270)
> 1b90ad27: 88145240 (console_drivers+0x40/0x40)
> d2c3d196: 8801db407858 (0x8801db407858)
> c4fe609c: 8801acaca4c0 (0x8801acaca4c0)
> 02ee263f: 8801db407798 (0x8801db407798)
> 0be75ad5: 11003b680eef (0x11003b680eef)
> 78f83b59: 8801db407880 (0x8801db407880)
> 43ac401f: 0082 (0x82)
> 6e6e547c: