[PATCH] ceph: fix NULL dereference of filelock_reply in ceph_lock_message()

Farhad Alemi posted 1 patch 2 weeks ago
[PATCH] ceph: fix NULL dereference of filelock_reply in ceph_lock_message()
Posted by Farhad Alemi 2 weeks ago
ceph_lock_message() dereferences req->r_reply_info.filelock_reply
unconditionally once a CEPH_MDS_OP_GETFILELOCK request completes
successfully, but a reply with a zero-length extra section leaves it NULL,
because parse_reply_info() calls parse_reply_info_extra() only when that
section is non-empty.  parse_reply_info_extra() also selects the parser
from info->head->op, the opcode the MDS echoes back rather than the one the
client sent, so a GETFILELOCK reply naming READDIR, LSSNAP, CREATE or
GETVXATTR is parsed as that op and leaves filelock_reply either NULL or,
since the extra results are a union, aliased onto the member that parser
wrote.  Reject a successful GETFILELOCK reply that left filelock_reply
NULL.  Select the parser from req->r_op instead, since only the op the
client sent identifies the live member of that union.

Closes: https://lore.kernel.org/all/CA+0ovCgZoX5yG35yvU1S2D0Fc35TYeEZKAFUnO-tFMqeXubfdw@mail.gmail.com/
Signed-off-by: Farhad Alemi <farhad.alemi@berkeley.edu>
---
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -822,7 +822,8 @@ static int parse_reply_info_extra(void **p, void *end,
 				  u64 features, struct ceph_mds_session *s)
 {
 	struct ceph_mds_reply_info_parsed *info = &req->r_reply_info;
-	u32 op = le32_to_cpu(info->head->op);
+	/* The extra section is a union; only our own op names the live member. */
+	u32 op = req->r_op;

 	if (op == CEPH_MDS_OP_GETFILELOCK)
 		return parse_reply_info_filelock(p, end, info, features);
@@ -872,6 +873,11 @@ static int parse_reply_info(struct
ceph_mds_session *s, struct ceph_msg *msg,
 			goto out_bad;
 	}

+	/* ceph_lock_message() dereferences filelock_reply on a success reply. */
+	if (req->r_op == CEPH_MDS_OP_GETFILELOCK &&
+	    !le32_to_cpu(info->head->result) && !info->filelock_reply)
+		goto bad;
+
 	/* snap blob */
 	ceph_decode_32_safe(&p, end, len, bad);
 	info->snapblob_len = len;