Bug #52407 for PAR-Packer: pp creates par-<user>/ in /, not /tmp

Thu Dec 03 15:00:35 2009 SWMCD [...] cpan.org - Ticket created

Subject:

pp creates par-<user>/ in /, not /tmp

pp tries to create the par-<user> directory in /, instead of /tmp If you happen to be root, it succeeds, but cluttering / with tmp dirs is poor practice. If you aren't root, it fails for lack of write on /. I tried a reinstall via cpan, and saw the same failure in the unit tests: PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0, 'inc', 'blib/lib', 'blib/arch')" t/00-pod.t t/10-parl-generation.t t/20-pp .t t/30-current_exec.t t/40-packer_cd_option.t t/00-pod.t ............... skipped: Set environment variable PERL_TEST_POD=1 to test POD t/10-parl-generation.t ... ok t/20-pp.t ................ ok t/30-current_exec.t ...... # Please wait t/30-current_exec.t ...... 2/4 # Failed test 'Respected PAR_GLOBAL_TMPDIR' # at t/30-current_exec.t line 46. I backed down to 0.991 and the problem went away. ~>uname -a Linux pstksd1 2.6.18-120.el5 #1 SMP Fri Oct 17 18:00:55 EDT 2008 x86_64 x86_64 x86_64 GNU/Linux ~>perl -V Summary of my perl5 (revision 5 version 8 subversion 8) configuration: Platform: osname=linux, osvers=2.6.9-78.0.1.elsmp, archname=x86_64-linux-thread-multi uname='linux hs20-bc1-7.build.redhat.com 2.6.9-78.0.1.elsmp #1 smp tue jul 22 18:01:05 edt 2008 x86_64 x86_64 x86_64 gnulinux ' config_args='-des -Doptimize=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -Dversion=5.8.8 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. -Dinstallprefix=/usr -Dprefix=/usr -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Dprivlib=/usr/lib/perl5/5.8.8 -Dsitelib=/usr/lib/perl5/site_perl/5.8.8 -Dvendorlib=/usr/lib/perl5/vendor_perl/5.8.8 -Darchlib=/usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi -Dsitearch=/usr/lib64/perl5/site_perl/5.8.8/x86_64-linux-thread-multi -Dvendorarch=/usr/lib64/perl5/vendor_perl/5.8.8/x86_64-linux-thread-multi -Darchname=x86_64-linux -Dvendorprefix=/usr -Dsiteprefix=/usr -Duseshrplib -Dusethreads -Duseithreads -Duselargefiles -Dd_dosuid -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto -Dinc_version_list=5.8.7 5.8.6 5.8.5 -Dscriptdir=/usr/bin' hint=recommended, useposix=true, d_sigaction=define usethreads=define use5005threads=undef useithreads=define usemultiplicity=define useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -Wdeclaration-after-statement -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm', optimize='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic', cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -Wdeclaration-after-statement -I/usr/local/include -I/usr/include/gdbm' ccversion='', gccversion='4.1.2 20071124 (Red Hat 4.1.2-41)', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='gcc', ldflags ='' libpth=/usr/local/lib64 /lib64 /usr/lib64 libs=-lresolv -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc perllibs=-lresolv -lnsl -ldl -lm -lcrypt -lutil -lpthread -lc libc=, so=so, useshrplib=true, libperl=libperl.so gnulibc_version='2.5' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi/CORE' cccdlflags='-fPIC', lddlflags='-shared -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic' Characteristics of this binary (from libperl): Compile-time options: MULTIPLICITY PERL_IMPLICIT_CONTEXT PERL_MALLOC_WRAP USE_64_BIT_ALL USE_64_BIT_INT USE_ITHREADS USE_LARGE_FILES USE_PERLIO USE_REENTRANT_API Built under linux Compiled at Sep 24 2008 11:41:17 @INC: /usr/lib64/perl5/site_perl/5.8.8/x86_64-linux-thread-multi /usr/lib64/perl5/site_perl/5.8.7/x86_64-linux-thread-multi /usr/lib64/perl5/site_perl/5.8.6/x86_64-linux-thread-multi /usr/lib64/perl5/site_perl/5.8.5/x86_64-linux-thread-multi /usr/lib/perl5/site_perl/5.8.8 /usr/lib/perl5/site_perl /usr/lib64/perl5/vendor_perl/5.8.8/x86_64-linux-thread-multi /usr/lib64/perl5/vendor_perl/5.8.7/x86_64-linux-thread-multi /usr/lib64/perl5/vendor_perl/5.8.6/x86_64-linux-thread-multi /usr/lib64/perl5/vendor_perl/5.8.5/x86_64-linux-thread-multi /usr/lib/perl5/vendor_perl/5.8.8 /usr/lib/perl5/vendor_perl/5.8.7 /usr/lib/perl5/vendor_perl/5.8.6 /usr/lib/perl5/vendor_perl/5.8.5 /usr/lib/perl5/vendor_perl /usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi /usr/lib/perl5/5.8.8 .

