Messages in this thread | | | From | Colin Ian King <> | Subject | re: NFS: handle source server reboot | Date | Fri, 15 Nov 2019 10:05:19 +0000 |
| |
Hi,
Static analysis with Coverity has detected a memory leak in the following commit:
commit 0e65a32c8a569db363048e17a708b1a0913adbef Author: Olga Kornievskaia <kolga@netapp.com> Date: Fri Jun 14 14:38:40 2019 -0400
NFS: handle source server reboot
In function __nfs4_copy_file_range() in fs/nfs/nfs4file.c, analysis is as follows:
155retry: 5. Condition !nfs42_files_from_same_server(file_in, file_out), taking false branch. 9. Condition !nfs42_files_from_same_server(file_in, file_out), taking false branch.
156 if (!nfs42_files_from_same_server(file_in, file_out)) { 157 /* for inter copy, if copy size if smaller than 12 RPC 158 * payloads, fallback to traditional copy. There are 159 * 14 RPCs during an NFSv4.x mount between source/dest 160 * servers. 161 */ 162 if (sync || 163 count <= 14 * NFS_SERVER(file_inode(file_in))->rsize) 164 return -EOPNOTSUPP; 165 cn_resp = kzalloc(sizeof(struct nfs42_copy_notify_res), 166 GFP_NOFS); 167 if (unlikely(cn_resp == NULL)) 168 return -ENOMEM; 169 170 ret = nfs42_proc_copy_notify(file_in, file_out, cn_resp); 171 if (ret) { 172 ret = -EOPNOTSUPP; 173 goto out; 174 } 175 nss = &cn_resp->cnr_src; 176 cnrs = &cn_resp->cnr_stateid; 177 } 178 ret = nfs42_proc_copy(file_in, pos_in, file_out, pos_out, count, 179 nss, cnrs, sync); 180out: 6. freed_arg: kfree frees cn_resp.
CID 91571 (#1 of 1): Double free (USE_AFTER_FREE)10. double_free: Calling kfree frees pointer cn_resp which has already been freed.
181 kfree(cn_resp);
7. Condition ret == -11, taking true branch.
182 if (ret == -EAGAIN) 8. Jumping to label retry.
183 goto retry; 184 return ret; 185} 186
On the 2nd iteration of the retry loop, cn_resp is being free'd twice if the call to nfs42_files_from_same_server() returns zero since cn_resp is not kalloc'd in the 2nd iteration. A naive fix would be to set cn_resp to NULL after the kfree on line 181, but I'm not sure if there is a better way to resolve this.
Colin
| |