Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Amir Goldstein <amir73il@gmail.com>
To:     Jan Kara <jack@suse.cz>
Cc:     Jeff Layton <jlayton@kernel.org>,
        Chuck Lever <chuck.lever@oracle.com>,
        linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org,
        Dan Carpenter <dan.carpenter@linaro.org>
Subject: [PATCH] exportfs: check for error return value from exportfs_encode_*()
Date:   Wed, 24 May 2023 18:48:25 +0300
Message-Id: <20230524154825.881414-1-amir73il@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

The exportfs_encode_*() helpers call the filesystem ->encode_fh()
method which returns a signed int.

All the in-tree implementations of ->encode_fh() return a positive
integer and FILEID_INVALID (255) for error.

Fortify the callers for possible future ->encode_fh() implementation
that will return a negative error value.

name_to_handle_at() would propagate the returned error to the users
if filesystem ->encode_fh() method returns an error.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Link: https://lore.kernel.org/linux-fsdevel/ca02955f-1877-4fde-b453-3c1d22794740@kili.mountain/
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Jan,

This patch is on top of the patches you have queued on fsnotify branch.

I am not sure about the handling of negative value in nfsfh.c.

Jeff/Chuck,

Could you please take a look.

I've test this patch with fanotify LTP tests, xfstest -g exportfs tests
and some sanity xfstest nfs tests, but I did not try to inject errors
in encode_fh().

Please let me know what you think.

Thanks,
Amir.


 fs/fhandle.c                  | 5 +++--
 fs/nfsd/nfsfh.c               | 4 +++-
 fs/notify/fanotify/fanotify.c | 2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/fhandle.c b/fs/fhandle.c
index 4a635cf787fc..fd0d6a3b3699 100644
--- a/fs/fhandle.c
+++ b/fs/fhandle.c
@@ -57,18 +57,19 @@ static long do_sys_name_to_handle(const struct path *path,
 	handle_bytes = handle_dwords * sizeof(u32);
 	handle->handle_bytes = handle_bytes;
 	if ((handle->handle_bytes > f_handle.handle_bytes) ||
-	    (retval == FILEID_INVALID) || (retval == -ENOSPC)) {
+	    (retval == FILEID_INVALID) || (retval < 0)) {
 		/* As per old exportfs_encode_fh documentation
 		 * we could return ENOSPC to indicate overflow
 		 * But file system returned 255 always. So handle
 		 * both the values
 		 */
+		if (retval == FILEID_INVALID || retval == -ENOSPC)
+			retval = -EOVERFLOW;
 		/*
 		 * set the handle size to zero so we copy only
 		 * non variable part of the file_handle
 		 */
 		handle_bytes = 0;
-		retval = -EOVERFLOW;
 	} else
 		retval = 0;
 	/* copy the mount id */
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index 31e4505c0df3..0f5eacae5f43 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -416,9 +416,11 @@ static void _fh_update(struct svc_fh *fhp, struct svc_export *exp,
 		int maxsize = (fhp->fh_maxsize - fhp->fh_handle.fh_size)/4;
 		int fh_flags = (exp->ex_flags & NFSEXP_NOSUBTREECHECK) ? 0 :
 				EXPORT_FH_CONNECTABLE;
+		int fileid_type =
+			exportfs_encode_fh(dentry, fid, &maxsize, fh_flags);
 
 		fhp->fh_handle.fh_fileid_type =
-			exportfs_encode_fh(dentry, fid, &maxsize, fh_flags);
+			fileid_type > 0 ? fileid_type : FILEID_INVALID;
 		fhp->fh_handle.fh_size += maxsize * 4;
 	} else {
 		fhp->fh_handle.fh_fileid_type = FILEID_ROOT;
diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index d2bbf1445a9e..9dac7f6e72d2 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -445,7 +445,7 @@ static int fanotify_encode_fh(struct fanotify_fh *fh, struct inode *inode,
 	dwords = fh_len >> 2;
 	type = exportfs_encode_fid(inode, buf, &dwords);
 	err = -EINVAL;
-	if (!type || type == FILEID_INVALID || fh_len != dwords << 2)
+	if (type <= 0 || type == FILEID_INVALID || fh_len != dwords << 2)
 		goto out_err;
 
 	fh->type = type;
-- 
2.34.1