Fri Dec 04 11:19:14 2009 RSCHUPP [...] cpan.org - Taken

Fri Dec 04 11:24:50 2009 RSCHUPP [...] cpan.org - Correspondence added

I know what's goin' on: does a C construct like sprintf(stmpdir, "%s other %stuff", stmpdir, ...); (in mlydr/mktempdir.c) ring a bell? The big question is: this code has been in there for ages - why does it break now?

Fri Dec 04 11:24:50 2009 The RT System itself - Status changed from 'new' to 'open'

Sat Dec 05 06:41:48 2009 smueller [...] cpan.org - Correspondence added

Hi, On Fri Dec 04 11:24:50 2009, RSCHUPP wrote: Show quoted text

> I know what's goin' on: does a C construct like > > sprintf(stmpdir, "%s other %stuff", stmpdir, ...); > > (in mlydr/mktempdir.c) ring a bell? > The big question is: this code has been in there for ages - > why does it break now?

Quite honestly... I have no good idea. Looking at the differences to 0.991, there is some change to myldr/mktempdir.c: --- myldr/mktmpdir.c (revision 7467) +++ myldr/mktmpdir.c (local) @@ -160,6 +160,29 @@ if (progname == NULL) progname = argv[0]; + /* If invoked as "/usr/bin/parl foo.par myscript.pl" then progname should + * be ".../parl", and we don't want to base our checksum on that, but + * rather on "foo.par". + */ + { +#ifdef WIN32 +#define STREQ(a,b) (strcasecmp(a,b) == 0) +#else +#define STREQ(a,b) (strcmp(a,b) == 0) +#endif + int prog_len = strlen(progname); + int parl_len = strlen(PARL_EXE); + + if (prog_len >= parl_len + && STREQ(progname + prog_len - parl_len, PARL_EXE) + && (prog_len == parl_len || progname[prog_len - parl_len - 1] == dir_sep[0]) + && argv[1] + && strlen(argv[1]) >= 4 + && STREQ(argv[1] + strlen(argv[1]) - 4, ".par")) + progname = argv[1]; +#undef STREQ + } + if ( !par_env_clean() && (f = open( progname, O_RDONLY | OPEN_O_BINARY ))) { lseek(f, -18, 2); read(f, buf, 6); @@ -176,6 +199,7 @@ } else { /* "$TEMP/par-$USER/cache-$SHA1" */ + lseek(f, 0, 0); sha_init( &sha_info ); while( ( j = read( f, buf, sizeof( buf ) ) ) > 0 ) {

Sat Dec 05 12:46:58 2009 RSCHUPP [...] cpan.org - Correspondence added

On Sat Dec 05 06:41:48 2009, SMUELLER wrote: Show quoted text

> Quite honestly... I have no good idea. Looking at the differences to > 0.991, there is some change to myldr/mktempdir.c:

I know, it did these :) But I don't think they're responsible. I was able to reproduce Steven's problem on a machine running Novell SLES 10 64bit (with the system's Perl 5.8.8). In fact, already the build failed there, running myldir/static aborted with "creation of private temporary subdirectory /cache-XXX failed". In this environment the statement sprintf( stmpdir, "%s%scache-%s%s", stmpdir, dir_sep, sha1, subdirbuf_suffix ); (in the alternative headed by /* "$TEMP/par-$USER/cache-$SHA1" */) had the expected contents of stmpdir before the statement, but afterwards stmpdir didn't have dir_sep+sha1+subdirbuf_suffix appended (as that seems to be the intent of the author), but stmpdir had been replaced by the latter. On the other hand, this works as intended on e.g. Debian sid, with Perl 5.10.1 (and probably lots of other systems, otherwise cpantesters results would be mostly in the red). Now my old C99 standard clearly states that you should not expect any particular outcome from the above since the output buffer overlaps with some input arguments. In fact, a few lines above this statement, we find the following comment: /* stmpdir is what we are going to return stmpdir2 is the top $TEMP/par-$USER, needed to build stmpdir. We need 2 buffers because snprintf() can't write to a buffer it's reading from. */ Unfortunately, this advice is not heeded and stmpdir2 isn't found in the code except for this comment. I'll post a fix along these lines shortly, but I'm still puzzled that this hasn't shown up earlier. All three alternatives to compute stmpdir in par_mktmpdir() use the same broken call to snprintf. And this code has been there for a long time. Maybe it's dependent on a particular implementation of snprintf? Steven's post shows (from "perl -V") gnulibc_version='2.5' I'm currently away from the machine I was able to reproduce it on, hence can't doublecheck. Unfortunately I don't see an easy way to extract this information from the existing test reports (i.e. without going over each one manually). Cheers, Roderich

