[PATCH v2 1/2] smb: client: fix create context out-of-bounds reads
Zihan Xi <[email protected]>
| Newsgroups | gmane.linux.kernel.stable,gmane.linux.kernel.cifs,gmane.network.samba.internals,gmane.linux.kernel |
|---|---|
| Message-ID | <[email protected]> |
smb2_parse_contexts() validates the complete create-context area but does
not bound each context record by its Next field before dispatching to a
handler. A malformed chain can therefore expose bytes past one context to
the handler. The QFid handler also used a full response-structure cast even
though it only consumes DiskFileId.
Bound each context by Next and reject malformed chains. Read the QFid
DiskFileId only when the context data covers that field, and do not call the
lease parser unless the record contains every field it reads, including
LeaseFlags.
Fixes: b8c32dbb0deb ("CIFS: Request SMB2.1 leases")
Fixes: 89a5bfa350fa ("smb3: optimize open to not send query file internal info")
Cc: [email protected]
Reported-by: Vega <[email protected]>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zihan Xi <[email protected]>
---
changes in v2:
- Bound each response context by Next and reject malformed chains.
- Read QFid DiskFileId only when DataLength covers the payload.
- Extend the SMB2 lease minimum through the LeaseFlags field.
- Correct Fixes history for the pre-existing Next/lease path and the later QFid path.
- v1 Link: https://lore.kernel.org/all/eb1bc35611f91bd10a4772400b37fac26f660956.1782579150.git.xizh2024@lzu.edu.cn/
---
fs/smb/client/smb2pdu.c | 38 ++++++++++++++++++++++++++++++++------
1 file changed, 32 insertions(+), 6 deletions(-)
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 4ce165e40657..95d862a1241b 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -2375,14 +2375,29 @@ create_reconnect_durable_buf(struct cifs_fid *fid)
return buf;
}
+static size_t smb2_create_lease_min_cc_len(struct TCP_Server_Info *server)
+{
+ if (server->vals->create_lease_size == sizeof(struct create_lease_v2))
+ return offsetof(struct create_lease_v2, lcontext.Epoch) +
+ sizeof(__le16);
+ return offsetof(struct create_lease, lcontext.LeaseFlags) +
+ sizeof(__le32);
+}
+
static void
parse_query_id_ctxt(struct create_context *cc, struct smb2_file_all_info *buf)
{
- struct create_disk_id_rsp *pdisk_id = (struct create_disk_id_rsp *)cc;
+ u16 doff = le16_to_cpu(cc->DataOffset);
+ u32 dlen = le32_to_cpu(cc->DataLength);
+ u8 *beg;
+
+ if (dlen < sizeof(__le64))
+ return;
- cifs_dbg(FYI, "parse query id context 0x%llx 0x%llx\n",
- pdisk_id->DiskFileId, pdisk_id->VolumeId);
- buf->IndexNumber = pdisk_id->DiskFileId;
+ beg = (u8 *)cc + doff;
+ memcpy(&buf->IndexNumber, beg, sizeof(__le64));
+ cifs_dbg(FYI, "parse query id context 0x%llx\n",
+ le64_to_cpu(buf->IndexNumber));
}
static void
@@ -2430,6 +2445,7 @@ int smb2_parse_contexts(struct TCP_Server_Info *server,
struct smb2_create_rsp *rsp = rsp_iov->iov_base;
struct create_context *cc;
size_t rem, off, len;
+ size_t cc_len;
size_t doff, dlen;
size_t noff, nlen;
char *name;
@@ -2452,9 +2468,18 @@ int smb2_parse_contexts(struct TCP_Server_Info *server,
buf->IndexNumber = 0;
while (rem >= sizeof(*cc)) {
+ off = le32_to_cpu(cc->Next);
+ if (off) {
+ if ((off & 0x7) || off > rem || off < sizeof(*cc))
+ return -EINVAL;
+ cc_len = off;
+ } else {
+ cc_len = rem;
+ }
+
doff = le16_to_cpu(cc->DataOffset);
dlen = le32_to_cpu(cc->DataLength);
- if (check_add_overflow(doff, dlen, &len) || len > rem)
+ if (check_add_overflow(doff, dlen, &len) || len > cc_len)
return -EINVAL;
noff = le16_to_cpu(cc->NameOffset);
@@ -2466,7 +2491,8 @@ int smb2_parse_contexts(struct TCP_Server_Info *server,
switch (nlen) {
case 4:
if (!strncmp(name, SMB2_CREATE_REQUEST_LEASE, 4)) {
- *oplock = server->ops->parse_lease_buf(cc, epoch,
+ if (cc_len >= smb2_create_lease_min_cc_len(server))
+ *oplock = server->ops->parse_lease_buf(cc, epoch,
lease_key);
} else if (buf &&
!strncmp(name, SMB2_CREATE_QUERY_ON_DISK_ID, 4)) {