On 23/08/2019 12:10, Naohiro Aota wrote:
[...]
> +int btrfs_get_dev_zone_info(struct btrfs_device *device)
> +{
> +     struct btrfs_zoned_device_info *zone_info = NULL;
> +     struct block_device *bdev = device->bdev;
> +     sector_t nr_sectors = bdev->bd_part->nr_sects;
> +     sector_t sector = 0;
> +     struct blk_zone *zones = NULL;
> +     unsigned int i, nreported = 0, nr_zones;
> +     unsigned int zone_sectors;
> +     int ret;
> +
> +     if (!bdev_is_zoned(bdev))
> +             return 0;
> +
> +     zone_info = kzalloc(sizeof(*zone_info), GFP_KERNEL);
> +     if (!zone_info)
> +             return -ENOMEM;
> +
> +     zone_sectors = bdev_zone_sectors(bdev);
> +     ASSERT(is_power_of_2(zone_sectors));
> +     zone_info->zone_size = (u64)zone_sectors << SECTOR_SHIFT;
> +     zone_info->zone_size_shift = ilog2(zone_info->zone_size);
> +     zone_info->nr_zones = nr_sectors >> ilog2(bdev_zone_sectors(bdev));
> +     if (nr_sectors & (bdev_zone_sectors(bdev) - 1))
> +             zone_info->nr_zones++;
> +
> +     zone_info->seq_zones = kcalloc(BITS_TO_LONGS(zone_info->nr_zones),
> +                                    sizeof(*zone_info->seq_zones),
> +                                    GFP_KERNEL);
> +     if (!zone_info->seq_zones) {
> +             ret = -ENOMEM;
> +             goto out;
> +     }
> +
> +     zone_info->empty_zones = kcalloc(BITS_TO_LONGS(zone_info->nr_zones),
> +                                      sizeof(*zone_info->empty_zones),
> +                                      GFP_KERNEL);
> +     if (!zone_info->empty_zones) {
> +             ret = -ENOMEM;
> +             goto out;
> +     }
> +
> +
> +     zones = kcalloc(BTRFS_REPORT_NR_ZONES,
> +                     sizeof(struct blk_zone), GFP_KERNEL);
> +     if (!zones)
> +             return -ENOMEM;

Won't this will leak zone_info, zone_info->seq_zones and
zone_info->empty_zones.


[...]

> +out:
> +     kfree(zones);
> +
> +     if (ret) {
> +             kfree(zone_info->seq_zones);
> +             kfree(zone_info->empty_zones);
> +             kfree(zone_info);
> +     }

Which is why I think it would be more clear to have:
free_zones:
        kfree(zones);
free_zi_emp_zones:
        kfree(zone_info->empty_zones);
free_zi_seq_zones:
        kfree(zone_info->seq_zones);
free_zi:
        kfree(zone_info);



-- 
Johannes Thumshirn                            SUSE Labs Filesystems
[email protected]                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Reply via email to