5.10.220-rc1 review

PATCH In review Greg Kroah-Hartman 14 participants 802 messages Jun 18, 2024 – Jul 24, 2024 Page 8 of 8

Subsystems (7)

Stable branch VFS NFS client NFS server +3 more subsystems

lore

Share & tools

lore

Greg Kroah-Hartman

author maintainer

2024-06-18 13:20 UTC #768 lore raw

PATCH 755/770 nfsd: make a copy of struct iattr before calling notify_change

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: Jeff Layton <[email protected]>

[ Upstream commit d53d70084d27f56bcdf5074328f2c9ec861be596 ]

notify_change can modify the iattr structure. In particular it can end up setting ATTR_MODE when ATTR_KILL_SUID is already set, causing a BUG() if the same iattr is passed to notify_change more than once.

Make a copy of the struct iattr before calling notify_change.

Reported-by Zhi Li <[email protected]>

Link https://bugzilla.redhat.com/show_bug.cgi?id=2207969

Tested-by Zhi Li <[email protected]>

Fixes 34b91dda7124 ("NFSD: Make nfsd4_setattr() wait before returning NFS4ERR_DELAY")

Signed-off-by Jeff Layton <[email protected]>

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfsd/vfs.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

fs/nfsd/vfs.c +9 -1

index abc682854507b..542a1adbbf2a1 100644
@@ -545,7 +545,15 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 
 	inode_lock(inode);
 	for (retries = 1;;) {
-		host_err = __nfsd_setattr(dentry, iap);
+		struct iattr attrs;
+
+		/*
+		 * notify_change() can alter its iattr argument, making
+		 * @iap unsuitable for submission multiple times. Make a
+		 * copy for every loop iteration.
+		 */
+		attrs = *iap;
+		host_err = __nfsd_setattr(dentry, &attrs);
 		if (host_err != -EAGAIN || !retries--)
 			break;
 		if (!nfsd_wait_for_delegreturn(rqstp, inode))

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:20 UTC #769 lore raw

PATCH 756/770 nfsd: fix double fget() bug in __write_ports_addfd()

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: Dan Carpenter <[email protected]>

[ Upstream commit c034203b6a9dae6751ef4371c18cb77983e30c28 ]

The bug here is that you cannot rely on getting the same socket from multiple calls to fget() because userspace can influence that. This is a kind of double fetch bug.

The fix is to delete the svc_alien_sock() function and instead do the checking inside the svc_addsock() function.

Fixes 3064639423c4 ("nfsd: check passed socket's net matches NFSd superblock's one")

Signed-off-by Dan Carpenter <[email protected]>

Reviewed-by NeilBrown <[email protected]>

Reviewed-by Jeff Layton <[email protected]>

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfsd/nfsctl.c               |  7 +------
 include/linux/sunrpc/svcsock.h |  7 +++----
 net/sunrpc/svcsock.c           | 24 ++++++------------------
 3 files changed, 10 insertions(+), 28 deletions(-)

fs/nfsd/nfsctl.c +1 -6

index c2577ee7ffb22..76a60e7a75097 100644
@@ -714,16 +714,11 @@ static ssize_t __write_ports_addfd(char *buf, struct net *net, const struct cred
 	if (err != 0 || fd < 0)
 		return -EINVAL;
 
-	if (svc_alien_sock(net, fd)) {
-		printk(KERN_ERR "%s: socket net is different to NFSd's one\n", __func__);
-		return -EINVAL;
-	}
-
 	err = nfsd_create_serv(net);
 	if (err != 0)
 		return err;
 
-	err = svc_addsock(nn->nfsd_serv, fd, buf, SIMPLE_TRANSACTION_LIMIT, cred);
+	err = svc_addsock(nn->nfsd_serv, net, fd, buf, SIMPLE_TRANSACTION_LIMIT, cred);
 
 	if (err >= 0 &&
 	    !nn->nfsd_serv->sv_nrthreads && !xchg(&nn->keep_active, 1))

include/linux/sunrpc/svcsock.h +3 -4

index b7ac7fe683067..a366d3eb05315 100644
@@ -57,10 +57,9 @@ int		svc_recv(struct svc_rqst *, long);
 int		svc_send(struct svc_rqst *);
 void		svc_drop(struct svc_rqst *);
 void		svc_sock_update_bufs(struct svc_serv *serv);
-bool		svc_alien_sock(struct net *net, int fd);
-int		svc_addsock(struct svc_serv *serv, const int fd,
-					char *name_return, const size_t len,
-					const struct cred *cred);
+int		svc_addsock(struct svc_serv *serv, struct net *net,
+			    const int fd, char *name_return, const size_t len,
+			    const struct cred *cred);
 void		svc_init_xprt_sock(void);
 void		svc_cleanup_xprt_sock(void);
 struct svc_xprt *svc_sock_create(struct svc_serv *serv, int prot);

net/sunrpc/svcsock.c +6 ~~-18~~

index 90f6231d6ed67..cb0cfcd8a8141 100644
@@ -1342,25 +1342,10 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
 	return svsk;
 }
 
-bool svc_alien_sock(struct net *net, int fd)
-{
-	int err;
-	struct socket *sock = sockfd_lookup(fd, &err);
-	bool ret = false;
-
-	if (!sock)
-		goto out;
-	if (sock_net(sock->sk) != net)
-		ret = true;
-	sockfd_put(sock);
-out:
-	return ret;
-}
-EXPORT_SYMBOL_GPL(svc_alien_sock);
-
 /**
  * svc_addsock - add a listener socket to an RPC service
  * @serv: pointer to RPC service to which to add a new listener
+ * @net: caller's network namespace
  * @fd: file descriptor of the new listener
  * @name_return: pointer to buffer to fill in with name of listener
  * @len: size of the buffer

@@ -1370,8 +1355,8 @@ EXPORT_SYMBOL_GPL(svc_alien_sock);
  * Name is terminated with '\n'.  On error, returns a negative errno
  * value.
  */
-int svc_addsock(struct svc_serv *serv, const int fd, char *name_return,
-		const size_t len, const struct cred *cred)
+int svc_addsock(struct svc_serv *serv, struct net *net, const int fd,
+		char *name_return, const size_t len, const struct cred *cred)
 {
 	int err = 0;
 	struct socket *so = sockfd_lookup(fd, &err);

@@ -1382,6 +1367,9 @@ int svc_addsock(struct svc_serv *serv, const int fd, char *name_return,
 
 	if (!so)
 		return err;
+	err = -EINVAL;
+	if (sock_net(so->sk) != net)
+		goto out;
 	err = -EAFNOSUPPORT;
 	if ((so->sk->sk_family != PF_INET) && (so->sk->sk_family != PF_INET6))
 		goto out;

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:20 UTC #770 lore raw

PATCH 757/770 lockd: drop inappropriate svc_get() from locked_get()

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: NeilBrown <[email protected]>

[ Upstream commit 665e89ab7c5af1f2d260834c861a74b01a30f95f ]

The below-mentioned patch was intended to simplify refcounting on the svc_serv used by locked. The goal was to only ever have a single reference from the single thread. To that end we dropped a call to lockd_start_svc() (except when creating thread) which would take a reference, and dropped the svc_put(serv) that would drop that reference.

Unfortunately we didn't also remove the svc_get() from lockd_create_svc() in the case where the svc_serv already existed. So after the patch:
- on the first call the svc_serv was allocated and the one reference was given to the thread, so there are no extra references
- on subsequent calls svc_get() was called so there is now an extra reference.
This is clearly not consistent.

The inconsistency is also clear in the current code in lockd_get() takes *two* references, one on nlmsvc_serv and one by incrementing
nlmsvc_users. This clearly does not match lockd_put().

So: drop that svc_get() from lockd_get() (which used to be in lockd_create_svc().

Reported-by Ido Schimmel <[email protected]>

Closes https://lore.kernel.org/linux-nfs/ZHsI%2FH16VX9kJQX1@shredder/T/#u open here

Fixes b73a2972041b ("lockd: move lockd_start_svc() call into lockd_create_svc()")

Signed-off-by NeilBrown <[email protected]>

Tested-by Ido Schimmel <[email protected]>

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/lockd/svc.c | 1 -
 1 file changed, 1 deletion(-)

fs/lockd/svc.c +0 -1

index 59ef8a1f843f3..5579e67da17db 100644
@@ -355,7 +355,6 @@ static int lockd_get(void)
 	int error;
 
 	if (nlmsvc_serv) {
-		svc_get(nlmsvc_serv);
 		nlmsvc_users++;
 		return 0;
 	}

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:20 UTC #771 lore raw

PATCH 758/770 NFSD: Add an nfsd4_encode_nfstime4() helper

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: Chuck Lever <[email protected]>

[ Upstream commit 262176798b18b12fd8ab84c94cfece0a6a652476 ]

Clean up: de-duplicate some common code.

Reviewed-by Jeff Layton <[email protected]>

Acked-by Tom Talpey <[email protected]>

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfsd/nfs4xdr.c | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

fs/nfsd/nfs4xdr.c +26 ~~-20~~

index d62382dfc135e..a81938c1e3efb 100644
@@ -2541,6 +2541,20 @@ static __be32 *encode_change(__be32 *p, struct kstat *stat, struct inode *inode,
 	return p;
 }
 
+static __be32 nfsd4_encode_nfstime4(struct xdr_stream *xdr,
+				    struct timespec64 *tv)
+{
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, XDR_UNIT * 3);
+	if (!p)
+		return nfserr_resource;
+
+	p = xdr_encode_hyper(p, (s64)tv->tv_sec);
+	*p = cpu_to_be32(tv->tv_nsec);
+	return nfs_ok;
+}
+
 /*
  * ctime (in NFSv4, time_metadata) is not writeable, and the client
  * doesn't really care what resolution could theoretically be stored by

@@ -3346,11 +3360,9 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 		p = xdr_encode_hyper(p, dummy64);
 	}
 	if (bmval1 & FATTR4_WORD1_TIME_ACCESS) {
-		p = xdr_reserve_space(xdr, 12);
-		if (!p)
-			goto out_resource;
-		p = xdr_encode_hyper(p, (s64)stat.atime.tv_sec);
-		*p++ = cpu_to_be32(stat.atime.tv_nsec);
+		status = nfsd4_encode_nfstime4(xdr, &stat.atime);
+		if (status)
+			goto out;
 	}
 	if (bmval1 & FATTR4_WORD1_TIME_DELTA) {
 		p = xdr_reserve_space(xdr, 12);

@@ -3359,25 +3371,19 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 		p = encode_time_delta(p, d_inode(dentry));
 	}
 	if (bmval1 & FATTR4_WORD1_TIME_METADATA) {
-		p = xdr_reserve_space(xdr, 12);
-		if (!p)
-			goto out_resource;
-		p = xdr_encode_hyper(p, (s64)stat.ctime.tv_sec);
-		*p++ = cpu_to_be32(stat.ctime.tv_nsec);
+		status = nfsd4_encode_nfstime4(xdr, &stat.ctime);
+		if (status)
+			goto out;
 	}
 	if (bmval1 & FATTR4_WORD1_TIME_MODIFY) {
-		p = xdr_reserve_space(xdr, 12);
-		if (!p)
-			goto out_resource;
-		p = xdr_encode_hyper(p, (s64)stat.mtime.tv_sec);
-		*p++ = cpu_to_be32(stat.mtime.tv_nsec);
+		status = nfsd4_encode_nfstime4(xdr, &stat.mtime);
+		if (status)
+			goto out;
 	}
 	if (bmval1 & FATTR4_WORD1_TIME_CREATE) {
-		p = xdr_reserve_space(xdr, 12);
-		if (!p)
-			goto out_resource;
-		p = xdr_encode_hyper(p, (s64)stat.btime.tv_sec);
-		*p++ = cpu_to_be32(stat.btime.tv_nsec);
+		status = nfsd4_encode_nfstime4(xdr, &stat.btime);
+		if (status)
+			goto out;
 	}
 	if (bmval1 & FATTR4_WORD1_MOUNTED_ON_FILEID) {
 		u64 ino = stat.ino;

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:20 UTC #772 lore raw

PATCH 759/770 nfsd: Fix creation time serialization order

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: Tavian Barnes <[email protected]>

[ Upstream commit d7dbed457c2ef83709a2a2723a2d58de43623449 ]

In nfsd4_encode_fattr(), TIME_CREATE was being written out after all other times. However, they should be written out in an order that matches the bit flags in bmval1, which in this case are

    #define FATTR4_WORD1_TIME_ACCESS        (1UL << 15)
    #define FATTR4_WORD1_TIME_CREATE        (1UL << 18)
    #define FATTR4_WORD1_TIME_DELTA         (1UL << 19)
    #define FATTR4_WORD1_TIME_METADATA      (1UL << 20)
    #define FATTR4_WORD1_TIME_MODIFY        (1UL << 21)

so TIME_CREATE should come second.

I noticed this on a FreeBSD NFSv4.2 client, which supports creation times. On this client, file times were weirdly permuted. With this patch applied on the server, times looked normal on the client.

Fixes e377a3e698fb ("nfsd: Add support for the birth time attribute")

Link https://unix.stackexchange.com/q/749605/56202

Signed-off-by Tavian Barnes <[email protected]>

Reviewed-by Jeff Layton <[email protected]>

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfsd/nfs4xdr.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

fs/nfsd/nfs4xdr.c +5 -5

index a81938c1e3efb..5a68c62864925 100644
@@ -3364,6 +3364,11 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 		if (status)
 			goto out;
 	}
+	if (bmval1 & FATTR4_WORD1_TIME_CREATE) {
+		status = nfsd4_encode_nfstime4(xdr, &stat.btime);
+		if (status)
+			goto out;
+	}
 	if (bmval1 & FATTR4_WORD1_TIME_DELTA) {
 		p = xdr_reserve_space(xdr, 12);
 		if (!p)

@@ -3380,11 +3385,6 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 		if (status)
 			goto out;
 	}
-	if (bmval1 & FATTR4_WORD1_TIME_CREATE) {
-		status = nfsd4_encode_nfstime4(xdr, &stat.btime);
-		if (status)
-			goto out;
-	}
 	if (bmval1 & FATTR4_WORD1_MOUNTED_ON_FILEID) {
 		u64 ino = stat.ino;

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:20 UTC #773 lore raw

PATCH 760/770 nfsd: dont allow nfsd threads to be signalled.

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: Chuck Lever <[email protected]>

[ Upstream commit 3903902401451b1cd9d797a8c79769eb26ac7fe5 ]

The original implementation of nfsd used signals to stop threads during shutdown.
In Linux 2.3.46pre5 nfsd gained the ability to shutdown threads internally it if was asked to run "0" threads. After this user-space transitioned to using "rpc.nfsd 0" to stop nfsd and sending signals to threads was no longer an important part of the API.

In commit 3ebdbe5203a8 ("SUNRPC: discard svo_setup and rename svc_set_num_threads_sync()") (v5.17-rc1~75^2~41) we finally removed the use of signals for stopping threads, using kthread_stop() instead.

This patch makes the "obvious" next step and removes the ability to signal nfsd threads - or any svc threads. nfsd stops allowing signals and we don't check for their delivery any more.

This will allow for some simplification in later patches.

A change worth noting is in nfsd4_ssc_setup_dul(). There was previously a signal_pending() check which would only succeed when the thread was being shut down. It should really have tested kthread_should_stop() as well. Now it just does the latter, not the former.

Signed-off-by NeilBrown <[email protected]>

Reviewed-by Jeff Layton <[email protected]>

[ cel: adjusted to apply to v5.10.y ]

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfs/callback.c     | 11 ++---------
 fs/nfsd/nfs4proc.c    |  8 ++++----
 fs/nfsd/nfssvc.c      | 12 ------------
 net/sunrpc/svc_xprt.c | 20 ++++++++------------
 4 files changed, 14 insertions(+), 37 deletions(-)

fs/nfs/callback.c +2 -9

index 456af7d230cf1..8fe143cad4a2b 100644
@@ -80,9 +80,6 @@ nfs4_callback_svc(void *vrqstp)
 	set_freezable();
 
 	while (!kthread_freezable_should_stop(NULL)) {
-
-		if (signal_pending(current))
-			flush_signals(current);
 		/*
 		 * Listen for a request on the socket
 		 */

@@ -112,11 +109,7 @@ nfs41_callback_svc(void *vrqstp)
 	set_freezable();
 
 	while (!kthread_freezable_should_stop(NULL)) {
-
-		if (signal_pending(current))
-			flush_signals(current);
-
-		prepare_to_wait(&serv->sv_cb_waitq, &wq, TASK_INTERRUPTIBLE);
+		prepare_to_wait(&serv->sv_cb_waitq, &wq, TASK_IDLE);
 		spin_lock_bh(&serv->sv_cb_lock);
 		if (!list_empty(&serv->sv_cb_list)) {
 			req = list_first_entry(&serv->sv_cb_list,

@@ -131,7 +124,7 @@ nfs41_callback_svc(void *vrqstp)
 		} else {
 			spin_unlock_bh(&serv->sv_cb_lock);
 			if (!kthread_should_stop())
-				schedule();
+				freezable_schedule();
 			finish_wait(&serv->sv_cb_waitq, &wq);
 		}
 	}

fs/nfsd/nfs4proc.c +4 -4

index 9c51c10bcf080..8560a11daa47d 100644
@@ -38,6 +38,7 @@
 #include <linux/slab.h>
 #include <linux/kthread.h>
 #include <linux/namei.h>
+#include <linux/freezer.h>
 
 #include <linux/sunrpc/addr.h>
 #include <linux/nfs_ssc.h>

@@ -1313,13 +1314,12 @@ static __be32 nfsd4_ssc_setup_dul(struct nfsd_net *nn, char *ipaddr,
 		/* found a match */
 		if (ni->nsui_busy) {
 			/*  wait - and try again */
-			prepare_to_wait(&nn->nfsd_ssc_waitq, &wait,
-				TASK_INTERRUPTIBLE);
+			prepare_to_wait(&nn->nfsd_ssc_waitq, &wait, TASK_IDLE);
 			spin_unlock(&nn->nfsd_ssc_lock);
 
 			/* allow 20secs for mount/unmount for now - revisit */
-			if (signal_pending(current) ||
-					(schedule_timeout(20*HZ) == 0)) {
+			if (kthread_should_stop() ||
+					(freezable_schedule_timeout(20*HZ) == 0)) {
 				finish_wait(&nn->nfsd_ssc_waitq, &wait);
 				kfree(work);
 				return nfserr_eagain;

fs/nfsd/nfssvc.c +0 ~~-12~~

index a0ecec54d3d7d..8063fab2c0279 100644
@@ -952,15 +952,6 @@ nfsd(void *vrqstp)
 
 	current->fs->umask = 0;
 
-	/*
-	 * thread is spawned with all signals set to SIG_IGN, re-enable
-	 * the ones that will bring down the thread
-	 */
-	allow_signal(SIGKILL);
-	allow_signal(SIGHUP);
-	allow_signal(SIGINT);
-	allow_signal(SIGQUIT);
-
 	atomic_inc(&nfsdstats.th_cnt);
 
 	set_freezable();

@@ -985,9 +976,6 @@ nfsd(void *vrqstp)
 		validate_process_creds();
 	}
 
-	/* Clear signals before calling svc_exit_thread() */
-	flush_signals(current);
-
 	atomic_dec(&nfsdstats.th_cnt);
 
 out:

net/sunrpc/svc_xprt.c +8 ~~-12~~

index 000b737784bd9..d1eacf3358b81 100644
@@ -674,13 +674,13 @@ static int svc_alloc_arg(struct svc_rqst *rqstp)
 		while (rqstp->rq_pages[i] == NULL) {
 			struct page *p = alloc_page(GFP_KERNEL);
 			if (!p) {
-				set_current_state(TASK_INTERRUPTIBLE);
-				if (signalled() || kthread_should_stop()) {
+				set_current_state(TASK_IDLE);
+				if (kthread_should_stop()) {
 					set_current_state(TASK_RUNNING);
 					return -EINTR;
 				}
-				schedule_timeout(msecs_to_jiffies(500));
 			}
+			freezable_schedule_timeout(msecs_to_jiffies(500));
 			rqstp->rq_pages[i] = p;
 		}
 	rqstp->rq_page_end = &rqstp->rq_pages[i];

@@ -713,7 +713,7 @@ rqst_should_sleep(struct svc_rqst *rqstp)
 		return false;
 
 	/* are we shutting down? */
-	if (signalled() || kthread_should_stop())
+	if (kthread_should_stop())
 		return false;
 
 	/* are we freezing? */

@@ -735,18 +735,14 @@ static struct svc_xprt *svc_get_next_xprt(struct svc_rqst *rqstp, long timeout)
 	if (rqstp->rq_xprt)
 		goto out_found;
 
-	/*
-	 * We have to be able to interrupt this wait
-	 * to bring down the daemons ...
-	 */
-	set_current_state(TASK_INTERRUPTIBLE);
+	set_current_state(TASK_IDLE);
 	smp_mb__before_atomic();
 	clear_bit(SP_CONGESTED, &pool->sp_flags);
 	clear_bit(RQ_BUSY, &rqstp->rq_flags);
 	smp_mb__after_atomic();
 
 	if (likely(rqst_should_sleep(rqstp)))
-		time_left = schedule_timeout(timeout);
+		time_left = freezable_schedule_timeout(timeout);
 	else
 		__set_current_state(TASK_RUNNING);

@@ -761,7 +757,7 @@ static struct svc_xprt *svc_get_next_xprt(struct svc_rqst *rqstp, long timeout)
 	if (!time_left)
 		atomic_long_inc(&pool->sp_stats.threads_timedout);
 
-	if (signalled() || kthread_should_stop())
+	if (kthread_should_stop())
 		return ERR_PTR(-EINTR);
 	return ERR_PTR(-EAGAIN);
 out_found:

@@ -860,7 +856,7 @@ int svc_recv(struct svc_rqst *rqstp, long timeout)
 	try_to_freeze();
 	cond_resched();
 	err = -EINTR;
-	if (signalled() || kthread_should_stop())
+	if (kthread_should_stop())
 		goto out;
 
 	xprt = svc_get_next_xprt(rqstp, timeout);

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:19 UTC #774 lore raw

PATCH 761/770 nfsd: Simplify code around svc_exit_thread() call in nfsd()

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: NeilBrown <[email protected]>

[ Upstream commit 18e4cf915543257eae2925671934937163f5639b ]

Previously a thread could exit asynchronously (due to a signal) so some care was needed to hold nfsd_mutex over the last svc_put() call. Now a thread can only exit when svc_set_num_threads() is called, and this is always called under nfsd_mutex. So no care is needed.

Not only is the mutex held when a thread exits now, but the svc refcount is elevated, so the svc_put() in svc_exit_thread() will never be a final put, so the mutex isn't even needed at this point in the code.

Signed-off-by NeilBrown <[email protected]>

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfsd/nfssvc.c           | 23 -----------------------
 include/linux/sunrpc/svc.h | 13 -------------
 2 files changed, 36 deletions(-)

fs/nfsd/nfssvc.c +0 ~~-23~~

index 8063fab2c0279..8907dba22c3f2 100644
@@ -979,31 +979,8 @@ nfsd(void *vrqstp)
 	atomic_dec(&nfsdstats.th_cnt);
 
 out:
-	/* Take an extra ref so that the svc_put in svc_exit_thread()
-	 * doesn't call svc_destroy()
-	 */
-	svc_get(nn->nfsd_serv);
-
 	/* Release the thread */
 	svc_exit_thread(rqstp);
-
-	/* We need to drop a ref, but may not drop the last reference
-	 * without holding nfsd_mutex, and we cannot wait for nfsd_mutex as that
-	 * could deadlock with nfsd_shutdown_threads() waiting for us.
-	 * So three options are:
-	 * - drop a non-final reference,
-	 * - get the mutex without waiting
-	 * - sleep briefly andd try the above again
-	 */
-	while (!svc_put_not_last(nn->nfsd_serv)) {
-		if (mutex_trylock(&nfsd_mutex)) {
-			nfsd_put(net);
-			mutex_unlock(&nfsd_mutex);
-			break;
-		}
-		msleep(20);
-	}
-
 	return 0;
 }

include/linux/sunrpc/svc.h +0 ~~-13~~

index 5cf6543d3c8e5..1cf7a7799cc04 100644
@@ -123,19 +123,6 @@ static inline void svc_put(struct svc_serv *serv)
 	kref_put(&serv->sv_refcnt, svc_destroy);
 }
 
-/**
- * svc_put_not_last - decrement non-final reference count on SUNRPC serv
- * @serv:  the svc_serv to have count decremented
- *
- * Returns: %true is refcount was decremented.
- *
- * If the refcount is 1, it is not decremented and instead failure is reported.
- */
-static inline bool svc_put_not_last(struct svc_serv *serv)
-{
-	return refcount_dec_not_one(&serv->sv_refcnt.refcount);
-}
-
 /*
  * Maximum payload size supported by a kernel RPC server.
  * This is use to determine the max number of pages nfsd is

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:19 UTC #775 lore raw

PATCH 762/770 nfsd: separate nfsd_last_thread() from nfsd_put()

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: NeilBrown <[email protected]>

[ Upstream commit 9f28a971ee9fdf1bf8ce8c88b103f483be610277 ]

Now that the last nfsd thread is stopped by an explicit act of calling svc_set_num_threads() with a count of zero, we only have a limited number of places that can happen, and don't need to call nfsd_last_thread() in nfsd_put()

So separate that out and call it at the two places where the number of threads is set to zero.

Move the clearing of ->nfsd_serv and the call to svc_xprt_destroy_all() into nfsd_last_thread(), as they are really part of the same action.

nfsd_put() is now a thin wrapper around svc_put(), so make it a static inline.

nfsd_put() cannot be called after nfsd_last_thread(), so in a couple of places we have to use svc_put() instead.

Signed-off-by NeilBrown <[email protected]>

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfsd/nfsd.h   |  7 ++++++-
 fs/nfsd/nfssvc.c | 52 ++++++++++++++++++------------------------------
 2 files changed, 25 insertions(+), 34 deletions(-)

fs/nfsd/nfsd.h +6 -1

index fa0144a742678..867dcfd64d426 100644
@@ -96,7 +96,12 @@ 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);
 
-void		nfsd_put(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);

fs/nfsd/nfssvc.c +19 ~~-33~~

114 lines not on this page · raw message

Signature · 1 line

2.43.0

Dominique Martinet

2024-06-24 04:15 UTC #776 lore raw

Re: [PATCH 5.10 762/770] nfsd: separate nfsd_last_thread() from nfsd_put()

Hi Greg,

(+Jeff & linux-nfs in Ccs)

Greg Kroah-Hartman wrote on Tue, Jun 18, 2024 at 02:40:15PM +0200:

Quoted 1 line

[ Upstream commit 9f28a971ee9fdf1bf8ce8c88b103f483be610277 ]

Playing with dyad in the 'vulns' repo, I noticed this commit got reverted in the 6.1 tree by pure chance as I just happened to test it on a related commit and wondered why the 6.1 kernel was listed twice:
b2c545c39877 ("Revert "nfsd: separate nfsd_last_thread() from nfsd_put()"") db5f2f4db8b7 ("Revert "nfsd: call nfsd_last_thread() before final nfsd_put()"")

See this thread for the discussion that caused that revert:
https://lore.kernel.org/all/[email protected]/

What made me look is that they got in 5.10/15 (without revert):

5.10 tree (since v5.10.220)
838a602db75d ("nfsd: call nfsd_last_thread() before final nfsd_put()") d31cd25f5501 ("nfsd: separate nfsd_last_thread() from nfsd_put()")

5.15 tree (since v5.15.154)
c52fee7a1f98 ("nfsd: call nfsd_last_thread() before final nfsd_put()") 56e5eeff6cfa ("nfsd: separate nfsd_last_thread() from nfsd_put()")

I considered trying to revert them as well, but it looks like they've been fixed by this commit (upstream id):
64e6304169f1 ("nfsd: drop the nfsd_put helper")
which wasn't in 6.1, so perhaps that's all there is to it and I'm worried too much?

Jeff, you're the one who suggested reverting the two back then, sorry to dump it on you but do you remember the kind of problems you ran into? Is there any chance it would have gone unoticed in the 5.15 tree for 2.5 months? (5.15.154 was April 2024)

(Bonus question: if that is really all there is, would that make sense / should we take the commits back in 6.1 with that extra fix?)

Thanks,

Signature · 1 line

Dominique Martinet | Asmadeus

Jeff Layton

maintainer

2024-06-25 15:45 UTC #777 lore raw

Re: [PATCH 5.10 762/770] nfsd: separate nfsd_last_thread() from nfsd_put()

Quoted 39 lines · Dominique Martinet

Hi Greg,
(+Jeff & linux-nfs in Ccs)
Greg Kroah-Hartman wrote on Tue, Jun 18, 2024 at 02:40:15PM +0200:
Quoted 1 line
[ Upstream commit 9f28a971ee9fdf1bf8ce8c88b103f483be610277 ]
Playing with dyad in the 'vulns' repo, I noticed this commit got reverted in the 6.1 tree by pure chance as I just happened to test it on
a related commit and wondered why the 6.1 kernel was listed twice:
b2c545c39877 ("Revert "nfsd: separate nfsd_last_thread() from nfsd_put()"")
db5f2f4db8b7 ("Revert "nfsd: call nfsd_last_thread() before final nfsd_put()"")
See this thread for the discussion that caused that revert:
https://lore.kernel.org/all/[email protected]/
What made me look is that they got in 5.10/15 (without revert):
5.10 tree (since v5.10.220)
838a602db75d ("nfsd: call nfsd_last_thread() before final nfsd_put()")
d31cd25f5501 ("nfsd: separate nfsd_last_thread() from nfsd_put()")
5.15 tree (since v5.15.154)
c52fee7a1f98 ("nfsd: call nfsd_last_thread() before final nfsd_put()")
56e5eeff6cfa ("nfsd: separate nfsd_last_thread() from nfsd_put()")
I considered trying to revert them as well, but it looks like they've been fixed by this commit (upstream id):
64e6304169f1 ("nfsd: drop the nfsd_put helper")
which wasn't in 6.1, so perhaps that's all there is to it and I'm worried too much?

Quoted 6 lines

Jeff, you're the one who suggested reverting the two back then, sorry to
dump it on you but do you remember the kind of problems you ran into? Is there any chance it would have gone unoticed in the 5.15 tree for 2.5 months? (5.15.154 was April 2024)

Sorry, I don't think I kept a record of that panic that I hit at the time. I do think that I looked at the original bug report and it looked like it was probably the same problem, but I don't remember the details.

I think I just mentioned reverting them because I didn't see the benefit in taking those into an old kernel. These are privileged anyway, so even if they are bugs I don't seem them as particularly critical.

Quoted 6 lines

(Bonus question: if that is really all there is, would that make sense
/ should we take the commits back in 6.1 with that extra fix?)

Maybe? The problem is that someone has to do the testing for this. These interfaces aren't currently part of any testsuite, so a lot of that tends to be a manual effort.

Signature · 1 line

Jeff Layton <[email protected]>

Dominique Martinet

2024-07-18 07:21 UTC #778 lore raw

Re: [PATCH 5.10 762/770] nfsd: separate nfsd_last_thread() from nfsd_put()

(Sorry for the slow reply)

Jeff Layton wrote on Tue, Jun 25, 2024 at 11:45:22AM -0400:

Quoted 11 lines

Quoted 5 lines
Jeff, you're the one who suggested reverting the two back then, sorry to
dump it on you but do you remember the kind of problems you ran into? Is there any chance it would have gone unoticed in the 5.15 tree for 2.5 months? (5.15.154 was April 2024)
Sorry, I don't think I kept a record of that panic that I hit at the time. I do think that I looked at the original bug report and it looked like it was probably the same problem, but I don't remember the details.

Quoted 4 lines

I think I just mentioned reverting them because I didn't see the benefit in taking those into an old kernel. These are privileged anyway, so even if they are bugs I don't seem them as particularly critical.

Right, normal users can't stop the nfsd so you're correct it probably doesn't matter as far as security goes (and this correctly doesn't have any of the shiny new CVEs assigned); I'm now curious why it got backported to all these trees at all if nothing needs it.

Anyway, I really just started looking at it because it got reverted in 6.1 so was wondering why it didn't get reverted in other trees, but if it hadn't made it to any tree I wouldn't have cared at all...

As long as there doesn't seem to be a problem with older trees (and at least I'm not getting any weird hang or crash on shutdown on my 5.10) let's leave things as they are.

Quoted 7 lines

Quoted 3 lines
(Bonus question: if that is really all there is, would that make sense
/ should we take the commits back in 6.1 with that extra fix?)
Maybe? The problem is that someone has to do the testing for this. These interfaces aren't currently part of any testsuite, so a lot of that tends to be a manual effort.

Agreed, let's not add more work there if no-one can quickly test this.

If it turns out some later commit that actually needs this gets backported to 6.1 we can always bring the pair of commits back when someone notices.

Thanks!

Signature · 1 line

Dominique

Chuck Lever

2024-07-18 14:53 UTC #779 lore raw

Re: [PATCH 5.10 762/770] nfsd: separate nfsd_last_thread() from nfsd_put()

Quoted 19 lines · Dominique Martinet

(Sorry for the slow reply)
Jeff Layton wrote on Tue, Jun 25, 2024 at 11:45:22AM -0400:
Quoted 11 lines
Quoted 5 lines
Jeff, you're the one who suggested reverting the two back then, sorry to
dump it on you but do you remember the kind of problems you ran into? Is there any chance it would have gone unoticed in the 5.15 tree for 2.5 months? (5.15.154 was April 2024)
Sorry, I don't think I kept a record of that panic that I hit at the time. I do think that I looked at the original bug report and it looked like it was probably the same problem, but I don't remember the details.
Quoted 4 lines
I think I just mentioned reverting them because I didn't see the benefit in taking those into an old kernel. These are privileged anyway, so even if they are bugs I don't seem them as particularly critical.

Quoted 4 lines

Right, normal users can't stop the nfsd so you're correct it probably doesn't matter as far as security goes (and this correctly doesn't have any of the shiny new CVEs assigned); I'm now curious why it got backported to all these trees at all if nothing needs it.

We wanted to backport the NFSD file cache fixes from v5.19 through v6.3 to LTS v6.1, v5.15, and v5.10. Since it wasn't just a handful of specific patches that fixed the NFSD file cache, Sasha and Greg asked me to backport the full range of NFSD commits up until v6.3 to these 3 kernels. The goals were:

1. To fix the file cache
2. To create a platform in the LTS code bases to make it easier to backport the more complex upstream NFSD fixes going forward

Thus the backport of course included some commits that would not normally have been backported to an LTS kernel. That has surprised a few folks.

Quoted 7 lines

Anyway, I really just started looking at it because it got reverted in 6.1 so was wondering why it didn't get reverted in other trees, but if it hadn't made it to any tree I wouldn't have cared at all...
As long as there doesn't seem to be a problem with older trees (and at least I'm not getting any weird hang or crash on shutdown on my 5.10) let's leave things as they are.

The only bothersome thing seems to be the inconsistency between the LTS kernels. I haven't heard a report of a problem.

Quoted 8 lines

Quoted 7 lines
Quoted 3 lines
(Bonus question: if that is really all there is, would that make sense
/ should we take the commits back in 6.1 with that extra fix?)
Maybe? The problem is that someone has to do the testing for this. These interfaces aren't currently part of any testsuite, so a lot of that tends to be a manual effort.

Quoted 5 lines

Agreed, let's not add more work there if no-one can quickly test this.
If it turns out some later commit that actually needs this gets backported to 6.1 we can always bring the pair of commits back when someone notices.

Sounds like a plan.

Signature · 1 line

Chuck Lever

Greg Kroah-Hartman

author maintainer

2024-06-18 13:19 UTC #780 lore raw

PATCH 763/770 Documentation: Add missing documentation for EXPORT_OP flags

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: Chuck Lever <[email protected]>

[ Upstream commit b38a6023da6a12b561f0421c6a5a1f7624a1529c ]

The commits that introduced these flags neglected to update the Documentation/filesystems/nfs/exporting.rst file.

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 Documentation/filesystems/nfs/exporting.rst | 26 +++++++++++++++++++++
 1 file changed, 26 insertions(+)

Documentation/filesystems/nfs/exporting.rst +26 -0

index 0e98edd353b5f..6f59a364f84cd 100644
@@ -215,3 +215,29 @@ following flags are defined:
     This flag causes nfsd to close any open files for this inode _before_
     calling into the vfs to do an unlink or a rename that would replace
     an existing file.
+
+  EXPORT_OP_REMOTE_FS - Backing storage for this filesystem is remote
+    PF_LOCAL_THROTTLE exists for loopback NFSD, where a thread needs to
+    write to one bdi (the final bdi) in order to free up writes queued
+    to another bdi (the client bdi). Such threads get a private balance
+    of dirty pages so that dirty pages for the client bdi do not imact
+    the daemon writing to the final bdi. For filesystems whose durable
+    storage is not local (such as exported NFS filesystems), this
+    constraint has negative consequences. EXPORT_OP_REMOTE_FS enables
+    an export to disable writeback throttling.
+
+  EXPORT_OP_NOATOMIC_ATTR - Filesystem does not update attributes atomically
+    EXPORT_OP_NOATOMIC_ATTR indicates that the exported filesystem
+    cannot provide the semantics required by the "atomic" boolean in
+    NFSv4's change_info4. This boolean indicates to a client whether the
+    returned before and after change attributes were obtained atomically
+    with the respect to the requested metadata operation (UNLINK,
+    OPEN/CREATE, MKDIR, etc).
+
+  EXPORT_OP_FLUSH_ON_CLOSE - Filesystem flushes file data on close(2)
+    On most filesystems, inodes can remain under writeback after the
+    file is closed. NFSD relies on client activity or local flusher
+    threads to handle writeback. Certain filesystems, such as NFS, flush
+    all of an inode's dirty data on last close. Exports that behave this
+    way should set EXPORT_OP_FLUSH_ON_CLOSE so that NFSD knows to skip
+    waiting for writeback when closing such files.

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:19 UTC #781 lore raw

PATCH 764/770 NFSD: fix possible oops when nfsd/pool_stats is closed.

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: NeilBrown <[email protected]>

[ Upstream commit 88956eabfdea7d01d550535af120d4ef265b1d02 ]

If /proc/fs/nfsd/pool_stats is open when the last nfsd thread exits, then when the file is closed a NULL pointer is dereferenced.
This is because nfsd_pool_stats_release() assumes that the pointer to the svc_serv cannot become NULL while a reference is held.

This used to be the case but a recent patch split nfsd_last_thread() out from nfsd_put(), and clearing the pointer is done in nfsd_last_thread().

This is easily reproduced by running
rpc.nfsd 8 ; ( rpc.nfsd 0;true) < /proc/fs/nfsd/pool_stats

Fortunately nfsd_pool_stats_release() has easy access to the svc_serv pointer, and so can call svc_put() on it directly.

Fixes 9f28a971ee9f ("nfsd: separate nfsd_last_thread() from nfsd_put()")

Signed-off-by NeilBrown <[email protected]>

Reviewed-by Jeff Layton <[email protected]>

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfsd/nfssvc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

fs/nfsd/nfssvc.c +3 -2

index ee5713fca1870..2a1dd580dfb94 100644
@@ -1084,11 +1084,12 @@ int nfsd_pool_stats_open(struct inode *inode, struct file *file)
 
 int nfsd_pool_stats_release(struct inode *inode, struct file *file)
 {
+	struct seq_file *seq = file->private_data;
+	struct svc_serv *serv = seq->private;
 	int ret = seq_release(inode, file);
-	struct net *net = inode->i_sb->s_fs_info;
 
 	mutex_lock(&nfsd_mutex);
-	nfsd_put(net);
+	svc_put(serv);
 	mutex_unlock(&nfsd_mutex);
 	return ret;
 }

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:19 UTC #782 lore raw

PATCH 765/770 nfsd: call nfsd_last_thread() before final nfsd_put()

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: NeilBrown <[email protected]>

[ Upstream commit 2a501f55cd641eb4d3c16a2eab0d678693fac663 ]

If write_ports_addfd or write_ports_addxprt fail, they call nfsd_put() without calling nfsd_last_thread(). This leaves nn->nfsd_serv pointing to a structure that has been freed.

So remove 'static' from nfsd_last_thread() and call it when the nfsd_serv is about to be destroyed.

Fixes ec52361df99b ("SUNRPC: stop using ->sv_nrthreads as a refcount")

Signed-off-by NeilBrown <[email protected]>

Reviewed-by Jeff Layton <[email protected]>

Cc <[email protected]>

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfsd/nfsctl.c | 9 +++++++--
 fs/nfsd/nfsd.h   | 1 +
 fs/nfsd/nfssvc.c | 2 +-
 3 files changed, 9 insertions(+), 3 deletions(-)

fs/nfsd/nfsctl.c +7 -2

index 76a60e7a75097..eec442edb6556 100644
@@ -720,8 +720,10 @@ static ssize_t __write_ports_addfd(char *buf, struct net *net, const struct cred
 
 	err = svc_addsock(nn->nfsd_serv, net, fd, buf, SIMPLE_TRANSACTION_LIMIT, cred);
 
-	if (err >= 0 &&
-	    !nn->nfsd_serv->sv_nrthreads && !xchg(&nn->keep_active, 1))
+	if (err < 0 && !nn->nfsd_serv->sv_nrthreads && !nn->keep_active)
+		nfsd_last_thread(net);
+	else if (err >= 0 &&
+		 !nn->nfsd_serv->sv_nrthreads && !xchg(&nn->keep_active, 1))
 		svc_get(nn->nfsd_serv);
 
 	nfsd_put(net);

@@ -771,6 +773,9 @@ static ssize_t __write_ports_addxprt(char *buf, struct net *net, const struct cr
 		svc_xprt_put(xprt);
 	}
 out_err:
+	if (!nn->nfsd_serv->sv_nrthreads && !nn->keep_active)
+		nfsd_last_thread(net);
+
 	nfsd_put(net);
 	return err;
 }

fs/nfsd/nfsd.h +1 -0

index 867dcfd64d426..3796015dc7656 100644
@@ -138,6 +138,7 @@ 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_reset_versions(struct nfsd_net *nn);
 int nfsd_create_serv(struct net *net);
+void nfsd_last_thread(struct net *net);
 
 extern int nfsd_max_blksize;

fs/nfsd/nfssvc.c +1 -1

index 2a1dd580dfb94..3d4fd40c987bd 100644
@@ -529,7 +529,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;

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:19 UTC #783 lore raw

PATCH 766/770 nfsd: drop the nfsd_put helper

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: Jeff Layton <[email protected]>

[ Upstream commit 64e6304169f1e1f078e7f0798033f80a7fb0ea46 ]

It's not safe to call nfsd_put once nfsd_last_thread has been called, as that function will zero out the nn->nfsd_serv pointer.

Drop the nfsd_put helper altogether and open-code the svc_put in its callers instead. That allows us to not be reliant on the value of that pointer when handling an error.

Fixes 2a501f55cd64 ("nfsd: call nfsd_last_thread() before final nfsd_put()")

Reported-by Zhi Li <[email protected]>

Cc NeilBrown <[email protected]>

Signed-off-by Jeffrey Layton <[email protected]>

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfsd/nfsctl.c | 31 +++++++++++++++++--------------
 fs/nfsd/nfsd.h   |  7 -------
 2 files changed, 17 insertions(+), 21 deletions(-)

fs/nfsd/nfsctl.c +17 ~~-14~~

index eec442edb6556..f77f00c931723 100644
@@ -709,6 +709,7 @@ static ssize_t __write_ports_addfd(char *buf, struct net *net, const struct cred
 	char *mesg = buf;
 	int fd, err;
 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+	struct svc_serv *serv;
 
 	err = get_int(&mesg, &fd);
 	if (err != 0 || fd < 0)

@@ -718,15 +719,15 @@ static ssize_t __write_ports_addfd(char *buf, struct net *net, const struct cred
 	if (err != 0)
 		return err;
 
-	err = svc_addsock(nn->nfsd_serv, net, fd, buf, SIMPLE_TRANSACTION_LIMIT, cred);
+	serv = nn->nfsd_serv;
+	err = svc_addsock(serv, net, fd, buf, SIMPLE_TRANSACTION_LIMIT, cred);
 
-	if (err < 0 && !nn->nfsd_serv->sv_nrthreads && !nn->keep_active)
+	if (err < 0 && !serv->sv_nrthreads && !nn->keep_active)
 		nfsd_last_thread(net);
-	else if (err >= 0 &&
-		 !nn->nfsd_serv->sv_nrthreads && !xchg(&nn->keep_active, 1))
-		svc_get(nn->nfsd_serv);
+	else if (err >= 0 && !serv->sv_nrthreads && !xchg(&nn->keep_active, 1))
+		svc_get(serv);
 
-	nfsd_put(net);
+	svc_put(serv);
 	return err;
 }

@@ -740,6 +741,7 @@ static ssize_t __write_ports_addxprt(char *buf, struct net *net, const struct cr
 	struct svc_xprt *xprt;
 	int port, err;
 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+	struct svc_serv *serv;
 
 	if (sscanf(buf, "%15s %5u", transport, &port) != 2)
 		return -EINVAL;

@@ -751,32 +753,33 @@ static ssize_t __write_ports_addxprt(char *buf, struct net *net, const struct cr
 	if (err != 0)
 		return err;
 
-	err = svc_xprt_create(nn->nfsd_serv, transport, net,
+	serv = nn->nfsd_serv;
+	err = svc_xprt_create(serv, transport, net,
 			      PF_INET, port, SVC_SOCK_ANONYMOUS, cred);
 	if (err < 0)
 		goto out_err;
 
-	err = svc_xprt_create(nn->nfsd_serv, transport, net,
+	err = svc_xprt_create(serv, transport, net,
 			      PF_INET6, port, SVC_SOCK_ANONYMOUS, cred);
 	if (err < 0 && err != -EAFNOSUPPORT)
 		goto out_close;
 
-	if (!nn->nfsd_serv->sv_nrthreads && !xchg(&nn->keep_active, 1))
-		svc_get(nn->nfsd_serv);
+	if (!serv->sv_nrthreads && !xchg(&nn->keep_active, 1))
+		svc_get(serv);
 
-	nfsd_put(net);
+	svc_put(serv);
 	return 0;
 out_close:
-	xprt = svc_find_xprt(nn->nfsd_serv, transport, net, PF_INET, port);
+	xprt = svc_find_xprt(serv, transport, net, PF_INET, port);
 	if (xprt != NULL) {
 		svc_xprt_close(xprt);
 		svc_xprt_put(xprt);
 	}
 out_err:
-	if (!nn->nfsd_serv->sv_nrthreads && !nn->keep_active)
+	if (!serv->sv_nrthreads && !nn->keep_active)
 		nfsd_last_thread(net);
 
-	nfsd_put(net);
+	svc_put(serv);
 	return err;
 }

fs/nfsd/nfsd.h +0 -7

index 3796015dc7656..013bfa24ced21 100644
@@ -96,13 +96,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 {

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:19 UTC #784 lore raw

PATCH 767/770 nfsd: fix RELEASE_LOCKOWNER

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: NeilBrown <[email protected]>

[ Upstream commit edcf9725150e42beeca42d085149f4c88fa97afd ]

The test on so_count in nfsd4_release_lockowner() is nonsense and harmful. Revert to using check_for_locks(), changing that to not sleep.

First: harmful.
As is documented in the kdoc comment for nfsd4_release_lockowner(), the test on so_count can transiently return a false positive resulting in a return of NFS4ERR_LOCKS_HELD when in fact no locks are held. This is clearly a protocol violation and with the Linux NFS client it can cause incorrect behaviour.

If RELEASE_LOCKOWNER is sent while some other thread is still processing a LOCK request which failed because, at the time that request was received, the given owner held a conflicting lock, then the nfsd thread processing that LOCK request can hold a reference (conflock) to the lock owner that causes nfsd4_release_lockowner() to return an incorrect error.

The Linux NFS client ignores that NFS4ERR_LOCKS_HELD error because it never sends NFS4_RELEASE_LOCKOWNER without first releasing any locks, so it knows that the error is impossible. It assumes the lock owner was in fact released so it feels free to use the same lock owner identifier in some later locking request.

When it does reuse a lock owner identifier for which a previous RELEASE failed, it will naturally use a lock_seqid of zero. However the server, which didn't release the lock owner, will expect a larger lock_seqid and so will respond with NFS4ERR_BAD_SEQID.

So clearly it is harmful to allow a false positive, which testing so_count allows.

The test is nonsense because ... well... it doesn't mean anything.

so_count is the sum of three different counts.
1/ the set of states listed on so_stateids
2/ the set of active vfs locks owned by any of those states 3/ various transient counts such as for conflicting locks.

When it is tested against '2' it is clear that one of these is the transient reference obtained by find_lockowner_str_locked(). It is not clear what the other one is expected to be.

In practice, the count is often 2 because there is precisely one state on so_stateids. If there were more, this would fail.

In my testing I see two circumstances when RELEASE_LOCKOWNER is called. In one case, CLOSE is called before RELEASE_LOCKOWNER. That results in all the lock states being removed, and so the lockowner being discarded (it is removed when there are no more references which usually happens when the lock state is discarded). When nfsd4_release_lockowner() finds that the lock owner doesn't exist, it returns success.

The other case shows an so_count of '2' and precisely one state listed in so_stateid. It appears that the Linux client uses a separate lock owner for each file resulting in one lock state per lock owner, so this test on '2' is safe. For another client it might not be safe.

So this patch changes check_for_locks() to use the (newish) find_any_file_locked() so that it doesn't take a reference on the nfs4_file and so never calls nfsd_file_put(), and so never sleeps. With this check is it safe to restore the use of check_for_locks() rather than testing so_count against the mysterious '2'.

Fixes ce3c4ad7f4ce ("NFSD: Fix possible sleep during nfsd4_release_lockowner()")

Signed-off-by NeilBrown <[email protected]>

Reviewed-by Jeff Layton <[email protected]>

Cc [email protected] # v6.2+

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfsd/nfs4state.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

fs/nfsd/nfs4state.c +15 ~~-11~~

index e8e642e5ec8f1..c073cc23c5285 100644
@@ -7836,14 +7836,16 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner)
 {
 	struct file_lock *fl;
 	int status = false;
-	struct nfsd_file *nf = find_any_file(fp);
+	struct nfsd_file *nf;
 	struct inode *inode;
 	struct file_lock_context *flctx;
 
+	spin_lock(&fp->fi_lock);
+	nf = find_any_file_locked(fp);
 	if (!nf) {
 		/* Any valid lock stateid should have some sort of access */
 		WARN_ON_ONCE(1);
-		return status;
+		goto out;
 	}
 
 	inode = locks_inode(nf->nf_file);

@@ -7859,7 +7861,8 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner)
 		}
 		spin_unlock(&flctx->flc_lock);
 	}
-	nfsd_file_put(nf);
+out:
+	spin_unlock(&fp->fi_lock);
 	return status;
 }

@@ -7869,10 +7872,8 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner)
  * @cstate: NFSv4 COMPOUND state
  * @u: RELEASE_LOCKOWNER arguments
  *