Mon Dec 07 11:02:34 2009 RSCHUPP [...] cpan.org - Correspondence added

Steven, can you try the attached patch? It corrects the broken usage of sprintf in myldr/mktmpdir.c.

--- PAR-Packer-1.001/myldr/mktmpdir.c 2009-11-13 09:29:29.000000000 +0100 +++ PAR-Packer-1.001/myldr/mktmpdir.c.fixed 2009-12-06 17:38:29.000000000 +0100 @@ -65,7 +65,7 @@ const char *subdirbuf_suffix = ""; char *progname = NULL, *username = NULL; - char *stmpdir = NULL; + char *stmpdir = NULL, *stmpdir2 = NULL; int f, j, k, stmp_len = 0; char sha1[41]; SHA_INFO sha_info; @@ -150,8 +150,9 @@ need 2 buffers because snprintf() can't write to a buffer it's reading from. */ stmpdir = malloc( stmp_len ); - sprintf(stmpdir, "%s%s%s%s", tmpdir, dir_sep, subdirbuf_prefix, username); - my_mkdir(stmpdir, 0755); + stmpdir2 = malloc( stmp_len ); + sprintf(stmpdir2, "%s%s%s%s", tmpdir, dir_sep, subdirbuf_prefix, username); + my_mkdir(stmpdir2, 0755); /* Doesn't really work - XXX */ val = (char *)par_getenv( "PATH" ); @@ -194,7 +195,7 @@ sprintf( stmpdir, "%s%scache-%s%s", - stmpdir, dir_sep, buf, subdirbuf_suffix + stmpdir2, dir_sep, buf, subdirbuf_suffix ); } else { @@ -215,13 +216,12 @@ sprintf( stmpdir, "%s%scache-%s%s", - stmpdir, dir_sep, sha1, subdirbuf_suffix + stmpdir2, dir_sep, sha1, subdirbuf_suffix ); } } else { int i = 0; - size_t len = strlen(stmpdir); /* "$TEMP/par-$USER/temp-$PID" */ @@ -229,7 +229,7 @@ sprintf( stmpdir, "%s%stemp-%u%s", - stmpdir, dir_sep, getpid(), subdirbuf_suffix + stmpdir2, dir_sep, getpid(), subdirbuf_suffix ); /* Ensure we pick an unused directory each time. If the directory @@ -239,15 +239,16 @@ might interfere. */ while (my_mkdir(stmpdir, 0755) == -1 && errno == EEXIST) { - stmpdir[len] = 0; sprintf( stmpdir, "%s%stemp-%u-%u%s", - stmpdir, dir_sep, getpid(), ++i, subdirbuf_suffix + stmpdir2, dir_sep, getpid(), ++i, subdirbuf_suffix ); } } + free(stmpdir2); + /* set dynamic loading path */ par_setenv(PAR_TEMP, stmpdir);

