Re: [PATCH 2/2] sunrpc: add bounds checking to svc_rqst_replace_page
From: Chuck Lever III <hidden>
Date: 2023-03-17 13:44:31
quoted hunk ↗ jump to hunk
On Mar 17, 2023, at 6:56 AM, Jeff Layton [off-list ref] wrote: There's no good way to handle this gracefully, but if rq_next_page ends up pointing outside the array, we can at least crash the box before it scribbles over too much else. Signed-off-by: Jeff Layton <jlayton@kernel.org> --- net/sunrpc/svc.c | 10 ++++++++++ 1 file changed, 10 insertions(+)diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c index fea7ce8fba14..864e62945647 100644 --- a/net/sunrpc/svc.c +++ b/net/sunrpc/svc.c@@ -845,6 +845,16 @@ EXPORT_SYMBOL_GPL(svc_set_num_threads); */void 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]; + BUG_ON(rqstp->rq_next_page < begin || rqstp->rq_next_page > end);
Linus has stated clearly that he does not want BUG_ON assertions if the system is not actually in danger... and this is clearly the result of a software bug, so a crash will occur anyway. Can you make this a pr_warn_once() ?
+
if (*rqstp->rq_next_page) {
if (!pagevec_space(&rqstp->rq_pvec))
__pagevec_release(&rqstp->rq_pvec);
--
2.39.2-- Chuck Lever