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

On 5/30/26 15:52, Dan Carpenter wrote:
> 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.
> 
Correct. Thanks for pointing this out.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
[email protected]                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich