nfs_clsubs.c: Fix ncl_getcookie() when "pos" is negative

In ncl_getcookie(), a very large value for "off" for
a directory can result in "pos" being set to a bogus
value (including a negative one), due to truncation.
When "pos" is negative, is can skip past the
while (pos >= NFSNUMCOOKIES) loop and return a
bogus pointer instead of NULL.

This patch changes the type to u_int and also adds
a sanity check for a very large "off" to ensure that
a NULL pointer is returned for this case.

This bug has been in the code for decades and I am
not aware of any report of it causing a problem for
users.

Reviewed by:	markj
Reported by:	Joshua Rogers of AISLE Research Team
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D56779

2 files changed, 10 insertions, 2 deletions

diff --git a/sys/fs/nfsclient/nfs_clsubs.c b/sys/fs/nfsclient/nfs_clsubs.c
index ae9fa51947cc..6361ae5f2901 100644
--- a/sys/fs/nfsclient/nfs_clsubs.c
+++ b/sys/fs/nfsclient/nfs_clsubs.c
@@ -263,9 +263,17 @@ nfsuint64 *
 ncl_getcookie(struct nfsnode *np, off_t off, int add)
 {
 	struct nfsdmap *dp, *dp2;
-	int pos;
+	u_int pos;
 	nfsuint64 *retval = NULL;
 
+	/*
+	 * Limiting "off" to 50Gbytes sets a limit of 100 million directory
+	 * entries of maximum filename length.  Much more with shorter
+	 * file names.  This limit ensures "pos" will not be truncated
+	 * in the devision below.
+	 */
+	if (off > 53687091200ull)
+		goto out;
 	pos = (uoff_t)off / NFS_DIRBLKSIZ;
 	if (pos == 0 || off < 0) {
 		KASSERT(!add, ("nfs getcookie add at <= 0"));
diff --git a/sys/fs/nfsclient/nfsnode.h b/sys/fs/nfsclient/nfsnode.h
index 07c7ccb0ff10..cd9ded943c2f 100644
--- a/sys/fs/nfsclient/nfsnode.h
+++ b/sys/fs/nfsclient/nfsnode.h
@@ -61,7 +61,7 @@ struct sillyrename {
 
 struct nfsdmap {
 	LIST_ENTRY(nfsdmap)	ndm_list;
-	int			ndm_eocookie;
+	u_int			ndm_eocookie;
 	union {
 		nfsuint64	ndmu3_cookies[NFSNUMCOOKIES];
 		uint64_t	ndmu4_cookies[NFSNUMCOOKIES];