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
