Skip Menu |

This queue is for tickets about the version CPAN distribution.

Report information
The Basics
Id: 91868
Status: resolved
Priority: 0/
Queue: version
People
Owner: Nobody in particular
Requestors: BULKDD [...] cpan.org
Cc:
AdminCc:
Bug Information
Severity: (no value)
Broken in: 0.9906
Fixed in: 0.9906

History Show all quoted text
  Sat Jan 04 22:41:43 2014 BULKDD [...] cpan.org - Ticket created
Subject: char * leak if fatal warnings are on and misc in Perl_upg_version
In Perl_upg_version at https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.c?at=default#cl-662 a warning check is done, if fatal warnings are on, "Safefree(version);" will never be called. Probably save stack should be used instead. Also same style leak at https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.c?at=default#cl-662 . Another issue, I'm not sure what your policy is on GETMAGIC, but at https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.c?at=default#cl-662 (" if ( SvNOK(ver) && !( SvPOK(ver) && sv_len(ver) == 3 ) )") flags are checked before any get magic call. Also on that line, why is sv_len (a function) called instead of SvCUR (a function call free macro) if its SvPOK?
  Sat Jan 04 23:34:42 2014 jpeacock [...] cpan.org - Correspondence added
Is this better? https://bitbucket.org/jpeacock/version/commits/7a76ab3bbf84e519ee9da878734d999db27be6da?at=default I removed all usage of Safefree() and paired any use of savepvn() with a SAVEFREEPV() call. That should free only the malloc'd strings that the version code itself creates, right?
  Sat Jan 04 23:34:42 2014 The RT System itself - Status changed from 'new' to 'open'
  Sat Jan 04 23:41:16 2014 jpeacock [...] cpan.org - Correspondence added
On Sat Jan 04 22:41:43 2014, BULKDD wrote: Show quoted text
> Another issue, I'm not sure what your policy is on GETMAGIC, but at > https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.c?at=default#cl- > 662 (" if ( SvNOK(ver) && !( SvPOK(ver) && sv_len(ver) == 3 ) )") > flags are checked before any get magic call.
That code path will never get called directly on a magic variable; it will always be a copy of some sort which will trigger any magic when copied. I did make the change to SvCUR as you suggested.
  Sun Jan 05 00:46:13 2014 BULKDD [...] cpan.org - Correspondence added
On Sat Jan 04 23:34:42 2014, JPEACOCK wrote: Show quoted text
> Is this better? > > https://bitbucket.org/jpeacock/version/commits/7a76ab3bbf84e519ee9da878734d999db27be6da?at=default > > I removed all usage of Safefree() and paired any use of savepvn() with > a SAVEFREEPV() call. That should free only the malloc'd strings that > the version code itself creates, right?
Correct. If its a char * coming in on C stack from caller, its not yours to free unless it is documented very very carefully. If its an existing function call, cpan grep should be checked and exterminated for bad usage (passing a non-freeable char *) of the call, or a new function call created. A little list of typically seen memory allocators in a process I've made up over time "Windows has many memory allocators, each CRT (VS 2, 3, 4, 5, NT/6, 7.0, 7.1, 8, 9, 10) malloc, LocalAlloc, GlobalAlloc, HeapAlloc, (each version of C++ Runtime Library) "new", CoGetMalloc, CoTaskMemAlloc, NetApiBufferAllocate, VirtualAlloc, CryptMemAlloc, AllocADsMem, SHAlloc, SnmpUtilMemAlloc." , I'll also add SysAllocString and SHGetMalloc to that list, and perl's own internal, win32_malloc, Perl_malloc, Perl_safesysmalloc, PerlMem_malloc, PerlMemShared_malloc, PerlMemParse_malloc, CPerlHost::Malloc, CPerlHost::MallocShared, CPerlHost::MallocParse, and VMem::Malloc. Which of those 10 functions Newx maps to (if any) is none of an XS authors business (unless they want pain).
  Mon Jan 06 01:13:55 2014 BULKDD [...] cpan.org - Correspondence added
On Sat Jan 04 23:34:42 2014, JPEACOCK wrote: Show quoted text
> Is this better? > > https://bitbucket.org/jpeacock/version/commits/7a76ab3bbf84e519ee9da878734d999db27be6da?at=default > > I removed all usage of Safefree() and paired any use of savepvn() with > a SAVEFREEPV() call. That should free only the malloc'd strings that > the version code itself creates, right?
https://bitbucket.org/jpeacock/version/src/649e700fadaef7d06b1f8f3252cd8d596d52ec8b/vutil/vutil.c?at=default#cl-602 missed one, this is beyond help, I'll start writing code myself
  Mon Jan 13 06:45:07 2014 jpeacock [...] cpan.org - Correspondence added
Fixed in 0.9906
  Mon Jan 13 06:45:07 2014 jpeacock [...] cpan.org - Status changed from 'open' to 'resolved'
  Mon Jan 13 06:45:07 2014 jpeacock [...] cpan.org - Fixed in 0.9906 added