[PATCH] lockd: don't allow locking on reexported NFSv2/3

[PATCH] lockd: don't allow locking on reexported NFSv2/3

[Jeff Layton]

Since commit 9254c8ae9b81 ("nfsd: disallow file locking and delegations
for NFSv4 reexport"), file locking when reexporting an NFS mount via
NFSv4 is expressly prohibited by nfsd. Do the same in lockd:

Add a new  nlmsvc_file_cannot_lock() helper that will test whether file
locking is allowed for a given file, and return nlm_lck_denied_nolocks
if it isn't.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Regardless of how we fix the bug that Olga found recently, I think we
need to do this as well. We don't allow locking when reexporting via v4,
and I don't think we want to allow it when reexporting via v2/3 either.
---
 fs/lockd/svclock.c          | 12 ++++++++++++
 fs/lockd/svcshare.c         |  6 ++++++
 include/linux/lockd/lockd.h |  9 ++++++++-
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index c1315df4b350bbd753305b5c08550d50f67b92aa..cd42e480a7000f1b3ec7fca5ecc7fb8dc4755a09 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -495,6 +495,9 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
 				(long long)lock->fl.fl_end,
 				wait);
 
+	if (nlmsvc_file_cannot_lock(file))
+		return nlm_lck_denied_nolocks;
+
 	if (!locks_can_async_lock(nlmsvc_file_file(file)->f_op)) {
 		async_block = wait;
 		wait = 0;
@@ -621,6 +624,9 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
 				(long long)lock->fl.fl_start,
 				(long long)lock->fl.fl_end);
 
+	if (nlmsvc_file_cannot_lock(file))
+		return nlm_lck_denied_nolocks;
+
 	if (locks_in_grace(SVC_NET(rqstp))) {
 		ret = nlm_lck_denied_grace_period;
 		goto out;
@@ -678,6 +684,9 @@ nlmsvc_unlock(struct net *net, struct nlm_file *file, struct nlm_lock *lock)
 				(long long)lock->fl.fl_start,
 				(long long)lock->fl.fl_end);
 
+	if (nlmsvc_file_cannot_lock(file))
+		return nlm_lck_denied_nolocks;
+
 	/* First, cancel any lock that might be there */
 	nlmsvc_cancel_blocked(net, file, lock);
 
@@ -715,6 +724,9 @@ nlmsvc_cancel_blocked(struct net *net, struct nlm_file *file, struct nlm_lock *l
 				(long long)lock->fl.fl_start,
 				(long long)lock->fl.fl_end);
 
+	if (nlmsvc_file_cannot_lock(file))
+		return nlm_lck_denied_nolocks;
+
 	if (locks_in_grace(net))
 		return nlm_lck_denied_grace_period;
 
