Skip to content

Commit fab81c5

Browse files
manizadanamjaejeon
authored andcommitted
ksmbd: fix heap OOB write in QUERY_INFO(Security) for compound requests
When a compound request such as READ + QUERY_INFO(Security) is received, and the first command (READ) consumes most of the response buffer, ksmbd could write beyond the allocated buffer while building a security descriptor. The root cause was that smb2_get_info_sec() checked buffer space using ppntsd_size from xattr, while build_sec_desc() often synthesized a significantly larger descriptor from POSIX ACLs. This patch introduces smb_acl_sec_desc_scratch_len() to accurately compute the final descriptor size beforehand, performs proper buffer checking with smb2_calc_max_out_buf_len(), and uses exact-sized allocation + iov pinning. Signed-off-by: Asim Viladi Oglu Manizada <manizada@pm.me> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
1 parent fcca011 commit fab81c5

3 files changed

Lines changed: 88 additions & 30 deletions

File tree

smb2pdu.c

Lines changed: 56 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3539,23 +3539,20 @@ int smb2_open(struct ksmbd_work *work)
35393539
KSMBD_SHARE_FLAG_ACL_XATTR)) {
35403540
struct smb_fattr fattr;
35413541
struct smb_ntsd *pntsd;
3542-
int pntsd_size, ace_num = 0;
3542+
int pntsd_size;
3543+
size_t scratch_len;
35433544

35443545
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 3, 0)
35453546
ksmbd_acls_fattr(&fattr, idmap, inode);
35463547
#else
35473548
ksmbd_acls_fattr(&fattr, user_ns, inode);
35483549
#endif
3549-
if (fattr.cf_acls)
3550-
ace_num = fattr.cf_acls->a_count;
3551-
if (fattr.cf_dacls)
3552-
ace_num += fattr.cf_dacls->a_count;
3553-
3554-
pntsd = kmalloc(sizeof(struct smb_ntsd) +
3555-
sizeof(struct smb_sid) * 3 +
3556-
sizeof(struct smb_acl) +
3557-
sizeof(struct smb_ace) * ace_num * 2,
3558-
KSMBD_DEFAULT_GFP);
3550+
scratch_len = smb_acl_sec_desc_scratch_len(&fattr,
3551+
NULL, 0,
3552+
OWNER_SECINFO | GROUP_SECINFO |
3553+
DACL_SECINFO);
3554+
3555+
pntsd = kvmalloc(scratch_len, KSMBD_DEFAULT_GFP);
35593556
if (!pntsd) {
35603557
posix_acl_release(fattr.cf_acls);
35613558
posix_acl_release(fattr.cf_dacls);
@@ -3575,7 +3572,7 @@ int smb2_open(struct ksmbd_work *work)
35753572
posix_acl_release(fattr.cf_acls);
35763573
posix_acl_release(fattr.cf_dacls);
35773574
if (rc) {
3578-
kfree(pntsd);
3575+
kvfree(pntsd);
35793576
goto err_out;
35803577
}
35813578

@@ -3589,7 +3586,7 @@ int smb2_open(struct ksmbd_work *work)
35893586
pntsd,
35903587
pntsd_size,
35913588
false);
3592-
kfree(pntsd);
3589+
kvfree(pntsd);
35933590
if (rc)
35943591
pr_err("failed to store ntacl in xattr : %d\n",
35953592
rc);
@@ -5734,6 +5731,11 @@ static int smb2_get_info_file(struct ksmbd_work *work,
57345731
rc = buffer_check_err(le32_to_cpu(req->OutputBufferLength),
57355732
rsp, work->response_buf);
57365733
ksmbd_fd_put(work, fp);
5734+
5735+
if (!rc)
5736+
rc = ksmbd_iov_pin_rsp(work, (void *)rsp,
5737+
offsetof(struct smb2_query_info_rsp, Buffer) +
5738+
le32_to_cpu(rsp->OutputBufferLength));
57375739
return rc;
57385740
}
57395741

