Linux NFS development
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: chuck.lever@oracle.com
Cc: bfields@fieldses.org, linux-nfs@vger.kernel.org,
	Jan Kasiak <j.kasiak@gmail.com>
Subject: [PATCH] lockd: detect and reject lock arguments that overflow
Date: Mon,  1 Aug 2022 13:18:19 -0400	[thread overview]
Message-ID: <20220801171819.125532-1-jlayton@kernel.org> (raw)

lockd doesn't currently vet the start and length in nlm4 requests like
it should, and can end up requesting locks with arguments that overflow
to the filesystem.

The NLM4 protocol unsigned 64-bit arguments for both start and length,
whereas struct file_lock tracks the start and end as loff_t values. By
the time we get around to calling nlm4svc_retrieve_args, we've lost the
information that would allow us to determine if there was an overflow.

Track whether the arguments will overflow as a boolean in struct
nlm_lock, and test for that in nlm4svc_retrieve_args. While we're in
here, get rid of the useless s64_to_loff_t helper and just directly cast
the values.

Link: https://bugzilla.linux-nfs.org/show_bug.cgi?id=392
Reported-by: Jan Kasiak <j.kasiak@gmail.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/lockd/svc4proc.c       |  3 +++
 fs/lockd/xdr4.c           | 26 ++++++++++++--------------
 include/linux/lockd/xdr.h |  1 +
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
index 176b468a61c7..e781ac747961 100644
--- a/fs/lockd/svc4proc.c
+++ b/fs/lockd/svc4proc.c
@@ -32,6 +32,9 @@ nlm4svc_retrieve_args(struct svc_rqst *rqstp, struct nlm_args *argp,
 	if (!nlmsvc_ops)
 		return nlm_lck_denied_nolocks;
 
+	if (lock->overflow)
+		return nlm4_fbig;
+
 	/* Obtain host handle */
 	if (!(host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len))
 	 || (argp->monitor && nsm_monitor(host) < 0))
diff --git a/fs/lockd/xdr4.c b/fs/lockd/xdr4.c
index 856267c0864b..fd5de5481a6f 100644
--- a/fs/lockd/xdr4.c
+++ b/fs/lockd/xdr4.c
@@ -20,13 +20,6 @@
 
 #include "svcxdr.h"
 
-static inline loff_t
-s64_to_loff_t(__s64 offset)
-{
-	return (loff_t)offset;
-}
-
-
 static inline s64
 loff_t_to_s64(loff_t offset)
 {
@@ -71,7 +64,6 @@ svcxdr_decode_lock(struct xdr_stream *xdr, struct nlm_lock *lock)
 {
 	struct file_lock *fl = &lock->fl;
 	u64 len, start;
-	s64 end;
 
 	if (!svcxdr_decode_string(xdr, &lock->caller, &lock->len))
 		return false;
@@ -86,15 +78,21 @@ svcxdr_decode_lock(struct xdr_stream *xdr, struct nlm_lock *lock)
 	if (xdr_stream_decode_u64(xdr, &len) < 0)
 		return false;
 
+	/*
+	 * We don't preserve the actual arguments in the call, so once
+	 * the decoding is done, it's too late to detect overflow. Thus,
+	 * we track it here and check for it later.
+	 */
+	if (start > OFFSET_MAX || (len && ((len - 1) > (OFFSET_MAX - start)))) {
+		lock->overflow = true;
+		return true;
+	}
+
 	locks_init_lock(fl);
 	fl->fl_flags = FL_POSIX;
 	fl->fl_type  = F_RDLCK;
-	end = start + len - 1;
-	fl->fl_start = s64_to_loff_t(start);
-	if (len == 0 || end < 0)
-		fl->fl_end = OFFSET_MAX;
-	else
-		fl->fl_end = s64_to_loff_t(end);
+	fl->fl_start = (loff_t)start;
+	fl->fl_end = len ? (loff_t)(start + len - 1) : OFFSET_MAX;
 
 	return true;
 }
diff --git a/include/linux/lockd/xdr.h b/include/linux/lockd/xdr.h
index 398f70093cd3..a80112d18658 100644
--- a/include/linux/lockd/xdr.h
+++ b/include/linux/lockd/xdr.h
@@ -39,6 +39,7 @@ struct nlm_lock {
 	char *			caller;
 	unsigned int		len; 	/* length of "caller" */
 	struct nfs_fh		fh;
+	bool			overflow; /* lock range overflow */
 	struct xdr_netobj	oh;
 	u32			svid;
 	struct file_lock	fl;
-- 
2.37.1


             reply	other threads:[~2022-08-01 17:18 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-01 17:18 Jeff Layton [this message]
2022-08-01 19:18 ` [PATCH] lockd: detect and reject lock arguments that overflow Chuck Lever III

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20220801171819.125532-1-jlayton@kernel.org \
    --to=jlayton@kernel.org \
    --cc=bfields@fieldses.org \
    --cc=chuck.lever@oracle.com \
    --cc=j.kasiak@gmail.com \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

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

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