Opened 5 years ago

Closed 5 years ago

#9021 closed bug (fixed)

[CID43168] rts/linker.c has a memory leak in the dlopen/dlerror code

Reported by: hgolden Owned by: simonmar
Priority: low Milestone: 7.8.3
Component: Runtime System Version: 7.8.2
Keywords: linker, memory leak, coverity Cc: simonmar
Operating System: POSIX Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

The Coverity Scan of ghc highlighted a memory leak (CID 43168) in the internal_dlopen function in rts/linker.c. I wrote this code originally and I recall that there was a memory leak in certain situations, but I didn't fix it at the time because it seemed to be rather minor and only would occur in unusual cases. However, since Coverity identified it, I have thought about it and know how to fix the leak, so I am creating this ticket.

The leak occurs because the error message from dlerror must be copied to allocated memory, since the dlerror message is not guaranteed to be stable, and its code is not guaranteed to be reentrant (see POSIX dlopen/dlerror documentation). As I originally coded Trac ticket #2615, I didn't bother to free the allocated memory. The fix is to make sure that the allocated memory is freed once the error message is displayed. Memory is only allocated if there is an error, so it must be freed only when an error has been identified and processed. As far as I know, this bug only exists on a POSIX system.

Attachments (1)

Linker.c.patch (1008 bytes) - added by hgolden 5 years ago.
Patch for Ticket #9021

Download all attachments as: .zip

Change History (9)

comment:1 Changed 5 years ago by nicolast

Keywords: coverity added

comment:2 Changed 5 years ago by nicolast

Summary: rts/linker.c has a memory leak in the dlopen/dlerror code[CID43168] rts/linker.c has a memory leak in the dlopen/dlerror code

Changed 5 years ago by hgolden

Attachment: Linker.c.patch added

Patch for Ticket #9021

comment:3 Changed 5 years ago by hgolden

Status: newpatch

comment:4 Changed 5 years ago by hgolden

Owner: changed from hgolden to simonmar

comment:5 Changed 5 years ago by hgolden

I am withdrawing my patch. The one posted to Coverity Scan was better. Please use that instead. (I don't know who wrote the Coverity Scan patch, but it may have been nicolast.)

comment:6 Changed 5 years ago by thoughtpolice

Resolution: fixed
Status: patchclosed

That was me, thanks!

comment:7 Changed 5 years ago by thoughtpolice

Status: closedmerge

comment:8 Changed 5 years ago by thoughtpolice

Status: mergeclosed

This was already merged, it seems.

Note: See TracTickets for help on using tickets.