bugGNU nano - Bugs: bug #47003, memory leak w/position history

 
 

bug #47003: memory leak w/position history

Submitted by:  Mike Frysinger <vapier>
Submitted on:  Thu 28 Jan 2016 10:27:35 PM UTC  
 
Severity: 3 - NormalStatus: Fixed
Assigned to: Benno Schulenberg <bens>Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Wed 10 Feb 2016 12:34:22 PM UTC, comment #4:

Thanks for the hint.

Fixed in SVN, r5624.

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Mon 01 Feb 2016 07:35:12 AM UTC, comment #3:

realloc() itself will not leak memory as long as you follow the rule: the pointer passed in should be replaced by the pointer returned. so if you follow the pattern:
foo = realloc(foo, ...)
you should be fine.

when you look at the traceback, keep in mind that the problem isn't at the last point. that's just where the memory was allocated. you have to back up a few points to see where the memory is actually being used. realloc() is in the C lib, nrealloc() is in nano, so look at the caller of that. in this case, the place to focus is get_full_path in files.c. i think the issue is around here:
--- a/src/files.c
+++ b/src/files.c
@@ -1466,6 +1466,7 @@ char get_full_path(const char origpath)
if (chdir(d_there) == -1) {
free(d_there);
d_there = NULL;
+free(d_here);
} else {
free(d_there);

that seems to fix the leak for me, but i haven't fully analyzed that func ... there's many code paths :x

Mike Frysinger <vapier>
Project Member
Fri 29 Jan 2016 08:50:39 PM UTC, comment #2:

Things should be better when updating to r5600. It maybe fixes the leak you're seeing.

I'm still seeing one, which gets "fixed" with the attached patch -- but that patch changes the behaviour: it doesn't open a buffer for any file that produces an error message, except when it is the only file given.

(file #36200)

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Fri 29 Jan 2016 01:12:08 PM UTC, comment #1:

At first I thought: how can it check position history when you use -I (that is: --ignorercfiles)? But apparently it does. That is a separate bug. Will fix that first.

The leak you are seeing is in get_full_path(). I've seen it before but can't understand ho it happens, how a realloc (in files.c at line 1389) can lose memory. A reallocation can never leak any memory, can it?

Benno Schulenberg <bens>
Project AdministratorIn charge of this item.
Thu 28 Jan 2016 10:27:35 PM UTC, original submission:

i'm having a hard time coming with a reliable test case, but here's the tracebacks i'm getting. maybe that's enough ?

this is using svn r5595

$ nano -I files/libintl-0.19.5-langinfo.patch /var/tmp/portage/intl/localename.c
<ctrl+x to close>
<ctrl+x to close>
=================================================================
==17334==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 52 byte(s) in 1 object(s) allocated from:
#0 0x7f134dc0736a in realloc (/usr/lib/gcc/x86_64-pc-linux-gnu/5.3.0/libasan.so.2+0x9d36a)
#1 0x455db4 in nrealloc .../nano/src/utils.c:400
#2 0x415939 in get_full_path .../nano/src/files.c:1389
#3 0x413a70 in open_file .../nano/src/files.c:958
#4 0x411328 in open_buffer .../nano/src/files.c:429
#5 0x433e67 in main .../nano/src/nano.c:2621
#6 0x7f134cf295ef in __libc_start_main (/lib64/libc.so.6+0x205ef)

Direct leak of 52 byte(s) in 1 object(s) allocated from:
#0 0x7f134dc0736a in realloc (/usr/lib/gcc/x86_64-pc-linux-gnu/5.3.0/libasan.so.2+0x9d36a)
#1 0x455db4 in nrealloc .../nano/src/utils.c:400
#2 0x415939 in get_full_path .../nano/src/files.c:1389
#3 0x41d8bc in check_poshistory .../nano/src/files.c:3234
#4 0x433f18 in main .../nano/src/nano.c:2634
#5 0x7f134cf295ef in __libc_start_main (/lib64/libc.so.6+0x205ef)

SUMMARY: AddressSanitizer: 104 byte(s) leaked in 2 allocation(s).

Mike Frysinger <vapier>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #36200:  avoid-a-fullpath-leak.patch added by bens (501B - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by prongs
  • -unavailable- added by bens (Posted a comment)
  • -unavailable- added by vapier (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 13 Feb 2016 10:36:24 AM UTCbensOpen/ClosedOpen=>Closed
    Wed 10 Feb 2016 12:34:22 PM UTCbensStatusNone=>Fixed
    Tue 02 Feb 2016 06:07:10 PM UTCprongsCarbon-Copy-=>Added prongs
    Tue 02 Feb 2016 06:06:52 PM UTCprongsCarbon-CopyRemoved prongs=>-
    Tue 02 Feb 2016 06:06:23 PM UTCprongsCarbon-Copy-=>Added prongs
    Fri 29 Jan 2016 08:50:39 PM UTCbensAttached File-=>Added avoid-a-fullpath-leak.patch, #36200
    Fri 29 Jan 2016 01:12:08 PM UTCbensAssigned toNone=>bens

    Back to the top


    Powered by Savane 3.1-cleanup1