From nobody Mon Sep 29 22:48:17 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D76A0C00140 for ; Mon, 15 Aug 2022 22:22:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346270AbiHOWWQ (ORCPT ); Mon, 15 Aug 2022 18:22:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57468 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1350444AbiHOWRu (ORCPT ); Mon, 15 Aug 2022 18:17:50 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BC3D5109A33; Mon, 15 Aug 2022 12:41:00 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 59098610A3; Mon, 15 Aug 2022 19:40:52 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 28B08C433C1; Mon, 15 Aug 2022 19:40:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1660592451; bh=EX2MJfbCX1AGJaX8enALDFqcrPjd3k8EjiGL/KAlHlg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=cjZHdIEMLOE/VWqbFRi/lc4KnR3hPtcDh+py3kMNxs936h5MwqwmBxKR0qwnSPN/A k+nSbAs2rBUvkvQONIkIlAZCdFMfKC/kIDfjt6USgLqKepbBzqdgwRglVdk3CK0E+S +7Ca+YcFxc/byM2MS89Zt0658dP48X/TwlEIC8bc= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Hyunchul Lee , Namjae Jeon , Steve French , zdi-disclosures@trendmicro.com Subject: [PATCH 5.19 0112/1157] ksmbd: fix heap-based overflow in set_ntacl_dacl() Date: Mon, 15 Aug 2022 19:51:08 +0200 Message-Id: <20220815180444.118313318@linuxfoundation.org> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20220815180439.416659447@linuxfoundation.org> References: <20220815180439.416659447@linuxfoundation.org> User-Agent: quilt/0.67 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" From: Namjae Jeon commit 8f0541186e9ad1b62accc9519cc2b7a7240272a7 upstream. The testcase use SMB2_SET_INFO_HE command to set a malformed file attribute under the label `security.NTACL`. SMB2_QUERY_INFO_HE command in testcase trigger the following overflow. [ 4712.003781] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D [ 4712.003790] BUG: KASAN: slab-out-of-bounds in build_sec_desc+0x842/0x1dd= 0 [ksmbd] [ 4712.003807] Write of size 1060 at addr ffff88801e34c068 by task kworker/= 0:0/4190 [ 4712.003813] CPU: 0 PID: 4190 Comm: kworker/0:0 Not tainted 5.19.0-rc5 #1 [ 4712.003850] Workqueue: ksmbd-io handle_ksmbd_work [ksmbd] [ 4712.003867] Call Trace: [ 4712.003870] [ 4712.003873] dump_stack_lvl+0x49/0x5f [ 4712.003935] print_report.cold+0x5e/0x5cf [ 4712.003972] ? ksmbd_vfs_get_sd_xattr+0x16d/0x500 [ksmbd] [ 4712.003984] ? cmp_map_id+0x200/0x200 [ 4712.003988] ? build_sec_desc+0x842/0x1dd0 [ksmbd] [ 4712.004000] kasan_report+0xaa/0x120 [ 4712.004045] ? build_sec_desc+0x842/0x1dd0 [ksmbd] [ 4712.004056] kasan_check_range+0x100/0x1e0 [ 4712.004060] memcpy+0x3c/0x60 [ 4712.004064] build_sec_desc+0x842/0x1dd0 [ksmbd] [ 4712.004076] ? parse_sec_desc+0x580/0x580 [ksmbd] [ 4712.004088] ? ksmbd_acls_fattr+0x281/0x410 [ksmbd] [ 4712.004099] smb2_query_info+0xa8f/0x6110 [ksmbd] [ 4712.004111] ? psi_group_change+0x856/0xd70 [ 4712.004148] ? update_load_avg+0x1c3/0x1af0 [ 4712.004152] ? asym_cpu_capacity_scan+0x5d0/0x5d0 [ 4712.004157] ? xas_load+0x23/0x300 [ 4712.004162] ? smb2_query_dir+0x1530/0x1530 [ksmbd] [ 4712.004173] ? _raw_spin_lock_bh+0xe0/0xe0 [ 4712.004179] handle_ksmbd_work+0x30e/0x1020 [ksmbd] [ 4712.004192] process_one_work+0x778/0x11c0 [ 4712.004227] ? _raw_spin_lock_irq+0x8e/0xe0 [ 4712.004231] worker_thread+0x544/0x1180 [ 4712.004234] ? __cpuidle_text_end+0x4/0x4 [ 4712.004239] kthread+0x282/0x320 [ 4712.004243] ? process_one_work+0x11c0/0x11c0 [ 4712.004246] ? kthread_complete_and_exit+0x30/0x30 [ 4712.004282] ret_from_fork+0x1f/0x30 This patch add the buffer validation for security descriptor that is stored by malformed SMB2_SET_INFO_HE command. and allocate large response buffer about SMB2_O_INFO_SECURITY file info class. Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-17771 Reviewed-by: Hyunchul Lee Signed-off-by: Namjae Jeon Signed-off-by: Steve French Signed-off-by: Greg Kroah-Hartman --- fs/ksmbd/smb2pdu.c | 39 ++++++++++----- fs/ksmbd/smbacl.c | 130 +++++++++++++++++++++++++++++++++++-------------= ----- fs/ksmbd/smbacl.h | 2=20 fs/ksmbd/vfs.c | 5 ++ 4 files changed, 119 insertions(+), 57 deletions(-) --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -535,9 +535,10 @@ int smb2_allocate_rsp_buf(struct ksmbd_w struct smb2_query_info_req *req; =20 req =3D smb2_get_msg(work->request_buf); - if (req->InfoType =3D=3D SMB2_O_INFO_FILE && - (req->FileInfoClass =3D=3D FILE_FULL_EA_INFORMATION || - req->FileInfoClass =3D=3D FILE_ALL_INFORMATION)) + if ((req->InfoType =3D=3D SMB2_O_INFO_FILE && + (req->FileInfoClass =3D=3D FILE_FULL_EA_INFORMATION || + req->FileInfoClass =3D=3D FILE_ALL_INFORMATION)) || + req->InfoType =3D=3D SMB2_O_INFO_SECURITY) sz =3D large_sz; } =20 @@ -2974,7 +2975,7 @@ int smb2_open(struct ksmbd_work *work) goto err_out; =20 rc =3D build_sec_desc(user_ns, - pntsd, NULL, + pntsd, NULL, 0, OWNER_SECINFO | GROUP_SECINFO | DACL_SECINFO, @@ -3819,6 +3820,15 @@ static int verify_info_level(int info_le return 0; } =20 +static int smb2_resp_buf_len(struct ksmbd_work *work, unsigned short hdr2_= len) +{ + int free_len; + + free_len =3D (int)(work->response_sz - + (get_rfc1002_len(work->response_buf) + 4)) - hdr2_len; + return free_len; +} + static int smb2_calc_max_out_buf_len(struct ksmbd_work *work, unsigned short hdr2_len, unsigned int out_buf_len) @@ -3828,9 +3838,7 @@ static int smb2_calc_max_out_buf_len(str if (out_buf_len > work->conn->vals->max_trans_size) return -EINVAL; =20 - free_len =3D (int)(work->response_sz - - (get_rfc1002_len(work->response_buf) + 4)) - - hdr2_len; + free_len =3D smb2_resp_buf_len(work, hdr2_len); if (free_len < 0) return -EINVAL; =20 @@ -5093,10 +5101,10 @@ static int smb2_get_info_sec(struct ksmb struct smb_ntsd *pntsd =3D (struct smb_ntsd *)rsp->Buffer, *ppntsd =3D NU= LL; struct smb_fattr fattr =3D {{0}}; struct inode *inode; - __u32 secdesclen; + __u32 secdesclen =3D 0; unsigned int id =3D KSMBD_NO_FID, pid =3D KSMBD_NO_FID; int addition_info =3D le32_to_cpu(req->AdditionalInformation); - int rc; + int rc =3D 0, ppntsd_size =3D 0; =20 if (addition_info & ~(OWNER_SECINFO | GROUP_SECINFO | DACL_SECINFO | PROTECTED_DACL_SECINFO | @@ -5142,11 +5150,14 @@ static int smb2_get_info_sec(struct ksmb =20 if (test_share_config_flag(work->tcon->share_conf, KSMBD_SHARE_FLAG_ACL_XATTR)) - ksmbd_vfs_get_sd_xattr(work->conn, user_ns, - fp->filp->f_path.dentry, &ppntsd); - - rc =3D build_sec_desc(user_ns, pntsd, ppntsd, addition_info, - &secdesclen, &fattr); + ppntsd_size =3D ksmbd_vfs_get_sd_xattr(work->conn, user_ns, + fp->filp->f_path.dentry, + &ppntsd); + + /* Check if sd buffer size exceeds response buffer size */ + if (smb2_resp_buf_len(work, 8) > ppntsd_size) + rc =3D build_sec_desc(user_ns, pntsd, ppntsd, ppntsd_size, + addition_info, &secdesclen, &fattr); posix_acl_release(fattr.cf_acls); posix_acl_release(fattr.cf_dacls); kfree(ppntsd); --- a/fs/ksmbd/smbacl.c +++ b/fs/ksmbd/smbacl.c @@ -690,6 +690,7 @@ posix_default_acl: static void set_ntacl_dacl(struct user_namespace *user_ns, struct smb_acl *pndacl, struct smb_acl *nt_dacl, + unsigned int aces_size, const struct smb_sid *pownersid, const struct smb_sid *pgrpsid, struct smb_fattr *fattr) @@ -703,9 +704,19 @@ static void set_ntacl_dacl(struct user_n if (nt_num_aces) { ntace =3D (struct smb_ace *)((char *)nt_dacl + sizeof(struct smb_acl)); for (i =3D 0; i < nt_num_aces; i++) { - memcpy((char *)pndace + size, ntace, le16_to_cpu(ntace->size)); - size +=3D le16_to_cpu(ntace->size); - ntace =3D (struct smb_ace *)((char *)ntace + le16_to_cpu(ntace->size)); + unsigned short nt_ace_size; + + if (offsetof(struct smb_ace, access_req) > aces_size) + break; + + nt_ace_size =3D le16_to_cpu(ntace->size); + if (nt_ace_size > aces_size) + break; + + memcpy((char *)pndace + size, ntace, nt_ace_size); + size +=3D nt_ace_size; + aces_size -=3D nt_ace_size; + ntace =3D (struct smb_ace *)((char *)ntace + nt_ace_size); num_aces++; } } @@ -878,7 +889,7 @@ int parse_sec_desc(struct user_namespace /* Convert permission bits from mode to equivalent CIFS ACL */ int build_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd, struct smb_ntsd *ppntsd, - int addition_info, __u32 *secdesclen, + int ppntsd_size, int addition_info, __u32 *secdesclen, struct smb_fattr *fattr) { int rc =3D 0; @@ -938,15 +949,25 @@ int build_sec_desc(struct user_namespace =20 if (!ppntsd) { set_mode_dacl(user_ns, dacl_ptr, fattr); - } else if (!ppntsd->dacloffset) { - goto out; } else { struct smb_acl *ppdacl_ptr; + unsigned int dacl_offset =3D le32_to_cpu(ppntsd->dacloffset); + int ppdacl_size, ntacl_size =3D ppntsd_size - dacl_offset; + + if (!dacl_offset || + (dacl_offset + sizeof(struct smb_acl) > ppntsd_size)) + goto out; + + ppdacl_ptr =3D (struct smb_acl *)((char *)ppntsd + dacl_offset); + ppdacl_size =3D le16_to_cpu(ppdacl_ptr->size); + if (ppdacl_size > ntacl_size || + ppdacl_size < sizeof(struct smb_acl)) + goto out; =20 - ppdacl_ptr =3D (struct smb_acl *)((char *)ppntsd + - le32_to_cpu(ppntsd->dacloffset)); set_ntacl_dacl(user_ns, dacl_ptr, ppdacl_ptr, - nowner_sid_ptr, ngroup_sid_ptr, fattr); + ntacl_size - sizeof(struct smb_acl), + nowner_sid_ptr, ngroup_sid_ptr, + fattr); } pntsd->dacloffset =3D cpu_to_le32(offset); offset +=3D le16_to_cpu(dacl_ptr->size); @@ -980,24 +1001,31 @@ int smb_inherit_dacl(struct ksmbd_conn * struct smb_sid owner_sid, group_sid; struct dentry *parent =3D path->dentry->d_parent; struct user_namespace *user_ns =3D mnt_user_ns(path->mnt); - int inherited_flags =3D 0, flags =3D 0, i, ace_cnt =3D 0, nt_size =3D 0; - int rc =3D 0, num_aces, dacloffset, pntsd_type, acl_len; + int inherited_flags =3D 0, flags =3D 0, i, ace_cnt =3D 0, nt_size =3D 0, = pdacl_size; + int rc =3D 0, num_aces, dacloffset, pntsd_type, pntsd_size, acl_len, aces= _size; char *aces_base; bool is_dir =3D S_ISDIR(d_inode(path->dentry)->i_mode); =20 - acl_len =3D ksmbd_vfs_get_sd_xattr(conn, user_ns, - parent, &parent_pntsd); - if (acl_len <=3D 0) + pntsd_size =3D ksmbd_vfs_get_sd_xattr(conn, user_ns, + parent, &parent_pntsd); + if (pntsd_size <=3D 0) return -ENOENT; dacloffset =3D le32_to_cpu(parent_pntsd->dacloffset); - if (!dacloffset) { + if (!dacloffset || (dacloffset + sizeof(struct smb_acl) > pntsd_size)) { rc =3D -EINVAL; goto free_parent_pntsd; } =20 parent_pdacl =3D (struct smb_acl *)((char *)parent_pntsd + dacloffset); + acl_len =3D pntsd_size - dacloffset; num_aces =3D le32_to_cpu(parent_pdacl->num_aces); pntsd_type =3D le16_to_cpu(parent_pntsd->type); + pdacl_size =3D le16_to_cpu(parent_pdacl->size); + + if (pdacl_size > acl_len || pdacl_size < sizeof(struct smb_acl)) { + rc =3D -EINVAL; + goto free_parent_pntsd; + } =20 aces_base =3D kmalloc(sizeof(struct smb_ace) * num_aces * 2, GFP_KERNEL); if (!aces_base) { @@ -1008,11 +1036,23 @@ int smb_inherit_dacl(struct ksmbd_conn * aces =3D (struct smb_ace *)aces_base; parent_aces =3D (struct smb_ace *)((char *)parent_pdacl + sizeof(struct smb_acl)); + aces_size =3D acl_len - sizeof(struct smb_acl); =20 if (pntsd_type & DACL_AUTO_INHERITED) inherited_flags =3D INHERITED_ACE; =20 for (i =3D 0; i < num_aces; i++) { + int pace_size; + + if (offsetof(struct smb_ace, access_req) > aces_size) + break; + + pace_size =3D le16_to_cpu(parent_aces->size); + if (pace_size > aces_size) + break; + + aces_size -=3D pace_size; + flags =3D parent_aces->flags; if (!smb_inherit_flags(flags, is_dir)) goto pass; @@ -1057,8 +1097,7 @@ int smb_inherit_dacl(struct ksmbd_conn * aces =3D (struct smb_ace *)((char *)aces + le16_to_cpu(aces->size)); ace_cnt++; pass: - parent_aces =3D - (struct smb_ace *)((char *)parent_aces + le16_to_cpu(parent_aces->size)= ); + parent_aces =3D (struct smb_ace *)((char *)parent_aces + pace_size); } =20 if (nt_size > 0) { @@ -1153,7 +1192,7 @@ int smb_check_perm_dacl(struct ksmbd_con struct smb_ntsd *pntsd =3D NULL; struct smb_acl *pdacl; struct posix_acl *posix_acls; - int rc =3D 0, acl_size; + int rc =3D 0, pntsd_size, acl_size, aces_size, pdacl_size, dacl_offset; struct smb_sid sid; int granted =3D le32_to_cpu(*pdaccess & ~FILE_MAXIMAL_ACCESS_LE); struct smb_ace *ace; @@ -1162,37 +1201,33 @@ int smb_check_perm_dacl(struct ksmbd_con struct smb_ace *others_ace =3D NULL; struct posix_acl_entry *pa_entry; unsigned int sid_type =3D SIDOWNER; - char *end_of_acl; + unsigned short ace_size; =20 ksmbd_debug(SMB, "check permission using windows acl\n"); - acl_size =3D ksmbd_vfs_get_sd_xattr(conn, user_ns, - path->dentry, &pntsd); - if (acl_size <=3D 0 || !pntsd || !pntsd->dacloffset) { - kfree(pntsd); - return 0; - } + pntsd_size =3D ksmbd_vfs_get_sd_xattr(conn, user_ns, + path->dentry, &pntsd); + if (pntsd_size <=3D 0 || !pntsd) + goto err_out; + + dacl_offset =3D le32_to_cpu(pntsd->dacloffset); + if (!dacl_offset || + (dacl_offset + sizeof(struct smb_acl) > pntsd_size)) + goto err_out; =20 pdacl =3D (struct smb_acl *)((char *)pntsd + le32_to_cpu(pntsd->dacloffse= t)); - end_of_acl =3D ((char *)pntsd) + acl_size; - if (end_of_acl <=3D (char *)pdacl) { - kfree(pntsd); - return 0; - } + acl_size =3D pntsd_size - dacl_offset; + pdacl_size =3D le16_to_cpu(pdacl->size); =20 - if (end_of_acl < (char *)pdacl + le16_to_cpu(pdacl->size) || - le16_to_cpu(pdacl->size) < sizeof(struct smb_acl)) { - kfree(pntsd); - return 0; - } + if (pdacl_size > acl_size || pdacl_size < sizeof(struct smb_acl)) + goto err_out; =20 if (!pdacl->num_aces) { - if (!(le16_to_cpu(pdacl->size) - sizeof(struct smb_acl)) && + if (!(pdacl_size - sizeof(struct smb_acl)) && *pdaccess & ~(FILE_READ_CONTROL_LE | FILE_WRITE_DAC_LE)) { rc =3D -EACCES; goto err_out; } - kfree(pntsd); - return 0; + goto err_out; } =20 if (*pdaccess & FILE_MAXIMAL_ACCESS_LE) { @@ -1200,11 +1235,16 @@ int smb_check_perm_dacl(struct ksmbd_con DELETE; =20 ace =3D (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl)); + aces_size =3D acl_size - sizeof(struct smb_acl); for (i =3D 0; i < le32_to_cpu(pdacl->num_aces); i++) { + if (offsetof(struct smb_ace, access_req) > aces_size) + break; + ace_size =3D le16_to_cpu(ace->size); + if (ace_size > aces_size) + break; + aces_size -=3D ace_size; granted |=3D le32_to_cpu(ace->access_req); ace =3D (struct smb_ace *)((char *)ace + le16_to_cpu(ace->size)); - if (end_of_acl < (char *)ace) - goto err_out; } =20 if (!pdacl->num_aces) @@ -1216,7 +1256,15 @@ int smb_check_perm_dacl(struct ksmbd_con id_to_sid(uid, sid_type, &sid); =20 ace =3D (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl)); + aces_size =3D acl_size - sizeof(struct smb_acl); for (i =3D 0; i < le32_to_cpu(pdacl->num_aces); i++) { + if (offsetof(struct smb_ace, access_req) > aces_size) + break; + ace_size =3D le16_to_cpu(ace->size); + if (ace_size > aces_size) + break; + aces_size -=3D ace_size; + if (!compare_sids(&sid, &ace->sid) || !compare_sids(&sid_unix_NFS_mode, &ace->sid)) { found =3D 1; @@ -1226,8 +1274,6 @@ int smb_check_perm_dacl(struct ksmbd_con others_ace =3D ace; =20 ace =3D (struct smb_ace *)((char *)ace + le16_to_cpu(ace->size)); - if (end_of_acl < (char *)ace) - goto err_out; } =20 if (*pdaccess & FILE_MAXIMAL_ACCESS_LE && found) { --- a/fs/ksmbd/smbacl.h +++ b/fs/ksmbd/smbacl.h @@ -193,7 +193,7 @@ struct posix_acl_state { int parse_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd, int acl_len, struct smb_fattr *fattr); int build_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd, - struct smb_ntsd *ppntsd, int addition_info, + struct smb_ntsd *ppntsd, int ppntsd_size, int addition_info, __u32 *secdesclen, struct smb_fattr *fattr); int init_acl_state(struct posix_acl_state *state, int cnt); void free_acl_state(struct posix_acl_state *state); --- a/fs/ksmbd/vfs.c +++ b/fs/ksmbd/vfs.c @@ -1540,6 +1540,11 @@ int ksmbd_vfs_get_sd_xattr(struct ksmbd_ } =20 *pntsd =3D acl.sd_buf; + if (acl.sd_size < sizeof(struct smb_ntsd)) { + pr_err("sd size is invalid\n"); + goto out_free; + } + (*pntsd)->osidoffset =3D cpu_to_le32(le32_to_cpu((*pntsd)->osidoffset) - NDR_NTSD_OFFSETOF); (*pntsd)->gsidoffset =3D cpu_to_le32(le32_to_cpu((*pntsd)->gsidoffset) -