nfsd: Use MD5 library instead of crypto_shash

Update NFSD's support for "legacy client tracking" (which uses MD5) to use the MD5 library instead of crypto_shash. This has several benefits:

- Simpler code. Notably, much of the error-handling code is no longer needed, since the library functions can't fail.

- Improved performance due to reduced overhead. A microbenchmark of nfs4_make_rec_clidname() shows a speedup from 1455 cycles to 425.

- The MD5 code can now safely be built as a loadable module when nfsd is built as a loadable module. (Previously, nfsd forced the MD5 code to built-in, presumably to work around the unreliablity of the name-based loading.) Thus, select MD5 from the tristate option NFSD if NFSD_LEGACY_CLIENT_TRACKING, instead of from the bool option NFSD_V4.

To preserve the existing behavior of legacy client tracking support being disabled when the kernel is booted with "fips=1", make nfsd4_legacy_tracking_init() return an error if fips_enabled. I don't know if this is truly needed, but it preserves the existing behavior.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/nfsd/Kconfig       |  3 +-
 fs/nfsd/nfs4recover.c | 82 ++++++++-----------------------------------
 2 files changed, 16 insertions(+), 69 deletions(-)

fs/nfsd/Kconfig +1 -2

index e134dce45e350..380a4caa33a73 100644
@@ -3,10 +3,11 @@ config NFSD
 	tristate "NFS server support"
 	depends on INET
 	depends on FILE_LOCKING
 	depends on FSNOTIFY
 	select CRC32
+	select CRYPTO_LIB_MD5 if NFSD_LEGACY_CLIENT_TRACKING
 	select CRYPTO_LIB_SHA256 if NFSD_V4
 	select LOCKD
 	select SUNRPC
 	select EXPORTFS
 	select NFS_COMMON

@@ -75,12 +76,10 @@ config NFSD_V3_ACL
 config NFSD_V4
 	bool "NFS server support for NFS version 4"
 	depends on NFSD && PROC_FS
 	select FS_POSIX_ACL
 	select RPCSEC_GSS_KRB5
-	select CRYPTO
-	select CRYPTO_MD5
 	select GRACE_PERIOD
 	select NFS_V4_2_SSC_HELPER if NFS_V4_2
 	help
 	  This option enables support in your system's NFS server for
 	  version 4 of the NFS protocol (RFC 3530).

fs/nfsd/nfs4recover.c +15 -67

index e2b9472e5c78c..dbc0aecef95e3 100644
@@ -30,13 +30,14 @@
 *  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 *
 */
 
-#include <crypto/hash.h>
+#include <crypto/md5.h>
 #include <crypto/sha2.h>
 #include <linux/file.h>
+#include <linux/fips.h>
 #include <linux/slab.h>
 #include <linux/namei.h>
 #include <linux/sched.h>
 #include <linux/fs.h>
 #include <linux/module.h>

@@ -90,61 +91,22 @@ static void
 nfs4_reset_creds(const struct cred *original)
 {
 	put_cred(revert_creds(original));
 }
 
-static int
+static void
 nfs4_make_rec_clidname(char dname[HEXDIR_LEN], const struct xdr_netobj *clname)
 {
 	u8 digest[MD5_DIGEST_SIZE];
-	struct crypto_shash *tfm;
-	int status;
 
 	dprintk("NFSD: nfs4_make_rec_clidname for %.*s\n",
 			clname->len, clname->data);
-	tfm = crypto_alloc_shash("md5", 0, 0);
-	if (IS_ERR(tfm)) {
-		status = PTR_ERR(tfm);
-		goto out_no_tfm;
-	}
 
-	status = crypto_shash_tfm_digest(tfm, clname->data, clname->len,
-					 digest);
-	if (status)
-		goto out;
+	md5(clname->data, clname->len, digest);
 
 	static_assert(HEXDIR_LEN == 2 * MD5_DIGEST_SIZE + 1);
 	sprintf(dname, "%*phN", MD5_DIGEST_SIZE, digest);
-
-	status = 0;
-out:
-	crypto_free_shash(tfm);
-out_no_tfm:
-	return status;
-}
-
-/*
- * If we had an error generating the recdir name for the legacy tracker
- * then warn the admin. If the error doesn't appear to be transient,
- * then disable recovery tracking.
- */
-static void
-legacy_recdir_name_error(struct nfs4_client *clp, int error)
-{
-	printk(KERN_ERR "NFSD: unable to generate recoverydir "
-			"name (%d).\n", error);
-
-	/*
-	 * if the algorithm just doesn't exist, then disable the recovery
-	 * tracker altogether. The crypto libs will generally return this if
-	 * FIPS is enabled as well.
-	 */
-	if (error == -ENOENT) {
-		printk(KERN_ERR "NFSD: disabling legacy clientid tracking. "
-			"Reboot recovery will not function correctly!\n");
-		nfsd4_client_tracking_exit(clp->net);
-	}
 }
 
 static void
 __nfsd4_create_reclaim_record_grace(struct nfs4_client *clp,
 		const char *dname, int len, struct nfsd_net *nn)

