2022-08-04 23:33 GMT+09:00, Dan Carpenter <dan.carpenter@xxxxxxxxxx>: > Hello Namjae Jeon, Hi Dan, > > The patch 982979772f2b: "ksmbd: fix heap-based overflow in > set_ntacl_dacl()" from Jul 28, 2022, leads to the following Smatch > static checker warning: > > fs/ksmbd/smb2pdu.c:5182 smb2_get_info_sec() > error: uninitialized symbol 'secdesclen'. This was fixed on v4 patch : https://marc.info/?l=linux-cifs&m=165939383203779&w=2 Thanks for your report! > > fs/ksmbd/smb2pdu.c > 5109 static int smb2_get_info_sec(struct ksmbd_work *work, > 5110 struct smb2_query_info_req *req, > 5111 struct smb2_query_info_rsp *rsp) > 5112 { > 5113 struct ksmbd_file *fp; > 5114 struct user_namespace *user_ns; > 5115 struct smb_ntsd *pntsd = (struct smb_ntsd *)rsp->Buffer, > *ppntsd = NULL; > 5116 struct smb_fattr fattr = {{0}}; > 5117 struct inode *inode; > 5118 __u32 secdesclen; > 5119 unsigned int id = KSMBD_NO_FID, pid = KSMBD_NO_FID; > 5120 int addition_info = > le32_to_cpu(req->AdditionalInformation); > 5121 int rc = 0, ppntsd_size = 0; > 5122 > 5123 if (addition_info & ~(OWNER_SECINFO | GROUP_SECINFO | > DACL_SECINFO | > 5124 PROTECTED_DACL_SECINFO | > 5125 UNPROTECTED_DACL_SECINFO)) { > 5126 ksmbd_debug(SMB, "Unsupported addition info: > 0x%x)\n", > 5127 addition_info); > 5128 > 5129 pntsd->revision = cpu_to_le16(1); > 5130 pntsd->type = cpu_to_le16(SELF_RELATIVE | > DACL_PROTECTED); > 5131 pntsd->osidoffset = 0; > 5132 pntsd->gsidoffset = 0; > 5133 pntsd->sacloffset = 0; > 5134 pntsd->dacloffset = 0; > 5135 > 5136 secdesclen = sizeof(struct smb_ntsd); > 5137 rsp->OutputBufferLength = cpu_to_le32(secdesclen); > 5138 inc_rfc1001_len(work->response_buf, secdesclen); > 5139 > 5140 return 0; > 5141 } > 5142 > 5143 if (work->next_smb2_rcv_hdr_off) { > 5144 if (!has_file_id(req->VolatileFileId)) { > 5145 ksmbd_debug(SMB, "Compound request set FID > = %llu\n", > 5146 work->compound_fid); > 5147 id = work->compound_fid; > 5148 pid = work->compound_pfid; > 5149 } > 5150 } > 5151 > 5152 if (!has_file_id(id)) { > 5153 id = req->VolatileFileId; > 5154 pid = req->PersistentFileId; > 5155 } > 5156 > 5157 fp = ksmbd_lookup_fd_slow(work, id, pid); > 5158 if (!fp) > 5159 return -ENOENT; > 5160 > 5161 user_ns = file_mnt_user_ns(fp->filp); > 5162 inode = file_inode(fp->filp); > 5163 ksmbd_acls_fattr(&fattr, user_ns, inode); > 5164 > 5165 if (test_share_config_flag(work->tcon->share_conf, > 5166 KSMBD_SHARE_FLAG_ACL_XATTR)) > 5167 ppntsd_size = ksmbd_vfs_get_sd_xattr(work->conn, > user_ns, > 5168 > fp->filp->f_path.dentry, > 5169 &ppntsd); > 5170 > 5171 /* Check if sd buffer size exceeds response buffer size */ > 5172 if (smb2_resp_buf_len(work, 8) > ppntsd_size) > 5173 rc = build_sec_desc(user_ns, pntsd, ppntsd, > ppntsd_size, > 5174 addition_info, &secdesclen, > &fattr); > > "secdesclen" is not initialized on else path. > > 5175 posix_acl_release(fattr.cf_acls); > 5176 posix_acl_release(fattr.cf_dacls); > 5177 kfree(ppntsd); > 5178 ksmbd_fd_put(work, fp); > 5179 if (rc) > 5180 return rc; > 5181 > --> 5182 rsp->OutputBufferLength = cpu_to_le32(secdesclen); > 5183 inc_rfc1001_len(work->response_buf, secdesclen); > 5184 return 0; > 5185 } > > regards, > dan carpenter >
