Bug 691557

Summary: Memory corruption in function gp_defaultpapersize (file: 'base/gp_upapr.c')
Product: Ghostscript Reporter: Vladimir Lomov <lomov.vl>
Component: PS InterpreterAssignee: Alex Cherepanov <alex>
Status: RESOLVED FIXED    
Severity: critical CC: alex
Priority: P4    
Version: master   
Hardware: PC   
OS: Linux   
Customer: Word Size: ---
Attachments: Patch for base/gp_upapr.c to remove redundant 'free()'
patch

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.