Re: [PATCH 1/4] block: fix use-after-free on cached last_lookup partition

Bart Van Assche
2020-02-21T04:03:56Z

On 2020-01-08 22:21, Ming Lei wrote:
> delete_partition() clears the cached last_lookup partition. However
> the .last_lookup cache may be overwritten by one IO path after
> it is cleared from delete_partition(). Then another IO path may
> use the cached deleting partition after __delete_partition() is
> called, then use-after-free is triggered on the cached partition.
>
> Fixes the issue by the following approach:
>
> 1) always get the partition's refcount via hd_struct_try_get() before
> setting .last_lookup
>
> 2) move clearing .last_lookup from delete_partition() to
> __delete_partition() which is release handle of the partition's
> percpu-refcount, so that no IO path can overwrite .last_lookup after it
> is cleared in __delete_partition().
>
> It is one candidate approach of Yufen's patch[1] which adds overhead
> in fast path by indirect lookup which may introduce one extra cacheline
> in IO path. Also this patch relies on percpu-refcount's protection, and
> it is easier to understand and verify.
>
> [1] https://lore.kernel.org/linux-block/20200109013551.GB9655@ming.t460p/T/#t

Hi Ming,

disk_map_sector_rcu() is called from the I/O path only and hence with
q->q_usage_counter > 0. Has it been considered to freeze disk->queue
from delete_partition() before deleting a partition and unfreezing
disk->queue after partition deletion has finished? Would that approach
allow to eliminate partition reference counting and thereby improve the
performance of the hot path?

Thanks,

Bart.