Re: coverity cherrypick candidates for 5.20.1

On 22 July 2014 01:28, Tony Cook <t...@develop-help.com> wrote:
> On Mon, Jul 21, 2014 at 09:04:02AM +0100, Steve Hay wrote:>> On 19 July 2014 22:21, Jarkko Hietaniemi <j...@iki.fi> wrote:>> > I went through the Coverity-induced changes and found the following that>> > might be suitable for 5.20.1.  One of them was second-order change>> > (c3caa5c), and one a fixup (d4825b2), the rest were first-order>> > (Coverity-caused). Links, quick descriptions (not always the first line of>> > commit message, since I suck at commit messages), and quick categorization.>> >>> > http://perl5.git.perl.org/perl.git/commitdiff/00b25eff1415ab27d5829f30fea1fecd57a7934e>> > | si_names access one past the end. | security (off-by-one)>> +1>>> >>> >>> > http://perl5.git.perl.org/perl.git/commitdiff/0ac78434bf60027f078daa1108d6c7bcec1ad6e4>> > | Leaked string in failure path. | security (off-by-one)>> +1>>> >>> > http://perl5.git.perl.org/perl.git/commitdiff/adc2d0c9de764f1cb892860df8ecc93dc8909b39>> > | Overrunning array PL_reg_intflags | security (off-by-one)>> +1>>> >>> > http://perl5.git.perl.org/perl.git/commitdiff/53673d98756218ddd125311548c0f73c714722f7>> > | Overrunning array anyofs | security (off-by-one)>> +1>>> >>> > http://perl5.git.perl.org/perl.git/commitdiff/d37662c0fabaaa893c0d695034fa83b9235c6872>> > | Passing freed pointer | security (use-after-free)>> +1>>> > http://perl5.git.perl.org/perl.git/commitdiff/c3caa5c3bdbd0ad0bc7ce5e7cd1a8eb5b7ca6a69>> > | Use the C_ARRAY_LENGTH | security/portability (C_ARRAY_LENGTH)>> +1>>> > http://perl5.git.perl.org/perl.git/commitdiff/60f7fc1ea42054e92f34b4ce9d608efd14357392>> > | Calling mkstemp() without securely setting umask first. | security (umask)>> I'm not so sure about this one.  In my comment on the original ticket> I suggested there was a race condition where you could be changing the> umask under another thread:>>   T1: umask(0600)>   T2: create some file with unexpected permissions>   T1: umask(original)>> I can see another possible bad interaction:>>   T1: umask(0600)>   T2: umask(0600)>   T1: umask(original)>   T2: create file (with the wrong permissions)>> (T1 is the thread calling PerlIO_tmpfile() in both cases)>> That said, I don't know of a better solution.>>> > http://perl5.git.perl.org/perl.git/commitdiff/e76fdebf5815ffaf53ebcfd2c7b78b0e9eacbfd2>> > | Do not invert a NULL cp_list. | security (potential NULL deref)>> +1>>> > http://perl5.git.perl.org/perl.git/commitdiff/b35b96b6f8e35207d18b15dfcdbd0d08a7c6437c>> > | Uninitialized tmbuf. | security (uninit)>> +1>> This doesn't add two new warnings, it just documents and tests for> them, if I understand the code correctly.>>> > http://perl5.git.perl.org/perl.git/commitdiff/9b56a01971980348bbaf5753e47fcb59dee1ef49>> > | Using uninitialized value slen | security (uninit)>> +1>>> > http://perl5.git.perl.org/perl.git/commitdiff/0a20f69bae04ff02616da2f0128de4e842151093>> > | Handle variable fd | fd leak>> +1>>> >>> > (d4825b builds on 3ed3a8)>> >>> > http://perl5.git.perl.org/perl.git/commitdiff/3ed3a8afebd64616aef147205403b96b30a4b4ee>> > | add va_end() calls | portability>> +1>>> >>> > http://perl5.git.perl.org/perl.git/commitdiff/d4825b278e28006bdc9c3f36ab174eade62d6c4c>> > | put va_end() in the right place | portability>> +1 (necessary if 3ed3a8 included)>>> > http://perl5.git.perl.org/perl.git/commitdiff/b00d10dc3d9b54e2ef58f2627b02bfe99daeae47>> > | perlfunc chdir | doc>> +1>>> >>> >>>>> Many thanks for going through these for me. It's a great help.>>>> +1 to all of these from me, EXCEPT for:>>>> http://perl5.git.perl.org/perl.git/commitdiff/b35b96b6f8e35207d18b15dfcdbd0d08a7c6437c>> | Uninitialized tmbuf. | security (uninit)>>>> because that adds new warnings, which perlpolicy does not allow for maint.>> See my note above.>

Thanks. All now cherry-picked, including b35b96b6f8 (sorry I didn't
look closely enough to notice the warnings weren't new), except for
60f7fc1ea4 which you weren't sure about, and the following four which
it turns out were committed before 5.20.0 was made and are hence in
that release already anyway:

http://perl5.git.perl.org/perl.git/commitdiff/adc2d0c9de764f1cb892860df8ecc93dc8909b39
| Overrunning array PL_reg_intflags | security (off-by-one)
http://perl5.git.perl.org/perl.git/commitdiff/53673d98756218ddd125311548c0f73c714722f7
| Overrunning array anyofs | security (off-by-one)
http://perl5.git.perl.org/perl.git/commitdiff/d37662c0fabaaa893c0d695034fa83b9235c6872
| Passing freed pointer | security (use-after-free)
http://perl5.git.perl.org/perl.git/commitdiff/b00d10dc3d9b54e2ef58f2627b02bfe99daeae47
| perlfunc chdir | doc