@@ -180,13 +142,11 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
 	if (test_and_set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return;
 	if (!nn->rec_file)
 		return;
 
-	status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-	if (status)
-		return legacy_recdir_name_error(clp, status);
+	nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 	status = nfs4_save_creds(&original_cred);
 	if (status < 0)
 		return;
 

@@ -374,13 +334,11 @@ nfsd4_remove_clid_dir(struct nfs4_client *clp)
 	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 
 	if (!nn->rec_file || !test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return;
 
-	status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-	if (status)
-		return legacy_recdir_name_error(clp, status);
+	nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 	status = mnt_want_write_file(nn->rec_file);
 	if (status)
 		goto out;
 	clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);

@@ -601,10 +559,15 @@ nfsd4_legacy_tracking_init(struct net *net)
 	if (net != &init_net) {
 		pr_warn("NFSD: attempt to initialize legacy client tracking in a container ignored.\n");
 		return -EINVAL;
 	}
 
+	if (fips_enabled) {
+		pr_warn("NFSD: legacy client tracking is disabled due to FIPS\n");
+		return -EINVAL;
+	}
+
 	status = nfs4_legacy_state_init(net);
 	if (status)
 		return status;
 
 	status = nfsd4_load_reboot_recovery_data(net);

@@ -657,25 +620,20 @@ nfs4_recoverydir(void)
 }
 
 static int
 nfsd4_check_legacy_client(struct nfs4_client *clp)
 {
-	int status;
 	char dname[HEXDIR_LEN];
 	struct nfs4_client_reclaim *crp;
 	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 	struct xdr_netobj name;
 
 	/* did we already find that this client is stable? */
 	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return 0;
 
-	status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-	if (status) {
-		legacy_recdir_name_error(clp, status);
-		return status;
-	}
+	nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 	/* look for it in the reclaim hashtable otherwise */
 	name.data = kmemdup(dname, HEXDIR_LEN, GFP_KERNEL);
 	if (!name.data) {
 		dprintk("%s: failed to allocate memory for name.data!\n",

@@ -1264,17 +1222,14 @@ nfsd4_cld_check(struct nfs4_client *clp)
 	if (crp)
 		goto found;
 
 #ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
 	if (nn->cld_net->cn_has_legacy) {
-		int status;
 		char dname[HEXDIR_LEN];
 		struct xdr_netobj name;
 
-		status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-		if (status)
-			return -ENOENT;
+		nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 		name.data = kmemdup(dname, HEXDIR_LEN, GFP_KERNEL);
 		if (!name.data) {
 			dprintk("%s: failed to allocate memory for name.data!\n",
 				__func__);

@@ -1315,15 +1270,12 @@ nfsd4_cld_check_v2(struct nfs4_client *clp)
 
 #ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
 	if (cn->cn_has_legacy) {
 		struct xdr_netobj name;
 		char dname[HEXDIR_LEN];
-		int status;
 
-		status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-		if (status)
-			return -ENOENT;
+		nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 		name.data = kmemdup(dname, HEXDIR_LEN, GFP_KERNEL);
 		if (!name.data) {
 			dprintk("%s: failed to allocate memory for name.data\n",
 					__func__);

@@ -1692,15 +1644,11 @@ nfsd4_cltrack_legacy_recdir(const struct xdr_netobj *name)
 		/* just return nothing if output will be truncated */
 		kfree(result);
 		return NULL;
 	}
 
-	copied = nfs4_make_rec_clidname(result + copied, name);
-	if (copied) {
-		kfree(result);
-		return NULL;
-	}
+	nfs4_make_rec_clidname(result + copied, name);
 
 	return result;
 }
 
 static char *

base-commit: 0739473694c4878513031006829f1030ec850bc2

Quoted 15 lines · Eric Biggers

Update NFSD's support for "legacy client tracking" (which uses MD5) to use the MD5 library instead of crypto_shash. This has several benefits:

- Simpler code. Notably, much of the error-handling code is no longer needed, since the library functions can't fail.

- Improved performance due to reduced overhead. A microbenchmark of nfs4_make_rec_clidname() shows a speedup from 1455 cycles to 425.

- The MD5 code can now safely be built as a loadable module when nfsd is built as a loadable module. (Previously, nfsd forced the MD5 code to built-in, presumably to work around the unreliablity of the name-based loading.) Thus, select MD5 from the tristate option NFSD if NFSD_LEGACY_CLIENT_TRACKING, instead of from the bool option NFSD_V4.

Quoted 5 lines

To preserve the existing behavior of legacy client tracking support being disabled when the kernel is booted with "fips=1", make nfsd4_legacy_tracking_init() return an error if fips_enabled. I don't know if this is truly needed, but it preserves the existing behavior.

FIPS is pretty draconian about algorithms, AIUI. We're not using MD5 in a cryptographically significant way here, but the FIPS gods won't bless a kernel that uses MD5 at all, so I think it is needed.

Quoted 22 lines · Jeff Layton

Quoted 15 lines · Eric Biggers

Update NFSD's support for "legacy client tracking" (which uses MD5) to use the MD5 library instead of crypto_shash. This has several benefits:

- Simpler code. Notably, much of the error-handling code is no longer needed, since the library functions can't fail.

- Improved performance due to reduced overhead. A microbenchmark of nfs4_make_rec_clidname() shows a speedup from 1455 cycles to 425.

- The MD5 code can now safely be built as a loadable module when nfsd is built as a loadable module. (Previously, nfsd forced the MD5 code to built-in, presumably to work around the unreliablity of the name-based loading.) Thus, select MD5 from the tristate option NFSD if NFSD_LEGACY_CLIENT_TRACKING, instead of from the bool option NFSD_V4.

Quoted 5 lines

To preserve the existing behavior of legacy client tracking support being disabled when the kernel is booted with "fips=1", make nfsd4_legacy_tracking_init() return an error if fips_enabled. I don't know if this is truly needed, but it preserves the existing behavior.

Quoted 3 lines

FIPS is pretty draconian about algorithms, AIUI. We're not using MD5 in a cryptographically significant way here, but the FIPS gods won't bless a kernel that uses MD5 at all, so I think it is needed.

If it's not being used for a security purpose, then I think you can just drop the fips_enabled check. People are used to the old API where MD5 was always forbidden when fips_enabled, but it doesn't actually need to be that strict. For this patch I wasn't certain about the use case though, so I just opted to preserve the existing behavior for now. A follow-on patch to remove the check could make sense.

Quoted 22 lines · Jeff Layton

Quoted 15 lines · Eric Biggers

Update NFSD's support for "legacy client tracking" (which uses MD5) to use the MD5 library instead of crypto_shash. This has several benefits:

- Simpler code. Notably, much of the error-handling code is no longer needed, since the library functions can't fail.

- Improved performance due to reduced overhead. A microbenchmark of nfs4_make_rec_clidname() shows a speedup from 1455 cycles to 425.

- The MD5 code can now safely be built as a loadable module when nfsd is built as a loadable module. (Previously, nfsd forced the MD5 code to built-in, presumably to work around the unreliablity of the name-based loading.) Thus, select MD5 from the tristate option NFSD if NFSD_LEGACY_CLIENT_TRACKING, instead of from the bool option NFSD_V4.

Quoted 5 lines

To preserve the existing behavior of legacy client tracking support being disabled when the kernel is booted with "fips=1", make nfsd4_legacy_tracking_init() return an error if fips_enabled. I don't know if this is truly needed, but it preserves the existing behavior.

Quoted 3 lines

FIPS is pretty draconian about algorithms, AIUI. We're not using MD5 in a cryptographically significant way here, but the FIPS gods won't bless a kernel that uses MD5 at all, so I think it is needed.

If it's not being used for a security purpose, then I think you can just drop the fips_enabled check. People are used to the old API where MD5 was always forbidden when fips_enabled, but it doesn't actually need to be that strict. For this patch I wasn't certain about the use case though, so I just opted to preserve the existing behavior for now. A follow-on patch to remove the check could make sense.

Quoted 22 lines · Jeff Layton

Quoted 15 lines · Eric Biggers

Update NFSD's support for "legacy client tracking" (which uses MD5) to use the MD5 library instead of crypto_shash. This has several benefits:

- Simpler code. Notably, much of the error-handling code is no longer needed, since the library functions can't fail.

- Improved performance due to reduced overhead. A microbenchmark of nfs4_make_rec_clidname() shows a speedup from 1455 cycles to 425.

- The MD5 code can now safely be built as a loadable module when nfsd is built as a loadable module. (Previously, nfsd forced the MD5 code to built-in, presumably to work around the unreliablity of the name-based loading.) Thus, select MD5 from the tristate option NFSD if NFSD_LEGACY_CLIENT_TRACKING, instead of from the bool option NFSD_V4.

Quoted 5 lines

To preserve the existing behavior of legacy client tracking support being disabled when the kernel is booted with "fips=1", make nfsd4_legacy_tracking_init() return an error if fips_enabled. I don't know if this is truly needed, but it preserves the existing behavior.

Quoted 3 lines

FIPS is pretty draconian about algorithms, AIUI. We're not using MD5 in a cryptographically significant way here, but the FIPS gods won't bless a kernel that uses MD5 at all, so I think it is needed.

If it's not being used for a security purpose, then I think you can just drop the fips_enabled check. People are used to the old API where MD5 was always forbidden when fips_enabled, but it doesn't actually need to be that strict. For this patch I wasn't certain about the use case though, so I just opted to preserve the existing behavior for now. A follow-on patch to remove the check could make sense.

Quoted 27 lines · Eric Biggers

Quoted 25 lines · Jeff Layton

Quoted 15 lines · Eric Biggers

Update NFSD's support for "legacy client tracking" (which uses MD5) to use the MD5 library instead of crypto_shash. This has several benefits:

- Simpler code. Notably, much of the error-handling code is no longer needed, since the library functions can't fail.

- Improved performance due to reduced overhead. A microbenchmark of nfs4_make_rec_clidname() shows a speedup from 1455 cycles to 425.

- The MD5 code can now safely be built as a loadable module when nfsd is built as a loadable module. (Previously, nfsd forced the MD5 code to built-in, presumably to work around the unreliablity of the name-based loading.) Thus, select MD5 from the tristate option NFSD if NFSD_LEGACY_CLIENT_TRACKING, instead of from the bool option NFSD_V4.

Quoted 5 lines

To preserve the existing behavior of legacy client tracking support being disabled when the kernel is booted with "fips=1", make nfsd4_legacy_tracking_init() return an error if fips_enabled. I don't know if this is truly needed, but it preserves the existing behavior.

FIPS is pretty draconian about algorithms, AIUI. We're not using MD5 in a cryptographically significant way here, but the FIPS gods won't bless a kernel that uses MD5 at all, so I think it is needed.

Quoted 6 lines

If it's not being used for a security purpose, then I think you can just drop the fips_enabled check. People are used to the old API where MD5 was always forbidden when fips_enabled, but it doesn't actually need to be that strict. For this patch I wasn't certain about the use case though, so I just opted to preserve the existing behavior for now. A follow-on patch to remove the check could make sense.

Eric, were you going to follow up with a fresh revision that drops the fips_enabled check?

Quoted 34 lines · Chuck Lever

Quoted 33 lines · Eric Biggers

Quoted 22 lines · Jeff Layton

Quoted 15 lines · Eric Biggers

Update NFSD's support for "legacy client tracking" (which uses MD5) to use the MD5 library instead of crypto_shash. This has several benefits:

- Simpler code. Notably, much of the error-handling code is no longer needed, since the library functions can't fail.

- Improved performance due to reduced overhead. A microbenchmark of nfs4_make_rec_clidname() shows a speedup from 1455 cycles to 425.

- The MD5 code can now safely be built as a loadable module when nfsd is built as a loadable module. (Previously, nfsd forced the MD5 code to built-in, presumably to work around the unreliablity of the name-based loading.) Thus, select MD5 from the tristate option NFSD if NFSD_LEGACY_CLIENT_TRACKING, instead of from the bool option NFSD_V4.

Quoted 5 lines

To preserve the existing behavior of legacy client tracking support being disabled when the kernel is booted with "fips=1", make nfsd4_legacy_tracking_init() return an error if fips_enabled. I don't know if this is truly needed, but it preserves the existing behavior.

Quoted 3 lines

FIPS is pretty draconian about algorithms, AIUI. We're not using MD5 in a cryptographically significant way here, but the FIPS gods won't bless a kernel that uses MD5 at all, so I think it is needed.

If it's not being used for a security purpose, then I think you can just drop the fips_enabled check. People are used to the old API where MD5 was always forbidden when fips_enabled, but it doesn't actually need to be that strict. For this patch I wasn't certain about the use case though, so I just opted to preserve the existing behavior for now. A follow-on patch to remove the check could make sense.

Quoted 2 lines

Eric, were you going to follow up with a fresh revision that drops the fips_enabled check?

Sure, if you want. I see you're also planning to revert my prerequisite patch "SUNRPC: Make RPCSEC_GSS_KRB5 select CRYPTO instead of depending on it". So I also need to work around that by keeping the 'select CRYPTO' in NFSD_V4.

Update NFSD's support for "legacy client tracking" (which uses MD5) to use the MD5 library instead of crypto_shash. This has several benefits:

- Simpler code. Notably, much of the error-handling code is no longer needed, since the library functions can't fail.

- Improved performance due to reduced overhead. A microbenchmark of nfs4_make_rec_clidname() shows a speedup from 1455 cycles to 425.

- The MD5 code can now safely be built as a loadable module when nfsd is built as a loadable module. (Previously, nfsd forced the MD5 code to built-in, presumably to work around the unreliablity of the name-based loading.) Thus, select MD5 from the tristate option NFSD if NFSD_LEGACY_CLIENT_TRACKING, instead of from the bool option NFSD_V4.

To preserve the existing behavior of legacy client tracking support being disabled when the kernel is booted with "fips=1", make nfsd4_legacy_tracking_init() return an error if fips_enabled. I don't know if this is truly needed, but it preserves the existing behavior.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/nfsd/Kconfig       |  3 +-
 fs/nfsd/nfs4recover.c | 82 ++++++++-----------------------------------
 2 files changed, 16 insertions(+), 69 deletions(-)

fs/nfsd/Kconfig +1 -2

index e134dce45e350..380a4caa33a73 100644
@@ -3,10 +3,11 @@ config NFSD
 	tristate "NFS server support"
 	depends on INET
 	depends on FILE_LOCKING
 	depends on FSNOTIFY
 	select CRC32
+	select CRYPTO_LIB_MD5 if NFSD_LEGACY_CLIENT_TRACKING
 	select CRYPTO_LIB_SHA256 if NFSD_V4
 	select LOCKD
 	select SUNRPC
 	select EXPORTFS
 	select NFS_COMMON

@@ -75,12 +76,10 @@ config NFSD_V3_ACL
 config NFSD_V4
 	bool "NFS server support for NFS version 4"
 	depends on NFSD && PROC_FS
 	select FS_POSIX_ACL
 	select RPCSEC_GSS_KRB5
-	select CRYPTO
-	select CRYPTO_MD5
 	select GRACE_PERIOD
 	select NFS_V4_2_SSC_HELPER if NFS_V4_2
 	help
 	  This option enables support in your system's NFS server for
 	  version 4 of the NFS protocol (RFC 3530).

fs/nfsd/nfs4recover.c +15 -67

index e2b9472e5c78c..dbc0aecef95e3 100644
@@ -30,13 +30,14 @@
 *  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 *
 */
 
-#include <crypto/hash.h>
+#include <crypto/md5.h>
 #include <crypto/sha2.h>
 #include <linux/file.h>
+#include <linux/fips.h>
 #include <linux/slab.h>
 #include <linux/namei.h>
 #include <linux/sched.h>
 #include <linux/fs.h>
 #include <linux/module.h>

@@ -90,61 +91,22 @@ static void
 nfs4_reset_creds(const struct cred *original)
 {
 	put_cred(revert_creds(original));
 }
 
-static int
+static void
 nfs4_make_rec_clidname(char dname[HEXDIR_LEN], const struct xdr_netobj *clname)
 {
 	u8 digest[MD5_DIGEST_SIZE];
-	struct crypto_shash *tfm;
-	int status;
 
 	dprintk("NFSD: nfs4_make_rec_clidname for %.*s\n",
 			clname->len, clname->data);
-	tfm = crypto_alloc_shash("md5", 0, 0);
-	if (IS_ERR(tfm)) {
-		status = PTR_ERR(tfm);
-		goto out_no_tfm;
-	}
 
-	status = crypto_shash_tfm_digest(tfm, clname->data, clname->len,
-					 digest);
-	if (status)
-		goto out;
+	md5(clname->data, clname->len, digest);
 
 	static_assert(HEXDIR_LEN == 2 * MD5_DIGEST_SIZE + 1);
 	sprintf(dname, "%*phN", MD5_DIGEST_SIZE, digest);
-
-	status = 0;
-out:
-	crypto_free_shash(tfm);
-out_no_tfm:
-	return status;
-}
-
-/*
- * If we had an error generating the recdir name for the legacy tracker
- * then warn the admin. If the error doesn't appear to be transient,
- * then disable recovery tracking.
- */
-static void
-legacy_recdir_name_error(struct nfs4_client *clp, int error)
-{
-	printk(KERN_ERR "NFSD: unable to generate recoverydir "
-			"name (%d).\n", error);
-
-	/*
-	 * if the algorithm just doesn't exist, then disable the recovery
-	 * tracker altogether. The crypto libs will generally return this if
-	 * FIPS is enabled as well.
-	 */
-	if (error == -ENOENT) {
-		printk(KERN_ERR "NFSD: disabling legacy clientid tracking. "
-			"Reboot recovery will not function correctly!\n");
-		nfsd4_client_tracking_exit(clp->net);
-	}
 }
 
 static void
 __nfsd4_create_reclaim_record_grace(struct nfs4_client *clp,
 		const char *dname, int len, struct nfsd_net *nn)

@@ -180,13 +142,11 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
 	if (test_and_set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return;
 	if (!nn->rec_file)
 		return;
 
-	status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-	if (status)
-		return legacy_recdir_name_error(clp, status);
+	nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 	status = nfs4_save_creds(&original_cred);
 	if (status < 0)
 		return;
 

@@ -374,13 +334,11 @@ nfsd4_remove_clid_dir(struct nfs4_client *clp)
 	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 
 	if (!nn->rec_file || !test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return;
 
-	status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-	if (status)
-		return legacy_recdir_name_error(clp, status);
+	nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 	status = mnt_want_write_file(nn->rec_file);
 	if (status)
 		goto out;
 	clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);

@@ -601,10 +559,15 @@ nfsd4_legacy_tracking_init(struct net *net)
 	if (net != &init_net) {
 		pr_warn("NFSD: attempt to initialize legacy client tracking in a container ignored.\n");
 		return -EINVAL;
 	}
 
+	if (fips_enabled) {
+		pr_warn("NFSD: legacy client tracking is disabled due to FIPS\n");
+		return -EINVAL;
+	}
+
 	status = nfs4_legacy_state_init(net);
 	if (status)
 		return status;
 
 	status = nfsd4_load_reboot_recovery_data(net);

@@ -657,25 +620,20 @@ nfs4_recoverydir(void)
 }
 
 static int
 nfsd4_check_legacy_client(struct nfs4_client *clp)
 {
-	int status;
 	char dname[HEXDIR_LEN];
 	struct nfs4_client_reclaim *crp;
 	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 	struct xdr_netobj name;
 
 	/* did we already find that this client is stable? */
 	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return 0;
 
-	status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-	if (status) {
-		legacy_recdir_name_error(clp, status);
-		return status;
-	}
+	nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 	/* look for it in the reclaim hashtable otherwise */
 	name.data = kmemdup(dname, HEXDIR_LEN, GFP_KERNEL);
 	if (!name.data) {
 		dprintk("%s: failed to allocate memory for name.data!\n",

@@ -1264,17 +1222,14 @@ nfsd4_cld_check(struct nfs4_client *clp)
 	if (crp)
 		goto found;
 
 #ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
 	if (nn->cld_net->cn_has_legacy) {
-		int status;
 		char dname[HEXDIR_LEN];
 		struct xdr_netobj name;
 
-		status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-		if (status)
-			return -ENOENT;
+		nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 		name.data = kmemdup(dname, HEXDIR_LEN, GFP_KERNEL);
 		if (!name.data) {
 			dprintk("%s: failed to allocate memory for name.data!\n",
 				__func__);

@@ -1315,15 +1270,12 @@ nfsd4_cld_check_v2(struct nfs4_client *clp)
 
 #ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
 	if (cn->cn_has_legacy) {
 		struct xdr_netobj name;
 		char dname[HEXDIR_LEN];
-		int status;
 
-		status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-		if (status)
-			return -ENOENT;
+		nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 		name.data = kmemdup(dname, HEXDIR_LEN, GFP_KERNEL);
 		if (!name.data) {
 			dprintk("%s: failed to allocate memory for name.data\n",
 					__func__);

@@ -1692,15 +1644,11 @@ nfsd4_cltrack_legacy_recdir(const struct xdr_netobj *name)
 		/* just return nothing if output will be truncated */
 		kfree(result);
 		return NULL;
 	}
 
-	copied = nfs4_make_rec_clidname(result + copied, name);
-	if (copied) {
-		kfree(result);
-		return NULL;
-	}
+	nfs4_make_rec_clidname(result + copied, name);
 
 	return result;
 }
 
 static char *

base-commit: 0739473694c4878513031006829f1030ec850bc2

Quoted 20 lines · Eric Biggers

Update NFSD's support for "legacy client tracking" (which uses MD5) to use the MD5 library instead of crypto_shash. This has several benefits:

- Simpler code. Notably, much of the error-handling code is no longer needed, since the library functions can't fail.

- Improved performance due to reduced overhead. A microbenchmark of nfs4_make_rec_clidname() shows a speedup from 1455 cycles to 425.

- The MD5 code can now safely be built as a loadable module when nfsd is built as a loadable module. (Previously, nfsd forced the MD5 code to built-in, presumably to work around the unreliablity of the name-based loading.) Thus, select MD5 from the tristate option NFSD if NFSD_LEGACY_CLIENT_TRACKING, instead of from the bool option NFSD_V4.

To preserve the existing behavior of legacy client tracking support being disabled when the kernel is booted with "fips=1", make nfsd4_legacy_tracking_init() return an error if fips_enabled. I don't know if this is truly needed, but it preserves the existing behavior.

Quoted 1 line

Signed-off-by Eric Biggers <[email protected]>

No objection, but let's cross our t's and dot our i's. Scott, when you have recovered from bake-a-thon, can you have a look at this one?

Thanks!

Quoted 248 lines

---
 fs/nfsd/Kconfig       |  3 +-
 fs/nfsd/nfs4recover.c | 82 ++++++++-----------------------------------
 2 files changed, 16 insertions(+), 69 deletions(-)

fs/nfsd/Kconfig +1 -2

index e134dce45e350..380a4caa33a73 100644
@@ -3,10 +3,11 @@ config NFSD
 	tristate "NFS server support"
 	depends on INET
 	depends on FILE_LOCKING
 	depends on FSNOTIFY
 	select CRC32
+	select CRYPTO_LIB_MD5 if NFSD_LEGACY_CLIENT_TRACKING
 	select CRYPTO_LIB_SHA256 if NFSD_V4
 	select LOCKD
 	select SUNRPC
 	select EXPORTFS
 	select NFS_COMMON

@@ -75,12 +76,10 @@ config NFSD_V3_ACL
 config NFSD_V4
 	bool "NFS server support for NFS version 4"
 	depends on NFSD && PROC_FS
 	select FS_POSIX_ACL
 	select RPCSEC_GSS_KRB5
-	select CRYPTO
-	select CRYPTO_MD5
 	select GRACE_PERIOD
 	select NFS_V4_2_SSC_HELPER if NFS_V4_2
 	help
 	  This option enables support in your system's NFS server for
 	  version 4 of the NFS protocol (RFC 3530).

fs/nfsd/nfs4recover.c +15 -67

index e2b9472e5c78c..dbc0aecef95e3 100644
@@ -30,13 +30,14 @@
 *  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 *
 */
 
-#include <crypto/hash.h>
+#include <crypto/md5.h>
 #include <crypto/sha2.h>
 #include <linux/file.h>
+#include <linux/fips.h>
 #include <linux/slab.h>
 #include <linux/namei.h>
 #include <linux/sched.h>
 #include <linux/fs.h>
 #include <linux/module.h>

@@ -90,61 +91,22 @@ static void
 nfs4_reset_creds(const struct cred *original)
 {
 	put_cred(revert_creds(original));
 }
 
-static int
+static void
 nfs4_make_rec_clidname(char dname[HEXDIR_LEN], const struct xdr_netobj *clname)
 {
 	u8 digest[MD5_DIGEST_SIZE];
-	struct crypto_shash *tfm;
-	int status;
 
 	dprintk("NFSD: nfs4_make_rec_clidname for %.*s\n",
 			clname->len, clname->data);
-	tfm = crypto_alloc_shash("md5", 0, 0);
-	if (IS_ERR(tfm)) {
-		status = PTR_ERR(tfm);
-		goto out_no_tfm;
-	}
 
-	status = crypto_shash_tfm_digest(tfm, clname->data, clname->len,
-					 digest);
-	if (status)
-		goto out;
+	md5(clname->data, clname->len, digest);
 
 	static_assert(HEXDIR_LEN == 2 * MD5_DIGEST_SIZE + 1);
 	sprintf(dname, "%*phN", MD5_DIGEST_SIZE, digest);
-
-	status = 0;
-out:
-	crypto_free_shash(tfm);
-out_no_tfm:
-	return status;
-}
-
-/*
- * If we had an error generating the recdir name for the legacy tracker
- * then warn the admin. If the error doesn't appear to be transient,
- * then disable recovery tracking.
- */
-static void
-legacy_recdir_name_error(struct nfs4_client *clp, int error)
-{
-	printk(KERN_ERR "NFSD: unable to generate recoverydir "
-			"name (%d).\n", error);
-
-	/*
-	 * if the algorithm just doesn't exist, then disable the recovery
-	 * tracker altogether. The crypto libs will generally return this if
-	 * FIPS is enabled as well.
-	 */
-	if (error == -ENOENT) {
-		printk(KERN_ERR "NFSD: disabling legacy clientid tracking. "
-			"Reboot recovery will not function correctly!\n");
-		nfsd4_client_tracking_exit(clp->net);
-	}
 }
 
 static void
 __nfsd4_create_reclaim_record_grace(struct nfs4_client *clp,
 		const char *dname, int len, struct nfsd_net *nn)

@@ -180,13 +142,11 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
 	if (test_and_set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return;
 	if (!nn->rec_file)
 		return;
 
-	status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-	if (status)
-		return legacy_recdir_name_error(clp, status);
+	nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 	status = nfs4_save_creds(&original_cred);
 	if (status < 0)
 		return;
 

@@ -374,13 +334,11 @@ nfsd4_remove_clid_dir(struct nfs4_client *clp)
 	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 
 	if (!nn->rec_file || !test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return;
 
-	status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-	if (status)
-		return legacy_recdir_name_error(clp, status);
+	nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 	status = mnt_want_write_file(nn->rec_file);
 	if (status)
 		goto out;
 	clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);

@@ -601,10 +559,15 @@ nfsd4_legacy_tracking_init(struct net *net)
 	if (net != &init_net) {
 		pr_warn("NFSD: attempt to initialize legacy client tracking in a container ignored.\n");
 		return -EINVAL;
 	}
 
+	if (fips_enabled) {
+		pr_warn("NFSD: legacy client tracking is disabled due to FIPS\n");
+		return -EINVAL;
+	}
+
 	status = nfs4_legacy_state_init(net);
 	if (status)
 		return status;
 
 	status = nfsd4_load_reboot_recovery_data(net);

@@ -657,25 +620,20 @@ nfs4_recoverydir(void)
 }
 
 static int
 nfsd4_check_legacy_client(struct nfs4_client *clp)
 {
-	int status;
 	char dname[HEXDIR_LEN];
 	struct nfs4_client_reclaim *crp;
 	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 	struct xdr_netobj name;
 
 	/* did we already find that this client is stable? */
 	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return 0;
 
-	status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-	if (status) {
-		legacy_recdir_name_error(clp, status);
-		return status;
-	}
+	nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 	/* look for it in the reclaim hashtable otherwise */
 	name.data = kmemdup(dname, HEXDIR_LEN, GFP_KERNEL);
 	if (!name.data) {
 		dprintk("%s: failed to allocate memory for name.data!\n",

@@ -1264,17 +1222,14 @@ nfsd4_cld_check(struct nfs4_client *clp)
 	if (crp)
 		goto found;
 
 #ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
 	if (nn->cld_net->cn_has_legacy) {
-		int status;
 		char dname[HEXDIR_LEN];
 		struct xdr_netobj name;
 
-		status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-		if (status)
-			return -ENOENT;
+		nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 		name.data = kmemdup(dname, HEXDIR_LEN, GFP_KERNEL);
 		if (!name.data) {
 			dprintk("%s: failed to allocate memory for name.data!\n",
 				__func__);

@@ -1315,15 +1270,12 @@ nfsd4_cld_check_v2(struct nfs4_client *clp)
 
 #ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
 	if (cn->cn_has_legacy) {
 		struct xdr_netobj name;
 		char dname[HEXDIR_LEN];
-		int status;
 
-		status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-		if (status)
-			return -ENOENT;
+		nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 		name.data = kmemdup(dname, HEXDIR_LEN, GFP_KERNEL);
 		if (!name.data) {
 			dprintk("%s: failed to allocate memory for name.data\n",
 					__func__);

@@ -1692,15 +1644,11 @@ nfsd4_cltrack_legacy_recdir(const struct xdr_netobj *name)
 		/* just return nothing if output will be truncated */
 		kfree(result);
 		return NULL;
 	}
 
-	copied = nfs4_make_rec_clidname(result + copied, name);
-	if (copied) {
-		kfree(result);
-		return NULL;
-	}
+	nfs4_make_rec_clidname(result + copied, name);
 
 	return result;
 }
 
 static char *

base-commit: 0739473694c4878513031006829f1030ec850bc2

Signature · 1 line

Chuck Lever

Quoted 23 lines · Chuck Lever

Quoted 21 lines · Eric Biggers

Update NFSD's support for "legacy client tracking" (which uses MD5) to use the MD5 library instead of crypto_shash. This has several benefits:

- Simpler code. Notably, much of the error-handling code is no longer needed, since the library functions can't fail.

- Improved performance due to reduced overhead. A microbenchmark of nfs4_make_rec_clidname() shows a speedup from 1455 cycles to 425.

- The MD5 code can now safely be built as a loadable module when nfsd is built as a loadable module. (Previously, nfsd forced the MD5 code to built-in, presumably to work around the unreliablity of the name-based loading.) Thus, select MD5 from the tristate option NFSD if NFSD_LEGACY_CLIENT_TRACKING, instead of from the bool option NFSD_V4.

To preserve the existing behavior of legacy client tracking support being disabled when the kernel is booted with "fips=1", make nfsd4_legacy_tracking_init() return an error if fips_enabled. I don't know if this is truly needed, but it preserves the existing behavior.

Signed-off-by Eric Biggers <[email protected]>

Quoted 4 lines

No objection, but let's cross our t's and dot our i's. Scott, when you have recovered from bake-a-thon, can you have a look at this one?

Thanks!

Looks fine to me.

Reviewed-by Scott Mayhew <[email protected]>

I agree with Jeff - it would be nice to just remove the legacy tracking.

I'm guessing it's still used in smaller/embedded setups? RHEL and Fedora haven't had it enabled for years. Looking at a few other distros... it's not enabled in OpenSUSE Leap or Tumbleweed. It's not enabled in Debian Sid (but it is enabled in Trixie).

Quoted 251 lines

Quoted 246 lines

---
 fs/nfsd/Kconfig       |  3 +-
 fs/nfsd/nfs4recover.c | 82 ++++++++-----------------------------------
 2 files changed, 16 insertions(+), 69 deletions(-)

fs/nfsd/Kconfig +1 -2

index e134dce45e350..380a4caa33a73 100644
@@ -3,10 +3,11 @@ config NFSD
 	tristate "NFS server support"
 	depends on INET
 	depends on FILE_LOCKING
 	depends on FSNOTIFY
 	select CRC32
+	select CRYPTO_LIB_MD5 if NFSD_LEGACY_CLIENT_TRACKING
 	select CRYPTO_LIB_SHA256 if NFSD_V4
 	select LOCKD
 	select SUNRPC
 	select EXPORTFS
 	select NFS_COMMON

@@ -75,12 +76,10 @@ config NFSD_V3_ACL
 config NFSD_V4
 	bool "NFS server support for NFS version 4"
 	depends on NFSD && PROC_FS
 	select FS_POSIX_ACL
 	select RPCSEC_GSS_KRB5
-	select CRYPTO
-	select CRYPTO_MD5
 	select GRACE_PERIOD
 	select NFS_V4_2_SSC_HELPER if NFS_V4_2
 	help
 	  This option enables support in your system's NFS server for
 	  version 4 of the NFS protocol (RFC 3530).

fs/nfsd/nfs4recover.c +15 -67

index e2b9472e5c78c..dbc0aecef95e3 100644
@@ -30,13 +30,14 @@
 *  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 *
 */
 
-#include <crypto/hash.h>
+#include <crypto/md5.h>
 #include <crypto/sha2.h>
 #include <linux/file.h>
+#include <linux/fips.h>
 #include <linux/slab.h>
 #include <linux/namei.h>
 #include <linux/sched.h>
 #include <linux/fs.h>
 #include <linux/module.h>

@@ -90,61 +91,22 @@ static void
 nfs4_reset_creds(const struct cred *original)
 {
 	put_cred(revert_creds(original));
 }
 
-static int
+static void
 nfs4_make_rec_clidname(char dname[HEXDIR_LEN], const struct xdr_netobj *clname)
 {
 	u8 digest[MD5_DIGEST_SIZE];
-	struct crypto_shash *tfm;
-	int status;
 
 	dprintk("NFSD: nfs4_make_rec_clidname for %.*s\n",
 			clname->len, clname->data);
-	tfm = crypto_alloc_shash("md5", 0, 0);
-	if (IS_ERR(tfm)) {
-		status = PTR_ERR(tfm);
-		goto out_no_tfm;
-	}
 
-	status = crypto_shash_tfm_digest(tfm, clname->data, clname->len,
-					 digest);
-	if (status)
-		goto out;
+	md5(clname->data, clname->len, digest);
 
 	static_assert(HEXDIR_LEN == 2 * MD5_DIGEST_SIZE + 1);
 	sprintf(dname, "%*phN", MD5_DIGEST_SIZE, digest);
-
-	status = 0;
-out:
-	crypto_free_shash(tfm);
-out_no_tfm:
-	return status;
-}
-
-/*
- * If we had an error generating the recdir name for the legacy tracker
- * then warn the admin. If the error doesn't appear to be transient,
- * then disable recovery tracking.
- */
-static void
-legacy_recdir_name_error(struct nfs4_client *clp, int error)
-{
-	printk(KERN_ERR "NFSD: unable to generate recoverydir "
-			"name (%d).\n", error);
-
-	/*
-	 * if the algorithm just doesn't exist, then disable the recovery
-	 * tracker altogether. The crypto libs will generally return this if
-	 * FIPS is enabled as well.
-	 */
-	if (error == -ENOENT) {
-		printk(KERN_ERR "NFSD: disabling legacy clientid tracking. "
-			"Reboot recovery will not function correctly!\n");
-		nfsd4_client_tracking_exit(clp->net);
-	}
 }
 
 static void
 __nfsd4_create_reclaim_record_grace(struct nfs4_client *clp,
 		const char *dname, int len, struct nfsd_net *nn)

@@ -180,13 +142,11 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
 	if (test_and_set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return;
 	if (!nn->rec_file)
 		return;
 
-	status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-	if (status)
-		return legacy_recdir_name_error(clp, status);
+	nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 	status = nfs4_save_creds(&original_cred);
 	if (status < 0)
 		return;
 

@@ -374,13 +334,11 @@ nfsd4_remove_clid_dir(struct nfs4_client *clp)
 	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 
 	if (!nn->rec_file || !test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return;
 
-	status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-	if (status)
-		return legacy_recdir_name_error(clp, status);
+	nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 	status = mnt_want_write_file(nn->rec_file);
 	if (status)
 		goto out;
 	clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);

@@ -601,10 +559,15 @@ nfsd4_legacy_tracking_init(struct net *net)
 	if (net != &init_net) {
 		pr_warn("NFSD: attempt to initialize legacy client tracking in a container ignored.\n");
 		return -EINVAL;
 	}
 
+	if (fips_enabled) {
+		pr_warn("NFSD: legacy client tracking is disabled due to FIPS\n");
+		return -EINVAL;
+	}
+
 	status = nfs4_legacy_state_init(net);
 	if (status)
 		return status;
 
 	status = nfsd4_load_reboot_recovery_data(net);

@@ -657,25 +620,20 @@ nfs4_recoverydir(void)
 }
 
 static int
 nfsd4_check_legacy_client(struct nfs4_client *clp)
 {
-	int status;
 	char dname[HEXDIR_LEN];
 	struct nfs4_client_reclaim *crp;
 	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 	struct xdr_netobj name;
 
 	/* did we already find that this client is stable? */
 	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return 0;
 
-	status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-	if (status) {
-		legacy_recdir_name_error(clp, status);
-		return status;
-	}
+	nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 	/* look for it in the reclaim hashtable otherwise */
 	name.data = kmemdup(dname, HEXDIR_LEN, GFP_KERNEL);
 	if (!name.data) {
 		dprintk("%s: failed to allocate memory for name.data!\n",

@@ -1264,17 +1222,14 @@ nfsd4_cld_check(struct nfs4_client *clp)
 	if (crp)
 		goto found;
 
 #ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
 	if (nn->cld_net->cn_has_legacy) {
-		int status;
 		char dname[HEXDIR_LEN];
 		struct xdr_netobj name;
 
-		status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-		if (status)
-			return -ENOENT;
+		nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 		name.data = kmemdup(dname, HEXDIR_LEN, GFP_KERNEL);
 		if (!name.data) {
 			dprintk("%s: failed to allocate memory for name.data!\n",
 				__func__);

@@ -1315,15 +1270,12 @@ nfsd4_cld_check_v2(struct nfs4_client *clp)
 
 #ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
 	if (cn->cn_has_legacy) {
 		struct xdr_netobj name;
 		char dname[HEXDIR_LEN];
-		int status;
 
-		status = nfs4_make_rec_clidname(dname, &clp->cl_name);
-		if (status)
-			return -ENOENT;
+		nfs4_make_rec_clidname(dname, &clp->cl_name);
 
 		name.data = kmemdup(dname, HEXDIR_LEN, GFP_KERNEL);
 		if (!name.data) {
 			dprintk("%s: failed to allocate memory for name.data\n",
 					__func__);

@@ -1692,15 +1644,11 @@ nfsd4_cltrack_legacy_recdir(const struct xdr_netobj *name)
 		/* just return nothing if output will be truncated */
 		kfree(result);
 		return NULL;
 	}
 
-	copied = nfs4_make_rec_clidname(result + copied, name);
-	if (copied) {
-		kfree(result);
-		return NULL;
-	}
+	nfs4_make_rec_clidname(result + copied, name);
 
 	return result;
 }
 
 static char *

base-commit: 0739473694c4878513031006829f1030ec850bc2

Signature · 1 line

Chuck Lever

Update NFSD's support for "legacy client tracking" (which uses MD5) to use the MD5 library instead of crypto_shash. This has several benefits:

- Simpler code. Notably, much of the error-handling code is no longer needed, since the library functions can't fail.

- Improved performance due to reduced overhead. A microbenchmark of nfs4_make_rec_clidname() shows a speedup from 1455 cycles to 425.

- The MD5 code can now safely be built as a loadable module when nfsd is built as a loadable module. (Previously, nfsd forced the MD5 code to built-in, presumably to work around the unreliablity of the name-based loading.) Thus, select MD5 from the tristate option NFSD if NFSD_LEGACY_CLIENT_TRACKING, instead of from the bool option NFSD_V4.

To preserve the existing behavior of legacy client tracking support being disabled when the kernel is booted with "fips=1", make nfsd4_legacy_tracking_init() return an error if fips_enabled. I don't know if this is truly needed, but it preserves the existing behavior.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/nfsd/Kconfig       |  3 +-
 fs/nfsd/nfs4recover.c | 82 ++++++++-----------------------------------
 2 files changed, 16 insertions(+), 69 deletions(-)