[PATCH] nfs: add missing selections of CONFIG_CRC32

[PATCH] nfs: add missing selections of CONFIG_CRC32

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

nfs.ko, nfsd.ko, and lockd.ko all use crc32_le(), which is available
only when CONFIG_CRC32 is enabled.  But the only NFS kconfig option that
selected CONFIG_CRC32 was CONFIG_NFS_DEBUG, which is client-specific and
did not actually guard the use of crc32_le() even on the client.

The code worked around this bug by only actually calling crc32_le() when
CONFIG_CRC32 is built-in, instead hard-coding '0' in other cases.  This
avoided randconfig build errors, and in real kernels the fallback code
was unlikely to be reached since CONFIG_CRC32 is 'default y'.  But, this
really needs to just be done properly, especially now that I'm planning
to update CONFIG_CRC32 to not be 'default y'.

Therefore, make CONFIG_NFS_FS, CONFIG_NFSD, and CONFIG_LOCKD select
CONFIG_CRC32.  Then remove the fallback code that becomes unnecessary,
as well as the selection of CONFIG_CRC32 from CONFIG_NFS_DEBUG.

Fixes: 1264a2f053a3 ("NFS: refactor code for calculating the crc32 hash of a filehandle")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/Kconfig           | 1 +
 fs/nfs/Kconfig       | 2 +-
 fs/nfs/internal.h    | 7 -------
 fs/nfs/nfs4session.h | 4 ----
 fs/nfsd/Kconfig      | 1 +
 fs/nfsd/nfsfh.h      | 7 -------
 include/linux/nfs.h  | 7 -------
 7 files changed, 3 insertions(+), 26 deletions(-)

diff --git a/fs/Kconfig b/fs/Kconfig
index c718b2e2de0e..5b4847bd2fbb 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -366,10 +366,11 @@ config GRACE_PERIOD
 	tristate
 
 config LOCKD
 	tristate
 	depends on FILE_LOCKING
+	select CRC32
 	select GRACE_PERIOD
 
 config LOCKD_V4
 	bool
 	depends on NFSD || NFS_V3
diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index d3f76101ad4b..07932ce9246c 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -1,9 +1,10 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config NFS_FS
 	tristate "NFS client support"
 	depends on INET && FILE_LOCKING && MULTIUSER
+	select CRC32
 	select LOCKD
 	select SUNRPC
 	select NFS_COMMON
 	select NFS_ACL_SUPPORT if NFS_V3_ACL
 	help
@@ -194,11 +195,10 @@ config NFS_USE_KERNEL_DNS
 	default y
 
 config NFS_DEBUG
 	bool
 	depends on NFS_FS && SUNRPC_DEBUG
-	select CRC32
 	default y
 
 config NFS_DISABLE_UDP_SUPPORT
        bool "NFS: Disable NFS UDP protocol support"
        depends on NFS_FS
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 1ac1d3eec517..0d6eb632dfcf 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -897,22 +897,15 @@ static inline
 u64 nfs_timespec_to_change_attr(const struct timespec64 *ts)
 {
 	return ((u64)ts->tv_sec << 30) + ts->tv_nsec;
 }
 
-#ifdef CONFIG_CRC32
 static inline u32 nfs_stateid_hash(const nfs4_stateid *stateid)
 {
 	return ~crc32_le(0xFFFFFFFF, &stateid->other[0],
 				NFS4_STATEID_OTHER_SIZE);
 }