@@ -5953,6 +5955,11 @@ static int smb2_get_info_filesystem(struct ksmbd_work *work,
59535955
rc = buffer_check_err(le32_to_cpu(req->OutputBufferLength),
59545956
rsp, work->response_buf);
59555957
path_put(&path);
5958+
5959+
if (!rc)
5960+
rc = ksmbd_iov_pin_rsp(work, (void *)rsp,
5961+
offsetof(struct smb2_query_info_rsp, Buffer) +
5962+
le32_to_cpu(rsp->OutputBufferLength));
59565963
return rc;
59575964
}
59585965

@@ -5966,13 +5973,14 @@ static int smb2_get_info_sec(struct ksmbd_work *work,
59665973
#else
59675974
struct user_namespace *user_ns;
59685975
#endif
5969-
struct smb_ntsd *pntsd = (struct smb_ntsd *)rsp->Buffer, *ppntsd = NULL;
5976+
struct smb_ntsd *pntsd, *ppntsd = NULL;
59705977
struct smb_fattr fattr = {{0}};
59715978
struct inode *inode;
59725979
__u32 secdesclen = 0;
59735980
unsigned int id = KSMBD_NO_FID, pid = KSMBD_NO_FID;
59745981
int addition_info = le32_to_cpu(req->AdditionalInformation);
5975-
int rc = 0, ppntsd_size = 0;
5982+
int rc = 0, ppntsd_size = 0, max_len;
5983+
size_t scratch_len;
59765984

59775985
if (addition_info & ~(OWNER_SECINFO | GROUP_SECINFO | DACL_SECINFO |
59785986
PROTECTED_DACL_SECINFO |
@@ -6033,23 +6041,44 @@ static int smb2_get_info_sec(struct ksmbd_work *work,
60336041
fp->filp->f_path.dentry,
60346042
&ppntsd);
60356043

6036-
/* Check if sd buffer size exceeds response buffer size */
6037-
if (smb2_resp_buf_len(work, 8) > ppntsd_size)
6044+
max_len = smb2_calc_max_out_buf_len(work,
6045+
offsetof(struct smb2_query_info_rsp, Buffer),
6046+
le32_to_cpu(req->OutputBufferLength));
6047+
if (max_len < 0) {
6048+
rc = -EINVAL;
6049+
goto out;
6050+
}
6051+
6052+
scratch_len = smb_acl_sec_desc_scratch_len(&fattr, ppntsd,
6053+
ppntsd_size, addition_info);
6054+
pntsd = kvmalloc(scratch_len, KSMBD_DEFAULT_GFP);
6055+
if (!pntsd) {
6056+
rc = -ENOMEM;
6057+
goto out;
6058+
}
6059+
60386060
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 3, 0)
6039-
rc = build_sec_desc(idmap, pntsd, ppntsd, ppntsd_size,
6061+
rc = build_sec_desc(idmap, pntsd, ppntsd, ppntsd_size,
6062+
addition_info, &secdesclen, &fattr);
60406063
#else
6041-
rc = build_sec_desc(user_ns, pntsd, ppntsd, ppntsd_size,
6064+
rc = build_sec_desc(user_ns, pntsd, ppntsd, ppntsd_size,
6065+
addition_info, &secdesclen, &fattr);
60426066
#endif
6043-
addition_info, &secdesclen, &fattr);
6067+
6068+
out:
60446069
posix_acl_release(fattr.cf_acls);
60456070
posix_acl_release(fattr.cf_dacls);
60466071
kfree(ppntsd);
60476072
ksmbd_fd_put(work, fp);
6048-
if (rc)
6073+
if (rc) {
6074+
kvfree(pntsd);
60496075
return rc;
6076+
}
60506077

60516078
rsp->OutputBufferLength = cpu_to_le32(secdesclen);
6052-
return 0;
6079+
return ksmbd_iov_pin_rsp_read(work, (void *)rsp,
6080+
offsetof(struct smb2_query_info_rsp, Buffer),
6081+
pntsd, secdesclen);
60536082
}
60546083

60556084
/**
@@ -6073,6 +6102,9 @@ int smb2_query_info(struct ksmbd_work *work)
60736102
goto err_out;
60746103
}
60756104

6105+
rsp->StructureSize = cpu_to_le16(9);
6106+
rsp->OutputBufferOffset = cpu_to_le16(72);
6107+
60766108
switch (req->InfoType) {
60776109
case SMB2_O_INFO_FILE:
60786110
ksmbd_debug(SMB, "GOT SMB2_O_INFO_FILE\n");
@@ -6093,14 +6125,6 @@ int smb2_query_info(struct ksmbd_work *work)
60936125
}
60946126
ksmbd_revert_fsids(work);
60956127

6096-
if (!rc) {
6097-
rsp->StructureSize = cpu_to_le16(9);
6098-
rsp->OutputBufferOffset = cpu_to_le16(72);
6099-
rc = ksmbd_iov_pin_rsp(work, (void *)rsp,
6100-
offsetof(struct smb2_query_info_rsp, Buffer) +
6101-
le32_to_cpu(rsp->OutputBufferLength));
6102-
}
6103-
61046128
err_out:
61056129
if (rc < 0) {
61066130
if (rc == -EACCES)
@@ -6111,6 +6135,8 @@ int smb2_query_info(struct ksmbd_work *work)
61116135
rsp->hdr.Status = STATUS_UNEXPECTED_IO_ERROR;
61126136
else if (rc == -ENOMEM)
61136137
rsp->hdr.Status = STATUS_INSUFFICIENT_RESOURCES;
6138+
else if (rc == -EINVAL)
6139+
rsp->hdr.Status = STATUS_INVALID_PARAMETER;
61146140
else if (rc == -EOPNOTSUPP || rsp->hdr.Status == 0)
61156141
rsp->hdr.Status = STATUS_INVALID_INFO_CLASS;
61166142
smb2_set_err_rsp(work);

smbacl.c

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,6 +1029,36 @@ int parse_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd,
10291029
return 0;
10301030
}
10311031

1032+
size_t smb_acl_sec_desc_scratch_len(struct smb_fattr *fattr,
1033+
struct smb_ntsd *ppntsd, int ppntsd_size, int addition_info)
1034+
{
1035+
size_t len = sizeof(struct smb_ntsd);
1036+
1037+
if (addition_info & OWNER_SECINFO)
1038+
len += sizeof(struct smb_sid);
1039+
if (addition_info & GROUP_SECINFO)
1040+
len += sizeof(struct smb_sid);
1041+
if (!(addition_info & DACL_SECINFO))
1042+
return len;
1043+
1044+
len += sizeof(struct smb_acl);
1045+
if (ppntsd && ppntsd_size > 0) {
1046+
unsigned int dacl_offset = le32_to_cpu(ppntsd->dacloffset);
1047+
1048+
if (dacl_offset < ppntsd_size)
1049+
len += ppntsd_size - dacl_offset;
1050+
}
1051+
1052+
if (fattr->cf_acls)
1053+
len += (size_t)fattr->cf_acls->a_count * 2 * sizeof(struct smb_ace);
1054+
else
1055+
len += 5 * sizeof(struct smb_ace);
1056+
1057+
if (fattr->cf_dacls)
1058+
len += (size_t)fattr->cf_dacls->a_count * sizeof(struct smb_ace);
1059+
return len;
1060+
}
1061+
10321062
/* Convert permission bits from mode to equivalent CIFS ACL */
10331063
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 3, 0)
10341064
int build_sec_desc(struct mnt_idmap *idmap,

smbacl.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,8 @@ int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
222222
bool type_check, bool get_write);
223223
void id_to_sid(unsigned int cid, uint sidtype, struct smb_sid *ssid);
224224
void ksmbd_init_domain(u32 *sub_auth);
225+
size_t smb_acl_sec_desc_scratch_len(struct smb_fattr *fattr,
226+
struct smb_ntsd *ppntsd, int ppntsd_size, int addition_info);
225227

226228
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 3, 0)
227229
static inline uid_t posix_acl_uid_translate(struct mnt_idmap *idmap,

0 commit comments

Comments
 (0)