479759 - memory leak and unallocated memory access possibility in universalchardet.

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[takasumi_iwamoto]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.9.0.6) Gecko/2009011913 Firefox/3.0.6 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.9.0.6) Gecko/2009011913 Firefox/3.0.6 (.NET CLR 3.5.30729)

I found memory leak and unallocated memory access possibility in universalchardet.
You can confirm these problem easily if you use valgrind (memory profiler) on x86 linux.

i) memory leak
Steps To Reproduce:
Memory leak occur if you call the following APIs sequentially.
- nsUniversalDetector::HandleData().
- nsUniversalDetector::Reset().
- nsUniversalDetector::HandleData(). /* memory leak occure in this function */

I confirmed this problem the following file.
http://mxr.mozilla.org/seamonkey/source/extensions/universalchardet/src/base/nsUniversalDetector.cpp#187
I guess nsUniversalDetector::mCharSetProbers[2] variable isn't free when nsUniversalDetector::Reset() is called in the above-mentioned step.

ii) unallocated memory access possibility

I confirmed this problem the following file.
http://mxr.mozilla.org/seamonkey/source/extensions/universalchardet/src/base/nsMBCSGroupProber.cpp#159

I think aLen variable means string length of aBuf array.
In nsMBCSGroupProber.cpp#159, aLen + 1 is given to mProbers[i]->HandleData() API.
So, I guess it has a possibility of accessing unallocated area of aBuf,
because aBuf[aLen] is outside of string user specified.

Reproducible: Always

Actual Results:  
i) memory leak
ii) A bad phenomenon of anything might not be able to be confirmed. 

Expected Results:  
i) no memory leak
ii) no unallocated memory access

Comment 1

[Simon Montagu :smontagu]

Attachment: Patch

Comment 2

[Simon Montagu :smontagu]

http://hg.mozilla.org/mozilla-central/rev/d0d2850eb91e

Not marking fixed yet because I wasn't able to reproduce the valgrind errors so I can't be sure that this fixes them.

Comment 3

[Simon Montagu :smontagu]

Comment on attachment 366281 [details] [diff] [review]
Patch

Asking branch approval since there is reason to believe that this fixes a crash - see 453631 comment 3

Comment 4

[Samuel Sidler (old account; do not CC)]

Comment on attachment 366281 [details] [diff] [review]
Patch

Please nominate this for 1.9.0.9 after it lands on 1.9.1.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 366281 [details] [diff] [review]
Patch

a191=beltzner, and please just mark this FIXED as well?

Comment 6

[Simon Montagu :smontagu]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/924b80de55be

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 366281 [details] [diff] [review]
Patch

Approved for 1.9.0.10, a=dveditz for release-drivers

Comment 8

[masinick]

I cannot be 100% positive, but I believe that there may yet be slight memory leaks still out there.  I am running the nightly trunk builds, which I have been updating each day.  When I start a new browser and start an Email client, memory usage is usually about 60-65M of resident memory and about 23M of shared memory, and slightly over 200M virtual memory.  My system has plenty of unused memory, but with very nominal use, one Email client, one browser window, mostly only one site, very occasionally two or three tabs, then back down to one, and home at google.com with minimal content on the page, memory usage in this scenario consistently creeps up near 100M of resident memory, sometimes 103-105M per day, and shared memory climbs to around 27M.

Perhaps this is simply that resources are not being released because there is no demand from the system that resources get trimmed, but it looks suspiciously like a moderate memory leak to me.  It would be helpful to have a real sleuth go in there and run memory usage scans throughout the code if that has not been done recently.