Red Hat Bugzilla – Bug 866989
double free in _nc_scroll_optimize()
Last modified: 2012-12-20 10:29:36 EST
Version-Release number of selected component:
libreport version: 2.0.16
cmdline: nano Git/FlatZinc/NFloat.Literal.cpp
:Thread no. 1 (7 frames)
: #5 _nc_scroll_optimize at ../../ncurses/tty/hardscroll.c:208
: #6 doupdate at ../../ncurses/tty/tty_update.c:927
: #7 get_key_buffer at winio.c:122
: #8 get_input at winio.c:259
: #9 parse_kbinput at winio.c:342
: #10 get_kbinput at winio.c:314
: #11 do_input at nano.c:1537
Created attachment 628163 [details]
Created attachment 628164 [details]
Created attachment 628165 [details]
Created attachment 628166 [details]
Created attachment 628167 [details]
Created attachment 628168 [details]
Created attachment 628169 [details]
Created attachment 628170 [details]
Created attachment 628171 [details]
Created attachment 628172 [details]
Created attachment 628173 [details]
This looks like a bug in ncurses to me:
int *new_oldnums = typeRealloc(int,
If a pointer is given to realloc(), you should not free() the pointer afterwards.
The ncurses version from the dso_list is is 5.9-20121013.
Should I do something? Is it solved?
No, I've just CCed the upstream maintainer. He may know if there were changes around this code recently.
Created attachment 628479 [details]
last diff, with checkin comment
From this discussion, this seems to be an incorrect bug-fix :-)
Ok.. ..let me verify if I got it right:
1. It reallocs a chunk of memory, which may or may not move the original block of data depending on the segmentation level of the heap.
2. Tests the returned pointer (Here i don't really get why he uses !new_oldnums. It's clear that if new_oldnums == 0 then !new_oldnums == 0xFF. But when new_oldnums == RandomNumber, the "complement" given by ! negation operator, does really return 0?)
3. Frees the memory previously associated to the pointer. Here two things may happen:
A- the original chunk of memory wasn't moved for memory issues, so he is freeing the newly reallocated chunk of memory
B- the original chunk of memory was moved, so he is freeing a piece of memory which has already been freed automatically by realloc.
In both cases it's an error, assuming those functions behave as standard free/realloc. I've tried to search for typeRealloc and FreeIfNeeded, but found nothing.. do you know where i can check their implementation? Ncurses?
The solution should be to simply remove line
which is not needed at all. Though i don't really understand if it's safe to return the memory "not resized" in case typeRealloc fails, as he does. :-/
By the way, how did you get that interesting diff? :-)
hmm - no - the problem that I see is not checking to ensure that
the old/new pointers are really different before doing the free
(as well as looking for other pitfalls of the same type).
The "interesting diff" is a chunk from rcshist.
...but I agree that removing the line is the fix needed :-)
well.. I don't get why you think the problem is only checking that the pointers are different..
..the pointers are different only if the chunk of memory is moved at another address for memory management issues, so the original memory address should be already "freed" automatically by realloc.
But I'm not an expert. :-/
On the other hand, I had a long day, and today's similar :-)
I'll put out a patch for this, and review the other realloc's
to fix the (already known) issue that using the result from
realloc requires checking it before keeping it.
...if realloc fails, the original memory is supposed to be intact.
I had a report on this a while back, and fixed the existing cases.
There are some new ones that I see. I'll put out the short fix this
morning, and work through the others (perhaps tomorrow - won't be
around this weekend).
Yep, I totally agree with you on this. But still don't get why you suggested "the problem that I see is not checking to ensure that the old/new pointers are really different before doing the free".
A) realloc fails: memory untouched, returns NULL: the original pointer should not immediately overwritten to not cause a memory leakage, nor freed;
B) realloc is successfull, but doesn't move the object in the heap (just adjusts its end): returns the same address passed as a parameter, hence freeing it is an error;
C) realloc is successful, and moves the object in the heap: the new address must be stored in the old variable pointer. The old pointer must not be freed, since realloc has already done it.
So, even if the pointers are indeed different, you should never free anything at all. No?
See comment #21 (long day -> increased time to wake up).
Oh, ok.. thank you for your patience..
ncurses-5.9-7.20121017.fc18 has been submitted as an update for Fedora 18.
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing ncurses-5.9-7.20121017.fc18'
as soon as you are able to.
Please go to the following url:
then log in and leave karma (feedback).
That's amazing.. but I've got a problem.
This morning was released an update to rhythmbox, right after which I was able to launch it again.. so note that by leaving a karma I'm saying that that update + your bugfix (which was proven to exist) have solved the launch problem.
I've seen that the IM plugin does not instead still work. Do you think that it's better to contact the plugin maker, wait some time for library stabilization or do something else?
I'm not sure about the IM plugin. If I'm finding some particular
problem with a program, I do check if there's similar reports
(and for instance look to see if it's being handled).
ncurses-5.9-7.20121017.fc18 has been pushed to the Fedora 18 stable repository. If problems still persist, please make note of it in this bug report.