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

Ming Lei
2020-05-08T01:54:49Z

On Thu, May 07, 2020 at 07:16:26AM -0700, Christoph Hellwig wrote:
> 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.

OK, not did it because I thought the logic is straightforward.

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

Looks I did miss this function, :-)

Thanks,
Ming