[PATCH] afs: Fix NULL stream dereference in afs_retry_request()

[Li Ximo]

From: Wang Haoran <haoranwangsec at gmail.com>

afs_retry_request() is registered unconditionally as the netfs
.retry_request op and serves both the write and the read retry paths.
The netfs read-side retry path, netfs_retry_read_subrequests()
(fs/netfs/read_retry.c), invokes the op with stream == NULL. However,
afs_retry_request() computes
list_first_entry(&stream->subrequests, ...) *before* it checks
wreq->origin, so a read retry dereferences NULL plus
offsetof(struct netfs_io_stream, subrequests), producing a near-NULL
wild pointer dereference and a kernel crash. The read path is reachable
from userspace through an ordinary AFS file read: a failed fscache read
(source NETFS_READ_FROM_CACHE) or a short/partial server read marks the
read subrequest with NETFS_SREQ_NEED_RETRY (fs/netfs/read_collect.c),
which drives netfs_retry_reads() into netfs_retry_read_subrequests() and
hence into afs_retry_request(rreq, NULL). The existing switch on
wreq->origin already rejects all read origins, but only after the stream
has been dereferenced - too late to help. This issue was found during a
code audit.

Fix this by moving the origin check to the top of the function, before
any use of stream: read requests return early exactly as they do today,
and list_first_entry() is only reached for write requests, whose callers
(netfs_retry_write_stream(), which explicitly skips empty subrequest
lists, and the unbuffered/DIO write path, which holds a live subrequest)
guarantee a valid stream with a non-empty subrequest list. Behaviour on
the write path is unchanged.

Reported-by: Wang Haoran <haoranwangsec at gmail.com>
Reported-by: Li Ximo <lixicaicai at gmail.com>
Signed-off-by: Wang Haoran <haoranwangsec at gmail.com>
---
 fs/afs/write.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/afs/write.c b/fs/afs/write.c
index 7f34b9397..4b4922858 100644
--- a/fs/afs/write.c
+++ b/fs/afs/write.c
@@ -191,10 +191,12 @@ void afs_begin_writeback(struct netfs_io_request *wreq)
  */
 void afs_retry_request(struct netfs_io_request *wreq, struct netfs_io_stream *stream)
 {
-	struct netfs_io_subrequest *subreq =
-		list_first_entry(&stream->subrequests,
-				 struct netfs_io_subrequest, rreq_link);
+	struct netfs_io_subrequest *subreq;
 
+	/*
+	 * The read-side retry path calls this with a NULL stream, so check
+	 * the origin before dereferencing stream.
+	 */
 	switch (wreq->origin) {
 	case NETFS_READAHEAD:
 	case NETFS_READPAGE:
@@ -208,6 +210,9 @@ void afs_retry_request(struct netfs_io_request *wreq, struct netfs_io_stream *st
 		break;
 	}
 
+	subreq = list_first_entry(&stream->subrequests,
+				  struct netfs_io_subrequest, rreq_link);
+
 	switch (subreq->error) {
 	case -EACCES:
 	case -EPERM:
-- 
2.30.2