[bug report] dm zoned: allocate temporary superblock for tertiary devices

Hello Hannes Reinecke,

Commit 5d2c74f3ddc0 ("dm zoned: allocate temporary superblock for
tertiary devices") from Jun 2, 2020 (linux-next), leads to the
following Smatch static checker warning:

	drivers/md/dm-zoned-metadata.c:1332 dmz_load_sb()
	error: double free of 'sb->mblk' (line 1336)

drivers/md/dm-zoned-metadata.c
    1310         if (zmd->sb_version > 1) {
    1311                 int i;
    1312                 struct dmz_sb *sb;
    1313 
    1314                 sb = kzalloc_obj(struct dmz_sb);
    1315                 if (!sb)
    1316                         return -ENOMEM;
    1317                 for (i = 1; i < zmd->nr_devs; i++) {
    1318                         sb->block = 0;
    1319                         sb->zone = dmz_get(zmd, zmd->dev[i].zone_offset);
    1320                         sb->dev = &zmd->dev[i];
    1321                         if (!dmz_is_meta(sb->zone)) {
    1322                                 dmz_dev_err(sb->dev,
    1323                                             "Tertiary super block zone %u not marked as metadata zone",
    1324                                             sb->zone->id);
    1325                                 ret = -EINVAL;
    1326                                 goto out_kfree;
    1327                         }
    1328                         ret = dmz_get_sb(zmd, sb, i + 1);

This sets sb->mblk on the success path, but if the
dmz_get_sb() allocation fails then sb->mblk is left as-is.

    1329                         if (ret) {
    1330                                 dmz_dev_err(sb->dev,
    1331                                             "Read tertiary super block failed");
--> 1332                                 dmz_free_mblock(zmd, sb->mblk);
                                                              ^^^^^^^^
On the second iteration throug the loop then this is a double free.
I think this dmz_free_mblock() should just be deleted.  If
dmz_get_sb() fails then there shouldn't be anything to free
here.

    1333                                 goto out_kfree;
    1334                         }
    1335                         ret = dmz_check_sb(zmd, sb, true);
    1336                         dmz_free_mblock(zmd, sb->mblk);

On the first iteration then sb->mblk is freed.

    1337                         if (ret == -EINVAL)
    1338                                 goto out_kfree;
    1339                 }
    1340 out_kfree:
    1341                 kfree(sb);
    1342         }
    1343         return ret;
    1344 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter