Re: Maybe inappropriate use BUG_ON() in CONFIG_SLAB_FREELIST_HARDENED

[Alexander Popov <alex.popov@linux.com>]

Hello!

Thanks for adding me to this discussion.
Let me also add Jann Horn.

On 17.02.2020 18:15, Andrey Konovalov wrote:
> On Thu, Feb 13, 2020 at 4:43 PM zerons <zeronsaxm@gmail.com> wrote:
>> In slub.c(https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/mm/slub.c?h=v5.4.19#n305),
>> for SLAB_FREELIST_HARDENED, an extra detection of the double free bug has been added.
>>
>> This patch can (maybe only) detect something like this: kfree(a) kfree(a).
>> However, it does nothing when another process calls kfree(b) between the two kfree above.

Yes, that's correct.

>> The problem is, if the panic_on_oops option is not set(Ubuntu 16.04/18.04 default option),
>> for a bug which kfree an object twice in a row, if another process can preempt the process
>> triggered this bug and then call kmalloc() to get the object, the patch doesn't work.

In theory, that is true.

However, let me show a counterexample from practice.

I developed this check after I exploited CVE-2017-2636 (race condition causing
double free). Please see the detailed write-up about the exploit:
https://a13xp0p0v.github.io/2017/03/24/CVE-2017-2636.html

There was a linked list with data buffers, and one of these buffers was added to
the list twice. Double free happened when the driver cleaned up its resources
and freed the buffers in this list. So double kfree() happened quite close to
each other.

I spent a lot of time trying to insert some kmalloc() between these kfree(), but
didn't succeed. That is difficult because slab caches are per-CPU, and heap
spray on other CPUs doesn't overwrite the needed kernel address.

The vulnerable kernel task didn't call scheduler between double kfree(). I
didn't manage to preempt it. But I solved that trouble by spraying _after_
double kfree().

>> Without this extra detection, the kernel could be unstable while the attacker
>> trying to do the race.

Could you bring more details? Which kind of instability do you mean?

When I did heap spray with sk_buffs after double kfree(), I got two sk_buff
items with sk_buff.head pointing to the same memory. Receiving one sk_buff
created use-after-free on another one. That is how double free turns into
use-after-free.

The check which we discuss now breaks that method.

Best regards,
Alexander