[PATCH v3] xfs: deprecate the V4 format

[PATCH v3] xfs: deprecate the V4 format

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

The V4 filesystem format contains known weaknesses in the on-disk format
that make metadata verification diffiult.  In addition, the format will
does not support dates past 2038 and will not be upgraded to do so.
Therefore, we should start the process of retiring the old format to
close off attack surfaces and to encourage users to migrate onto V5.

Therefore, make XFS V4 support a configurable option.  For the first
period it will be default Y in case some distributors want to withdraw
support early; for the second period it will be default N so that anyone
who wishes to continue support can do so; and after that, support will
be removed from the kernel.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v3: be a little more helpful about old xfsprogs and warn more loudly
about deprecation
v2: define what is a V4 filesystem, update the administrator guide
---
 Documentation/admin-guide/xfs.rst |   23 +++++++++++++++++++++++
 fs/xfs/Kconfig                    |   24 ++++++++++++++++++++++++
 fs/xfs/xfs_super.c                |   13 +++++++++++++
 3 files changed, 60 insertions(+)

diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
index f461d6c33534..b6deea9ec073 100644
--- a/Documentation/admin-guide/xfs.rst
+++ b/Documentation/admin-guide/xfs.rst
@@ -210,6 +210,28 @@ When mounting an XFS filesystem, the following options are accepted.
 	inconsistent namespace presentation during or after a
 	failover event.
 
+Deprecation of V4 Format
+========================
+
+The V4 filesystem format lacks certain features that are supported by
+the V5 format, such as metadata checksumming, strengthened metadata
+verification, and the ability to store timestamps past the year 2038.
+Because of this, the V4 format is deprecated.  All users should upgrade
+by backing up their files, reformatting, and restoring from the backup.
+
+Administrators and users can detect a V4 filesystem by running xfs_info
+against a filesystem mountpoint and checking for a string beginning with
+"crc=".  If no such string is found, please upgrade xfsprogs to the
+latest version and try again.
+
+The deprecation will take place in two parts.  Support for mounting V4
+filesystems can now be disabled at kernel build time via Kconfig option.
+The option will default to yes until September 2025, at which time it
+will be changed to default to no.  In September 2030, support will be
+removed from the codebase entirely.
+
+Note: Distributors may choose to withdraw V4 format support earlier than
+the dates listed above.
 
 Deprecated Mount Options
 ========================
@@ -217,6 +239,7 @@ Deprecated Mount Options
 ===========================     ================
   Name				Removal Schedule
 ===========================     ================
+Mounting with V4 filesystem     September 2030
 ===========================     ================
 
 
diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig
index e685299eb3d2..5422227e9e93 100644
--- a/fs/xfs/Kconfig
+++ b/fs/xfs/Kconfig
@@ -22,6 +22,30 @@ config XFS_FS
 	  system of your root partition is compiled as a module, you'll need
 	  to use an initial ramdisk (initrd) to boot.
 
