| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <35874d6a-d5bc-4f5f-a62c-c03a6e877588@oracle.com>
Date: Wed, 2 Apr 2025 09:51:05 -0400
From: Chuck Lever <chuck.lever@...cle.com>
To: Eric Biggers <ebiggers@...nel.org>, linux-nfs@...r.kernel.org,
Trond Myklebust <trondmy@...nel.org>, Anna Schumaker <anna@...nel.org>,
Jeff Layton <jlayton@...nel.org>
Cc: Neil Brown <neilb@...e.de>, Olga Kornievskaia <okorniev@...hat.com>,
Dai Ngo <Dai.Ngo@...cle.com>, Tom Talpey <tom@...pey.com>,
Ard Biesheuvel <ardb@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] nfs: add missing selections of CONFIG_CRC32
On 4/1/25 6:02 PM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@...gle.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@...cle.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@...gle.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
Powered by blists - more mailing lists