On 11/2/22 19:11, Johannes Thumshirn wrote:
> On 02.11.22 10:44, Damien Le Moal wrote:
>> On 11/2/22 18:28, Johannes Thumshirn wrote:
>>> On 31.10.22 04:00, Damien Le Moal wrote:
>>>> + /*
>>>> + * The only files that have more than one zone are conventional zone
>>>> + * files with aggregated conventional zones, for which the inode zone
>>>> + * size is always larger than the device zone size.
>>>> + */
>>>> + if (zi->i_zone_size > bdev_zone_sectors(sb->s_bdev))
>>>> + nr_zones = zi->i_zone_size >>
>>>> + (sbi->s_zone_sectors_shift + SECTOR_SHIFT);
>>>> +
>>>
>>> I wonder if we should also have a check/assertion like this somewhere:
>>> WARN_ON_ONCE(zi->i_zone_size > bdev_zone_sectors(sb->sbdev) &&
>>> !sbi->s_features & ZONEFS_F_AGGRCNV)
>>
>> Well, this is set when the inode is created on mount. So we could add the
>> check there, but I do not really see the point since we would be checking
>> exactly what we are doing. So the only chance warn ever showing would be
>> memory corruption, but then we'll likely have bigger problems anyway. No ?
>
> Something like this:
>
> From f90acf1ca3f84d37a3bdb570abf89e186697c0d4 Mon Sep 17 00:00:00 2001
> Message-Id: <f90acf1ca3f84d37a3bdb570abf89e186697c0d4.1667383842.git.johannes.thumshirn@xxxxxxx>
> From: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
> Date: Wed, 2 Nov 2022 02:57:35 -0700
> Subject: [PATCH] zonefs: add sanity check for aggregated conventional zones
>
> 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 a warning if this case happens and we do not have the mount option
> set. Also if the -oerror=read-only mount option is set, mark the
> filesystem as read-only.
>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
> ---
> fs/zonefs/super.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
> index 860f0b1032c6..7c0b776a7bc4 100644
> --- a/fs/zonefs/super.c
> +++ b/fs/zonefs/super.c
> @@ -1407,6 +1407,15 @@ 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 (WARN_ON(zi->i_zone_size > bdev_zone_sectors(sb->s_bdev) &&
> + !sbi->s_features & ZONEFS_F_AGGRCNV)) {
> + if ((sbi->s_mount_opts & ZONEFS_MNTOPT_ERRORS_RO) &&
> + !sb_rdonly(sb)) {
> + zonefs_warn(sb, "remounting filesystem read-only\n");
> + sb->s_flags |= SB_RDONLY;
This is during mount. So let's fail the mount...
> + }
> + return -EINVAL;
> + }
>
> zi->i_max_size = min_t(loff_t, MAX_LFS_FILESIZE,
> zone->capacity << SECTOR_SHIFT);
--
Damien Le Moal
Western Digital Research