+config XFS_SUPPORT_V4
+	bool "Support deprecated V4 (crc=0) format"
+	default y
+	help
+	  The V4 filesystem format lacks certain features that are supported
+	  by the V5 format, such as metadata checksumming, strengthened
+	  metadata verification, and the ability to store timestamps past the
+	  year 2038.  Because of this, the V4 format is deprecated.  All users
+	  should upgrade by backing up their files, reformatting, and restoring
+	  from the backup.
+
+	  Administrators and users can detect a V4 filesystem by running
+	  xfs_info against a filesystem mountpoint and checking for a string
+	  beginning with "crc=".  If the string "crc=0" is found, the
+	  filesystem is a V4 filesystem.  If no such string is found, please
+	  upgrade xfsprogs to the latest version and try again.
+
+	  This option will become default N in September 2025.  Support for the
+	  V4 format will be removed entirely in September 2030.  Distributors
+	  can say N here to withdraw support earlier.
+
+	  To continue supporting the old V4 format (crc=0), say Y.
+	  To close off an attack surface, say N.
+
 config XFS_QUOTA
 	bool "XFS Quota support"
 	depends on XFS_FS
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index febd61ba071d..e2250c6d7251 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1451,6 +1451,19 @@ xfs_fc_fill_super(
 	if (error)
 		goto out_free_sb;
 
+	/* V4 support is undergoing deprecation. */
+	if (!xfs_sb_version_hascrc(&mp->m_sb)) {
+#ifdef CONFIG_XFS_SUPPORT_V4
+		xfs_warn_once(mp,
+	"Deprecated V4 format (crc=0) will not be supported after September 2030.");
+#else
+		xfs_warn(mp,
+	"Deprecated V4 format (crc=0) not supported by kernel.");
+		error = -EINVAL;
+		goto out_free_sb;
+#endif
+	}
+
 	/*
 	 * XFS block mappings use 54 bits to store the logical block offset.
 	 * This should suffice to handle the maximum file size that the VFS

Re: [PATCH v3] xfs: deprecate the V4 format

[Christoph Hellwig]

On Fri, Sep 11, 2020 at 09:43:11AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The V4 filesystem format contains known weaknesses in the on-disk format
> that make metadata verification diffiult.  In addition, the format will
> does not support dates past 2038 and will not be upgraded to do so.
> Therefore, we should start the process of retiring the old format to
> close off attack surfaces and to encourage users to migrate onto V5.
> 
> Therefore, make XFS V4 support a configurable option.  For the first
> period it will be default Y in case some distributors want to withdraw
> support early; for the second period it will be default N so that anyone
> who wishes to continue support can do so; and after that, support will
> be removed from the kernel.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> v3: be a little more helpful about old xfsprogs and warn more loudly
> about deprecation
> v2: define what is a V4 filesystem, update the administrator guide

Whie this patch itself looks good, I think the ifdef as is is rather
silly as it just prevents mounting v4 file systems without reaping any
benefits from that.

So at very least we should add a little helper like this:

static inline bool xfs_sb_is_v4(truct xfs_sb *sbp)
{
	if (IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
		return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4;
	return false;
}

and use it in all the feature test macros to let the compile eliminate
all the dead code.

Re: [PATCH v3] xfs: deprecate the V4 format

[Eric Sandeen]

On 9/14/20 2:29 AM, Christoph Hellwig wrote:
> On Fri, Sep 11, 2020 at 09:43:11AM -0700, Darrick J. Wong wrote:
>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>
>> The V4 filesystem format contains known weaknesses in the on-disk format
>> that make metadata verification diffiult.  In addition, the format will
>> does not support dates past 2038 and will not be upgraded to do so.
>> Therefore, we should start the process of retiring the old format to
>> close off attack surfaces and to encourage users to migrate onto V5.
>>
>> Therefore, make XFS V4 support a configurable option.  For the first
>> period it will be default Y in case some distributors want to withdraw
>> support early; for the second period it will be default N so that anyone
>> who wishes to continue support can do so; and after that, support will
>> be removed from the kernel.
>>
>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>> ---
>> v3: be a little more helpful about old xfsprogs and warn more loudly
>> about deprecation
>> v2: define what is a V4 filesystem, update the administrator guide
> 
> Whie this patch itself looks good, I think the ifdef as is is rather
> silly as it just prevents mounting v4 file systems without reaping any
> benefits from that.
> 
> So at very least we should add a little helper like this:
> 
> static inline bool xfs_sb_is_v4(truct xfs_sb *sbp)
> {
> 	if (IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
> 		return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4;
> 	return false;
> }
> 
> and use it in all the feature test macros to let the compile eliminate
> all the dead code.
> 

Makes sense, I think - something like this?

xfs: short-circuit version tests if V4 is disabled at compile time

Replace open-coded checks for == XFS_SB_VERSION_[45] with helpers
which can be compiled away if CONFIG_XFS_SUPPORT_V4 is disabled.

Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

NB: this is compile-tested only

Honestly I'd like to replace lots of the has_crc() checks with is_v5()
as well, unless the test is specifically related to CRC use. *shrug*


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 31b7ece985bb..18b187e38017 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -283,6 +283,23 @@ typedef struct xfs_dsb {
 /*
  * The first XFS version we support is a v4 superblock with V2 directories.
  */
+
+static inline bool xfs_sb_version_is_v4(struct xfs_sb *sbp)
+{
+	if (!IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
+		return false;
+
+	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4;
+}
+
+static inline bool xfs_sb_version_is_v5(struct xfs_sb *sbp)
+{
+	if (!IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
+		return true;
+
+	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
+}
+
 static inline bool xfs_sb_good_v4_features(struct xfs_sb *sbp)
 {
 	if (!(sbp->sb_versionnum & XFS_SB_VERSION_DIRV2BIT))
@@ -301,9 +318,9 @@ static inline bool xfs_sb_good_v4_features(struct xfs_sb *sbp)
 
 static inline bool xfs_sb_good_version(struct xfs_sb *sbp)
 {
-	if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5)
+	if (xfs_sb_version_is_v5(sbp))
 		return true;
-	if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4)
+	if (xfs_sb_version_is_v4(sbp))
 		return xfs_sb_good_v4_features(sbp);
 	return false;
 }
@@ -344,7 +361,7 @@ static inline void xfs_sb_version_addquota(struct xfs_sb *sbp)
 
 static inline bool xfs_sb_version_hasalign(struct xfs_sb *sbp)
 {
-	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 ||
+	return (xfs_sb_version_is_v5(sbp) ||
 		(sbp->sb_versionnum & XFS_SB_VERSION_ALIGNBIT));
 }
 
@@ -355,7 +372,7 @@ static inline bool xfs_sb_version_hasdalign(struct xfs_sb *sbp)
 
 static inline bool xfs_sb_version_haslogv2(struct xfs_sb *sbp)
 {
-	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 ||
+	return xfs_sb_version_is_v5(sbp) ||
 	       (sbp->sb_versionnum & XFS_SB_VERSION_LOGV2BIT);
 }
 
@@ -371,7 +388,7 @@ static inline bool xfs_sb_version_hasasciici(struct xfs_sb *sbp)
 
 static inline bool xfs_sb_version_hasmorebits(struct xfs_sb *sbp)
 {
-	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 ||
+	return xfs_sb_version_is_v5(sbp) ||
 	       (sbp->sb_versionnum & XFS_SB_VERSION_MOREBITSBIT);
 }
 
@@ -380,14 +397,14 @@ static inline bool xfs_sb_version_hasmorebits(struct xfs_sb *sbp)
  */
 static inline bool xfs_sb_version_haslazysbcount(struct xfs_sb *sbp)
 {
-	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) ||
+	return (xfs_sb_version_is_v5(sbp)) ||
 	       (xfs_sb_version_hasmorebits(sbp) &&
 		(sbp->sb_features2 & XFS_SB_VERSION2_LAZYSBCOUNTBIT));
 }
 
 static inline bool xfs_sb_version_hasattr2(struct xfs_sb *sbp)
 {
-	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) ||
+	return (xfs_sb_version_is_v5(sbp)) ||
 	       (xfs_sb_version_hasmorebits(sbp) &&
 		(sbp->sb_features2 & XFS_SB_VERSION2_ATTR2BIT));
 }
