Issue 67740

Summary:

double-free in xmlhelp.

Product:

utilities

Reporter:

caolanm

Component:

code

Assignee:

Status:

CLOSED DUPLICATE

QA Contact:

Unknown <non-migrated>

Severity:

Trivial

Priority:

CC:

hennes.rohling, issues, jialiang.cheng, matthias.huetsch, mmeeks, roland.edv

Version:

OOo 2.0.3

Target Milestone:

OOo 2.x

Hardware:

All

OS:

Linux, all

Issue Type:

DEFECT

Latest Confirmation in:

---

Developer Difficulty:

---

Attachments:

Description	Flags
valgrind log	none
workaround patch	none
crude double free detection in sal	none

Description caolanm 2006-07-25 09:00:02 UTC

Attached is valgrind log pointing to the two frees of the same memory in xmlhelp.

This (for me, your milage may vary) will cause a crash later on in writer, e.g.
Help->OpenOffice.org Help->Find, search for "scan", and double click on "General
Glossay" crashes OOo 2.0.3.

Attached is a patch which removes the first of the free's, but someone familiar
with the code will have to see what the correct course of action is.

cmc->mhu:
As an aside, this crashes in writer, because the new allocator is especially
sensitive to double-frees as it quickly re-uses free'd cache pointers. And if
something is double free'd then the pointer is pushed twice onto the free list,
and is easily re-used twice concurrently. Perhaps a little non-product build
crude double free detector like the final attachment might be a good idea ?

Comment 1 caolanm 2006-07-25 09:00:49 UTC

Created attachment 38006 [details]
valgrind log

Comment 2 caolanm 2006-07-25 09:01:26 UTC

Created attachment 38007 [details]
workaround patch

Comment 3 caolanm 2006-07-25 09:01:52 UTC

Created attachment 38008 [details]
crude double free detection in sal

Comment 4 caolanm 2006-07-25 09:02:16 UTC

set a target

Comment 5 caolanm 2006-07-27 12:10:25 UTC

cmc->mmeeks: you might be interested in this one. That might not be the right
fix, and I believe the ooo-build default is to use the system allocator ?, but
nevertheless in internal allocator mode double-freeing is a real serious problem.

Comment 6 matthias.huetsch 2006-07-27 13:18:19 UTC

Hi Caolan,

Of course your right in that debug support in the new (rtl_cache based)
allocator is very limited. Unfortunately, your "crude" patch will not generally
work (besides its performance impact) as the previously free'd object may not be
in the current magazine ("curr").

Please allow for a couple of days to work out a more general solution; the
pieces are already there (FLAG_NOMAGAZINE can force the cache to use the slab
layer only, and the slab layer can be made to keep track of all buffers in a
hash table) but I need to find some time to actually make these changes.

Of course, my proposed changes also have a negative performance impact, and will
also (possibly significantly) increase the amount of memory used (additional
hash table space). So, this can only be enabled in non-pro builds which are
probably not in wide spread use.

Probably, the only reliable way to detect such issues is through use of valgrind
(and friends).

Hope that helps,
Matthias

Comment 7 hennes.rohling 2006-08-07 10:29:34 UTC

Comment 8 hennes.rohling 2007-02-12 11:59:41 UTC

Comment 9 Oliver Specht 2007-02-12 14:20:06 UTC

->hro: Why me?

Reassigned to abi, who has the most entries in the cvs log ;-)

Comment 10 andreas.bille 2007-03-14 18:28:14 UTC

accepted

Comment 11 andreas.bille 2007-04-04 14:27:20 UTC

ABI->AB: As discussed ...

Comment 12 ab 2007-04-18 16:00:07 UTC

STARTED

Comment 13 caolanm 2007-05-08 14:10:28 UTC

*** Issue 77015 has been marked as a duplicate of this issue. ***

Comment 14 caolanm 2007-05-09 08:12:15 UTC

*** Issue 72381 has been marked as a duplicate of this issue. ***

Comment 15 caolanm 2007-09-01 14:46:48 UTC

The patch here was integrated into OOG680_m3 under issue 80952, so we can close
this now.

*** This issue has been marked as a duplicate of 80952 ***

Comment 16 caolanm 2007-09-01 14:49:09 UTC

closing, (yippee!)