FAQ SearchLogin
Tuxera Home
View unanswered posts | View active topics It is currently Fri May 07, 2021 00:47



Post new topic Reply to topic  [ 6 posts ] 
[Bug]Possible memory leak in function ntfs_index_ctx_free 
Author Message

Joined: Mon Sep 26, 2016 03:18
Posts: 3
Post [Bug]Possible memory leak in function ntfs_index_ctx_free
We found a memory leak in function ntfs_index_ctx_free at ntfs-3g_ntfsprogs-2016.2.22.
The call stack is like this:

ntfs_inode_sync_file_name -> ntfs_index_lookup -> ntfs_ir_lookup (this func alloc memory in &icx->actx)
-> ntfs_index_ctx_put (when icx->entry is null, the meory in &icx->actx was not free and cause memory leak)

int the function ntfs_index_lookup, when some I/O read error happened like below:
if (ntfs_ib_read(icx, vcn, ib))
goto err_out;
The icx->entry is null, and the func ntfs_index_ctx_put doesn`t free the meory alloc in icx->actx.

So,we wanna know why the func ntfs_index_ctx_free doesn`t call the ntfs_attr_push_searh_ctx when icx->entry is null.
static void ntfs_index_ctx_free(ntfs_index_context *icx)
{
ntfs_log_trace("Entering\n");

if (!icx->entry)
return;

if (icx->actx)
ntfs_attr_put_search_ctx(icx->actx);

if (!icx->is_in_root) {
if (icx->ib_dirty) {
/* FIXME: Error handling!!! */
ntfs_ib_write(icx, icx->ib);
}
free(icx->ib);
}

ntfs_attr_close(icx->ia_na);
}

How we could fix this memory leak? Thanks.


Mon Sep 26, 2016 13:26
Profile
NTFS-3G Lead Developer

Joined: Tue Sep 04, 2007 17:22
Posts: 1286
Post Re: [Bug]Possible memory leak in function ntfs_index_ctx_free
Hi,

Quote:
We found a memory leak in function ntfs_index_ctx_free at ntfs-3g_ntfsprogs-2016.2.22.

By triggering a random error on return of ntfs_ie_lookup() I could produce a leak condition, but that is not exactly a condition which can happen, so an important issue is : did you find the possible leak in a real condition which can happen or by running some source code analyzer ?

Anyway, my feeling is that ntfs_index_lookup() should do the housekeeping before returning an error. The problem is however that an ENOENT error is not a real error, it means a key not to be present and can be inserted into the index.

This may be contrary to the original developer's view, as he commented :
* If an error occurs return -1, set errno to error code and @icx is left
* untouched.

Can you please apply the attached patch and report ?

Regards

Jean-Pierre

*edit* : attachment deleted


Tue Sep 27, 2016 11:21
Profile

Joined: Mon Sep 26, 2016 03:18
Posts: 3
Post Re: [Bug]Possible memory leak in function ntfs_index_ctx_free
Hi,
Actually I found this issue on my demo board, every time I copy files sized about 100MB into a directory of my U-Disk, the system OOM.
This directory is somehow broken or what, nothing can be written into or removed from it, either in shell or by samba.
When copying files into it, nfts-3g outputs debug info:

Incomplete multi-sector transfer: magic: 0x58444e49 size: 4096 usa_ofs: 40 usa_count: 5 data: 5046 usn: 4606: Ir
Corrupt index block signature: vcn 8 inode 15053
Index lookup failed, inode 15053: Input/output error

Failed to sync FILE_NAME (inode 15055): Input/output error


I used the malloc--free pair and finally located this possible leak, I also tried to comment
if (!icx->entry)
return;

in ntfs_index_ctx_free, it turns out no memory leak.

But I'm not sure if it is the right solution, guess doing some housekeeping before returning an error, as you mentioned, is the right one.


Tue Sep 27, 2016 17:15
Profile
NTFS-3G Lead Developer

Joined: Tue Sep 04, 2007 17:22
Posts: 1286
Post Re: [Bug]Possible memory leak in function ntfs_index_ctx_free
Hi,

My previous patch is not satisfactory. In your case, a read error is detected in ntfs_ib_read(), after the allocation is done in ntfs_ie_lookup(), and other leaks are possible.

The original developer wanted icx->entry to track possible error conditions in ntfs_index_lookup(), but this is only true if the error occurs in ntfs_ie_lookup(), not in subsequent checks. After such errors icx->entry is still NULL, but several allocations took place, and they have to be freed in ntfs_index_ctx_free().

As a consequence, I suggest having a new field for keeping track of late errors and have ntfs_index_ctx_free() do the housekeeping.

See the new attached patch (I am deleting the previous one).

Your error case is triggered by a bad index node (probably a bad directory). If this is not a consequence of some hardware error, chkdsk should be able to rebuild the index.

Regards

Jean-Pierre


Attachments:
leak-v2.patch.gz [462 Bytes]
Downloaded 681 times
Wed Sep 28, 2016 11:45
Profile

Joined: Mon Sep 26, 2016 03:18
Posts: 3
Post Re: [Bug]Possible memory leak in function ntfs_index_ctx_free
Hi jpa:
Thank you very much for the leak-v2.patch.gz. The memory leak has been fixed.

BTW,we have one more question to discuss:
We worried about the function ntfs_ib_write called by ntfs_index_ctx_free, will this make some side effect for the normal situation?
If there is no side effect, will you merge this patch to the latest ntfs-3g version?

Regards


Wed Oct 12, 2016 12:05
Profile
NTFS-3G Lead Developer

Joined: Tue Sep 04, 2007 17:22
Posts: 1286
Post Re: [Bug]Possible memory leak in function ntfs_index_ctx_free
Hi,

Quote:
We worried about the function ntfs_ib_write called by ntfs_index_ctx_free, will this make some side effect for the normal situation?

AFAICT, the patch only acts on error cases (read errors or unconsistent data). I have made a small change to the patch to be even more sure there is no behavior change in normal situations.
Quote:
If there is no side effect, will you merge this patch to the latest ntfs-3g version?

The patch is part of ntfs-2016.2.22AR.2 (http://jp-andre.pagesperso-orange.fr/ad ... l#download), and is to be included in next stable versions unless some bad behavior emerges.

Regards

Jean-Pierre


Wed Oct 12, 2016 13:58
Profile
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 6 posts ] 


Who is online

Users browsing this forum: No registered users and 2 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
cron
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group.
Original forum style by Vjacheslav Trushkin.