- * The lockowner's so_count is bumped when a lock record is added
- * or when copying a conflicting lock. The latter case is brief,
- * but can lead to fleeting false positives when looking for
- * locks-in-use.
+ * Check if theree are any locks still held and if not - free the lockowner
+ * and any lock state that is owned.
  *
  * Return values:
  *   %nfs_ok: lockowner released or not found

@@ -7908,10 +7909,13 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
 		spin_unlock(&clp->cl_lock);
 		return nfs_ok;
 	}
-	if (atomic_read(&lo->lo_owner.so_count) != 2) {
-		spin_unlock(&clp->cl_lock);
-		nfs4_put_stateowner(&lo->lo_owner);
-		return nfserr_locks_held;
+
+	list_for_each_entry(stp, &lo->lo_owner.so_stateids, st_perstateowner) {
+		if (check_for_locks(stp->st_stid.sc_file, lo)) {
+			spin_unlock(&clp->cl_lock);
+			nfs4_put_stateowner(&lo->lo_owner);
+			return nfserr_locks_held;
+		}
 	}
 	unhash_lockowner_locked(lo);
 	while (!list_empty(&lo->lo_owner.so_stateids)) {

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:20 UTC #785 lore raw

PATCH 768/770 nfsd: dont take fi_lock in nfsd_break_deleg_cb()

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: NeilBrown <[email protected]>

[ Upstream commit 5ea9a7c5fe4149f165f0e3b624fe08df02b6c301 ]

A recent change to check_for_locks() changed it to take ->flc_lock while holding ->fi_lock. This creates a lock inversion (reported by lockdep) because there is a case where ->fi_lock is taken while holding ->flc_lock.

->flc_lock is held across ->fl_lmops callbacks, and nfsd_break_deleg_cb() is one of those and does take ->fi_lock. However it doesn't need to.

Prior to v4.17-rc1~110^2~22 ("nfsd: create a separate lease for each delegation") nfsd_break_deleg_cb() would walk the ->fi_delegations list and so needed the lock. Since then it doesn't walk the list and doesn't need the lock.

Two actions are performed under the lock. One is to call nfsd_break_one_deleg which calls nfsd4_run_cb(). These doesn't act on the nfs4_file at all, so don't need the lock.

The other is to set ->fi_had_conflict which is in the nfs4_file. This field is only ever set here (except when initialised to false) so there is no possible problem will multiple threads racing when setting it.

The field is tested twice in nfs4_set_delegation(). The first test does not hold a lock and is documented as an opportunistic optimisation, so it doesn't impose any need to hold ->fi_lock while setting ->fi_had_conflict.

The second test in nfs4_set_delegation() *is* make under ->fi_lock, so removing the locking when ->fi_had_conflict is set could make a change. The change could only be interesting if ->fi_had_conflict tested as false even though nfsd_break_one_deleg() ran before ->fi_lock was unlocked. i.e. while hash_delegation_locked() was running. As hash_delegation_lock() doesn't interact in any way with nfs4_run_cb() there can be no importance to this interaction.

So this patch removes the locking from nfsd_break_one_deleg() and moves the final test on ->fi_had_conflict out of the locked region to make it clear that locking isn't important to the test. It is still tested *after* vfs_setlease() has succeeded. This might be significant and as vfs_setlease() takes ->flc_lock, and nfsd_break_one_deleg() is called under ->flc_lock this "after" is a true ordering provided by a spinlock.

Fixes edcf9725150e ("nfsd: fix RELEASE_LOCKOWNER")

Signed-off-by NeilBrown <[email protected]>

Reviewed-by Jeff Layton <[email protected]>

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfsd/nfs4state.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

fs/nfsd/nfs4state.c +5 -6

index c073cc23c5285..165acd8138abe 100644
@@ -4946,10 +4946,8 @@ nfsd_break_deleg_cb(struct file_lock *fl)
 	 */
 	fl->fl_break_time = 0;
 
-	spin_lock(&fp->fi_lock);
 	fp->fi_had_conflict = true;
 	nfsd_break_one_deleg(dp);
-	spin_unlock(&fp->fi_lock);
 	return false;
 }

@@ -5537,12 +5535,13 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 	if (status)
 		goto out_unlock;
 
+	status = -EAGAIN;
+	if (fp->fi_had_conflict)
+		goto out_unlock;
+
 	spin_lock(&state_lock);
 	spin_lock(&fp->fi_lock);
-	if (fp->fi_had_conflict)
-		status = -EAGAIN;
-	else
-		status = hash_delegation_locked(dp, fp);
+	status = hash_delegation_locked(dp, fp);
 	spin_unlock(&fp->fi_lock);
 	spin_unlock(&state_lock);

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:20 UTC #786 lore raw

PATCH 769/770 nfsd: dont call locks_release_private() twice concurrently

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: NeilBrown <[email protected]>

[ Upstream commit 05eda6e75773592760285e10ac86c56d683be17f ]

It is possible for free_blocked_lock() to be called twice concurrently, once from nfsd4_lock() and once from nfsd4_release_lockowner() calling remove_blocked_locks(). This is why a kref was added.

It is perfectly safe for locks_delete_block() and kref_put() to be called in parallel as they use locking or atomicity respectively as protection. However locks_release_private() has no locking. It is safe for it to be called twice sequentially, but not concurrently.

This patch moves that call from free_blocked_lock() where it could race with itself, to free_nbl() where it cannot. This will slightly delay the freeing of private info or release of the owner - but not by much. It is arguably more natural for this freeing to happen in free_nbl() where the structure itself is freed.

This bug was found by code inspection - it has not been seen in practice.

Fixes 47446d74f170 ("nfsd4: add refcount for nfsd4_blocked_lock")

Signed-off-by NeilBrown <[email protected]>

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfsd/nfs4state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/nfsd/nfs4state.c +1 -1

index 165acd8138abe..228560f3fd0e0 100644
@@ -318,6 +318,7 @@ free_nbl(struct kref *kref)
 	struct nfsd4_blocked_lock *nbl;
 
 	nbl = container_of(kref, struct nfsd4_blocked_lock, nbl_kref);
+	locks_release_private(&nbl->nbl_lock);
 	kfree(nbl);
 }

