All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Chuck Lever <chuck.lever@oracle.com>, Neil Brown <neilb@suse.de>,
	Olga Kornievskaia <kolga@netapp.com>,
	Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	Zhi Li <yieli@redhat.com>, Jeff Layton <jlayton@kernel.org>
Subject: [PATCH] nfsd: properly tear down server when write_ports fails
Date: Mon, 11 Dec 2023 15:35:30 -0500	[thread overview]
Message-ID: <20231211-nfsd-fixes-v1-1-c87a802f4977@kernel.org> (raw)

When the initial write to the "portlist" file fails, we'll currently put
the reference to the nn->nfsd_serv, but leave the pointer intact. This
leads to a UAF if someone tries to write to "portlist" again.

Simple reproducer, from a host with nfsd shut down:

    # echo "foo 2049" > /proc/fs/nfsd/portlist
    # echo "foo 2049" > /proc/fs/nfsd/portlist

The kernel will oops on the second one when it trips over the dangling
nn->nfsd_serv pointer. There is a similar bug in __write_ports_addfd.

This patch fixes it by adding some extra logic to nfsd_put to ensure
that nfsd_last_thread is called prior to putting the reference when the
conditions are right.

Fixes: 9f28a971ee9f ("nfsd: separate nfsd_last_thread() from nfsd_put()")
Closes: https://issues.redhat.com/browse/RHEL-19081
Reported-by: Zhi Li <yieli@redhat.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
This should probably go to stable, but we'll need to backport for v6.6
since older kernels don't have nfsd_nl_rpc_status_get_done. We should
just be able to drop that hunk though.
---
 fs/nfsd/nfsctl.c | 32 ++++++++++++++++++++++++++++----
 fs/nfsd/nfsd.h   |  8 +-------
 fs/nfsd/nfssvc.c |  2 +-
 3 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 3e15b72f421d..1ceccf804e44 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -61,6 +61,30 @@ enum {
 	NFSD_MaxReserved
 };
 
+/**
+ * nfsd_put - put the reference to the nfsd_serv for given net
+ * @net: the net namespace for the serv
+ * @err: current error for the op
+ *
+ * When putting a reference to the nfsd_serv from a control operation
+ * we must first call nfsd_last_thread if all of these are true:
+ *
+ * - the configuration operation is going fail
+ * - there are no running threads
+ * - there are no successfully configured ports
+ *
+ * Otherwise, just put the serv reference.
+ */
+static inline void nfsd_put(struct net *net, int err)
+{
+	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+	struct svc_serv *serv = nn->nfsd_serv;
+
+	if (err < 0 && !nn->nfsd_serv->sv_nrthreads && !nn->keep_active)
+		nfsd_last_thread(net);
+	svc_put(serv);
+}
+
 /*
  * write() for these nodes.
  */
@@ -709,7 +733,7 @@ static ssize_t __write_ports_addfd(char *buf, struct net *net, const struct cred
 	    !nn->nfsd_serv->sv_nrthreads && !xchg(&nn->keep_active, 1))
 		svc_get(nn->nfsd_serv);
 
-	nfsd_put(net);
+	nfsd_put(net, err);
 	return err;
 }
 
@@ -748,7 +772,7 @@ static ssize_t __write_ports_addxprt(char *buf, struct net *net, const struct cr
 	if (!nn->nfsd_serv->sv_nrthreads && !xchg(&nn->keep_active, 1))
 		svc_get(nn->nfsd_serv);
 
-	nfsd_put(net);
+	nfsd_put(net, 0);
 	return 0;
 out_close:
 	xprt = svc_find_xprt(nn->nfsd_serv, transport, net, PF_INET, port);
@@ -757,7 +781,7 @@ static ssize_t __write_ports_addxprt(char *buf, struct net *net, const struct cr
 		svc_xprt_put(xprt);
 	}
 out_err:
-	nfsd_put(net);
+	nfsd_put(net, err);
 	return err;
 }
 
@@ -1687,7 +1711,7 @@ int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
 int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb)
 {
 	mutex_lock(&nfsd_mutex);
-	nfsd_put(sock_net(cb->skb->sk));
+	nfsd_put(sock_net(cb->skb->sk), 0);
 	mutex_unlock(&nfsd_mutex);
 
 	return 0;
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index f5ff42f41ee7..3aa8cd2c19ac 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -113,13 +113,6 @@ int		nfsd_pool_stats_open(struct inode *, struct file *);
 int		nfsd_pool_stats_release(struct inode *, struct file *);
 void		nfsd_shutdown_threads(struct net *net);
 
-static inline void nfsd_put(struct net *net)
-{
-	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
-
-	svc_put(nn->nfsd_serv);
-}
-
 bool		i_am_nfsd(void);
 
 struct nfsdfs_client {
@@ -153,6 +146,7 @@ struct nfsd_net;
 enum vers_op {NFSD_SET, NFSD_CLEAR, NFSD_TEST, NFSD_AVAIL };
 int nfsd_vers(struct nfsd_net *nn, int vers, enum vers_op change);
 int nfsd_minorversion(struct nfsd_net *nn, u32 minorversion, enum vers_op change);
+void nfsd_last_thread(struct net *net);
 void nfsd_reset_versions(struct nfsd_net *nn);
 int nfsd_create_serv(struct net *net);
 
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index fe61d9bbcc1f..d6939e23ffcf 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -542,7 +542,7 @@ static struct notifier_block nfsd_inet6addr_notifier = {
 /* Only used under nfsd_mutex, so this atomic may be overkill: */
 static atomic_t nfsd_notifier_refcount = ATOMIC_INIT(0);
 
-static void nfsd_last_thread(struct net *net)
+void nfsd_last_thread(struct net *net)
 {
 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
 	struct svc_serv *serv = nn->nfsd_serv;

---
base-commit: a39b6ac3781d46ba18193c9dbb2110f31e9bffe9
change-id: 20231211-nfsd-fixes-d9f21d5c12d7

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>


             reply	other threads:[~2023-12-11 20:35 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-11 20:35 Jeff Layton [this message]
2023-12-11 22:59 ` [PATCH] nfsd: properly tear down server when write_ports fails NeilBrown
2023-12-11 23:11   ` Jeff Layton
2023-12-12 13:50     ` Chuck Lever
2023-12-12 14:05       ` Chuck Lever
2023-12-13  3:45         ` NeilBrown
2023-12-13 14:26           ` Jeff Layton
2023-12-13 14:42           ` Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231211-nfsd-fixes-v1-1-c87a802f4977@kernel.org \
    --to=jlayton@kernel.org \
    --cc=Dai.Ngo@oracle.com \
    --cc=chuck.lever@oracle.com \
    --cc=kolga@netapp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=tom@talpey.com \
    --cc=yieli@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.