[PATCH v2 2/2] sunrpc: add bounds checking to svc_rqst_replace_page
From: Jeff Layton <jlayton@kernel.org>
Date: 2023-03-17 17:13:22
Subsystem:
filesystems (vfs and infrastructure), kernel nfsd, sunrpc, and lockd servers, networking [general], nfs, sunrpc, and lockd clients, the rest · Maintainers:
Alexander Viro, Christian Brauner, Chuck Lever, Jeff Layton, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Trond Myklebust, Anna Schumaker, Linus Torvalds
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