Re: [PATCH] f2fs: introduce on-disk layout version checking functionality

[Jaegeuk Kim <jaegeuk@kernel.org>]

Hi Slava,

On Thu, May 19, 2016 at 10:46:06AM -0700, Viacheslav Dubeyko wrote:
...
>  
> +#ifdef CONFIG_F2FS_16TB_VOLUME_SUPPORT
> +#define F2FS_MAX_SUPP_MAJOR_VERSION		(2)
> +#define F2FS_MIN_16TB_VOLUME_SUPPORT_VERSION	(2)
> +#else
> +#define F2FS_MAX_SUPP_MAJOR_VERSION		(1)
> +#endif
> +
...
>  
> +static int f2fs_check_version_and_features(struct super_block *sb,
> +					   struct f2fs_super_block *raw_super)
> +{
> +	u16 major_ver = le16_to_cpu(raw_super->major_ver);
> +	u32 feature = le32_to_cpu(raw_super->feature);
> +
> +	if (major_ver > F2FS_MAX_SUPP_MAJOR_VERSION) {

This means, for example, f2fs driver in v4.8 will deny to mount a partition
formatted by mkfs.f2fs v3.x, which doesn't make sense, IIUC.

As Christoph mentioned, how about checking the feature only like this?

1. if the feature is ON,
 - go 64 bits   , when compiled w/  F2FS_MIN_16TB_VOLUME_SUPPORT
 - fail to mount, when compiled w/o F2FS_MIN_16TB_VOLUME_SUPPORT

2. if the feature is OFF,
 - fail to mount, when compiled w/  F2FS_MIN_16TB_VOLUME_SUPPORT
 - go 32 bits   , when compiled w/o F2FS_MIN_16TB_VOLUME_SUPPORT

Thoughts?

Thanks,

> +		f2fs_msg(sb, KERN_CRIT,
> +			 "Failed to mount volume: "
> +			 "major version %u, max supported version %u",
> +			 major_ver,
> +			 F2FS_MAX_SUPP_MAJOR_VERSION);
> +		return -EOPNOTSUPP;
> +	}
> +
> +#ifdef CONFIG_F2FS_16TB_VOLUME_SUPPORT
> +
> +	if (major_ver < F2FS_MIN_16TB_VOLUME_SUPPORT_VERSION) {
> +		if (feature & F2FS_FEATURE_16TB_SUPPORT) {
> +			f2fs_msg(sb, KERN_CRIT,
> +				 "Failed to mount corrupted volume. "
> +				 "Please, check the volume by FSCK utility.");
> +			return -EOPNOTSUPP;
> +		}
> +	} else {
> +		if (!(feature & F2FS_FEATURE_16TB_SUPPORT)) {
> +			f2fs_msg(sb, KERN_CRIT,
> +				 "Failed to mount corrupted volume. "
> +				 "Please, check the volume by FSCK utility.");
> +			return -EOPNOTSUPP;
> +		}
> +	}
> +
> +#else
> +
> +	if (feature & F2FS_FEATURE_16TB_SUPPORT) {
> +		f2fs_msg(sb, KERN_CRIT,
> +			 "Failed to mount corrupted volume. "
> +			 "Please, check the volume by FSCK utility.");
> +		return -EOPNOTSUPP;
> +	}
> +
> +#endif
> +
> +	return 0;
> +}
> +
>  static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>  {
>  	struct f2fs_sb_info *sbi;
> @@ -1360,6 +1407,10 @@ try_onemore:
>  	if (err)
>  		goto free_sbi;
>  
> +	err = f2fs_check_version_and_features(sb, raw_super);
> +	if (err)
> +		goto free_sbi;
> +
>  	sb->s_fs_info = sbi;
>  	default_options(sbi);
>  	/* parse mount options */
> -- 
> 1.9.1
> 
> 

------------------------------------------------------------------------------
Mobile security can be enabling, not merely restricting. Employees who
bring their own devices (BYOD) to work are irked by the imposition of MDM
restrictions. Mobile Device Manager Plus allows you to control only the
apps on BYO-devices by containerizing them, leaving personal data untouched!
https://ad.doubleclick.net/ddm/clk/304595813;131938128;j