From:   Jeff Layton <jlayton@kernel.org>
To:     chuck.lever@oracle.com
Cc:     linux-nfs@vger.kernel.org, dcritch@redhat.com, d.lesca@solinos.it,
        viro@zeniv.linux.org.uk
Subject: [PATCH v2 2/2] sunrpc: add bounds checking to svc_rqst_replace_page
Date:   Fri, 17 Mar 2023 13:13:09 -0400
Message-Id: <20230317171309.73607-2-jlayton@kernel.org>
In-Reply-To: <20230317171309.73607-1-jlayton@kernel.org>
References: <20230317171309.73607-1-jlayton@kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

If rq_next_page ends up pointing outside the array, then we can corrupt
memory when we go to change its value. Ensure that it hasn't strayed
outside the array, and have svc_rqst_replace_page return -EIO without
changing anything if it has.

Fix up nfsd_splice_actor (the only caller) to handle this case by either
returning an error or a short splice when this happens.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/vfs.c              | 23 +++++++++++++++++------
 include/linux/sunrpc/svc.h |  2 +-
 net/sunrpc/svc.c           | 14 +++++++++++++-
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 97b38b47c563..0ebd7a65a9f0 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -939,6 +939,7 @@ nfsd_splice_actor(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
 	struct page *page = buf->page;	// may be a compound one
 	unsigned offset = buf->offset;
 	struct page *last_page;
+	int ret = 0, consumed = 0;
 
 	last_page = page + (offset + sd->len - 1) / PAGE_SIZE;
 	for (page += offset / PAGE_SIZE; page <= last_page; page++) {
@@ -946,13 +947,23 @@ nfsd_splice_actor(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
 		 * Skip page replacement when extending the contents
 		 * of the current page.
 		 */
-		if (page != *(rqstp->rq_next_page - 1))
-			svc_rqst_replace_page(rqstp, page);
+		if (page != *(rqstp->rq_next_page - 1)) {
+			ret = svc_rqst_replace_page(rqstp, page);
+			if (ret)
+				break;
+		}
+		consumed += min_t(int,
+				  PAGE_SIZE - offset_in_page(offset),
+				  sd->len - consumed);
+		offset = 0;
 	}
-	if (rqstp->rq_res.page_len == 0)	// first call
-		rqstp->rq_res.page_base = offset % PAGE_SIZE;
-	rqstp->rq_res.page_len += sd->len;
-	return sd->len;
+	if (consumed) {
+		if (rqstp->rq_res.page_len == 0)	// first call
+			rqstp->rq_res.page_base = offset % PAGE_SIZE;
+		rqstp->rq_res.page_len += consumed;
+		return consumed;
+	}
+	return ret;
 }
 
 static int nfsd_direct_splice_actor(struct pipe_inode_info *pipe,
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 877891536c2f..9ea52f143f49 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -422,7 +422,7 @@ struct svc_serv *svc_create(struct svc_program *, unsigned int,
 			    int (*threadfn)(void *data));
 struct svc_rqst *svc_rqst_alloc(struct svc_serv *serv,
 					struct svc_pool *pool, int node);
-void		   svc_rqst_replace_page(struct svc_rqst *rqstp,
+int		   svc_rqst_replace_page(struct svc_rqst *rqstp,
 					 struct page *page);
 void		   svc_rqst_free(struct svc_rqst *);
 void		   svc_exit_thread(struct svc_rqst *);
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index fea7ce8fba14..d624c02f09be 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -843,8 +843,19 @@ EXPORT_SYMBOL_GPL(svc_set_num_threads);
  * When replacing a page in rq_pages, batch the release of the
  * replaced pages to avoid hammering the page allocator.
  */
-void svc_rqst_replace_page(struct svc_rqst *rqstp, struct page *page)
+int svc_rqst_replace_page(struct svc_rqst *rqstp, struct page *page)
 {
+	struct page **begin, **end;
+
+	/*
+	 * Bounds check: make sure rq_next_page points into the rq_respages
+	 * part of the array.
+	 */
+	begin = rqstp->rq_pages;
+	end = &rqstp->rq_pages[RPCSVC_MAXPAGES];
+	if (WARN_ON_ONCE(rqstp->rq_next_page < begin || rqstp->rq_next_page > end))
+		return -EIO;
+
 	if (*rqstp->rq_next_page) {
 		if (!pagevec_space(&rqstp->rq_pvec))
 			__pagevec_release(&rqstp->rq_pvec);
@@ -853,6 +864,7 @@ void svc_rqst_replace_page(struct svc_rqst *rqstp, struct page *page)
 
 	get_page(page);
 	*(rqstp->rq_next_page++) = page;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(svc_rqst_replace_page);
 
-- 
2.39.2