@@ -325,7 +326,6 @@ static void
 free_blocked_lock(struct nfsd4_blocked_lock *nbl)
 {
 	locks_delete_block(&nbl->nbl_lock);
-	locks_release_private(&nbl->nbl_lock);
 	kref_put(&nbl->nbl_kref, free_nbl);
 }

Signature · 1 line

2.43.0

Greg Kroah-Hartman

author maintainer

2024-06-18 13:20 UTC #787 lore raw

PATCH 770/770 nfsd: Fix a regression in nfsd_setattr()

In reply to Greg Kroah-Hartman

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

From: Trond Myklebust <[email protected]>

[ Upstream commit 6412e44c40aaf8f1d7320b2099c5bdd6cb9126ac ]

Commit bb4d53d66e4b ("NFSD: use (un)lock_inode instead of fh_(un)lock for file operations") broke the NFSv3 pre/post op attributes behaviour when doing a SETATTR rpc call by stripping out the calls to fh_fill_pre_attrs() and fh_fill_post_attrs().

Fixes bb4d53d66e4b ("NFSD: use (un)lock_inode instead of fh_(un)lock for file operations")

Signed-off-by Trond Myklebust <[email protected]>

Reviewed-by Jeff Layton <[email protected]>

Reviewed-by NeilBrown <[email protected]>

