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

Christoph Hellwig
2020-05-07T14:16:30Z

On Thu, May 07, 2020 at 04:52:36PM +0800, Ming Lei wrote:
> for (i = 1; i < ptbl->len; i++) {
> part = rcu_dereference(ptbl->part[i]);
>
> if (part && sector_in_part(part, sector)) {
> + if (!hd_struct_try_get(part))
> + goto exit;

I think this needs the comment that was in blk_account_io_start to
explain the logic. Also no need for the goto, this can just be a break.

> - rcu_assign_pointer(ptbl->last_lookup, NULL);
> kobject_put(part->holder_dir);
> device_del(part_to_dev(part));
>
> @@ -393,6 +402,7 @@ static struct hd_struct *add_partition(struct gendisk *disk, int partno,
> p->nr_sects = len;
> p->partno = partno;
> p->policy = get_disk_ro(disk);
> + p->disk = disk;

Do we really need this pointer? Can't we use part_to_disk in the
free path for some reason?