@@ -407,7 +424,7 @@ static inline void xfs_sb_version_removeattr2(struct xfs_sb *sbp)
 
 static inline bool xfs_sb_version_hasprojid32bit(struct xfs_sb *sbp)
 {
-	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) ||
+	return (xfs_sb_version_is_v5(sbp)) ||
 	       (xfs_sb_version_hasmorebits(sbp) &&
 		(sbp->sb_features2 & XFS_SB_VERSION2_PROJID32BIT));
 }
@@ -494,7 +511,7 @@ xfs_sb_has_incompat_log_feature(
  */
 static inline bool xfs_sb_version_hascrc(struct xfs_sb *sbp)
 {
-	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
+	return xfs_sb_version_is_v5(sbp);
 }
 
 /*
@@ -503,7 +520,7 @@ static inline bool xfs_sb_version_hascrc(struct xfs_sb *sbp)
  */
 static inline bool xfs_sb_version_has_v3inode(struct xfs_sb *sbp)
 {
-	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
+	return xfs_sb_version_is_v5(sbp);
 }
 
 static inline bool xfs_dinode_good_version(struct xfs_sb *sbp,
@@ -516,12 +533,12 @@ static inline bool xfs_dinode_good_version(struct xfs_sb *sbp,
 
 static inline bool xfs_sb_version_has_pquotino(struct xfs_sb *sbp)
 {
-	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
+	return xfs_sb_version_is_v5(sbp);
 }
 
 static inline int xfs_sb_version_hasftype(struct xfs_sb *sbp)
 {
-	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
+	return (xfs_sb_version_is_v5(sbp) &&
 		xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_FTYPE)) ||
 	       (xfs_sb_version_hasmorebits(sbp) &&
 		 (sbp->sb_features2 & XFS_SB_VERSION2_FTYPE));
@@ -529,13 +546,13 @@ static inline int xfs_sb_version_hasftype(struct xfs_sb *sbp)
 
 static inline bool xfs_sb_version_hasfinobt(xfs_sb_t *sbp)
 {
-	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) &&
+	return (xfs_sb_version_is_v5(sbp)) &&
 		(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_FINOBT);
 }
 
 static inline bool xfs_sb_version_hassparseinodes(struct xfs_sb *sbp)
 {
-	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
+	return xfs_sb_version_is_v5(sbp) &&
 		xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_SPINODES);
 }
 
@@ -547,19 +564,19 @@ static inline bool xfs_sb_version_hassparseinodes(struct xfs_sb *sbp)
  */
 static inline bool xfs_sb_version_hasmetauuid(struct xfs_sb *sbp)
 {
-	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) &&
+	return (xfs_sb_version_is_v5(sbp)) &&
 		(sbp->sb_features_incompat & XFS_SB_FEAT_INCOMPAT_META_UUID);
 }
 
 static inline bool xfs_sb_version_hasrmapbt(struct xfs_sb *sbp)
 {
-	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) &&
+	return (xfs_sb_version_is_v5(sbp)) &&
 		(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_RMAPBT);
 }
 
 static inline bool xfs_sb_version_hasreflink(struct xfs_sb *sbp)
 {
-	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
+	return xfs_sb_version_is_v5(sbp) &&
 		(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_REFLINK);
 }
 
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index ae9aaf1f34bf..fcd1e674be73 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -97,7 +97,7 @@ xfs_validate_sb_read(
 	struct xfs_mount	*mp,
 	struct xfs_sb		*sbp)
 {
-	if (XFS_SB_VERSION_NUM(sbp) != XFS_SB_VERSION_5)
+	if (!xfs_sb_version_is_v5(sbp))
 		return 0;
 
 	/*
@@ -164,7 +164,7 @@ xfs_validate_sb_write(
 		return -EFSCORRUPTED;
 	}
 
-	if (XFS_SB_VERSION_NUM(sbp) != XFS_SB_VERSION_5)
+	if (!xfs_sb_version_is_v5(sbp))
 		return 0;
 
 	/*
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index e2ec91b2d0f4..503fc6baabaa 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3420,7 +3420,7 @@ xlog_recover(
 		 * (e.g. unsupported transactions, then simply reject the
 		 * attempt at recovery before touching anything.
 		 */
-		if (XFS_SB_VERSION_NUM(&log->l_mp->m_sb) == XFS_SB_VERSION_5 &&
+		if (xfs_sb_version_is_v5(&log->l_mp->m_sb) &&
 		    xfs_sb_has_incompat_log_feature(&log->l_mp->m_sb,
 					XFS_SB_FEAT_INCOMPAT_LOG_UNKNOWN)) {
 			xfs_warn(log->l_mp,
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 501b9e14ce38..fc44d9891f79 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1504,7 +1504,7 @@ xfs_fc_fill_super(
 	set_posix_acl_flag(sb);
 
 	/* version 5 superblocks support inode version counters. */
-	if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
+	if (xfs_sb_version_is_v5(&mp->m_sb))
 		sb->s_flags |= SB_I_VERSION;
 
 	if (mp->m_flags & XFS_MOUNT_DAX_ALWAYS) {
@@ -1622,7 +1622,7 @@ xfs_remount_rw(
 		return -EINVAL;
 	}
 
-	if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
+	if (xfs_sb_version_is_v5(&mp->m_sb) &&
 	    xfs_sb_has_ro_compat_feature(sbp, XFS_SB_FEAT_RO_COMPAT_UNKNOWN)) {
 		xfs_warn(mp,
 	"ro->rw transition prohibited on unknown (0x%x) ro-compat filesystem",
@@ -1735,7 +1735,7 @@ xfs_fc_reconfigure(
 	int			error;
 
 	/* version 5 superblocks always support version counters. */
-	if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
+	if (xfs_sb_version_is_v5(&mp->m_sb))
 		fc->sb_flags |= SB_I_VERSION;
 
 	error = xfs_fc_validate_params(new_mp);

Re: [PATCH v3] xfs: deprecate the V4 format

[Eric Sandeen]

On 9/14/20 2:48 PM, Eric Sandeen wrote:
> On 9/14/20 2:29 AM, Christoph Hellwig wrote:
>> On Fri, Sep 11, 2020 at 09:43:11AM -0700, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> The V4 filesystem format contains known weaknesses in the on-disk format
>>> that make metadata verification diffiult.  In addition, the format will
>>> does not support dates past 2038 and will not be upgraded to do so.
>>> Therefore, we should start the process of retiring the old format to
>>> close off attack surfaces and to encourage users to migrate onto V5.
>>>
>>> Therefore, make XFS V4 support a configurable option.  For the first
>>> period it will be default Y in case some distributors want to withdraw
>>> support early; for the second period it will be default N so that anyone
>>> who wishes to continue support can do so; and after that, support will
>>> be removed from the kernel.
>>>
>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>> ---
>>> v3: be a little more helpful about old xfsprogs and warn more loudly
>>> about deprecation
>>> v2: define what is a V4 filesystem, update the administrator guide
>> Whie this patch itself looks good, I think the ifdef as is is rather
>> silly as it just prevents mounting v4 file systems without reaping any
>> benefits from that.
>>
>> So at very least we should add a little helper like this:
>>
>> static inline bool xfs_sb_is_v4(truct xfs_sb *sbp)
>> {
>> 	if (IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
>> 		return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4;
>> 	return false;
>> }
>>
>> and use it in all the feature test macros to let the compile eliminate
>> all the dead code.
>>
> Makes sense, I think - something like this?
> 
> xfs: short-circuit version tests if V4 is disabled at compile time
> 
> Replace open-coded checks for == XFS_SB_VERSION_[45] with helpers
> which can be compiled away if CONFIG_XFS_SUPPORT_V4 is disabled.
> 
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> NB: this is compile-tested only
> 
> Honestly I'd like to replace lots of the has_crc() checks with is_v5()
> as well, unless the test is specifically related to CRC use. *shrug*
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index 31b7ece985bb..18b187e38017 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -283,6 +283,23 @@ typedef struct xfs_dsb {
>  /*
>   * The first XFS version we support is a v4 superblock with V2 directories.
>   */
> +
> +static inline bool xfs_sb_version_is_v4(struct xfs_sb *sbp)
> +{
> +	if (!IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
> +		return false;
> +
> +	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4;
> +}
> +
> +static inline bool xfs_sb_version_is_v5(struct xfs_sb *sbp)
> +{
> +	if (!IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
> +		return true;
> +
> +	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
> +}

Oh, I suppose the tests in the mount path would then need to be open-coded,
won't they.  After we've gotten past that then we can assume that we only have
V5 if we've mounted successfully ....

-Eric

Re: [PATCH v3] xfs: deprecate the V4 format

[Darrick J. Wong]

On Mon, Sep 14, 2020 at 08:29:09AM +0100, Christoph Hellwig wrote:
> On Fri, Sep 11, 2020 at 09:43:11AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > The V4 filesystem format contains known weaknesses in the on-disk format
> > that make metadata verification diffiult.  In addition, the format will
> > does not support dates past 2038 and will not be upgraded to do so.
> > Therefore, we should start the process of retiring the old format to
> > close off attack surfaces and to encourage users to migrate onto V5.
> > 
> > Therefore, make XFS V4 support a configurable option.  For the first
> > period it will be default Y in case some distributors want to withdraw
> > support early; for the second period it will be default N so that anyone
> > who wishes to continue support can do so; and after that, support will
> > be removed from the kernel.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > v3: be a little more helpful about old xfsprogs and warn more loudly
> > about deprecation
> > v2: define what is a V4 filesystem, update the administrator guide
> 
> Whie this patch itself looks good, I think the ifdef as is is rather
> silly as it just prevents mounting v4 file systems without reaping any
> benefits from that.
> 
> So at very least we should add a little helper like this:
> 
> static inline bool xfs_sb_is_v4(truct xfs_sb *sbp)
> {
> 	if (IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
> 		return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4;
> 	return false;
> }
> 
> and use it in all the feature test macros to let the compile eliminate
> all the dead code.

Oh, wait, you meant as a means for future patches to make various bits
of code disappear, not just as a weird one-off thing for this particular
patch?

I mean... maybe we should just stuff that into the hascrc predicate,
like Eric sort of implied on irc.  Hmm, I'll look into that.

--D

Re: [PATCH v3] xfs: deprecate the V4 format

[Dave Chinner]

On Mon, Sep 14, 2020 at 02:12:41PM -0700, Darrick J. Wong wrote:
> On Mon, Sep 14, 2020 at 08:29:09AM +0100, Christoph Hellwig wrote:
> > On Fri, Sep 11, 2020 at 09:43:11AM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > The V4 filesystem format contains known weaknesses in the on-disk format
> > > that make metadata verification diffiult.  In addition, the format will
> > > does not support dates past 2038 and will not be upgraded to do so.
> > > Therefore, we should start the process of retiring the old format to
> > > close off attack surfaces and to encourage users to migrate onto V5.
> > > 
> > > Therefore, make XFS V4 support a configurable option.  For the first
> > > period it will be default Y in case some distributors want to withdraw
> > > support early; for the second period it will be default N so that anyone
> > > who wishes to continue support can do so; and after that, support will
> > > be removed from the kernel.
> > > 
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > ---
> > > v3: be a little more helpful about old xfsprogs and warn more loudly
> > > about deprecation
> > > v2: define what is a V4 filesystem, update the administrator guide
> > 
> > Whie this patch itself looks good, I think the ifdef as is is rather
> > silly as it just prevents mounting v4 file systems without reaping any
> > benefits from that.
> > 
> > So at very least we should add a little helper like this:
> > 
> > static inline bool xfs_sb_is_v4(truct xfs_sb *sbp)
> > {
> > 	if (IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
> > 		return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4;
> > 	return false;
> > }
> > 
> > and use it in all the feature test macros to let the compile eliminate
> > all the dead code.
> 
> Oh, wait, you meant as a means for future patches to make various bits
> of code disappear, not just as a weird one-off thing for this particular
> patch?
> 
> I mean... maybe we should just stuff that into the hascrc predicate,
> like Eric sort of implied on irc.  Hmm, I'll look into that.

Killing dead code is not the goal of this patch, getting the policy
in place and documenting it sufficiently is the goal of this patch.

Optimise the implementation in follow-on patches, don't obfuscate
this one by commingling it with wide-spread code changes...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v3] xfs: deprecate the V4 format

[Eric Sandeen]

On 9/14/20 4:54 PM, Dave Chinner wrote:
> On Mon, Sep 14, 2020 at 02:12:41PM -0700, Darrick J. Wong wrote:
>> On Mon, Sep 14, 2020 at 08:29:09AM +0100, Christoph Hellwig wrote:
>>> On Fri, Sep 11, 2020 at 09:43:11AM -0700, Darrick J. Wong wrote:
>>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>>
>>>> The V4 filesystem format contains known weaknesses in the on-disk format
>>>> that make metadata verification diffiult.  In addition, the format will
>>>> does not support dates past 2038 and will not be upgraded to do so.
>>>> Therefore, we should start the process of retiring the old format to
>>>> close off attack surfaces and to encourage users to migrate onto V5.
>>>>
>>>> Therefore, make XFS V4 support a configurable option.  For the first
>>>> period it will be default Y in case some distributors want to withdraw
>>>> support early; for the second period it will be default N so that anyone
>>>> who wishes to continue support can do so; and after that, support will
>>>> be removed from the kernel.
>>>>
>>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>>> ---
>>>> v3: be a little more helpful about old xfsprogs and warn more loudly
>>>> about deprecation
>>>> v2: define what is a V4 filesystem, update the administrator guide
>>>
>>> Whie this patch itself looks good, I think the ifdef as is is rather
>>> silly as it just prevents mounting v4 file systems without reaping any
>>> benefits from that.
>>>
>>> So at very least we should add a little helper like this:
>>>
>>> static inline bool xfs_sb_is_v4(truct xfs_sb *sbp)
>>> {
>>> 	if (IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
>>> 		return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4;
>>> 	return false;
>>> }
>>>
>>> and use it in all the feature test macros to let the compile eliminate
>>> all the dead code.
>>
>> Oh, wait, you meant as a means for future patches to make various bits
>> of code disappear, not just as a weird one-off thing for this particular
>> patch?
>>
>> I mean... maybe we should just stuff that into the hascrc predicate,
>> like Eric sort of implied on irc.  Hmm, I'll look into that.
> 
> Killing dead code is not the goal of this patch, getting the policy
> in place and documenting it sufficiently is the goal of this patch.
> 
> Optimise the implementation in follow-on patches, don't obfuscate
> this one by commingling it with wide-spread code changes...

Agreed - 

To be clear, the (messy) patch I sent was supposed to be a follow on
patch, not something to merge with the original.

-Eric

Re: [PATCH v3] xfs: deprecate the V4 format

[Eric Sandeen]

On 9/11/20 11:43 AM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The V4 filesystem format contains known weaknesses in the on-disk format
> that make metadata verification diffiult.  In addition, the format will

Editorial nitpicks you are free to take or leave:

drop "will" 

> does not support dates past 2038 and will not be upgraded to do so.
> Therefore, we should start the process of retiring the old format to
> close off attack surfaces and to encourage users to migrate onto V5.
> 
> Therefore, make XFS V4 support a configurable option.  For the first

Too many "Therefores":

"This patch makes XFS V4 support a ..."

> period it will be default Y in case some distributors want to withdraw
> support early; for the second period it will be default N so that anyone
> who wishes to continue support can do so; and after that, support will
> be removed from the kernel.

Maybe something like:

"Dates for these events are added to the XFS documentation in the kernel."

In any case,

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> v3: be a little more helpful about old xfsprogs and warn more loudly
> about deprecation
> v2: define what is a V4 filesystem, update the administrator guide
> ---
>  Documentation/admin-guide/xfs.rst |   23 +++++++++++++++++++++++
>  fs/xfs/Kconfig                    |   24 ++++++++++++++++++++++++
>  fs/xfs/xfs_super.c                |   13 +++++++++++++
>  3 files changed, 60 insertions(+)
> 
> diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
> index f461d6c33534..b6deea9ec073 100644
> --- a/Documentation/admin-guide/xfs.rst
> +++ b/Documentation/admin-guide/xfs.rst
> @@ -210,6 +210,28 @@ When mounting an XFS filesystem, the following options are accepted.
>  	inconsistent namespace presentation during or after a
>  	failover event.
>  
> +Deprecation of V4 Format
> +========================
> +
> +The V4 filesystem format lacks certain features that are supported by
> +the V5 format, such as metadata checksumming, strengthened metadata
> +verification, and the ability to store timestamps past the year 2038.
> +Because of this, the V4 format is deprecated.  All users should upgrade
> +by backing up their files, reformatting, and restoring from the backup.
> +
> +Administrators and users can detect a V4 filesystem by running xfs_info
> +against a filesystem mountpoint and checking for a string beginning with
> +"crc=".  If no such string is found, please upgrade xfsprogs to the
> +latest version and try again.
> +
> +The deprecation will take place in two parts.  Support for mounting V4
> +filesystems can now be disabled at kernel build time via Kconfig option.
> +The option will default to yes until September 2025, at which time it
> +will be changed to default to no.  In September 2030, support will be
> +removed from the codebase entirely.
> +
> +Note: Distributors may choose to withdraw V4 format support earlier than
> +the dates listed above.
>  
>  Deprecated Mount Options
>  ========================
> @@ -217,6 +239,7 @@ Deprecated Mount Options
>  ===========================     ================
>    Name				Removal Schedule
>  ===========================     ================
> +Mounting with V4 filesystem     September 2030
>  ===========================     ================
>  
>  
> diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig
> index e685299eb3d2..5422227e9e93 100644
> --- a/fs/xfs/Kconfig
> +++ b/fs/xfs/Kconfig
> @@ -22,6 +22,30 @@ config XFS_FS
>  	  system of your root partition is compiled as a module, you'll need
>  	  to use an initial ramdisk (initrd) to boot.
>  
> +config XFS_SUPPORT_V4
> +	bool "Support deprecated V4 (crc=0) format"
> +	default y
> +	help
> +	  The V4 filesystem format lacks certain features that are supported
> +	  by the V5 format, such as metadata checksumming, strengthened
> +	  metadata verification, and the ability to store timestamps past the
> +	  year 2038.  Because of this, the V4 format is deprecated.  All users
> +	  should upgrade by backing up their files, reformatting, and restoring
> +	  from the backup.
> +
> +	  Administrators and users can detect a V4 filesystem by running
> +	  xfs_info against a filesystem mountpoint and checking for a string
> +	  beginning with "crc=".  If the string "crc=0" is found, the
> +	  filesystem is a V4 filesystem.  If no such string is found, please
> +	  upgrade xfsprogs to the latest version and try again.
> +
> +	  This option will become default N in September 2025.  Support for the
> +	  V4 format will be removed entirely in September 2030.  Distributors
> +	  can say N here to withdraw support earlier.
> +
> +	  To continue supporting the old V4 format (crc=0), say Y.
> +	  To close off an attack surface, say N.
> +
>  config XFS_QUOTA
>  	bool "XFS Quota support"
>  	depends on XFS_FS
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index febd61ba071d..e2250c6d7251 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1451,6 +1451,19 @@ xfs_fc_fill_super(
>  	if (error)
>  		goto out_free_sb;
>  
> +	/* V4 support is undergoing deprecation. */
> +	if (!xfs_sb_version_hascrc(&mp->m_sb)) {
> +#ifdef CONFIG_XFS_SUPPORT_V4
> +		xfs_warn_once(mp,
> +	"Deprecated V4 format (crc=0) will not be supported after September 2030.");
> +#else
> +		xfs_warn(mp,
> +	"Deprecated V4 format (crc=0) not supported by kernel.");
> +		error = -EINVAL;
> +		goto out_free_sb;
> +#endif
> +	}
> +
>  	/*
>  	 * XFS block mappings use 54 bits to store the logical block offset.
>  	 * This should suffice to handle the maximum file size that the VFS
> 

Re: [PATCH v3] xfs: deprecate the V4 format

[Dave Chinner]

On Fri, Sep 11, 2020 at 09:43:11AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The V4 filesystem format contains known weaknesses in the on-disk format
> that make metadata verification diffiult.  In addition, the format will
> does not support dates past 2038 and will not be upgraded to do so.
> Therefore, we should start the process of retiring the old format to
> close off attack surfaces and to encourage users to migrate onto V5.
> 
> Therefore, make XFS V4 support a configurable option.  For the first
> period it will be default Y in case some distributors want to withdraw
> support early; for the second period it will be default N so that anyone
> who wishes to continue support can do so; and after that, support will
> be removed from the kernel.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> v3: be a little more helpful about old xfsprogs and warn more loudly
> about deprecation
> v2: define what is a V4 filesystem, update the administrator guide
> ---
>  Documentation/admin-guide/xfs.rst |   23 +++++++++++++++++++++++
>  fs/xfs/Kconfig                    |   24 ++++++++++++++++++++++++
>  fs/xfs/xfs_super.c                |   13 +++++++++++++
>  3 files changed, 60 insertions(+)

I agree with the overall policy direction and that we should be
warning v4 filesystem users as early as possible.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com