| Message ID | 20241015102036.2882322-1-suhui@nfschina.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] smb: client: fix possible double free in smb2_set_ea() | expand |
On Tue, Oct 15, 2024 at 06:20:37PM +0800, Su Hui wrote: > Clang static checker(scan-build) warning： > fs/smb/client/smb2ops.c:1304:2: Attempt to free released memory. > 1304 | kfree(ea); > | ^~~~~~~~~ > > There is a double free in such case: > 'ea is initialized to NULL' -> 'first successful memory allocation for > ea' -> 'something failed, goto sea_exit' -> 'first memory release for ea' > -> 'goto replay_again' -> 'second goto sea_exit before allocate memory > for ea' -> 'second memory release for ea resulted in double free'. > > Re-initialie 'ea' to NULL near to the replay_again label, it can fix this > double free problem. > > Fixes: 4f1fffa23769 ("cifs: commands that are retried should have replay flag set") > Signed-off-by: Su Hui <suhui@nfschina.com> > --- > v2: > - Move 'ea = NULL' near to the replay_again label.(Dan's suggestion) Thanks! Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org> regards, dan carpenter
merged into cifs-2.6.git for-next pending additional testing On Tue, Oct 15, 2024 at 5:45 AM Dan Carpenter <dan.carpenter@linaro.org> wrote: > > On Tue, Oct 15, 2024 at 06:20:37PM +0800, Su Hui wrote: > > Clang static checker(scan-build) warning： > > fs/smb/client/smb2ops.c:1304:2: Attempt to free released memory. > > 1304 | kfree(ea); > > | ^~~~~~~~~ > > > > There is a double free in such case: > > 'ea is initialized to NULL' -> 'first successful memory allocation for > > ea' -> 'something failed, goto sea_exit' -> 'first memory release for ea' > > -> 'goto replay_again' -> 'second goto sea_exit before allocate memory > > for ea' -> 'second memory release for ea resulted in double free'. > > > > Re-initialie 'ea' to NULL near to the replay_again label, it can fix this > > double free problem. > > > > Fixes: 4f1fffa23769 ("cifs: commands that are retried should have replay flag set") > > Signed-off-by: Su Hui <suhui@nfschina.com> > > --- > > v2: > > - Move 'ea = NULL' near to the replay_again label.(Dan's suggestion) > > Thanks! > > Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org> > > regards, > dan carpenter > >
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index 6b385fce3f2a..24a2aa04a108 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -1158,7 +1158,7 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon, struct cifs_fid fid; unsigned int size[1]; void *data[1]; - struct smb2_file_full_ea_info *ea = NULL; + struct smb2_file_full_ea_info *ea; struct smb2_query_info_rsp *rsp; int rc, used_len = 0; int retries = 0, cur_sleep = 1; @@ -1179,6 +1179,7 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon, if (!utf16_path) return -ENOMEM; + ea = NULL; resp_buftype[0] = resp_buftype[1] = resp_buftype[2] = CIFS_NO_BUFFER; vars = kzalloc(sizeof(*vars), GFP_KERNEL); if (!vars) {
Clang static checker(scan-build) warning： fs/smb/client/smb2ops.c:1304:2: Attempt to free released memory. 1304 | kfree(ea); | ^~~~~~~~~ There is a double free in such case: 'ea is initialized to NULL' -> 'first successful memory allocation for ea' -> 'something failed, goto sea_exit' -> 'first memory release for ea' -> 'goto replay_again' -> 'second goto sea_exit before allocate memory for ea' -> 'second memory release for ea resulted in double free'. Re-initialie 'ea' to NULL near to the replay_again label, it can fix this double free problem. Fixes: 4f1fffa23769 ("cifs: commands that are retried should have replay flag set") Signed-off-by: Su Hui <suhui@nfschina.com> --- v2: - Move 'ea = NULL' near to the replay_again label.(Dan's suggestion) fs/smb/client/smb2ops.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)