* [bug report] ksmbd: fix heap-based overflow in set_ntacl_dacl()
@ 2022-08-04 14:33 Dan Carpenter
2022-08-04 23:03 ` Namjae Jeon
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2022-08-04 14:33 UTC (permalink / raw)
To: linkinjeon; +Cc: linux-cifs
Hello Namjae Jeon,
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'.
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] ksmbd: fix heap-based overflow in set_ntacl_dacl()
2022-08-04 14:33 [bug report] ksmbd: fix heap-based overflow in set_ntacl_dacl() Dan Carpenter
@ 2022-08-04 23:03 ` Namjae Jeon
2022-08-05 11:42 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Namjae Jeon @ 2022-08-04 23:03 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-cifs
2022-08-04 23:33 GMT+09:00, Dan Carpenter <dan.carpenter@oracle.com>:
> 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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] ksmbd: fix heap-based overflow in set_ntacl_dacl()
2022-08-04 23:03 ` Namjae Jeon
@ 2022-08-05 11:42 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2022-08-05 11:42 UTC (permalink / raw)
To: Namjae Jeon; +Cc: linux-cifs
On Fri, Aug 05, 2022 at 08:03:04AM +0900, Namjae Jeon wrote:
> 2022-08-04 23:33 GMT+09:00, Dan Carpenter <dan.carpenter@oracle.com>:
> > 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!
Oops. Sorry, I didn't realize it was part of the kbuild warning... The
kbuild warning didn't include that code so this was the first time I
was seeing the code myself.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-05 11:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-04 14:33 [bug report] ksmbd: fix heap-based overflow in set_ntacl_dacl() Dan Carpenter
2022-08-04 23:03 ` Namjae Jeon
2022-08-05 11:42 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).