Messages in this thread | | | From | Colin Ian King <> | Subject | Re: [PATCH][next] btrfs: fix memory leak of path on error return path | Date | Thu, 4 Jul 2019 17:47:50 +0100 |
| |
On 04/07/2019 17:37, David Sterba wrote: > On Tue, Jul 02, 2019 at 03:10:28PM +0100, Colin King wrote: >> From: Colin Ian King <colin.king@canonical.com> >> >> Currently if the allocation of roots or tmp_ulist fails the error handling >> does not free up the allocation of path causing a memory leak. Fix this by >> freeing path with a call to btrfs_free_path before taking the error return >> path. >> >> Addresses-Coverity: ("Resource leak") > > Does this have an id, that coverity uses?
Not a public one that I know of. These are based on private scans I run in Canonical.
> >> Fixes: 5911c8fe05c5 ("btrfs: fiemap: preallocate ulists for btrfs_check_shared") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> fs/btrfs/extent_io.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c >> index 1eb671c16ff1..d7f37a33d597 100644 >> --- a/fs/btrfs/extent_io.c >> +++ b/fs/btrfs/extent_io.c >> @@ -4600,6 +4600,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, >> tmp_ulist = ulist_alloc(GFP_KERNEL); >> if (!roots || !tmp_ulist) { >> ret = -ENOMEM; >> + btrfs_free_path(path); > > This fixes only one leak, therere are more that I spotted while > reviewing this patch. The gotos from the while-loop jump to > out_free_list but that leave the path behind> > That's why the exit block is a better place for the cleanups. This > requires proper nesting of the cleanup calls, that's slightly > inconvenient in this case. The free_path is before call to > unlock_extent_cached so when the ordre is switched and free_path moved > to out_free_ulist, then all the leaks are addressed in one go.
Oh, yes. Even static analysis missed that too!
> > Bummer that the leaks escaped sight of original patch author (me), 2 > reviewers and now 1 fix reviewer. > Given that you can see more issues, I'll leave the fix in your capable hands.
Colin
| |