From 59616d58214b46e535e17ef3b582a4ea0beab033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= Date: Wed, 3 Apr 2019 16:14:26 +0200 Subject: [PATCH] Fix a memory leak when croaking about a too deep recursion --- ....173-Data-Dumper-avoid-leak-on-croak.patch | 151 ++++++++++++++++++ perl-Data-Dumper.spec | 9 +- 2 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 Data-Dumper-2.173-Data-Dumper-avoid-leak-on-croak.patch diff --git a/Data-Dumper-2.173-Data-Dumper-avoid-leak-on-croak.patch b/Data-Dumper-2.173-Data-Dumper-avoid-leak-on-croak.patch new file mode 100644 index 0000000..d51ada9 --- /dev/null +++ b/Data-Dumper-2.173-Data-Dumper-avoid-leak-on-croak.patch @@ -0,0 +1,151 @@ +From d9c4b4ae5a1a17347ff5e3ecbf8e1d9da481f476 Mon Sep 17 00:00:00 2001 +From: David Mitchell +Date: Wed, 3 Apr 2019 13:23:24 +0100 +Subject: [PATCH] Data::Dumper - avoid leak on croak +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +v5.21.3-742-g19be3be696 added a facility to Dumper.xs to croak if the +recursion level became too deep (1000 by default). + +The trouble with this is that various parts of DD_dump() allocate +temporary SVs and buffers, which will leak if DD_dump() unceremoniously +just croaks(). + +This currently manifests as dist/Data-Dumper/t/recurse.t failing under +Address Sanitiser. + +This commit makes the depth checking code just set a sticky 'too deep' +boolean flag, and +a) on entry, DD_dump() just returns immediately if the flag is set; +b) the flag is checked by the top-level called of DD_dump() and croaks +if set. + +So the net effect is to defer croaking until the dump is complete, +and avoid any further recursion once the flag is set. + +This is a bit of a quick fix. More long-term solutions would be to +convert DD_dump() to be iterative rather than recursive, and/or make +sure all temporary SVs and buffers are suitably anchored somewhere so +that they get cleaned up on croak. + +Petr Písař: Ported from 6d65cb5d847ac93680949c4fa02111808207fbdc in +perl git tree. + +Signed-off-by: Petr Písař +--- + Dumper.pm | 2 +- + Dumper.xs | 27 ++++++++++++++++++++------- + 2 files changed, 21 insertions(+), 8 deletions(-) + +diff --git a/Dumper.pm b/Dumper.pm +index 40aeb7d..06af4c4 100644 +--- a/Dumper.pm ++++ b/Dumper.pm +@@ -1461,7 +1461,7 @@ be to use the C filter of Data::Dumper. + + Gurusamy Sarathy gsar@activestate.com + +-Copyright (c) 1996-2017 Gurusamy Sarathy. All rights reserved. ++Copyright (c) 1996-2019 Gurusamy Sarathy. All rights reserved. + This program is free software; you can redistribute it and/or + modify it under the same terms as Perl itself. + +diff --git a/Dumper.xs b/Dumper.xs +index 7f0b027..a324cb6 100644 +--- a/Dumper.xs ++++ b/Dumper.xs +@@ -61,9 +61,10 @@ + #endif + + /* This struct contains almost all the user's desired configuration, and it +- * is treated as constant by the recursive function. This arrangement has +- * the advantage of needing less memory than passing all of them on the +- * stack all the time (as was the case in an earlier implementation). */ ++ * is treated as mostly constant (except for maxrecursed) by the recursive ++ * function. This arrangement has the advantage of needing less memory ++ * than passing all of them on the stack all the time (as was the case in ++ * an earlier implementation). */ + typedef struct { + SV *pad; + SV *xpad; +@@ -74,6 +75,7 @@ typedef struct { + SV *toaster; + SV *bless; + IV maxrecurse; ++ bool maxrecursed; /* at some point we exceeded the maximum recursion level */ + I32 indent; + I32 purity; + I32 deepcopy; +@@ -97,7 +99,7 @@ static bool safe_decimal_number(const char *p, STRLEN len); + static SV *sv_x (pTHX_ SV *sv, const char *str, STRLEN len, I32 n); + static I32 DD_dump (pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, + HV *seenhv, AV *postav, const I32 level, SV *apad, +- const Style *style); ++ Style *style); + + #ifndef HvNAME_get + #define HvNAME_get HvNAME +@@ -615,7 +617,7 @@ deparsed_output(pTHX_ SV *val) + */ + static I32 + DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv, +- AV *postav, const I32 level, SV *apad, const Style *style) ++ AV *postav, const I32 level, SV *apad, Style *style) + { + char tmpbuf[128]; + Size_t i; +@@ -642,6 +644,9 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv, + if (!val) + return 0; + ++ if (style->maxrecursed) ++ return 0; ++ + /* If the output buffer has less than some arbitrary amount of space + remaining, then enlarge it. For the test case (25M of output), + *1.1 was slower, *2.0 was the same, so the first guess of 1.5 is +@@ -793,7 +798,7 @@ DD_dump(pTHX_ SV *val, const char *name, STRLEN namelen, SV *retval, HV *seenhv, + } + + if (style->maxrecurse > 0 && level >= style->maxrecurse) { +- croak("Recursion limit of %" IVdf " exceeded", style->maxrecurse); ++ style->maxrecursed = TRUE; + } + + if (realpack && !no_bless) { /* we have a blessed ref */ +@@ -1528,6 +1533,7 @@ Data_Dumper_Dumpxs(href, ...) + style.indent = 2; + style.quotekeys = 1; + style.maxrecurse = 1000; ++ style.maxrecursed = FALSE; + style.purity = style.deepcopy = style.useqq = style.maxdepth + = style.use_sparse_seen_hash = style.trailingcomma = 0; + style.pad = style.xpad = style.sep = style.pair = style.sortkeys +@@ -1675,7 +1681,7 @@ Data_Dumper_Dumpxs(href, ...) + DD_dump(aTHX_ val, SvPVX_const(name), SvCUR(name), valstr, seenhv, + postav, 0, newapad, &style); + SPAGAIN; +- ++ + if (style.indent >= 2 && !terse) + SvREFCNT_dec(newapad); + +@@ -1715,6 +1721,13 @@ Data_Dumper_Dumpxs(href, ...) + } + SvREFCNT_dec(postav); + SvREFCNT_dec(valstr); ++ ++ /* we defer croaking until here so that temporary SVs and ++ * buffers won't be leaked */ ++ if (style.maxrecursed) ++ croak("Recursion limit of %" IVdf " exceeded", ++ style.maxrecurse); ++ + } + else + croak("Call to new() method failed to return HASH ref"); +-- +2.20.1 + diff --git a/perl-Data-Dumper.spec b/perl-Data-Dumper.spec index 730d10e..a10215b 100644 --- a/perl-Data-Dumper.spec +++ b/perl-Data-Dumper.spec @@ -1,10 +1,13 @@ Name: perl-Data-Dumper Version: 2.173 -Release: 2%{?dist} +Release: 3%{?dist} Summary: Stringify perl data structures, suitable for printing and eval License: GPL+ or Artistic URL: https://metacpan.org/release/Data-Dumper Source0: https://cpan.metacpan.org/authors/id/X/XS/XSAWYERX/Data-Dumper-%{version}.tar.gz +# Fix a memory leak when croaking about a too deep recursion, +# fixed in perl after 5.29.9 +Patch0: Data-Dumper-2.173-Data-Dumper-avoid-leak-on-croak.patch BuildRequires: findutils BuildRequires: gcc BuildRequires: make @@ -52,6 +55,7 @@ structures correctly. %prep %setup -q -n Data-Dumper-%{version} +%patch0 -p1 %build perl Makefile.PL INSTALLDIRS=vendor NO_PACKLIST=1 OPTIMIZE="$RPM_OPT_FLAGS" @@ -74,6 +78,9 @@ make test %{_mandir}/man3/* %changelog +* Wed Apr 03 2019 Petr Pisar - 2.173-3 +- Fix a memory leak when croaking about a too deep recursion + * Fri Feb 01 2019 Fedora Release Engineering - 2.173-2 - Rebuilt for https://fedoraproject.org/wiki/Fedora_30_Mass_Rebuild