Re: Maybe inappropriate use BUG_ON() in CONFIG_SLAB_FREELIST_HARDENED

[zerons <zeronsaxm@gmail.com>]

> 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

I did this too. :D

> 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?
> 

Unlike cve-2017-2636 kind of double free bugs, assume that we have
another double free bug that would be stopped by this patch.

Now, the problem is, do we really have this kind of bugs that we could
win the race? I am not sure for now. Here is an example in Jann Horn's
latest article I read days ago. Though this example is not good enough
because the hlist operations may crash the kernel before double kfree(),
we should notice that some codes still can give us a chance.

(https://googleprojectzero.blogspot.com/2020/02/mitigations-are-attack-surface-too.html)
there could be a double free.
================================================================================
  spin_lock_irqsave(&table->pid_map_lock, irqsave_flags);
  hlist_for_each_entry(descr, &table->pid_map[hash_key], pid_map_node) {
    if (pid == descr->task->pid) {
      target_task_descr = descr;
      break;
    }
  }
  spin_unlock_irqrestore(&table->pid_map_lock, irqsave_flags);
================================================================================
The victim object is found in a hlist, and that's all we do in this critical section.
Later, it gets freed.

"As long as the double free happens in a user process context, you can
retry triggering it until you succeed in winning the race to reallocate
the object (without causing slab freelist corruption, as it would have
had happened before SLAB_FREELIST_HARDENED)." Andrey Konovalov said.

This patch does work for cve-2017-2636 case, it is barely impossible to win the
race. My concern is based on an assumption: we do have a double kfree() bug and
we can win the race.

Best regards,