Fri Dec 11 18:40:37 2009 IRONCAMEL [...] cpan.org - Correspondence added

I got the same error on my local ubuntu machine and several redhat servers we have. I fixed it by changing one line in a file in the 't' directory called test-proc: $ diff test-proc.orig test-proc 16c16 < print "PAR_TEMP = $ENV{PAR_TEMP}\n"; --- Show quoted text

> print "PAR_TEMP = $ENV{PAR_GLOBAL_TMPDIR}\n";

Is this a good solution?

Sun Dec 13 03:06:27 2009 IRONCAMEL [...] cpan.org - Correspondence added

On Fri Dec 11 18:40:37 2009, IRONCAMEL wrote: Show quoted text

> I got the same error on my local ubuntu machine and several redhat > servers we have. I fixed it by changing one line in a file in the 't' > directory called test-proc: > > $ diff test-proc.orig test-proc > 16c16 > < print "PAR_TEMP = $ENV{PAR_TEMP}\n"; > ---

> > print "PAR_TEMP = $ENV{PAR_GLOBAL_TMPDIR}\n";

> > Is this a good solution?

By the way, the tests fail even when I run them as root, because t/30-current_exec.t checks to see if the tmp dir was correctly set in the environment, which it wasn't. The above patch made the tests pass.

Wed Dec 16 11:17:59 2009 SWMCD [...] cpan.org - Correspondence added

On Mon Dec 07 11:02:34 2009, RSCHUPP wrote: Show quoted text

> Steven, can you try the attached patch? > > It corrects the broken usage of sprintf in myldr/mktmpdir.c.

This patch fixes the problem on our system.

Thu Dec 17 05:41:10 2009 RSCHUPP [...] cpan.org - Correspondence added

On Wed Dec 16 11:17:59 2009, SWMCD wrote: Show quoted text

> On Mon Dec 07 11:02:34 2009, RSCHUPP wrote:

> > Steven, can you try the attached patch? > > > > It corrects the broken usage of sprintf in myldr/mktmpdir.c.

> > This patch fixes the problem on our system.

Thanks for testing. Patch committed, will be in the next release.

Thu Dec 17 05:41:26 2009 RSCHUPP [...] cpan.org - Status changed from 'open' to 'resolved'

Thu Dec 17 08:13:34 2009 smueller [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #52407] pp creates par-<user>/ in /, not /tmp
Date:	Thu, 17 Dec 2009 14:13:05 +0100
To:	bug-PAR-Packer [...] rt.cpan.org
From:	Steffen Mueller <smueller [...] cpan.org>

RSCHUPP via RT wrote: Show quoted text

> Queue: PAR-Packer > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=52407 > > > On Wed Dec 16 11:17:59 2009, SWMCD wrote:

>> On Mon Dec 07 11:02:34 2009, RSCHUPP wrote:

>>> Steven, can you try the attached patch? >>> >>> It corrects the broken usage of sprintf in myldr/mktmpdir.c.

>> This patch fixes the problem on our system.

> > Thanks for testing. Patch committed, will be in the next release.

... which I'll try to push out ASAP, but not today. Thanks for fixing this particularly ugly issue. Best regards, Steffen

Thu Dec 17 08:13:35 2009 The RT System itself - Status changed from 'resolved' to 'open'

Thu Dec 17 16:01:48 2009 smueller [...] cpan.org - Correspondence added

Fixed PAR::Packer 1.002 was just uploaded to PAUSE. Thanks for reporting (and fixing!) this issue.

Thu Dec 17 16:01:50 2009 smueller [...] cpan.org - Status changed from 'open' to 'resolved'