diff --git a/fs/lockd/svcshare.c b/fs/lockd/svcshare.c
index ade4931b2da247abd23bd16923f1d2388dc6ce00..88c81ce1148d92bd29ec580ac399ac944ba5ecf8 100644
--- a/fs/lockd/svcshare.c
+++ b/fs/lockd/svcshare.c
@@ -32,6 +32,9 @@ nlmsvc_share_file(struct nlm_host *host, struct nlm_file *file,
 	struct xdr_netobj	*oh = &argp->lock.oh;
 	u8			*ohdata;
 
+	if (nlmsvc_file_cannot_lock(file))
+		return nlm_lck_denied_nolocks;
+
 	for (share = file->f_shares; share; share = share->s_next) {
 		if (share->s_host == host && nlm_cmp_owner(share, oh))
 			goto update;
@@ -72,6 +75,9 @@ nlmsvc_unshare_file(struct nlm_host *host, struct nlm_file *file,
 	struct nlm_share	*share, **shpp;
 	struct xdr_netobj	*oh = &argp->lock.oh;
 
+	if (nlmsvc_file_cannot_lock(file))
+		return nlm_lck_denied_nolocks;
+
 	for (shpp = &file->f_shares; (share = *shpp) != NULL;
 					shpp = &share->s_next) {
 		if (share->s_host == host && nlm_cmp_owner(share, oh)) {
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index c8f0f9458f2cc035fd9161f8f2486ba76084abf1..330e38776bb20d09c20697fc38a96c161ad0313a 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -12,6 +12,7 @@
 
 /* XXX: a lot of this should really be under fs/lockd. */
 
+#include <linux/exportfs.h>
 #include <linux/in.h>
 #include <linux/in6.h>
 #include <net/ipv6.h>
@@ -307,7 +308,7 @@ void		  nlmsvc_invalidate_all(void);
 int           nlmsvc_unlock_all_by_sb(struct super_block *sb);
 int           nlmsvc_unlock_all_by_ip(struct sockaddr *server_addr);
 
-static inline struct file *nlmsvc_file_file(struct nlm_file *file)
+static inline struct file *nlmsvc_file_file(const struct nlm_file *file)
 {
 	return file->f_file[O_RDONLY] ?
 	       file->f_file[O_RDONLY] : file->f_file[O_WRONLY];
@@ -318,6 +319,12 @@ static inline struct inode *nlmsvc_file_inode(struct nlm_file *file)
 	return file_inode(nlmsvc_file_file(file));
 }
 
+static inline bool
+nlmsvc_file_cannot_lock(const struct nlm_file *file)
+{
+	return exportfs_cannot_lock(nlmsvc_file_file(file)->f_path.dentry->d_sb->s_export_op);
+}
+
 static inline int __nlm_privileged_request4(const struct sockaddr *sap)
 {
 	const struct sockaddr_in *sin = (struct sockaddr_in *)sap;

---
base-commit: 316f960d9ffb8439e0876dc2eab812e55f3ccb2a
change-id: 20251023-lockd-owner-a529e45d8622

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

Re: [PATCH] lockd: don't allow locking on reexported NFSv2/3

[Olga Kornievskaia]

On Thu, Oct 23, 2025 at 9:13 AM Jeff Layton <jlayton@kernel.org> wrote:
>
> Since commit 9254c8ae9b81 ("nfsd: disallow file locking and delegations
> for NFSv4 reexport"), file locking when reexporting an NFS mount via
> NFSv4 is expressly prohibited by nfsd. Do the same in lockd:
>
> Add a new  nlmsvc_file_cannot_lock() helper that will test whether file
> locking is allowed for a given file, and return nlm_lck_denied_nolocks
> if it isn't.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Tested-by: Olga Kornievskaia <okorniev@redhat.com>

> ---
> Regardless of how we fix the bug that Olga found recently, I think we
> need to do this as well. We don't allow locking when reexporting via v4,
> and I don't think we want to allow it when reexporting via v2/3 either.
> ---
>  fs/lockd/svclock.c          | 12 ++++++++++++
>  fs/lockd/svcshare.c         |  6 ++++++
>  include/linux/lockd/lockd.h |  9 ++++++++-
>  3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> index c1315df4b350bbd753305b5c08550d50f67b92aa..cd42e480a7000f1b3ec7fca5ecc7fb8dc4755a09 100644
> --- a/fs/lockd/svclock.c
> +++ b/fs/lockd/svclock.c
> @@ -495,6 +495,9 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
>                                 (long long)lock->fl.fl_end,
>                                 wait);
>
> +       if (nlmsvc_file_cannot_lock(file))
> +               return nlm_lck_denied_nolocks;
> +
>         if (!locks_can_async_lock(nlmsvc_file_file(file)->f_op)) {
>                 async_block = wait;
>                 wait = 0;
> @@ -621,6 +624,9 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
>                                 (long long)lock->fl.fl_start,
>                                 (long long)lock->fl.fl_end);
>
> +       if (nlmsvc_file_cannot_lock(file))
> +               return nlm_lck_denied_nolocks;
> +
>         if (locks_in_grace(SVC_NET(rqstp))) {
>                 ret = nlm_lck_denied_grace_period;
>                 goto out;
> @@ -678,6 +684,9 @@ nlmsvc_unlock(struct net *net, struct nlm_file *file, struct nlm_lock *lock)
>                                 (long long)lock->fl.fl_start,
>                                 (long long)lock->fl.fl_end);
>
> +       if (nlmsvc_file_cannot_lock(file))
> +               return nlm_lck_denied_nolocks;
> +
>         /* First, cancel any lock that might be there */
>         nlmsvc_cancel_blocked(net, file, lock);
>
> @@ -715,6 +724,9 @@ nlmsvc_cancel_blocked(struct net *net, struct nlm_file *file, struct nlm_lock *l
>                                 (long long)lock->fl.fl_start,
>                                 (long long)lock->fl.fl_end);
>
> +       if (nlmsvc_file_cannot_lock(file))
> +               return nlm_lck_denied_nolocks;
> +
>         if (locks_in_grace(net))
>                 return nlm_lck_denied_grace_period;
>
> diff --git a/fs/lockd/svcshare.c b/fs/lockd/svcshare.c
> index ade4931b2da247abd23bd16923f1d2388dc6ce00..88c81ce1148d92bd29ec580ac399ac944ba5ecf8 100644
> --- a/fs/lockd/svcshare.c
> +++ b/fs/lockd/svcshare.c
> @@ -32,6 +32,9 @@ nlmsvc_share_file(struct nlm_host *host, struct nlm_file *file,
>         struct xdr_netobj       *oh = &argp->lock.oh;
>         u8                      *ohdata;
>
> +       if (nlmsvc_file_cannot_lock(file))
> +               return nlm_lck_denied_nolocks;
> +
>         for (share = file->f_shares; share; share = share->s_next) {
>                 if (share->s_host == host && nlm_cmp_owner(share, oh))
>                         goto update;
> @@ -72,6 +75,9 @@ nlmsvc_unshare_file(struct nlm_host *host, struct nlm_file *file,
>         struct nlm_share        *share, **shpp;
>         struct xdr_netobj       *oh = &argp->lock.oh;
>
> +       if (nlmsvc_file_cannot_lock(file))
> +               return nlm_lck_denied_nolocks;
> +
>         for (shpp = &file->f_shares; (share = *shpp) != NULL;
>                                         shpp = &share->s_next) {
>                 if (share->s_host == host && nlm_cmp_owner(share, oh)) {
> diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
> index c8f0f9458f2cc035fd9161f8f2486ba76084abf1..330e38776bb20d09c20697fc38a96c161ad0313a 100644
> --- a/include/linux/lockd/lockd.h
> +++ b/include/linux/lockd/lockd.h
> @@ -12,6 +12,7 @@
>
>  /* XXX: a lot of this should really be under fs/lockd. */
>
> +#include <linux/exportfs.h>
>  #include <linux/in.h>
>  #include <linux/in6.h>
>  #include <net/ipv6.h>
> @@ -307,7 +308,7 @@ void                  nlmsvc_invalidate_all(void);
>  int           nlmsvc_unlock_all_by_sb(struct super_block *sb);
>  int           nlmsvc_unlock_all_by_ip(struct sockaddr *server_addr);
>
> -static inline struct file *nlmsvc_file_file(struct nlm_file *file)
> +static inline struct file *nlmsvc_file_file(const struct nlm_file *file)
>  {
>         return file->f_file[O_RDONLY] ?
>                file->f_file[O_RDONLY] : file->f_file[O_WRONLY];
> @@ -318,6 +319,12 @@ static inline struct inode *nlmsvc_file_inode(struct nlm_file *file)
>         return file_inode(nlmsvc_file_file(file));
>  }
>
> +static inline bool
> +nlmsvc_file_cannot_lock(const struct nlm_file *file)
> +{
> +       return exportfs_cannot_lock(nlmsvc_file_file(file)->f_path.dentry->d_sb->s_export_op);
> +}
> +
>  static inline int __nlm_privileged_request4(const struct sockaddr *sap)
>  {
>         const struct sockaddr_in *sin = (struct sockaddr_in *)sap;
>
> ---
> base-commit: 316f960d9ffb8439e0876dc2eab812e55f3ccb2a
> change-id: 20251023-lockd-owner-a529e45d8622
>
> Best regards,
> --
> Jeff Layton <jlayton@kernel.org>
>
>

Re: [PATCH] lockd: don't allow locking on reexported NFSv2/3

[NeilBrown]

On Fri, 24 Oct 2025, Jeff Layton wrote:
> Since commit 9254c8ae9b81 ("nfsd: disallow file locking and delegations
> for NFSv4 reexport"), file locking when reexporting an NFS mount via
> NFSv4 is expressly prohibited by nfsd. Do the same in lockd:
> 
> Add a new  nlmsvc_file_cannot_lock() helper that will test whether file
> locking is allowed for a given file, and return nlm_lck_denied_nolocks
> if it isn't.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> Regardless of how we fix the bug that Olga found recently, I think we
> need to do this as well. We don't allow locking when reexporting via v4,
> and I don't think we want to allow it when reexporting via v2/3 either.

I would like to see more justification.  The two locking protocols have
substantial differences so it is not obvious that reasoning which
applies to one also applies to the other.

What is the reason that we disable locking for v4?  If we could state
that and justify that the same reasoning applies to v3, then certainly
this would be an appropriate fix.

My guess is that it relates to handling restart of the forwarding server
from the perspectives of both the ultimate client and the ultimate
server.  Restart handling is quite different in the two protocols, but
maybe they are equally unable to handle this situation?

Thanks,
NeilBrown

Re: [PATCH] lockd: don't allow locking on reexported NFSv2/3

[Jeff Layton]

On Fri, 2025-10-24 at 08:49 +1100, NeilBrown wrote:
> On Fri, 24 Oct 2025, Jeff Layton wrote:
> > Since commit 9254c8ae9b81 ("nfsd: disallow file locking and delegations
> > for NFSv4 reexport"), file locking when reexporting an NFS mount via
> > NFSv4 is expressly prohibited by nfsd. Do the same in lockd:
> > 
> > Add a new  nlmsvc_file_cannot_lock() helper that will test whether file
> > locking is allowed for a given file, and return nlm_lck_denied_nolocks
> > if it isn't.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > Regardless of how we fix the bug that Olga found recently, I think we
> > need to do this as well. We don't allow locking when reexporting via v4,
> > and I don't think we want to allow it when reexporting via v2/3 either.
> 
> I would like to see more justification.  The two locking protocols have
> substantial differences so it is not obvious that reasoning which
> applies to one also applies to the other.
> 
> What is the reason that we disable locking for v4?  If we could state
> that and justify that the same reasoning applies to v3, then certainly
> this would be an appropriate fix.
> 
> My guess is that it relates to handling restart of the forwarding server
> from the perspectives of both the ultimate client and the ultimate
> server.  Restart handling is quite different in the two protocols, but
> maybe they are equally unable to handle this situation?
> 

Yes, that's basically it. If the reexporting server crashes and
reboots, there is no mechanism to force the backend server into grace
and no coordination of their grace periods. That's true for all
versions of NFS.

Documentation/filesystems/nfs/reexport.rst has a section that talks
about reboot recovery which documents this, but we can certainly quote
that in the changelog.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH] lockd: don't allow locking on reexported NFSv2/3

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

On Thu, 23 Oct 2025 09:12:39 -0400, Jeff Layton wrote:
> Since commit 9254c8ae9b81 ("nfsd: disallow file locking and delegations
> for NFSv4 reexport"), file locking when reexporting an NFS mount via
> NFSv4 is expressly prohibited by nfsd. Do the same in lockd:
> 
> Add a new  nlmsvc_file_cannot_lock() helper that will test whether file
> locking is allowed for a given file, and return nlm_lck_denied_nolocks
> if it isn't.
> 
> [...]

Applied to nfsd-testing, thanks!

[1/1] lockd: don't allow locking on reexported NFSv2/3
      commit: 49d93cbe598561458a1da11dceb2e26891d019e3

--
Chuck Lever