Message-ID: <[email protected]>

[ cel: adjusted to apply to v5.10.y ]

Signed-off-by Chuck Lever <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/nfsd/nfs4proc.c | 4 ++++
 fs/nfsd/vfs.c      | 6 ++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

fs/nfsd/nfs4proc.c +4 -0

index 8560a11daa47d..2c0de247083a9 100644
@@ -1107,6 +1107,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	};
 	struct inode *inode;
 	__be32 status = nfs_ok;
+	bool save_no_wcc;
 	int err;
 
 	if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {

@@ -1132,8 +1133,11 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	if (status)
 		goto out;
+	save_no_wcc = cstate->current_fh.fh_no_wcc;
+	cstate->current_fh.fh_no_wcc = true;
 	status = nfsd_setattr(rqstp, &cstate->current_fh, &attrs,
 				0, (time64_t)0);
+	cstate->current_fh.fh_no_wcc = save_no_wcc;
 	if (!status)
 		status = nfserrno(attrs.na_labelerr);
 	if (!status)

fs/nfsd/vfs.c +4 -2

index 542a1adbbf2a1..0ea05ddff0d08 100644
@@ -486,7 +486,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	int		accmode = NFSD_MAY_SATTR;
 	umode_t		ftype = 0;
 	__be32		err;
-	int		host_err;
+	int		host_err = 0;
 	bool		get_write_count;
 	bool		size_change = (iap->ia_valid & ATTR_SIZE);
 	int		retries;

@@ -544,6 +544,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	}
 
 	inode_lock(inode);