-#else
-static inline u32 nfs_stateid_hash(nfs4_stateid *stateid)
-{
-	return 0;
-}
-#endif
 
 static inline bool nfs_error_is_fatal(int err)
 {
 	switch (err) {
 	case -ERESTARTSYS:
diff --git a/fs/nfs/nfs4session.h b/fs/nfs/nfs4session.h
index 351616c61df5..f9c291e2165c 100644
--- a/fs/nfs/nfs4session.h
+++ b/fs/nfs/nfs4session.h
@@ -146,20 +146,16 @@ static inline void nfs4_copy_sessionid(struct nfs4_sessionid *dst,
 		const struct nfs4_sessionid *src)
 {
 	memcpy(dst->data, src->data, NFS4_MAX_SESSIONID_LEN);
 }
 
-#ifdef CONFIG_CRC32
 /*
  * nfs_session_id_hash - calculate the crc32 hash for the session id
  * @session - pointer to session
  */
 #define nfs_session_id_hash(sess_id) \
 	(~crc32_le(0xFFFFFFFF, &(sess_id)->data[0], sizeof((sess_id)->data)))
-#else
-#define nfs_session_id_hash(session) (0)
-#endif
 #else /* defined(CONFIG_NFS_V4_1) */
 
 static inline int nfs4_init_session(struct nfs_client *clp)
 {
 	return 0;
diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index 792d3fed1b45..731a88f6313e 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -2,10 +2,11 @@
 config NFSD
 	tristate "NFS server support"
 	depends on INET
 	depends on FILE_LOCKING
 	depends on FSNOTIFY
+	select CRC32
 	select LOCKD
 	select SUNRPC
 	select EXPORTFS
 	select NFS_COMMON
 	select NFS_ACL_SUPPORT if NFSD_V2_ACL
diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
index 876152a91f12..5103c2f4d225 100644
--- a/fs/nfsd/nfsfh.h
+++ b/fs/nfsd/nfsfh.h
@@ -265,11 +265,10 @@ static inline bool fh_fsid_match(const struct knfsd_fh *fh1,
 	if (memcmp(fh1->fh_fsid, fh2->fh_fsid, key_len(fh1->fh_fsid_type)) != 0)
 		return false;
 	return true;
 }
 
-#ifdef CONFIG_CRC32
 /**
  * knfsd_fh_hash - calculate the crc32 hash for the filehandle
  * @fh - pointer to filehandle
  *
  * returns a crc32 hash for the filehandle that is compatible with
@@ -277,16 +276,10 @@ static inline bool fh_fsid_match(const struct knfsd_fh *fh1,
  */
 static inline u32 knfsd_fh_hash(const struct knfsd_fh *fh)
 {
 	return ~crc32_le(0xFFFFFFFF, fh->fh_raw, fh->fh_size);
 }
-#else
-static inline u32 knfsd_fh_hash(const struct knfsd_fh *fh)
-{
-	return 0;
-}
-#endif
 
 /**
  * fh_clear_pre_post_attrs - Reset pre/post attributes
  * @fhp: file handle to be updated
  *
diff --git a/include/linux/nfs.h b/include/linux/nfs.h
index 9ad727ddfedb..0906a0b40c6a 100644
--- a/include/linux/nfs.h
+++ b/include/linux/nfs.h
@@ -53,11 +53,10 @@ enum nfs3_stable_how {
 
 	/* used by direct.c to mark verf as invalid */
 	NFS_INVALID_STABLE_HOW = -1
 };
 
-#ifdef CONFIG_CRC32
 /**
  * nfs_fhandle_hash - calculate the crc32 hash for the filehandle
  * @fh - pointer to filehandle
  *
  * returns a crc32 hash for the filehandle that is compatible with
@@ -65,12 +64,6 @@ enum nfs3_stable_how {
  */
 static inline u32 nfs_fhandle_hash(const struct nfs_fh *fh)
 {
 	return ~crc32_le(0xFFFFFFFF, &fh->data[0], fh->size);
 }
-#else /* CONFIG_CRC32 */
-static inline u32 nfs_fhandle_hash(const struct nfs_fh *fh)
-{
-	return 0;
-}
-#endif /* CONFIG_CRC32 */
 #endif /* _LINUX_NFS_H */

base-commit: 91e5bfe317d8f8471fbaa3e70cf66cae1314a516
-- 
2.49.0

Re: [PATCH] nfs: add missing selections of CONFIG_CRC32

[Chuck Lever]

On 4/1/25 6:02 PM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> nfs.ko, nfsd.ko, and lockd.ko all use crc32_le(), which is available
> only when CONFIG_CRC32 is enabled.  But the only NFS kconfig option that
> selected CONFIG_CRC32 was CONFIG_NFS_DEBUG, which is client-specific and
> did not actually guard the use of crc32_le() even on the client.
> 
> The code worked around this bug by only actually calling crc32_le() when
> CONFIG_CRC32 is built-in, instead hard-coding '0' in other cases.  This
> avoided randconfig build errors, and in real kernels the fallback code
> was unlikely to be reached since CONFIG_CRC32 is 'default y'.  But, this
> really needs to just be done properly, especially now that I'm planning
> to update CONFIG_CRC32 to not be 'default y'.

It's interesting that no-one has noticed this before. dprintk is not the
only consumer of the FH hash function: NFS/NFSD trace points also use
it.

Eric, assuming you would like to carry this patch forward instead of us
taking it through one of the NFS client or server trees:

Acked-by: Chuck Lever <chuck.lever@oracle.com>

for the hunks related to nfsd and lockd.


> Therefore, make CONFIG_NFS_FS, CONFIG_NFSD, and CONFIG_LOCKD select
> CONFIG_CRC32.  Then remove the fallback code that becomes unnecessary,
> as well as the selection of CONFIG_CRC32 from CONFIG_NFS_DEBUG.
> 
> Fixes: 1264a2f053a3 ("NFS: refactor code for calculating the crc32 hash of a filehandle")
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  fs/Kconfig           | 1 +
>  fs/nfs/Kconfig       | 2 +-
>  fs/nfs/internal.h    | 7 -------
>  fs/nfs/nfs4session.h | 4 ----
>  fs/nfsd/Kconfig      | 1 +
>  fs/nfsd/nfsfh.h      | 7 -------
>  include/linux/nfs.h  | 7 -------
>  7 files changed, 3 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/Kconfig b/fs/Kconfig
> index c718b2e2de0e..5b4847bd2fbb 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -366,10 +366,11 @@ config GRACE_PERIOD
>  	tristate
>  
>  config LOCKD
>  	tristate
>  	depends on FILE_LOCKING
> +	select CRC32
>  	select GRACE_PERIOD
>  
>  config LOCKD_V4
>  	bool
>  	depends on NFSD || NFS_V3
> diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
> index d3f76101ad4b..07932ce9246c 100644
> --- a/fs/nfs/Kconfig
> +++ b/fs/nfs/Kconfig
> @@ -1,9 +1,10 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  config NFS_FS
>  	tristate "NFS client support"
>  	depends on INET && FILE_LOCKING && MULTIUSER
> +	select CRC32
>  	select LOCKD
>  	select SUNRPC
>  	select NFS_COMMON
>  	select NFS_ACL_SUPPORT if NFS_V3_ACL
>  	help
> @@ -194,11 +195,10 @@ config NFS_USE_KERNEL_DNS
>  	default y
>  
>  config NFS_DEBUG
>  	bool
>  	depends on NFS_FS && SUNRPC_DEBUG
> -	select CRC32
>  	default y
>  
>  config NFS_DISABLE_UDP_SUPPORT
>         bool "NFS: Disable NFS UDP protocol support"
>         depends on NFS_FS
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index 1ac1d3eec517..0d6eb632dfcf 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -897,22 +897,15 @@ static inline
>  u64 nfs_timespec_to_change_attr(const struct timespec64 *ts)
>  {
>  	return ((u64)ts->tv_sec << 30) + ts->tv_nsec;
>  }
>  
> -#ifdef CONFIG_CRC32
>  static inline u32 nfs_stateid_hash(const nfs4_stateid *stateid)
>  {
>  	return ~crc32_le(0xFFFFFFFF, &stateid->other[0],
>  				NFS4_STATEID_OTHER_SIZE);
>  }
> -#else
> -static inline u32 nfs_stateid_hash(nfs4_stateid *stateid)
> -{
> -	return 0;
> -}
> -#endif
>  
>  static inline bool nfs_error_is_fatal(int err)
>  {
>  	switch (err) {
>  	case -ERESTARTSYS:
> diff --git a/fs/nfs/nfs4session.h b/fs/nfs/nfs4session.h
> index 351616c61df5..f9c291e2165c 100644
> --- a/fs/nfs/nfs4session.h
> +++ b/fs/nfs/nfs4session.h
> @@ -146,20 +146,16 @@ static inline void nfs4_copy_sessionid(struct nfs4_sessionid *dst,
>  		const struct nfs4_sessionid *src)
>  {
>  	memcpy(dst->data, src->data, NFS4_MAX_SESSIONID_LEN);
>  }
>  
> -#ifdef CONFIG_CRC32
>  /*
>   * nfs_session_id_hash - calculate the crc32 hash for the session id
>   * @session - pointer to session
>   */
>  #define nfs_session_id_hash(sess_id) \
>  	(~crc32_le(0xFFFFFFFF, &(sess_id)->data[0], sizeof((sess_id)->data)))
> -#else
> -#define nfs_session_id_hash(session) (0)
> -#endif
>  #else /* defined(CONFIG_NFS_V4_1) */
>  
>  static inline int nfs4_init_session(struct nfs_client *clp)
>  {
>  	return 0;
> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> index 792d3fed1b45..731a88f6313e 100644
> --- a/fs/nfsd/Kconfig
> +++ b/fs/nfsd/Kconfig
> @@ -2,10 +2,11 @@
>  config NFSD
>  	tristate "NFS server support"
>  	depends on INET
>  	depends on FILE_LOCKING
>  	depends on FSNOTIFY
> +	select CRC32
>  	select LOCKD
>  	select SUNRPC
>  	select EXPORTFS
>  	select NFS_COMMON
>  	select NFS_ACL_SUPPORT if NFSD_V2_ACL
> diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
> index 876152a91f12..5103c2f4d225 100644
> --- a/fs/nfsd/nfsfh.h
> +++ b/fs/nfsd/nfsfh.h
> @@ -265,11 +265,10 @@ static inline bool fh_fsid_match(const struct knfsd_fh *fh1,
>  	if (memcmp(fh1->fh_fsid, fh2->fh_fsid, key_len(fh1->fh_fsid_type)) != 0)
>  		return false;
>  	return true;
>  }
>  
> -#ifdef CONFIG_CRC32
>  /**
>   * knfsd_fh_hash - calculate the crc32 hash for the filehandle
>   * @fh - pointer to filehandle
>   *
>   * returns a crc32 hash for the filehandle that is compatible with
> @@ -277,16 +276,10 @@ static inline bool fh_fsid_match(const struct knfsd_fh *fh1,
>   */
>  static inline u32 knfsd_fh_hash(const struct knfsd_fh *fh)
>  {
>  	return ~crc32_le(0xFFFFFFFF, fh->fh_raw, fh->fh_size);
>  }
> -#else
> -static inline u32 knfsd_fh_hash(const struct knfsd_fh *fh)
> -{
> -	return 0;
> -}
> -#endif
>  
>  /**
>   * fh_clear_pre_post_attrs - Reset pre/post attributes
>   * @fhp: file handle to be updated
>   *
> diff --git a/include/linux/nfs.h b/include/linux/nfs.h
> index 9ad727ddfedb..0906a0b40c6a 100644
> --- a/include/linux/nfs.h
> +++ b/include/linux/nfs.h
> @@ -53,11 +53,10 @@ enum nfs3_stable_how {
>  
>  	/* used by direct.c to mark verf as invalid */
>  	NFS_INVALID_STABLE_HOW = -1
>  };
>  
> -#ifdef CONFIG_CRC32
>  /**
>   * nfs_fhandle_hash - calculate the crc32 hash for the filehandle
>   * @fh - pointer to filehandle
>   *
>   * returns a crc32 hash for the filehandle that is compatible with
> @@ -65,12 +64,6 @@ enum nfs3_stable_how {
>   */
>  static inline u32 nfs_fhandle_hash(const struct nfs_fh *fh)
>  {
>  	return ~crc32_le(0xFFFFFFFF, &fh->data[0], fh->size);
>  }
> -#else /* CONFIG_CRC32 */
> -static inline u32 nfs_fhandle_hash(const struct nfs_fh *fh)
> -{
> -	return 0;
> -}
> -#endif /* CONFIG_CRC32 */
>  #endif /* _LINUX_NFS_H */
> 
> base-commit: 91e5bfe317d8f8471fbaa3e70cf66cae1314a516


-- 
Chuck Lever

Re: [PATCH] nfs: add missing selections of CONFIG_CRC32

[Eric Biggers]

On Wed, Apr 02, 2025 at 09:51:05AM -0400, Chuck Lever wrote:
> On 4/1/25 6:02 PM, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > nfs.ko, nfsd.ko, and lockd.ko all use crc32_le(), which is available
> > only when CONFIG_CRC32 is enabled.  But the only NFS kconfig option that
> > selected CONFIG_CRC32 was CONFIG_NFS_DEBUG, which is client-specific and
> > did not actually guard the use of crc32_le() even on the client.
> > 
> > The code worked around this bug by only actually calling crc32_le() when
> > CONFIG_CRC32 is built-in, instead hard-coding '0' in other cases.  This
> > avoided randconfig build errors, and in real kernels the fallback code
> > was unlikely to be reached since CONFIG_CRC32 is 'default y'.  But, this
> > really needs to just be done properly, especially now that I'm planning
> > to update CONFIG_CRC32 to not be 'default y'.
> 
> It's interesting that no-one has noticed this before. dprintk is not the
> only consumer of the FH hash function: NFS/NFSD trace points also use
> it.
> 
> Eric, assuming you would like to carry this patch forward instead of us
> taking it through one of the NFS client or server trees:
> 
> Acked-by: Chuck Lever <chuck.lever@oracle.com>
> 
> for the hunks related to nfsd and lockd.

Please go ahead and take it through one of the NFS trees.  Thanks!

- Eric

Re: [PATCH] nfs: add missing selections of CONFIG_CRC32

[Anna Schumaker]

On 4/1/25 6:02 PM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> nfs.ko, nfsd.ko, and lockd.ko all use crc32_le(), which is available
> only when CONFIG_CRC32 is enabled.  But the only NFS kconfig option that
> selected CONFIG_CRC32 was CONFIG_NFS_DEBUG, which is client-specific and
> did not actually guard the use of crc32_le() even on the client.
> 
> The code worked around this bug by only actually calling crc32_le() when
> CONFIG_CRC32 is built-in, instead hard-coding '0' in other cases.  This
> avoided randconfig build errors, and in real kernels the fallback code
> was unlikely to be reached since CONFIG_CRC32 is 'default y'.  But, this
> really needs to just be done properly, especially now that I'm planning
> to update CONFIG_CRC32 to not be 'default y'.
> 
> Therefore, make CONFIG_NFS_FS, CONFIG_NFSD, and CONFIG_LOCKD select
> CONFIG_CRC32.  Then remove the fallback code that becomes unnecessary,
> as well as the selection of CONFIG_CRC32 from CONFIG_NFS_DEBUG.
> 
> Fixes: 1264a2f053a3 ("NFS: refactor code for calculating the crc32 hash of a filehandle")
> Signed-off-by: Eric Biggers <ebiggers@google.com>

For the NFS client bits:

Acked-by: Anna Schumaker <anna.schumaker@oracle.com>

Thanks for looking at this!
Anna

> ---
>  fs/Kconfig           | 1 +
>  fs/nfs/Kconfig       | 2 +-
>  fs/nfs/internal.h    | 7 -------
>  fs/nfs/nfs4session.h | 4 ----
>  fs/nfsd/Kconfig      | 1 +
>  fs/nfsd/nfsfh.h      | 7 -------
>  include/linux/nfs.h  | 7 -------
>  7 files changed, 3 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/Kconfig b/fs/Kconfig
> index c718b2e2de0e..5b4847bd2fbb 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -366,10 +366,11 @@ config GRACE_PERIOD
>  	tristate
>  
>  config LOCKD
>  	tristate
>  	depends on FILE_LOCKING
> +	select CRC32
>  	select GRACE_PERIOD
>  
>  config LOCKD_V4
>  	bool
>  	depends on NFSD || NFS_V3
> diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
> index d3f76101ad4b..07932ce9246c 100644
> --- a/fs/nfs/Kconfig
> +++ b/fs/nfs/Kconfig
> @@ -1,9 +1,10 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  config NFS_FS
>  	tristate "NFS client support"
>  	depends on INET && FILE_LOCKING && MULTIUSER
> +	select CRC32
>  	select LOCKD
>  	select SUNRPC
>  	select NFS_COMMON
>  	select NFS_ACL_SUPPORT if NFS_V3_ACL
>  	help
> @@ -194,11 +195,10 @@ config NFS_USE_KERNEL_DNS
>  	default y
>  
>  config NFS_DEBUG
>  	bool
>  	depends on NFS_FS && SUNRPC_DEBUG
> -	select CRC32
>  	default y
>  
>  config NFS_DISABLE_UDP_SUPPORT
>         bool "NFS: Disable NFS UDP protocol support"
>         depends on NFS_FS
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index 1ac1d3eec517..0d6eb632dfcf 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -897,22 +897,15 @@ static inline
>  u64 nfs_timespec_to_change_attr(const struct timespec64 *ts)
>  {
>  	return ((u64)ts->tv_sec << 30) + ts->tv_nsec;
>  }
>  
> -#ifdef CONFIG_CRC32
>  static inline u32 nfs_stateid_hash(const nfs4_stateid *stateid)
>  {
>  	return ~crc32_le(0xFFFFFFFF, &stateid->other[0],
>  				NFS4_STATEID_OTHER_SIZE);
>  }
> -#else
> -static inline u32 nfs_stateid_hash(nfs4_stateid *stateid)
> -{
> -	return 0;
> -}
> -#endif
>  
>  static inline bool nfs_error_is_fatal(int err)
>  {
>  	switch (err) {
>  	case -ERESTARTSYS:
> diff --git a/fs/nfs/nfs4session.h b/fs/nfs/nfs4session.h
> index 351616c61df5..f9c291e2165c 100644
> --- a/fs/nfs/nfs4session.h
> +++ b/fs/nfs/nfs4session.h
> @@ -146,20 +146,16 @@ static inline void nfs4_copy_sessionid(struct nfs4_sessionid *dst,
>  		const struct nfs4_sessionid *src)
>  {
>  	memcpy(dst->data, src->data, NFS4_MAX_SESSIONID_LEN);
>  }
>  
> -#ifdef CONFIG_CRC32
>  /*
>   * nfs_session_id_hash - calculate the crc32 hash for the session id
>   * @session - pointer to session
>   */
>  #define nfs_session_id_hash(sess_id) \
>  	(~crc32_le(0xFFFFFFFF, &(sess_id)->data[0], sizeof((sess_id)->data)))
> -#else
> -#define nfs_session_id_hash(session) (0)
> -#endif
>  #else /* defined(CONFIG_NFS_V4_1) */
>  
>  static inline int nfs4_init_session(struct nfs_client *clp)
>  {
>  	return 0;
> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> index 792d3fed1b45..731a88f6313e 100644
> --- a/fs/nfsd/Kconfig
> +++ b/fs/nfsd/Kconfig
> @@ -2,10 +2,11 @@
>  config NFSD
>  	tristate "NFS server support"
>  	depends on INET
>  	depends on FILE_LOCKING
>  	depends on FSNOTIFY
> +	select CRC32
>  	select LOCKD
>  	select SUNRPC
>  	select EXPORTFS
>  	select NFS_COMMON
>  	select NFS_ACL_SUPPORT if NFSD_V2_ACL
> diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
> index 876152a91f12..5103c2f4d225 100644
> --- a/fs/nfsd/nfsfh.h
> +++ b/fs/nfsd/nfsfh.h
> @@ -265,11 +265,10 @@ static inline bool fh_fsid_match(const struct knfsd_fh *fh1,
>  	if (memcmp(fh1->fh_fsid, fh2->fh_fsid, key_len(fh1->fh_fsid_type)) != 0)
>  		return false;
>  	return true;
>  }
>  
> -#ifdef CONFIG_CRC32
>  /**
>   * knfsd_fh_hash - calculate the crc32 hash for the filehandle
>   * @fh - pointer to filehandle
>   *
>   * returns a crc32 hash for the filehandle that is compatible with
> @@ -277,16 +276,10 @@ static inline bool fh_fsid_match(const struct knfsd_fh *fh1,
>   */
>  static inline u32 knfsd_fh_hash(const struct knfsd_fh *fh)
>  {
>  	return ~crc32_le(0xFFFFFFFF, fh->fh_raw, fh->fh_size);
>  }
> -#else
> -static inline u32 knfsd_fh_hash(const struct knfsd_fh *fh)
> -{
> -	return 0;
> -}
> -#endif
>  
>  /**
>   * fh_clear_pre_post_attrs - Reset pre/post attributes
>   * @fhp: file handle to be updated
>   *
> diff --git a/include/linux/nfs.h b/include/linux/nfs.h
> index 9ad727ddfedb..0906a0b40c6a 100644
> --- a/include/linux/nfs.h
> +++ b/include/linux/nfs.h
> @@ -53,11 +53,10 @@ enum nfs3_stable_how {
>  
>  	/* used by direct.c to mark verf as invalid */
>  	NFS_INVALID_STABLE_HOW = -1
>  };
>  
> -#ifdef CONFIG_CRC32
>  /**
>   * nfs_fhandle_hash - calculate the crc32 hash for the filehandle
>   * @fh - pointer to filehandle
>   *
>   * returns a crc32 hash for the filehandle that is compatible with
> @@ -65,12 +64,6 @@ enum nfs3_stable_how {
>   */
>  static inline u32 nfs_fhandle_hash(const struct nfs_fh *fh)
>  {
>  	return ~crc32_le(0xFFFFFFFF, &fh->data[0], fh->size);
>  }
> -#else /* CONFIG_CRC32 */
> -static inline u32 nfs_fhandle_hash(const struct nfs_fh *fh)
> -{
> -	return 0;
> -}
> -#endif /* CONFIG_CRC32 */
>  #endif /* _LINUX_NFS_H */
> 
> base-commit: 91e5bfe317d8f8471fbaa3e70cf66cae1314a516

Re: [PATCH] nfs: add missing selections of CONFIG_CRC32

[cel]

From: Chuck Lever <chuck.lever@oracle.com>

On Tue, 01 Apr 2025 15:02:21 -0700, Eric Biggers wrote:
> nfs.ko, nfsd.ko, and lockd.ko all use crc32_le(), which is available
> only when CONFIG_CRC32 is enabled.  But the only NFS kconfig option that
> selected CONFIG_CRC32 was CONFIG_NFS_DEBUG, which is client-specific and
> did not actually guard the use of crc32_le() even on the client.
> 
> The code worked around this bug by only actually calling crc32_le() when
> CONFIG_CRC32 is built-in, instead hard-coding '0' in other cases.  This
> avoided randconfig build errors, and in real kernels the fallback code
> was unlikely to be reached since CONFIG_CRC32 is 'default y'.  But, this
> really needs to just be done properly, especially now that I'm planning
> to update CONFIG_CRC32 to not be 'default y'.
> 
> [...]

Applied to nfsd-testing, thanks!

[1/1] nfs: add missing selections of CONFIG_CRC32
      commit: 3d28468e53a519bb8adc0675e5000f56f11e0602

--
Chuck Lever

Re: [PATCH] nfs: add missing selections of CONFIG_CRC32

[Eric Biggers]

On Wed, Apr 02, 2025 at 09:29:41AM -0700, Eric Biggers wrote:
> On Wed, Apr 02, 2025 at 09:51:05AM -0400, Chuck Lever wrote:
> > On 4/1/25 6:02 PM, Eric Biggers wrote:
> > > From: Eric Biggers <ebiggers@google.com>
> > > 
> > > nfs.ko, nfsd.ko, and lockd.ko all use crc32_le(), which is available
> > > only when CONFIG_CRC32 is enabled.  But the only NFS kconfig option that
> > > selected CONFIG_CRC32 was CONFIG_NFS_DEBUG, which is client-specific and
> > > did not actually guard the use of crc32_le() even on the client.
> > > 
> > > The code worked around this bug by only actually calling crc32_le() when
> > > CONFIG_CRC32 is built-in, instead hard-coding '0' in other cases.  This
> > > avoided randconfig build errors, and in real kernels the fallback code
> > > was unlikely to be reached since CONFIG_CRC32 is 'default y'.  But, this
> > > really needs to just be done properly, especially now that I'm planning
> > > to update CONFIG_CRC32 to not be 'default y'.
> > 
> > It's interesting that no-one has noticed this before. dprintk is not the
> > only consumer of the FH hash function: NFS/NFSD trace points also use
> > it.
> > 
> > Eric, assuming you would like to carry this patch forward instead of us
> > taking it through one of the NFS client or server trees:
> > 
> > Acked-by: Chuck Lever <chuck.lever@oracle.com>
> > 
> > for the hunks related to nfsd and lockd.
> 
> Please go ahead and take it through one of the NFS trees.  Thanks!
> 

I ended up sending in the removal of 'default y' from CONFIG_CRC32 for 6.15.  So
I recommend that you send this NFS patch in for 6.15 as well, as it's now
slightly more likely that people can end up with CONFIG_CRC32 disabled (though
many other parts of the kernel select it anyway, so it still tends to be
enabled).

- Eric

Re: [PATCH] nfs: add missing selections of CONFIG_CRC32

[Chuck Lever]

On 4/8/25 4:45 PM, Eric Biggers wrote:
> On Wed, Apr 02, 2025 at 09:29:41AM -0700, Eric Biggers wrote:
>> On Wed, Apr 02, 2025 at 09:51:05AM -0400, Chuck Lever wrote:
>>> On 4/1/25 6:02 PM, Eric Biggers wrote:
>>>> From: Eric Biggers <ebiggers@google.com>
>>>>
>>>> nfs.ko, nfsd.ko, and lockd.ko all use crc32_le(), which is available
>>>> only when CONFIG_CRC32 is enabled.  But the only NFS kconfig option that
>>>> selected CONFIG_CRC32 was CONFIG_NFS_DEBUG, which is client-specific and
>>>> did not actually guard the use of crc32_le() even on the client.
>>>>
>>>> The code worked around this bug by only actually calling crc32_le() when
>>>> CONFIG_CRC32 is built-in, instead hard-coding '0' in other cases.  This
>>>> avoided randconfig build errors, and in real kernels the fallback code
>>>> was unlikely to be reached since CONFIG_CRC32 is 'default y'.  But, this
>>>> really needs to just be done properly, especially now that I'm planning
>>>> to update CONFIG_CRC32 to not be 'default y'.
>>>
>>> It's interesting that no-one has noticed this before. dprintk is not the
>>> only consumer of the FH hash function: NFS/NFSD trace points also use
>>> it.
>>>
>>> Eric, assuming you would like to carry this patch forward instead of us
>>> taking it through one of the NFS client or server trees:
>>>
>>> Acked-by: Chuck Lever <chuck.lever@oracle.com>
>>>
>>> for the hunks related to nfsd and lockd.
>>
>> Please go ahead and take it through one of the NFS trees.  Thanks!
>>
> 
> I ended up sending in the removal of 'default y' from CONFIG_CRC32 for 6.15.  So
> I recommend that you send this NFS patch in for 6.15 as well, as it's now
> slightly more likely that people can end up with CONFIG_CRC32 disabled (though
> many other parts of the kernel select it anyway, so it still tends to be
> enabled).

I've moved the patch to nfsd-next, which has exposure to community
automated testing robots. If no problems are reported, I will move the
patch to nfsd-fixes and push it to v6.15-rc in a couple of weeks.


-- 
Chuck Lever