Re: use-after-free in sctp_do_sm

[Dmitry Vyukov]

On Thu, Dec 3, 2015 at 6:02 PM, Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
> On Thu, Dec 3, 2015 at 7:55 AM, Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
>> On Thu, Dec 3, 2015 at 3:48 PM, Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
>>>>
>>>> No, I don't. But pr_debug always computes its arguments. See no_printk
>>>> in printk.h. So this use-after-free happens for all users.
>>>
>>> Hmm.
>>>
>>> pr_debug() should be a nop unless either DEBUG or CONFIG_DYNAMIC_DEBUG are set
>>>
>>> On our production kernels, pr_debug() is a nop.
>>>
>>> Can you double check ? Thanks !
>>
>>
>> Why should it be nop? no_printk thing in printk.h pretty much
>> explicitly makes it not a nop...
>>
>> Double-checked: debug_post_sfx leads to some generated code:
>>
>>         debug_post_sfx();
>> ffffffff8229f256:       48 8b 85 58 fe ff ff    mov    -0x1a8(%rbp),%rax
>> ffffffff8229f25d:       48 85 c0                test   %rax,%rax
>> ffffffff8229f260:       74 24                   je
>> ffffffff8229f286 <sctp_do_sm+0x176>
>> ffffffff8229f262:       8b b0 a8 00 00 00       mov    0xa8(%rax),%esi
>> ffffffff8229f268:       48 8b 85 60 fe ff ff    mov    -0x1a0(%rbp),%rax
>> ffffffff8229f26f:       44 89 85 74 fe ff ff    mov    %r8d,-0x18c(%rbp)
>> ffffffff8229f276:       48 8b 78 20             mov    0x20(%rax),%rdi
>> ffffffff8229f27a:       e8 71 28 01 00          callq
>> ffffffff822b1af0 <sctp_id2assoc>
>> ffffffff8229f27f:       44 8b 85 74 fe ff ff    mov    -0x18c(%rbp),%r8d
>>
>>         return error;
>> }
>> ffffffff8229f286:       48 81 c4 a0 01 00 00    add    $0x1a0,%rsp
>> ffffffff8229f28d:       44 89 c0                mov    %r8d,%eax
>> ffffffff8229f290:       5b                      pop    %rbx
>> ffffffff8229f291:       41 5c                   pop    %r12
>> ffffffff8229f293:       41 5d                   pop    %r13
>> ffffffff8229f295:       41 5e                   pop    %r14
>> ffffffff8229f297:       41 5f                   pop    %r15
>> ffffffff8229f299:       5d                      pop    %rbp
>> ffffffff8229f29a:       c3                      retq
>
> This is a serious concern, because we let in the past lot of patches
> converting traditional
>
> #ifdef DEBUG
> # define some_hand_coded_ugly_debug()  printk( ...._
> #else
> # define some_hand_coded_ugly_debug()
> #endif
>
> On the premise pr_debug() would be a nop.
>
> It seems it is not always the case. This is a very serious problem.
>
> We probably have hundred of potential bugs, because few people
> actually make sure all debugging stuff is correct,
> like comments can be wrong because they are not updated properly as time flies.


FWIW I enabled CONFIG_DYNAMIC_DEBUG on my fuzzer. Not that it gives
any particular guarantees, but still can catch some of these.



> It is definitely a nop for many cases.
>
> +void eric_test_pr_debug(struct sock *sk)
> +{
> +       if (atomic_read(&sk->sk_omem_alloc))
> +               pr_debug("%s: optmem leakage for sock %p\n",
> +                        __func__, sk);
> +}
>
> ->
>
> 0000000000004740 <eric_test_pr_debug>:
>     4740: e8 00 00 00 00       callq  4745 <eric_test_pr_debug+0x5>
> 4741: R_X86_64_PC32 __fentry__-0x4
>     4745: 55                   push   %rbp
>     4746: 8b 87 24 01 00 00     mov    0x124(%rdi),%eax     //
> atomic_read()  but nothing follows
>     474c: 48 89 e5             mov    %rsp,%rbp
>     474f: 5d                   pop    %rbp
>     4750: c3                   retq
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/