From: Colin Ian King <colin.king@canonical.com>
To: Olga Kornievskaia <kolga@netapp.com>,
Trond Myklebust <trond.myklebust@hammerspace.com>,
Anna Schumaker <anna.schumaker@netapp.com>,
linux-nfs@vger.kernel.org
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: re: NFS: handle source server reboot
Date: Fri, 15 Nov 2019 10:05:19 +0000 [thread overview]
Message-ID: <c64dfc7b-0c0b-e571-5a49-f32034eccaa5@canonical.com> (raw)
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
next reply other threads:[~2019-11-15 10:05 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-15 10:05 Colin Ian King [this message]
2019-11-15 19:51 ` NFS: handle source server reboot Olga Kornievskaia
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c64dfc7b-0c0b-e571-5a49-f32034eccaa5@canonical.com \
--to=colin.king@canonical.com \
--cc=anna.schumaker@netapp.com \
--cc=kolga@netapp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=trond.myklebust@hammerspace.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).