| 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: <aA_ezENy3YmMUzsU@aion>
Date: Mon, 28 Apr 2025 16:02:20 -0400
From: Scott Mayhew <smayhew@...hat.com>
To: Chuck Lever <chuck.lever@...cle.com>
Cc: Eric Biggers <ebiggers@...nel.org>, NeilBrown <neil@...wn.name>,
Olga Kornievskaia <okorniev@...hat.com>,
Jeff Layton <jlayton@...nel.org>, Dai Ngo <Dai.Ngo@...cle.com>,
Tom Talpey <tom@...pey.com>, linux-kernel@...r.kernel.org,
linux-crypto@...r.kernel.org, linux-nfs@...r.kernel.org
Subject: Re: [PATCH] nfsd: use SHA-256 library API instead of crypto_shash API
On Mon, 28 Apr 2025, Chuck Lever wrote:
> On 4/28/25 3:36 PM, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@...gle.com>
> >
> > This user of SHA-256 does not support any other algorithm, so the
> > crypto_shash abstraction provides no value. Just use the SHA-256
> > library API instead, which is much simpler and easier to use.
> >
> > Signed-off-by: Eric Biggers <ebiggers@...gle.com>
> > ---
> > fs/nfsd/Kconfig | 2 +-
> > fs/nfsd/nfs4recover.c | 57 ++++++++-----------------------------------
> > 2 files changed, 11 insertions(+), 48 deletions(-)
> >
> > diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> > index 731a88f6313eb..879e0b104d1c8 100644
> > --- a/fs/nfsd/Kconfig
> > +++ b/fs/nfsd/Kconfig
> > @@ -75,12 +75,12 @@ 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_LIB_SHA256
> > select CRYPTO_MD5
> > - select CRYPTO_SHA256
> > 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).
> > diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> > index acde3edab7336..c3840793261be 100644
> > --- a/fs/nfsd/nfs4recover.c
> > +++ b/fs/nfsd/nfs4recover.c
> > @@ -31,10 +31,11 @@
> > * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > *
> > */
> >
> > #include <crypto/hash.h>
> > +#include <crypto/sha2.h>
> > #include <linux/file.h>
> > #include <linux/slab.h>
> > #include <linux/namei.h>
> > #include <linux/sched.h>
> > #include <linux/fs.h>
> > @@ -735,11 +736,10 @@ static const struct nfsd4_client_tracking_ops nfsd4_legacy_tracking_ops = {
> > struct cld_net {
> > struct rpc_pipe *cn_pipe;
> > spinlock_t cn_lock;
> > struct list_head cn_list;
> > unsigned int cn_xid;
> > - struct crypto_shash *cn_tfm;
> > #ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
> > bool cn_has_legacy;
> > #endif
> > };
> >
> > @@ -1061,12 +1061,10 @@ nfsd4_remove_cld_pipe(struct net *net)
> > struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> > struct cld_net *cn = nn->cld_net;
> >
> > nfsd4_cld_unregister_net(net, cn->cn_pipe);
> > rpc_destroy_pipe_data(cn->cn_pipe);
> > - if (cn->cn_tfm)
> > - crypto_free_shash(cn->cn_tfm);
> > kfree(nn->cld_net);
> > nn->cld_net = NULL;
> > }
> >
> > static struct cld_upcall *
> > @@ -1156,12 +1154,10 @@ nfsd4_cld_create_v2(struct nfs4_client *clp)
> > int ret;
> > struct cld_upcall *cup;
> > struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
> > struct cld_net *cn = nn->cld_net;
> > struct cld_msg_v2 *cmsg;
> > - struct crypto_shash *tfm = cn->cn_tfm;
> > - struct xdr_netobj cksum;
> > char *principal = NULL;
> >
> > /* Don't upcall if it's already stored */
> > if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
> > return;
> > @@ -1180,36 +1176,22 @@ nfsd4_cld_create_v2(struct nfs4_client *clp)
> > if (clp->cl_cred.cr_raw_principal)
> > principal = clp->cl_cred.cr_raw_principal;
> > else if (clp->cl_cred.cr_principal)
> > principal = clp->cl_cred.cr_principal;
> > if (principal) {
> > - cksum.len = crypto_shash_digestsize(tfm);
> > - cksum.data = kmalloc(cksum.len, GFP_KERNEL);
> > - if (cksum.data == NULL) {
> > - ret = -ENOMEM;
> > - goto out;
> > - }
> > - ret = crypto_shash_tfm_digest(tfm, principal, strlen(principal),
> > - cksum.data);
> > - if (ret) {
> > - kfree(cksum.data);
> > - goto out;
> > - }
> > - cmsg->cm_u.cm_clntinfo.cc_princhash.cp_len = cksum.len;
> > - memcpy(cmsg->cm_u.cm_clntinfo.cc_princhash.cp_data,
> > - cksum.data, cksum.len);
> > - kfree(cksum.data);
> > + sha256(principal, strlen(principal),
> > + cmsg->cm_u.cm_clntinfo.cc_princhash.cp_data);
> > + cmsg->cm_u.cm_clntinfo.cc_princhash.cp_len = SHA256_DIGEST_SIZE;
> > } else
> > cmsg->cm_u.cm_clntinfo.cc_princhash.cp_len = 0;
> >
> > ret = cld_pipe_upcall(cn->cn_pipe, cmsg, nn);
> > if (!ret) {
> > ret = cmsg->cm_status;
> > set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
> > }
> >
> > -out:
> > free_cld_upcall(cup);
> > out_err:
> > if (ret)
> > pr_err("NFSD: Unable to create client record on stable storage: %d\n",
> > ret);
> > @@ -1347,13 +1329,10 @@ static int
> > nfsd4_cld_check_v2(struct nfs4_client *clp)
> > {
> > struct nfs4_client_reclaim *crp;
> > struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
> > struct cld_net *cn = nn->cld_net;
> > - int status;
> > - struct crypto_shash *tfm = cn->cn_tfm;
> > - struct xdr_netobj cksum;
> > char *principal = NULL;
> >
> > /* did we already find that this client is stable? */
> > if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
> > return 0;
> > @@ -1365,10 +1344,11 @@ 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;
> >
> > @@ -1387,32 +1367,22 @@ nfsd4_cld_check_v2(struct nfs4_client *clp)
> > }
> > #endif
> > return -ENOENT;
> > found:
> > if (crp->cr_princhash.len) {
> > + u8 digest[SHA256_DIGEST_SIZE];
> > +
> > if (clp->cl_cred.cr_raw_principal)
> > principal = clp->cl_cred.cr_raw_principal;
> > else if (clp->cl_cred.cr_principal)
> > principal = clp->cl_cred.cr_principal;
> > if (principal == NULL)
> > return -ENOENT;
> > - cksum.len = crypto_shash_digestsize(tfm);
> > - cksum.data = kmalloc(cksum.len, GFP_KERNEL);
> > - if (cksum.data == NULL)
> > - return -ENOENT;
> > - status = crypto_shash_tfm_digest(tfm, principal,
> > - strlen(principal), cksum.data);
> > - if (status) {
> > - kfree(cksum.data);
> > + sha256(principal, strlen(principal), digest);
> > + if (memcmp(crp->cr_princhash.data, digest,
> > + crp->cr_princhash.len))
> > return -ENOENT;
> > - }
> > - if (memcmp(crp->cr_princhash.data, cksum.data,
> > - crp->cr_princhash.len)) {
> > - kfree(cksum.data);
> > - return -ENOENT;
> > - }
> > - kfree(cksum.data);
> > }
> > crp->cr_clp = clp;
> > return 0;
> > }
> >
> > @@ -1588,11 +1558,10 @@ nfsd4_cld_tracking_init(struct net *net)
> > {
> > int status;
> > struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> > bool running;
> > int retries = 10;
> > - struct crypto_shash *tfm;
> >
> > status = nfs4_cld_state_init(net);
> > if (status)
> > return status;
> >
> > @@ -1613,16 +1582,10 @@ nfsd4_cld_tracking_init(struct net *net)
> >
> > if (!running) {
> > status = -ETIMEDOUT;
> > goto err_remove;
> > }
> > - tfm = crypto_alloc_shash("sha256", 0, 0);
> > - if (IS_ERR(tfm)) {
> > - status = PTR_ERR(tfm);
> > - goto err_remove;
> > - }
> > - nn->cld_net->cn_tfm = tfm;
> >
> > status = nfsd4_cld_get_version(nn);
> > if (status == -EOPNOTSUPP)
> > pr_warn("NFSD: nfsdcld GetVersion upcall failed. Please upgrade nfsdcld.\n");
> >
> >
> > base-commit: 33035b665157558254b3c21c3f049fd728e72368
>
> Make sense to me. Scott, can I get an R-b: from you?
Reviewed-by: Scott Mayhew <smayhew@...hat.com>
-Scott
>
> --
> Chuck Lever
>
Powered by blists - more mailing lists