| Message ID | fe0e42b533442766d941740697cd8e33fcad99ad.1668413972.git.johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] zonefs: add sanity check for aggregated conventional zones | expand |
On 11/14/22 17:19, Johannes Thumshirn wrote: > When initializing a file inode, check if the zone's size if bigger than > the number of device zone sectors. This can only be the case if we mount > the filesystem with the -oaggr_cnv mount option. > > Emit an error in case this case happens and fail the mount. > > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > --- > Changes to v1: > - Change IS_ERR_OR_NULL() to IS_ERR() (Damien) > - Add parentheses around 'sbi->s_features & ZONEFS_F_AGGRCNV' (Dan) > --- > fs/zonefs/super.c | 23 ++++++++++++++++------- > 1 file changed, 16 insertions(+), 7 deletions(-) > > diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c > index 860f0b1032c6..143bd018acd2 100644 > --- a/fs/zonefs/super.c > +++ b/fs/zonefs/super.c > @@ -1407,6 +1407,14 @@ static int zonefs_init_file_inode(struct inode *inode, struct blk_zone *zone, > zi->i_ztype = type; > zi->i_zsector = zone->start; > zi->i_zone_size = zone->len << SECTOR_SHIFT; > + if (zi->i_zone_size > bdev_zone_sectors(sb->s_bdev) << SECTOR_SHIFT && > + !(sbi->s_features & ZONEFS_F_AGGRCNV)) { > + zonefs_err(sb, > + "zone size %llu doesn't match device's zone sectors %llu\n", > + zi->i_zone_size, > + bdev_zone_sectors(sb->s_bdev) << SECTOR_SHIFT); > + return -EINVAL; > + } > > zi->i_max_size = min_t(loff_t, MAX_LFS_FILESIZE, > zone->capacity << SECTOR_SHIFT); > @@ -1456,11 +1464,11 @@ static struct dentry *zonefs_create_inode(struct dentry *parent, > struct inode *dir = d_inode(parent); > struct dentry *dentry; > struct inode *inode; > - int ret; > + int ret = -ENOMEM; > > dentry = d_alloc_name(parent, name); > if (!dentry) > - return NULL; > + return ERR_PTR(ret); > > inode = new_inode(parent->d_sb); > if (!inode) > @@ -1485,7 +1493,7 @@ static struct dentry *zonefs_create_inode(struct dentry *parent, > dput: > dput(dentry); > > - return NULL; > + return ERR_PTR(ret); > } > > struct zonefs_zone_data { > @@ -1523,8 +1531,8 @@ static int zonefs_create_zgroup(struct zonefs_zone_data *zd, > zgroup_name = "seq"; > > dir = zonefs_create_inode(sb->s_root, zgroup_name, NULL, type); > - if (!dir) { > - ret = -ENOMEM; > + if (IS_ERR(dir)) { > + ret = PTR_ERR(dir); > goto free; > } > > @@ -1570,8 +1578,9 @@ static int zonefs_create_zgroup(struct zonefs_zone_data *zd, > * Use the file number within its group as file name. > */ > snprintf(file_name, ZONEFS_NAME_MAX - 1, "%u", n); > - if (!zonefs_create_inode(dir, file_name, zone, type)) { > - ret = -ENOMEM; > + dir = zonefs_create_inode(dir, file_name, zone, type); This one is for file inodes but you are overwriting dir, which will totally mess things up for the next file inode to create. > + if (IS_ERR(dir)) { > + ret = PTR_ERR(dir); > goto free; > } >
On 14.11.22 12:44, Damien Le Moal wrote >> @@ -1570,8 +1578,9 @@ static int zonefs_create_zgroup(struct zonefs_zone_data *zd, >> * Use the file number within its group as file name. >> */ >> snprintf(file_name, ZONEFS_NAME_MAX - 1, "%u", n); >> - if (!zonefs_create_inode(dir, file_name, zone, type)) { >> - ret = -ENOMEM; >> + dir = zonefs_create_inode(dir, file_name, zone, type); > > This one is for file inodes but you are overwriting dir, which will > totally mess things up for the next file inode to create. > >> + if (IS_ERR(dir)) { >> + ret = PTR_ERR(dir); >> goto free; >> } >> > Indeed I'm sorry. Will send a v3 ASAP.
diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c index 860f0b1032c6..143bd018acd2 100644 --- a/fs/zonefs/super.c +++ b/fs/zonefs/super.c @@ -1407,6 +1407,14 @@ static int zonefs_init_file_inode(struct inode *inode, struct blk_zone *zone, zi->i_ztype = type; zi->i_zsector = zone->start; zi->i_zone_size = zone->len << SECTOR_SHIFT; + if (zi->i_zone_size > bdev_zone_sectors(sb->s_bdev) << SECTOR_SHIFT && + !(sbi->s_features & ZONEFS_F_AGGRCNV)) { + zonefs_err(sb, + "zone size %llu doesn't match device's zone sectors %llu\n", + zi->i_zone_size, + bdev_zone_sectors(sb->s_bdev) << SECTOR_SHIFT); + return -EINVAL; + } zi->i_max_size = min_t(loff_t, MAX_LFS_FILESIZE, zone->capacity << SECTOR_SHIFT); @@ -1456,11 +1464,11 @@ static struct dentry *zonefs_create_inode(struct dentry *parent, struct inode *dir = d_inode(parent); struct dentry *dentry; struct inode *inode; - int ret; + int ret = -ENOMEM; dentry = d_alloc_name(parent, name); if (!dentry) - return NULL; + return ERR_PTR(ret); inode = new_inode(parent->d_sb); if (!inode) @@ -1485,7 +1493,7 @@ static struct dentry *zonefs_create_inode(struct dentry *parent, dput: dput(dentry); - return NULL; + return ERR_PTR(ret); } struct zonefs_zone_data { @@ -1523,8 +1531,8 @@ static int zonefs_create_zgroup(struct zonefs_zone_data *zd, zgroup_name = "seq"; dir = zonefs_create_inode(sb->s_root, zgroup_name, NULL, type); - if (!dir) { - ret = -ENOMEM; + if (IS_ERR(dir)) { + ret = PTR_ERR(dir); goto free; } @@ -1570,8 +1578,9 @@ static int zonefs_create_zgroup(struct zonefs_zone_data *zd, * Use the file number within its group as file name. */ snprintf(file_name, ZONEFS_NAME_MAX - 1, "%u", n); - if (!zonefs_create_inode(dir, file_name, zone, type)) { - ret = -ENOMEM; + dir = zonefs_create_inode(dir, file_name, zone, type); + if (IS_ERR(dir)) { + ret = PTR_ERR(dir); goto free; }
When initializing a file inode, check if the zone's size if bigger than the number of device zone sectors. This can only be the case if we mount the filesystem with the -oaggr_cnv mount option. Emit an error in case this case happens and fail the mount. Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> --- Changes to v1: - Change IS_ERR_OR_NULL() to IS_ERR() (Damien) - Add parentheses around 'sbi->s_features & ZONEFS_F_AGGRCNV' (Dan) --- fs/zonefs/super.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)