+	fh_fill_pre_attrs(fhp);
 	for (retries = 1;;) {
 		struct iattr attrs;

@@ -569,13 +570,14 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	    !attr->na_aclerr && attr->na_dpacl && S_ISDIR(inode->i_mode))
 		attr->na_aclerr = set_posix_acl(inode, ACL_TYPE_DEFAULT,
 						attr->na_dpacl);
+	fh_fill_post_attrs(fhp);
 	inode_unlock(inode);
 	if (size_change)
 		put_write_access(inode);
 out:
 	if (!host_err)
 		host_err = commit_metadata(fhp);
-	return nfserrno(host_err);
+	return err != 0 ? err : nfserrno(host_err);
 }
 
 #if defined(CONFIG_NFSD_V4)

Signature · 1 line

2.43.0

Jon Hunter

maintainer Tested-by on the v1 series

2024-06-18 21:01 UTC #788 lore raw

In reply to Greg Kroah-Hartman

Quoted 9 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 20 Jun 2024 12:32:00 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:

Quoted 8 lines

	https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.10.220-rc1.gz
or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.10.y
and the diffstat can be found below.
thanks,
greg k-h

All tests passing for Tegra ...

Test results for stable-v5.10:

    10 builds:	10 pass, 0 fail
    26 boots:	26 pass, 0 fail
    68 tests:	68 pass, 0 fail

Linux version: 5.10.220-rc1-g7927147b02fc
Boards tested: tegra124-jetson-tk1, tegra186-p2771-0000,

                tegra194-p2972-0000, tegra194-p3509-0000+p3668-0000,
                tegra20-ventana, tegra210-p2371-2180,
                tegra210-p3450-0000, tegra30-cardhu-a04

Tested-by Jon Hunter <[email protected]>

Jon

Pavel Machek

Tested-by on the v1 series

2024-06-19 08:51 UTC #789 lore raw

In reply to Greg Kroah-Hartman

Hi!

Quoted 4 lines

This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Mentioning motivation behind 800 nfs patches would be welcome here.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-5.10.y

Tested-by Pavel Machek (CIP) <[email protected]>

Best regards,
Pavel

Signature · 2 lines

DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Dominique Martinet

Tested-by on the v1 series

2024-06-19 09:05 UTC #790 lore raw

In reply to Greg Kroah-Hartman

Greg Kroah-Hartman wrote on Tue, Jun 18, 2024 at 02:27:33PM +0200:

Quoted 5 lines

This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Quoted 8 lines

Responses should be made by Thu, 20 Jun 2024 12:32:00 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
	https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.10.220-rc1.gz
or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.10.y
and the diffstat can be found below.

