Bug 691557 - Memory corruption in function gp_defaultpapersize (file: 'base/gp_upapr.c')
Summary: Memory corruption in function gp_defaultpapersize (file: 'base/gp_upapr.c')
Status: RESOLVED FIXED
Alias: None
Product: Ghostscript
Classification: Unclassified
Component: PS Interpreter (show other bugs)
Version: master
Hardware: PC Linux
: P4 critical
Assignee: Alex Cherepanov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-14 07:59 UTC by Vladimir Lomov
Modified: 2010-08-14 21:14 UTC (History)
1 user (show)

See Also:
Customer:
Word Size: ---


Attachments
Patch for base/gp_upapr.c to remove redundant 'free()' (393 bytes, patch)
2010-08-14 07:59 UTC, Vladimir Lomov
Details | Diff
patch (786 bytes, patch)
2010-08-14 15:55 UTC, Alex Cherepanov
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vladimir Lomov 2010-08-14 07:59:41 UTC
Created attachment 6659 [details]
Patch for base/gp_upapr.c to remove redundant 'free()'

Hi.

The error that causes memory corruption was introduced in 11588.

I attach small patch file.

I noticed this only now. My system:
OS:         archlinux 86_64
compiler:   gcc 4.5.1

ghostscript was compiled with the following system libraries:
expat, jasper, jpeg, libpng, libz.

Before patching gs runs causes glibc report, running in valgrind confirm this corruption. After patching gs runs fine (valgrind doesn't report any errors).
Comment 1 Alex Cherepanov 2010-08-14 12:49:27 UTC
What version of libpaper are you using?
My version, 1.1.24, defiles paperdone() as a no-op, which
doesn't free anything. Without rev. 11588 I have a memory leak.
Comment 2 Vladimir Lomov 2010-08-14 14:15:30 UTC
(In reply to comment #1)
> What version of libpaper are you using?
> My version, 1.1.24, defiles paperdone() as a no-op, which
> doesn't free anything. Without rev. 11588 I have a memory leak.

On my system libpaper is of 1.1.23 version.

But this is irrelevant to version as far I could see.

1. I not sure what you mean
> version, 1.1.24, defiles paperdone() as a no-op
what is 'no-op'?

2. IMHO (I don't have big practice in C programming, more in C++ than in C :) free should be used _only_ if one call 'malloc' explicitly but I don't see 'malloc' use in 'base/gp_upapr.c'.

3. I tried to dig deeper into the problem and found that the "official" home page of libpaper is within Debian project (for my distro the site http://packages.debian.org/unstable/source/libpaper is mentioned as 'official' but google gives http://packages.qa.debian.org/libp/libpaper.html as first result for me).

I downloaded 1.1.23+nmu1 and 1.1.24 versions of libpaper, put them into local git repository (in that order) and for 'lib/' directory I get not so much differences (I don't post them because I think they are irrelevant here).

So I summarize:
1. According to 'lib/paper.c' in libpaper source the 'paperdone' function does nothing (just returns 0).

2. According the books I read about C programming, each explicit call to 'malloc' should be paired with respective 'free'.

3. The changes between 1.1.23 and 1.1.24 were minor and weren't concerned with functions definition.

P.S. Sorry for long post but I spent a half of day trying to found out the problem. I began with rev 11579 which works and I knew that rev. 11631 always gives me report from glibc about memory corruption (only gs --version works). Time to compile, time to switch between revisions and I got a half of day of enjoy :).
Comment 3 Alex Cherepanov 2010-08-14 15:55:30 UTC
Created attachment 6660 [details]
patch

Free the string returned systempapername() but don't free the static string
returned by defaultpapername() .
Comment 4 Alex Cherepanov 2010-08-14 21:14:06 UTC
The patch has been committed as a rev. 11633.
Vladimir, thank you for using and testing Ghostscript.
You've spotted a serious bug just before the release.