(that's a lot of NFS/FS patches... I normally don't test NFS, but given the content of this cycle I tested very basic client/server code by exporting something mounting it and reading/writing to a file, so at least it's not exploding immediately)

Tested 7927147b02fc ("Linux 5.10.220-rc1") on:
- arm i.MX6ULL (Armadillo 640)
- arm64 i.MX8MP (Armadillo G4)

No obvious regression in dmesg or basic tests:

Tested-by Dominique Martinet <[email protected]>

Signature · 1 line

Dominique Martinet | Asmadeus

Florian Fainelli

Tested-by on the v1 series

2024-06-19 10:17 UTC #791 lore raw

In reply to Greg Kroah-Hartman

Quoted 9 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 20 Jun 2024 12:32:00 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:

Quoted 8 lines

	https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.10.220-rc1.gz
or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.10.y
and the diffstat can be found below.
thanks,
greg k-h

On ARCH_BRCMSTB using 32-bit and 64-bit ARM kernels, build tested on BMIPS_GENERIC:

Tested-by Florian Fainelli <[email protected]>

Signature · 1 line

Florian

Naresh Kamboju

Tested-by Linux Kernel Functional Testing on the v1 series

2024-06-19 11:10 UTC #792 lore raw

In reply to Greg Kroah-Hartman

Quoted 10 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 20 Jun 2024 12:32:00 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:

Quoted 8 lines

        https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.10.220-rc1.gz
or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.10.y
and the diffstat can be found below.
thanks,
greg k-h

Results from Linaro’s test farm.
No regressions on arm64, arm, x86_64, and i386.

Tested-by Linux Kernel Functional Testing <[email protected]>

## Build
* kernel: 5.10.220-rc1
* git: https://gitlab.com/Linaro/lkft/mirrors/stable/linux-stable-rc
* git branch: linux-5.10.y
* git commit: 7927147b02fc59fa7d326be121ef2a599edda19d
* git describe: v5.10.219-771-g7927147b02fc
* test details:
https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-5.10.y/build/v5.10.219-771-g7927147b02fc

## Test Regressions (compared to v5.10.219)

## Metric Regressions (compared to v5.10.219)

## Test Fixes (compared to v5.10.219)

## Metric Fixes (compared to v5.10.219)

## Test result summary
total: 128385, pass: 101019, fail: 5547, skip: 21723, xfail: 96

## Build Summary
* arc: 5 total, 5 passed, 0 failed
* arm: 104 total, 104 passed, 0 failed
* arm64: 31 total, 31 passed, 0 failed
* i386: 25 total, 25 passed, 0 failed
* mips: 22 total, 22 passed, 0 failed
* parisc: 3 total, 0 passed, 3 failed
* powerpc: 23 total, 23 passed, 0 failed
* riscv: 9 total, 9 passed, 0 failed
* s390: 9 total, 9 passed, 0 failed
* sh: 10 total, 10 passed, 0 failed
* sparc: 6 total, 6 passed, 0 failed
* x86_64: 27 total, 27 passed, 0 failed

## Test suites summary
* boot
* kselftest-android
* kselftest-arm64
* kselftest-breakpoints
* kselftest-capabilities
* kselftest-cgroup
* kselftest-clone3
* kselftest-core
* kselftest-cpu-hotplug
* kselftest-cpufreq
* kselftest-drivers-dma-buf
* kselftest-efivarfs
* kselftest-exec
* kselftest-filesystems
* kselftest-filesystems-binderfs
* kselftest-filesystems-epoll
* kselftest-firmware
* kselftest-fpu
* kselftest-ftrace
* kselftest-futex
* kselftest-gpio
* kselftest-intel_pstate
* kselftest-ipc
* kselftest-ir
* kselftest-kcmp
* kselftest-kexec
* kselftest-kvm
* kselftest-lib
* kselftest-livepatch
* kselftest-membarrier
* kselftest-memfd
* kselftest-memory-hotplug
* kselftest-mincore
* kselftest-mm
* kselftest-mount
* kselftest-mqueue
* kselftest-net
* kselftest-net-forwarding
* kselftest-net-mptcp
* kselftest-netfilter
* kselftest-nsfs
* kselftest-openat2
* kselftest-pid_namespace
* kselftest-pidfd
* kselftest-proc
* kselftest-pstore
* kselftest-ptrace
* kselftest-rseq
* kselftest-rtc
* kselftest-sigaltstack
* kselftest-size
* kselftest-splice
* kselftest-static_keys
* kselftest-sync
* kselftest-sysctl
* kselftest-tc-testing
* kselftest-timens
* kselftest-timers
* kselftest-tmpfs
* kselftest-tpm2
* kselftest-user
* kselftest-user_events
* kselftest-vDSO
* kselftest-watchdog
* kselftest-x86
* kselftest-zram
* kunit
* kvm-unit-tests
* libgpiod
* log-parser-boot
* log-parser-test
* ltp-commands
* ltp-containers
* ltp-controllers
* ltp-cpuhotplug
* ltp-crypto
* ltp-cve
* ltp-dio
* ltp-fcntl-locktests
* ltp-fs
* ltp-fs_bind
* ltp-fs_perms_simple
* ltp-hugetlb
* ltp-ipc
* ltp-math
* ltp-mm
* ltp-nptl
* ltp-pty
* ltp-sched
* ltp-smoke
* ltp-smoketest
* ltp-syscalls
* ltp-tracing
* perf
* rcutorture

--
Linaro LKFT
https://lkft.linaro.org

Guenter Roeck

2024-06-25 14:48 UTC #793 lore raw

In reply to Greg Kroah-Hartman

Quoted 8 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 20 Jun 2024 12:32:00 +0000. Anything received after that time might be too late.

[ ... ]

Quoted 3 lines

Chuck Lever <[email protected]>
SUNRPC: Prepare for xdr_stream-style decoding on the server-side

The ChromeOS patches robot reports a number of fixes for the patches applied in 5.5.220. This is one example, later fixed with commit 90bfc37b5ab9 ("SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation"), but there are more. Are those fixes going to be applied in a subsequent release of v5.10.y, was there a reason to not include them, or did they get lost ?

Thanks,
Guenter

Greg Kroah-Hartman

author maintainer

2024-06-25 15:04 UTC #794 lore raw

Quoted 14 lines · Guenter Roeck

Quoted 8 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 20 Jun 2024 12:32:00 +0000. Anything received after that time might be too late.
[ ... ]
Quoted 3 lines
Chuck Lever <[email protected]>
SUNRPC: Prepare for xdr_stream-style decoding on the server-side

Quoted 6 lines

The ChromeOS patches robot reports a number of fixes for the patches applied in 5.5.220. This is one example, later fixed with commit 90bfc37b5ab9 ("SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation"), but there are more. Are those fixes going to be applied in a subsequent release of v5.10.y, was there a reason to not include them, or did they get lost ?

I saw this as well, but when I tried to apply a few, they didn't, so I was guessing that Chuck had merged them together into the series.

I'll defer to Chuck on this, this release was all his :)

thanks,

greg k-h

Chuck Lever III

2024-06-25 15:13 UTC #795 lore raw

Hi -

Quoted 24 lines · Greg Kroah-Hartman

Quoted 14 lines · Guenter Roeck
Quoted 8 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 20 Jun 2024 12:32:00 +0000. Anything received after that time might be too late.
[ ... ]
Quoted 3 lines
Chuck Lever <[email protected]>
SUNRPC: Prepare for xdr_stream-style decoding on the server-side
Quoted 6 lines
The ChromeOS patches robot reports a number of fixes for the patches applied in 5.5.220. This is one example, later fixed with commit 90bfc37b5ab9 ("SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation"), but there are more. Are those fixes going to be applied in a subsequent release of v5.10.y, was there a reason to not include them, or did they get lost ?

Quoted 4 lines

I saw this as well, but when I tried to apply a few, they didn't, so I was guessing that Chuck had merged them together into the series.
I'll defer to Chuck on this, this release was all his :)

I did this port months ago, I've been waiting for the dust to settle on the 6.1 and 5.15 NFSD backports, so I've all but forgotten the status of individual patches.

If you (Greg or Guenter) send me a list of what you believe is missing, I can have a look at the individual cases and then run the finished result through our NFSD CI gauntlet.

--
Chuck Lever

Guenter Roeck

2024-06-25 16:29 UTC #796 lore raw

Quoted 31 lines · Chuck Lever III

Hi -
Quoted 24 lines · Greg Kroah-Hartman
Quoted 14 lines · Guenter Roeck
Quoted 8 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 20 Jun 2024 12:32:00 +0000. Anything received after that time might be too late.
[ ... ]
Quoted 3 lines
Chuck Lever <[email protected]>
SUNRPC: Prepare for xdr_stream-style decoding on the server-side
Quoted 6 lines
The ChromeOS patches robot reports a number of fixes for the patches applied in 5.5.220. This is one example, later fixed with commit 90bfc37b5ab9 ("SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation"), but there are more. Are those fixes going to be applied in a subsequent release of v5.10.y, was there a reason to not include them, or did they get lost ?
Quoted 4 lines
I saw this as well, but when I tried to apply a few, they didn't, so I was guessing that Chuck had merged them together into the series.
I'll defer to Chuck on this, this release was all his :)

Quoted 8 lines

I did this port months ago, I've been waiting for the dust to settle on the 6.1 and 5.15 NFSD backports, so I've all but forgotten the status of individual patches.
If you (Greg or Guenter) send me a list of what you believe is missing, I can have a look at the individual cases and then run the finished result through our NFSD CI gauntlet.

This is what the robot reported so far:

1242a87da0d8 SUNRPC: Fix svcxdr_init_encode's buflen calculation

Fixes bddfdbcddbe2 ("NFSD: Extract the svcxdr_init_encode() helper")

90bfc37b5ab9 SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation

Fixes 5191955d6fc6 ("SUNRPC: Prepare for xdr_stream-style decoding on the server-side")

10396f4df8b7 nfsd: hold a lighter-weight client reference over CB_RECALL_ANY

Fixes 44df6f439a17 ("NFSD: add delegation reaper to react to low memory condition")

Guenter

Chuck Lever III

2024-06-25 19:09 UTC #797 lore raw

Quoted 41 lines · Guenter Roeck

Quoted 28 lines · Chuck Lever III
Hi -
Quoted 27 lines · Greg Kroah-Hartman
Quoted 14 lines · Guenter Roeck
Quoted 8 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 20 Jun 2024 12:32:00 +0000. Anything received after that time might be too late.
[ ... ]
Quoted 3 lines
Chuck Lever <[email protected]>
SUNRPC: Prepare for xdr_stream-style decoding on the server-side
Quoted 6 lines
The ChromeOS patches robot reports a number of fixes for the patches applied in 5.5.220. This is one example, later fixed with commit 90bfc37b5ab9 ("SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation"), but there are more. Are those fixes going to be applied in a subsequent release of v5.10.y, was there a reason to not include them, or did they get lost ?
I saw this as well, but when I tried to apply a few, they didn't, so I was guessing that Chuck had merged them together into the series.
Quoted 7 lines
Quoted 1 line
I'll defer to Chuck on this, this release was all his :)
I did this port months ago, I've been waiting for the dust to settle on the 6.1 and 5.15 NFSD backports, so I've all but forgotten the status of individual patches.
If you (Greg or Guenter) send me a list of what you believe is missing, I can have a look at the individual cases and then run the finished result through our NFSD CI gauntlet.

Quoted 8 lines

This is what the robot reported so far:
1242a87da0d8 SUNRPC: Fix svcxdr_init_encode's buflen calculation
Fixes bddfdbcddbe2 ("NFSD: Extract the svcxdr_init_encode() helper")
90bfc37b5ab9 SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation
Fixes 5191955d6fc6 ("SUNRPC: Prepare for xdr_stream-style decoding on the server-side")
10396f4df8b7 nfsd: hold a lighter-weight client reference over CB_RECALL_ANY
Fixes 44df6f439a17 ("NFSD: add delegation reaper to react to low memory condition")

My naive search found:

Checking commit 44df6f439a17 ...
upstream fix 10396f4df8b75ff6ab0aa2cd74296565466f2c8d not found
10396f4df8b75ff6ab0aa2cd74296565466f2c8d nfsd: hold a lighter-weight client reference over CB_RECALL_ANY
upstream fix f385f7d244134246f984975ed34cd75f77de479f is already applied
Checking commit a2071573d634 ...
upstream fix f1aa2eb5ea05ccd1fd92d235346e60e90a1ed949 not found
f1aa2eb5ea05ccd1fd92d235346e60e90a1ed949 sysctl: fix proc_dobool() usability
Checking commit bddfdbcddbe2 ...
upstream fix 1242a87da0d8cd2a428e96ca68e7ea899b0f4624 not found
1242a87da0d8cd2a428e96ca68e7ea899b0f4624 SUNRPC: Fix svcxdr_init_encode's buflen calculation
Checking commit 9fe61450972d ... upstream fix 2111c3c0124f7432fe908c036a50abe8733dbf38 not found
2111c3c0124f7432fe908c036a50abe8733dbf38 namei: fix kernel-doc for struct renamedata and more
Checking commit 013c1667cf78 ... upstream fix 2c0f0f3639562d6e38ee9705303c6457c4936eac not found
2c0f0f3639562d6e38ee9705303c6457c4936eac module: correctly exit module_kallsyms_on_each_symbol when fn() != 0
upstream fix 1e80d9cb579ed7edd121753eeccce82ff82521b4 not found
1e80d9cb579ed7edd121753eeccce82ff82521b4 module: potential uninitialized return in module_kallsyms_on_each_symbol() Checking commit 89ff87494c6e ...
upstream fix 5c11720767f70d34357d00a15ba5a0ad052c40fe not found
5c11720767f70d34357d00a15ba5a0ad052c40fe SUNRPC: Fix a NULL pointer deref in trace_svc_stats_latency() Checking commit 5191955d6fc6 ...
upstream fix 90bfc37b5ab91c1a6165e3e5cfc49bf04571b762 not found
90bfc37b5ab91c1a6165e3e5cfc49bf04571b762 SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation
upstream fix b9f83ffaa0c096b4c832a43964fe6bff3acffe10 not found
b9f83ffaa0c096b4c832a43964fe6bff3acffe10 SUNRPC: Fix null pointer dereference in svc_rqst_free()

I'll look into backporting the missing NFSD and SUNRPC patches.

--
Chuck Lever

Guenter Roeck

2024-06-25 19:35 UTC #798 lore raw

Quoted 77 lines · Chuck Lever III

Quoted 41 lines · Guenter Roeck
Quoted 28 lines · Chuck Lever III
Hi -
Quoted 27 lines · Greg Kroah-Hartman
Quoted 14 lines · Guenter Roeck
Quoted 8 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 20 Jun 2024 12:32:00 +0000. Anything received after that time might be too late.
[ ... ]
Quoted 3 lines
Chuck Lever <[email protected]>
SUNRPC: Prepare for xdr_stream-style decoding on the server-side
Quoted 6 lines
The ChromeOS patches robot reports a number of fixes for the patches applied in 5.5.220. This is one example, later fixed with commit 90bfc37b5ab9 ("SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation"), but there are more. Are those fixes going to be applied in a subsequent release of v5.10.y, was there a reason to not include them, or did they get lost ?
I saw this as well, but when I tried to apply a few, they didn't, so I was guessing that Chuck had merged them together into the series.
Quoted 7 lines
Quoted 1 line
I'll defer to Chuck on this, this release was all his :)
I did this port months ago, I've been waiting for the dust to settle on the 6.1 and 5.15 NFSD backports, so I've all but forgotten the status of individual patches.
If you (Greg or Guenter) send me a list of what you believe is missing, I can have a look at the individual cases and then run the finished result through our NFSD CI gauntlet.
Quoted 8 lines
This is what the robot reported so far:
1242a87da0d8 SUNRPC: Fix svcxdr_init_encode's buflen calculation
Fixes bddfdbcddbe2 ("NFSD: Extract the svcxdr_init_encode() helper")
90bfc37b5ab9 SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation
Fixes 5191955d6fc6 ("SUNRPC: Prepare for xdr_stream-style decoding on the server-side")
10396f4df8b7 nfsd: hold a lighter-weight client reference over CB_RECALL_ANY
Fixes 44df6f439a17 ("NFSD: add delegation reaper to react to low memory condition")
My naive search found:
Checking commit 44df6f439a17 ...
upstream fix 10396f4df8b75ff6ab0aa2cd74296565466f2c8d not found
10396f4df8b75ff6ab0aa2cd74296565466f2c8d nfsd: hold a lighter-weight client reference over CB_RECALL_ANY
upstream fix f385f7d244134246f984975ed34cd75f77de479f is already applied
Checking commit a2071573d634 ...
upstream fix f1aa2eb5ea05ccd1fd92d235346e60e90a1ed949 not found
f1aa2eb5ea05ccd1fd92d235346e60e90a1ed949 sysctl: fix proc_dobool() usability
Checking commit bddfdbcddbe2 ...
upstream fix 1242a87da0d8cd2a428e96ca68e7ea899b0f4624 not found
1242a87da0d8cd2a428e96ca68e7ea899b0f4624 SUNRPC: Fix svcxdr_init_encode's buflen calculation
Checking commit 9fe61450972d ... upstream fix 2111c3c0124f7432fe908c036a50abe8733dbf38 not found
2111c3c0124f7432fe908c036a50abe8733dbf38 namei: fix kernel-doc for struct renamedata and more
Checking commit 013c1667cf78 ... upstream fix 2c0f0f3639562d6e38ee9705303c6457c4936eac not found
2c0f0f3639562d6e38ee9705303c6457c4936eac module: correctly exit module_kallsyms_on_each_symbol when fn() != 0
upstream fix 1e80d9cb579ed7edd121753eeccce82ff82521b4 not found
1e80d9cb579ed7edd121753eeccce82ff82521b4 module: potential uninitialized return in module_kallsyms_on_each_symbol() Checking commit 89ff87494c6e ...
upstream fix 5c11720767f70d34357d00a15ba5a0ad052c40fe not found
5c11720767f70d34357d00a15ba5a0ad052c40fe SUNRPC: Fix a NULL pointer deref in trace_svc_stats_latency() Checking commit 5191955d6fc6 ...
upstream fix 90bfc37b5ab91c1a6165e3e5cfc49bf04571b762 not found
90bfc37b5ab91c1a6165e3e5cfc49bf04571b762 SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation
upstream fix b9f83ffaa0c096b4c832a43964fe6bff3acffe10 not found
b9f83ffaa0c096b4c832a43964fe6bff3acffe10 SUNRPC: Fix null pointer dereference in svc_rqst_free()

Quoted 2 lines

I'll look into backporting the missing NFSD and SUNRPC patches.

My list didn't include patches with conflicts. There are a lot of them. Our robot collects those, but doesn't focus on it. It also doesn't analyze just nfds/SUNRPC patches, but all of them. I started an analysis to list all the fixes with conflicts; so far I found about 100 of them. Three are tagged SUNRPC.

Upstream commit 8e088a20dbe3 ("SUNRPC: add a missing rpc_stat for TCP TLS")
upstream: v6.9-rc7

    Fixes: 1548036ef120 ("nfs: make the rpc_stat per net namespace")
      in linux-5.4.y: 19f51adc778f
      in linux-5.10.y: afdbc21a92a0
      in linux-5.15.y: 7ceb89f4016e
      in linux-6.1.y: 2b7f2d663a96
      in linux-6.6.y: 260333221cf0
      upstream: v6.9-rc1
    Affected branches:
      linux-5.4.y (conflicts - backport needed)
      linux-5.10.y (conflicts - backport needed)
      linux-5.15.y (conflicts - backport needed)
      linux-6.1.y (conflicts - backport needed)
      linux-6.6.y (already applied)

Upstream commit aed28b7a2d62 ("SUNRPC: Don't dereference xprt->snd_task if it's a cookie")
upstream: v5.17-rc2

    Fixes: e26d9972720e ("SUNRPC: Clean up scheduling of autoclose")
      in linux-5.4.y: 2d6f096476e6
      in linux-5.10.y: 2ab569edd883
      upstream: v5.15-rc1
    Affected branches:
      linux-5.4.y (conflicts - backport needed)
      linux-5.10.y (conflicts - backport needed)
      linux-5.15.y (already applied)

Upstream commit aad41a7d7cf6 ("SUNRPC: Don't leak sockets in xs_local_connect()")
upstream: v5.18-rc6

Fixes f00432063db1 ("SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()")

in linux-5.4.y: 2f8f6c393b11
in linux-5.10.y: e68b60ae29de
in linux-5.15.y: 54f6834b283d
upstream: v5.18-rc2
Affected branches:
linux-5.4.y (conflicts - backport needed)
linux-5.10.y (conflicts - backport needed)
linux-5.15.y (conflicts - backport needed)

I'll send a complete list after the analysis is done.

Guenter

Chuck Lever III

2024-06-25 19:41 UTC #799 lore raw

Quoted 118 lines · Guenter Roeck

Quoted 65 lines · Chuck Lever III
Quoted 39 lines · Guenter Roeck
Quoted 28 lines · Chuck Lever III
Hi -
Quoted 27 lines · Greg Kroah-Hartman
Quoted 14 lines · Guenter Roeck
Quoted 8 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 20 Jun 2024 12:32:00 +0000. Anything received after that time might be too late.
[ ... ]
Quoted 3 lines
Chuck Lever <[email protected]>
SUNRPC: Prepare for xdr_stream-style decoding on the server-side
Quoted 6 lines
The ChromeOS patches robot reports a number of fixes for the patches applied in 5.5.220. This is one example, later fixed with commit 90bfc37b5ab9 ("SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation"), but there are more. Are those fixes going to be applied in a subsequent release of v5.10.y, was there a reason to not include them, or did they get lost ?
I saw this as well, but when I tried to apply a few, they didn't, so I was guessing that Chuck had merged them together into the series.
Quoted 7 lines
Quoted 1 line
I'll defer to Chuck on this, this release was all his :)
I did this port months ago, I've been waiting for the dust to settle on the 6.1 and 5.15 NFSD backports, so I've all but forgotten the status of individual patches.
If you (Greg or Guenter) send me a list of what you believe is missing, I can have a look at the individual cases and then run the finished result through our NFSD CI gauntlet.
Quoted 8 lines
This is what the robot reported so far:
1242a87da0d8 SUNRPC: Fix svcxdr_init_encode's buflen calculation
Fixes bddfdbcddbe2 ("NFSD: Extract the svcxdr_init_encode() helper")
90bfc37b5ab9 SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation
Fixes 5191955d6fc6 ("SUNRPC: Prepare for xdr_stream-style decoding on the server-side")
10396f4df8b7 nfsd: hold a lighter-weight client reference over CB_RECALL_ANY
Fixes 44df6f439a17 ("NFSD: add delegation reaper to react to low memory condition")
My naive search found:
Checking commit 44df6f439a17 ...
upstream fix 10396f4df8b75ff6ab0aa2cd74296565466f2c8d not found
10396f4df8b75ff6ab0aa2cd74296565466f2c8d nfsd: hold a lighter-weight client reference over CB_RECALL_ANY
upstream fix f385f7d244134246f984975ed34cd75f77de479f is already applied
Checking commit a2071573d634 ...
upstream fix f1aa2eb5ea05ccd1fd92d235346e60e90a1ed949 not found
f1aa2eb5ea05ccd1fd92d235346e60e90a1ed949 sysctl: fix proc_dobool() usability
Checking commit bddfdbcddbe2 ...
upstream fix 1242a87da0d8cd2a428e96ca68e7ea899b0f4624 not found
1242a87da0d8cd2a428e96ca68e7ea899b0f4624 SUNRPC: Fix svcxdr_init_encode's buflen calculation
Checking commit 9fe61450972d ... upstream fix 2111c3c0124f7432fe908c036a50abe8733dbf38 not found
2111c3c0124f7432fe908c036a50abe8733dbf38 namei: fix kernel-doc for struct renamedata and more
Checking commit 013c1667cf78 ... upstream fix 2c0f0f3639562d6e38ee9705303c6457c4936eac not found
2c0f0f3639562d6e38ee9705303c6457c4936eac module: correctly exit module_kallsyms_on_each_symbol when fn() != 0
upstream fix 1e80d9cb579ed7edd121753eeccce82ff82521b4 not found
1e80d9cb579ed7edd121753eeccce82ff82521b4 module: potential uninitialized return in module_kallsyms_on_each_symbol() Checking commit 89ff87494c6e ...
Quoted 8 lines
upstream fix 5c11720767f70d34357d00a15ba5a0ad052c40fe not found
5c11720767f70d34357d00a15ba5a0ad052c40fe SUNRPC: Fix a NULL pointer deref in trace_svc_stats_latency() Checking commit 5191955d6fc6 ...
upstream fix 90bfc37b5ab91c1a6165e3e5cfc49bf04571b762 not found
90bfc37b5ab91c1a6165e3e5cfc49bf04571b762 SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation
upstream fix b9f83ffaa0c096b4c832a43964fe6bff3acffe10 not found
b9f83ffaa0c096b4c832a43964fe6bff3acffe10 SUNRPC: Fix null pointer dereference in svc_rqst_free() I'll look into backporting the missing NFSD and SUNRPC patches.
My list didn't include patches with conflicts. There are a lot of them. Our robot collects those, but doesn't focus on it. It also doesn't analyze just nfds/SUNRPC patches, but all of them. I started an analysis to list all the fixes with conflicts; so far I found about 100 of them. Three are tagged SUNRPC.
Upstream commit 8e088a20dbe3 ("SUNRPC: add a missing rpc_stat for TCP TLS")
upstream: v6.9-rc7
Fixes 1548036ef120 ("nfs: make the rpc_stat per net namespace")
     in linux-5.4.y: 19f51adc778f
     in linux-5.10.y: afdbc21a92a0
     in linux-5.15.y: 7ceb89f4016e
     in linux-6.1.y: 2b7f2d663a96
     in linux-6.6.y: 260333221cf0
     upstream: v6.9-rc1
Affected branches:
linux-5.4.y (conflicts - backport needed)
linux-5.10.y (conflicts - backport needed)
linux-5.15.y (conflicts - backport needed)
linux-6.1.y (conflicts - backport needed)
linux-6.6.y (already applied)
Upstream commit aed28b7a2d62 ("SUNRPC: Don't dereference xprt->snd_task if it's a cookie")
upstream: v5.17-rc2
Fixes e26d9972720e ("SUNRPC: Clean up scheduling of autoclose")
     in linux-5.4.y: 2d6f096476e6
     in linux-5.10.y: 2ab569edd883
     upstream: v5.15-rc1
Affected branches:
linux-5.4.y (conflicts - backport needed)
linux-5.10.y (conflicts - backport needed)
linux-5.15.y (already applied)
Upstream commit aad41a7d7cf6 ("SUNRPC: Don't leak sockets in xs_local_connect()")
upstream: v5.18-rc6
Fixes f00432063db1 ("SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()")
     in linux-5.4.y: 2f8f6c393b11
     in linux-5.10.y: e68b60ae29de
     in linux-5.15.y: 54f6834b283d
     upstream: v5.18-rc2

Quoted 6 lines

Affected branches:
linux-5.4.y (conflicts - backport needed)
linux-5.10.y (conflicts - backport needed)
linux-5.15.y (conflicts - backport needed)
I'll send a complete list after the analysis is done.

The "NFSD file cache fixes" backports focused on NFSD, not on SUNRPC, and only the NFS server side of affairs. The missing fixes you found are outside of one or both of those areas, so they can go through the usual stable backport process if the NFS client folks care to do that.

--
Chuck Lever

Chuck Lever III

2024-06-25 19:50 UTC #800 lore raw

Quoted 127 lines · Chuck Lever III

Quoted 124 lines · Guenter Roeck
Quoted 65 lines · Chuck Lever III
Quoted 39 lines · Guenter Roeck
Quoted 28 lines · Chuck Lever III
Hi -
Quoted 27 lines · Greg Kroah-Hartman
Quoted 14 lines · Guenter Roeck
Quoted 8 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 20 Jun 2024 12:32:00 +0000. Anything received after that time might be too late.
[ ... ]
Quoted 3 lines
Chuck Lever <[email protected]>
SUNRPC: Prepare for xdr_stream-style decoding on the server-side
Quoted 6 lines
The ChromeOS patches robot reports a number of fixes for the patches applied in 5.5.220. This is one example, later fixed with commit 90bfc37b5ab9 ("SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation"), but there are more. Are those fixes going to be applied in a subsequent release of v5.10.y, was there a reason to not include them, or did they get lost ?
I saw this as well, but when I tried to apply a few, they didn't, so I was guessing that Chuck had merged them together into the series.
Quoted 7 lines
Quoted 1 line
I'll defer to Chuck on this, this release was all his :)
I did this port months ago, I've been waiting for the dust to settle on the 6.1 and 5.15 NFSD backports, so I've all but forgotten the status of individual patches.
If you (Greg or Guenter) send me a list of what you believe is missing, I can have a look at the individual cases and then run the finished result through our NFSD CI gauntlet.
Quoted 8 lines
This is what the robot reported so far:
1242a87da0d8 SUNRPC: Fix svcxdr_init_encode's buflen calculation
Fixes bddfdbcddbe2 ("NFSD: Extract the svcxdr_init_encode() helper")
90bfc37b5ab9 SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation
Fixes 5191955d6fc6 ("SUNRPC: Prepare for xdr_stream-style decoding on the server-side")
10396f4df8b7 nfsd: hold a lighter-weight client reference over CB_RECALL_ANY
Fixes 44df6f439a17 ("NFSD: add delegation reaper to react to low memory condition")
My naive search found:
Checking commit 44df6f439a17 ...
upstream fix 10396f4df8b75ff6ab0aa2cd74296565466f2c8d not found
10396f4df8b75ff6ab0aa2cd74296565466f2c8d nfsd: hold a lighter-weight client reference over CB_RECALL_ANY
upstream fix f385f7d244134246f984975ed34cd75f77de479f is already applied
Checking commit a2071573d634 ...
upstream fix f1aa2eb5ea05ccd1fd92d235346e60e90a1ed949 not found
f1aa2eb5ea05ccd1fd92d235346e60e90a1ed949 sysctl: fix proc_dobool() usability
Checking commit bddfdbcddbe2 ...
upstream fix 1242a87da0d8cd2a428e96ca68e7ea899b0f4624 not found
1242a87da0d8cd2a428e96ca68e7ea899b0f4624 SUNRPC: Fix svcxdr_init_encode's buflen calculation
Checking commit 9fe61450972d ... upstream fix 2111c3c0124f7432fe908c036a50abe8733dbf38 not found
2111c3c0124f7432fe908c036a50abe8733dbf38 namei: fix kernel-doc for struct renamedata and more
Checking commit 013c1667cf78 ... upstream fix 2c0f0f3639562d6e38ee9705303c6457c4936eac not found
2c0f0f3639562d6e38ee9705303c6457c4936eac module: correctly exit module_kallsyms_on_each_symbol when fn() != 0
upstream fix 1e80d9cb579ed7edd121753eeccce82ff82521b4 not found
1e80d9cb579ed7edd121753eeccce82ff82521b4 module: potential uninitialized return in module_kallsyms_on_each_symbol() Checking commit 89ff87494c6e ...
Quoted 8 lines
upstream fix 5c11720767f70d34357d00a15ba5a0ad052c40fe not found
5c11720767f70d34357d00a15ba5a0ad052c40fe SUNRPC: Fix a NULL pointer deref in trace_svc_stats_latency() Checking commit 5191955d6fc6 ...
upstream fix 90bfc37b5ab91c1a6165e3e5cfc49bf04571b762 not found
90bfc37b5ab91c1a6165e3e5cfc49bf04571b762 SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation
upstream fix b9f83ffaa0c096b4c832a43964fe6bff3acffe10 not found
b9f83ffaa0c096b4c832a43964fe6bff3acffe10 SUNRPC: Fix null pointer dereference in svc_rqst_free() I'll look into backporting the missing NFSD and SUNRPC patches.
My list didn't include patches with conflicts. There are a lot of them. Our robot collects those, but doesn't focus on it. It also doesn't analyze just nfds/SUNRPC patches, but all of them. I started an analysis to list all the fixes with conflicts; so far I found about 100 of them. Three are tagged SUNRPC.
Upstream commit 8e088a20dbe3 ("SUNRPC: add a missing rpc_stat for TCP TLS")
upstream: v6.9-rc7
Fixes 1548036ef120 ("nfs: make the rpc_stat per net namespace")
    in linux-5.4.y: 19f51adc778f
    in linux-5.10.y: afdbc21a92a0
    in linux-5.15.y: 7ceb89f4016e
    in linux-6.1.y: 2b7f2d663a96
    in linux-6.6.y: 260333221cf0
    upstream: v6.9-rc1
Affected branches:
linux-5.4.y (conflicts - backport needed)
linux-5.10.y (conflicts - backport needed)
linux-5.15.y (conflicts - backport needed)
linux-6.1.y (conflicts - backport needed)
linux-6.6.y (already applied)
Upstream commit aed28b7a2d62 ("SUNRPC: Don't dereference xprt->snd_task if it's a cookie")
upstream: v5.17-rc2
Fixes e26d9972720e ("SUNRPC: Clean up scheduling of autoclose")
    in linux-5.4.y: 2d6f096476e6
    in linux-5.10.y: 2ab569edd883
    upstream: v5.15-rc1
Affected branches:
linux-5.4.y (conflicts - backport needed)
linux-5.10.y (conflicts - backport needed)
linux-5.15.y (already applied)
Upstream commit aad41a7d7cf6 ("SUNRPC: Don't leak sockets in xs_local_connect()")
upstream: v5.18-rc6
Fixes f00432063db1 ("SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()")
    in linux-5.4.y: 2f8f6c393b11
    in linux-5.10.y: e68b60ae29de
    in linux-5.15.y: 54f6834b283d
    upstream: v5.18-rc2
Affected branches:
linux-5.4.y (conflicts - backport needed)
linux-5.10.y (conflicts - backport needed)
linux-5.15.y (conflicts - backport needed)

Quoted 7 lines

Quoted 1 line
I'll send a complete list after the analysis is done.
The "NFSD file cache fixes" backports focused on NFSD, not on SUNRPC, and only the NFS server side of affairs. The missing fixes you found are outside of one or both of those areas, so they can go through the usual stable backport process if the NFS client folks care to do that.

Or, to cut this another way: I looked at only the patches that I submitted for v5.10.220; I will take responsibility for ensuring those all have the latest upstream fixes applied, where that is feasible.

Anything else (other subsystems, other LTS kernels) gets the normal stable backport treatment: those subsystem maintainers or their designees have to step up to handle the code work and testing if they view the fix as a priority.

--
Chuck Lever

Guenter Roeck

2024-06-25 20:43 UTC #801 lore raw

Quoted 140 lines · Chuck Lever III

Quoted 127 lines · Chuck Lever III
Quoted 124 lines · Guenter Roeck
Quoted 65 lines · Chuck Lever III
Quoted 39 lines · Guenter Roeck
Quoted 28 lines · Chuck Lever III
Hi -
Quoted 27 lines · Greg Kroah-Hartman
Quoted 14 lines · Guenter Roeck
Quoted 8 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.10.220 release. There are 770 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 20 Jun 2024 12:32:00 +0000. Anything received after that time might be too late.
[ ... ]
Quoted 3 lines
Chuck Lever <[email protected]>
SUNRPC: Prepare for xdr_stream-style decoding on the server-side
Quoted 6 lines
The ChromeOS patches robot reports a number of fixes for the patches applied in 5.5.220. This is one example, later fixed with commit 90bfc37b5ab9 ("SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation"), but there are more. Are those fixes going to be applied in a subsequent release of v5.10.y, was there a reason to not include them, or did they get lost ?
I saw this as well, but when I tried to apply a few, they didn't, so I was guessing that Chuck had merged them together into the series.
Quoted 7 lines
Quoted 1 line
I'll defer to Chuck on this, this release was all his :)
I did this port months ago, I've been waiting for the dust to settle on the 6.1 and 5.15 NFSD backports, so I've all but forgotten the status of individual patches.
If you (Greg or Guenter) send me a list of what you believe is missing, I can have a look at the individual cases and then run the finished result through our NFSD CI gauntlet.
Quoted 8 lines
This is what the robot reported so far:
1242a87da0d8 SUNRPC: Fix svcxdr_init_encode's buflen calculation
Fixes bddfdbcddbe2 ("NFSD: Extract the svcxdr_init_encode() helper")
90bfc37b5ab9 SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation
Fixes 5191955d6fc6 ("SUNRPC: Prepare for xdr_stream-style decoding on the server-side")
10396f4df8b7 nfsd: hold a lighter-weight client reference over CB_RECALL_ANY
Fixes 44df6f439a17 ("NFSD: add delegation reaper to react to low memory condition")
My naive search found:
Checking commit 44df6f439a17 ...
upstream fix 10396f4df8b75ff6ab0aa2cd74296565466f2c8d not found
10396f4df8b75ff6ab0aa2cd74296565466f2c8d nfsd: hold a lighter-weight client reference over CB_RECALL_ANY
upstream fix f385f7d244134246f984975ed34cd75f77de479f is already applied
Checking commit a2071573d634 ...
upstream fix f1aa2eb5ea05ccd1fd92d235346e60e90a1ed949 not found
f1aa2eb5ea05ccd1fd92d235346e60e90a1ed949 sysctl: fix proc_dobool() usability
Checking commit bddfdbcddbe2 ...
upstream fix 1242a87da0d8cd2a428e96ca68e7ea899b0f4624 not found
1242a87da0d8cd2a428e96ca68e7ea899b0f4624 SUNRPC: Fix svcxdr_init_encode's buflen calculation
Checking commit 9fe61450972d ... upstream fix 2111c3c0124f7432fe908c036a50abe8733dbf38 not found
2111c3c0124f7432fe908c036a50abe8733dbf38 namei: fix kernel-doc for struct renamedata and more
Checking commit 013c1667cf78 ... upstream fix 2c0f0f3639562d6e38ee9705303c6457c4936eac not found
2c0f0f3639562d6e38ee9705303c6457c4936eac module: correctly exit module_kallsyms_on_each_symbol when fn() != 0
upstream fix 1e80d9cb579ed7edd121753eeccce82ff82521b4 not found
1e80d9cb579ed7edd121753eeccce82ff82521b4 module: potential uninitialized return in module_kallsyms_on_each_symbol() Checking commit 89ff87494c6e ...
Quoted 8 lines
upstream fix 5c11720767f70d34357d00a15ba5a0ad052c40fe not found
5c11720767f70d34357d00a15ba5a0ad052c40fe SUNRPC: Fix a NULL pointer deref in trace_svc_stats_latency() Checking commit 5191955d6fc6 ...
upstream fix 90bfc37b5ab91c1a6165e3e5cfc49bf04571b762 not found
90bfc37b5ab91c1a6165e3e5cfc49bf04571b762 SUNRPC: Fix svcxdr_init_decode's end-of-buffer calculation
upstream fix b9f83ffaa0c096b4c832a43964fe6bff3acffe10 not found
b9f83ffaa0c096b4c832a43964fe6bff3acffe10 SUNRPC: Fix null pointer dereference in svc_rqst_free() I'll look into backporting the missing NFSD and SUNRPC patches.
My list didn't include patches with conflicts. There are a lot of them. Our robot collects those, but doesn't focus on it. It also doesn't analyze just nfds/SUNRPC patches, but all of them. I started an analysis to list all the fixes with conflicts; so far I found about 100 of them. Three are tagged SUNRPC.
Upstream commit 8e088a20dbe3 ("SUNRPC: add a missing rpc_stat for TCP TLS")
upstream: v6.9-rc7
Fixes 1548036ef120 ("nfs: make the rpc_stat per net namespace")
     in linux-5.4.y: 19f51adc778f
     in linux-5.10.y: afdbc21a92a0
     in linux-5.15.y: 7ceb89f4016e
     in linux-6.1.y: 2b7f2d663a96
     in linux-6.6.y: 260333221cf0
     upstream: v6.9-rc1
Affected branches:
linux-5.4.y (conflicts - backport needed)
linux-5.10.y (conflicts - backport needed)
linux-5.15.y (conflicts - backport needed)
linux-6.1.y (conflicts - backport needed)
linux-6.6.y (already applied)
Upstream commit aed28b7a2d62 ("SUNRPC: Don't dereference xprt->snd_task if it's a cookie")
upstream: v5.17-rc2
Fixes e26d9972720e ("SUNRPC: Clean up scheduling of autoclose")
     in linux-5.4.y: 2d6f096476e6
     in linux-5.10.y: 2ab569edd883
     upstream: v5.15-rc1
Affected branches:
linux-5.4.y (conflicts - backport needed)
linux-5.10.y (conflicts - backport needed)
linux-5.15.y (already applied)
Upstream commit aad41a7d7cf6 ("SUNRPC: Don't leak sockets in xs_local_connect()")
upstream: v5.18-rc6
Fixes f00432063db1 ("SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()")
     in linux-5.4.y: 2f8f6c393b11
     in linux-5.10.y: e68b60ae29de
     in linux-5.15.y: 54f6834b283d
     upstream: v5.18-rc2
Affected branches:
linux-5.4.y (conflicts - backport needed)
linux-5.10.y (conflicts - backport needed)
linux-5.15.y (conflicts - backport needed)
Quoted 7 lines
Quoted 1 line
I'll send a complete list after the analysis is done.
The "NFSD file cache fixes" backports focused on NFSD, not on SUNRPC, and only the NFS server side of affairs. The missing fixes you found are outside of one or both of those areas, so they can go through the usual stable backport process if the NFS client folks care to do that.
Or, to cut this another way: I looked at only the patches that I submitted for v5.10.220; I will take responsibility for ensuring those all have the latest upstream fixes applied, where that is feasible.

Quoted 5 lines

Anything else (other subsystems, other LTS kernels) gets the normal stable backport treatment: those subsystem maintainers or their designees have to step up to handle the code work and testing if they view the fix as a priority.

Sounds good to me.

Thanks,
Guenter