Fix a memory leak in package name lookup
This commit is contained in:
		
							parent
							
								
									4c2649c4bd
								
							
						
					
					
						commit
						3bd28c15ea
					
				| @ -0,0 +1,54 @@ | ||||
| From 06cbc317229e882f379e75eb3adf7cf9c071febd Mon Sep 17 00:00:00 2001 | ||||
| From: David Mitchell <davem@iabyn.com> | ||||
| Date: Wed, 3 Apr 2019 11:06:22 +0100 | ||||
| Subject: [PATCH] Fix recent double free in S_parse_gv_stash_name() | ||||
| MIME-Version: 1.0 | ||||
| Content-Type: text/plain; charset=UTF-8 | ||||
| Content-Transfer-Encoding: 8bit | ||||
| 
 | ||||
| RT #133977 | ||||
| 
 | ||||
| My recent commit v5.29.9-29-g657ed7c1c1 moved all buffer freeing to | ||||
| the end of the function, but missed removing one of the existing frees. | ||||
| 
 | ||||
| The problem was spotted by James E Keenan and diagnosed by Tony Cook; I just | ||||
| added a test. | ||||
| 
 | ||||
| A simple reproducer is | ||||
| 
 | ||||
| my $def = defined *{"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'x"}; | ||||
| 
 | ||||
| Signed-off-by: Petr Písař <ppisar@redhat.com> | ||||
| ---
 | ||||
|  gv.c                  | 1 - | ||||
|  t/op/stash_parse_gv.t | 2 +- | ||||
|  2 files changed, 1 insertion(+), 2 deletions(-) | ||||
| 
 | ||||
| diff --git a/gv.c b/gv.c
 | ||||
| index 61085f5c53..3b8759e88a 100644
 | ||||
| --- a/gv.c
 | ||||
| +++ b/gv.c
 | ||||
| @@ -1665,7 +1665,6 @@ S_parse_gv_stash_name(pTHX_ HV **stash, GV **gv, const char **name,
 | ||||
|                  gvp = (GV**)hv_fetch(*stash, key, is_utf8 ? -((I32)*len) : (I32)*len, add); | ||||
|                  *gv = gvp ? *gvp : NULL; | ||||
|                  if (!*gv || *gv == (const GV *)&PL_sv_undef) { | ||||
| -                    Safefree(tmpfullbuf); /* free our tmpfullbuf if it was used */
 | ||||
|                      goto notok; | ||||
|                  } | ||||
|                  /* here we know that *gv && *gv != &PL_sv_undef */ | ||||
| diff --git a/t/op/stash_parse_gv.t b/t/op/stash_parse_gv.t
 | ||||
| index 05694ca8ce..bd9e95cf37 100644
 | ||||
| --- a/t/op/stash_parse_gv.t
 | ||||
| +++ b/t/op/stash_parse_gv.t
 | ||||
| @@ -23,7 +23,7 @@ foreach my $t (@tests) {
 | ||||
|      my ( $sub, $name ) = @$t; | ||||
|   | ||||
|      fresh_perl_is( | ||||
| -        qq[sub $sub { print qq[ok\n]} &{"$sub"} ],
 | ||||
| +        qq[sub $sub { print qq[ok\n]} &{"$sub"}; my \$d = defined *{"foo$sub"} ],
 | ||||
|          q[ok], | ||||
|          { switches => ['-w'] }, | ||||
|          $name | ||||
| -- 
 | ||||
| 2.20.1 | ||||
| 
 | ||||
							
								
								
									
										73
									
								
								perl-5.29.9-fix-leak-in-package-name-lookup.patch
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										73
									
								
								perl-5.29.9-fix-leak-in-package-name-lookup.patch
									
									
									
									
									
										Normal file
									
								
							| @ -0,0 +1,73 @@ | ||||
| From 657ed7c1c190e7fad1bac2979944d07245bbeea4 Mon Sep 17 00:00:00 2001 | ||||
| From: David Mitchell <davem@iabyn.com> | ||||
| Date: Tue, 26 Mar 2019 08:56:55 +0000 | ||||
| Subject: [PATCH] fix leak in package name lookup | ||||
| MIME-Version: 1.0 | ||||
| Content-Type: text/plain; charset=UTF-8 | ||||
| Content-Transfer-Encoding: 8bit | ||||
| 
 | ||||
| S_parse_gv_stash_name() mallocs a temporary buffer when scanning package | ||||
| names longer than 64 bytes. Depending on how it exits the function, it | ||||
| doesn't always free the buffer afterwards. Change the function so that | ||||
| there are only two exit points (which free the buffer) and make other bits | ||||
| of code goto those two points. | ||||
| 
 | ||||
| Can be reproduced with e.g. | ||||
| 
 | ||||
| &{"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'x"} | ||||
| 
 | ||||
| Similar code is already present in t/op/stash_parse_gv.t | ||||
| 
 | ||||
| Signed-off-by: Petr Písař <ppisar@redhat.com> | ||||
| ---
 | ||||
|  gv.c | 13 +++++++++---- | ||||
|  1 file changed, 9 insertions(+), 4 deletions(-) | ||||
| 
 | ||||
| diff --git a/gv.c b/gv.c
 | ||||
| index ae7f2aa422..61085f5c53 100644
 | ||||
| --- a/gv.c
 | ||||
| +++ b/gv.c
 | ||||
| @@ -1636,7 +1636,7 @@ S_parse_gv_stash_name(pTHX_ HV **stash, GV **gv, const char **name,
 | ||||
|              if (!*stash) | ||||
|                  *stash = PL_defstash; | ||||
|              if (!*stash || !SvREFCNT(*stash)) /* symbol table under destruction */ | ||||
| -                return FALSE;
 | ||||
| +                goto notok;
 | ||||
|   | ||||
|              *len = name_cursor - *name; | ||||
|              if (name_cursor > nambeg) { /* Skip for initial :: or ' */ | ||||
| @@ -1666,7 +1666,7 @@ S_parse_gv_stash_name(pTHX_ HV **stash, GV **gv, const char **name,
 | ||||
|                  *gv = gvp ? *gvp : NULL; | ||||
|                  if (!*gv || *gv == (const GV *)&PL_sv_undef) { | ||||
|                      Safefree(tmpfullbuf); /* free our tmpfullbuf if it was used */ | ||||
| -                    return FALSE;
 | ||||
| +                    goto notok;
 | ||||
|                  } | ||||
|                  /* here we know that *gv && *gv != &PL_sv_undef */ | ||||
|                  if (SvTYPE(*gv) != SVt_PVGV) | ||||
| @@ -1707,15 +1707,20 @@ S_parse_gv_stash_name(pTHX_ HV **stash, GV **gv, const char **name,
 | ||||
|  			    MUTABLE_HV(SvREFCNT_inc_simple(PL_defstash)); | ||||
|  		    } | ||||
|  		} | ||||
| -                Safefree(tmpfullbuf); /* free our tmpfullbuf if it was used */
 | ||||
| -                return TRUE;
 | ||||
| +                goto ok;
 | ||||
|              } | ||||
|          } | ||||
|      } | ||||
|      *len = name_cursor - *name; | ||||
| +  ok:
 | ||||
| +    Safefree(tmpfullbuf); /* free our tmpfullbuf if it was used */
 | ||||
|      return TRUE; | ||||
| +  notok:
 | ||||
| +    Safefree(tmpfullbuf); /* free our tmpfullbuf if it was used */
 | ||||
| +    return FALSE;
 | ||||
|  } | ||||
|   | ||||
| +
 | ||||
|  /* Checks if an unqualified name is in the main stash */ | ||||
|  PERL_STATIC_INLINE bool | ||||
|  S_gv_is_in_main(pTHX_ const char *name, STRLEN len, const U32 is_utf8) | ||||
| -- 
 | ||||
| 2.20.1 | ||||
| 
 | ||||
| @ -295,6 +295,10 @@ Patch64:        perl-5.29.9-fix-leak-with-local-WARNING_BITS.patch | ||||
| # in upstream after 5.29.9 | ||||
| Patch65:        perl-5.28.1-fix-a-leak-with-indented-heredocs.patch | ||||
| 
 | ||||
| # Fix a memory leak in package name lookup, RT#133977, in upstream after 5.29.9 | ||||
| Patch66:        perl-5.29.9-fix-leak-in-package-name-lookup.patch | ||||
| Patch67:        perl-5.29.9-Fix-recent-double-free-in-S_parse_gv_stash_name.patch | ||||
| 
 | ||||
| # Link XS modules to libperl.so with EU::CBuilder on Linux, bug #960048 | ||||
| Patch200:       perl-5.16.3-Link-XS-modules-to-libperl.so-with-EU-CBuilder-on-Li.patch | ||||
| 
 | ||||
| @ -2916,6 +2920,8 @@ Perl extension for Version Objects | ||||
| %patch63 -p1 | ||||
| %patch64 -p1 | ||||
| %patch65 -p1 | ||||
| %patch66 -p1 | ||||
| %patch67 -p1 | ||||
| %patch200 -p1 | ||||
| %patch201 -p1 | ||||
| 
 | ||||
| @ -2973,6 +2979,7 @@ perl -x patchlevel.h \ | ||||
|     'Fedora Patch63: Fix a memory leak when assigning a regular expression to a non-copy-on-write string' \ | ||||
|     'Fedora Patch64: Fix a memory leak when assignig to a localized ${^WARNING_BITS}' \ | ||||
|     'Fedora Patch65: Fix a memory leak when parsing misindented here-documents' \ | ||||
|     'Fedora Patch66: Fix a memory leak in package name lookup (RT#133977)' \ | ||||
|     'Fedora Patch200: Link XS modules to libperl.so with EU::CBuilder on Linux' \ | ||||
|     'Fedora Patch201: Link XS modules to libperl.so with EU::MM on Linux' \ | ||||
|     %{nil} | ||||
| @ -5272,6 +5279,7 @@ popd | ||||
| - Fix a memory leak when assigning a regular expression to a non-copy-on-write string | ||||
| - Fix a memory leak when assignig to a localized ${^WARNING_BITS} | ||||
| - Fix a memory leak when parsing misindented here-documents | ||||
| - Fix a memory leak in package name lookup (RT#133977) | ||||
| 
 | ||||
| * Tue Mar 05 2019 Björn Esser <besser82@fedoraproject.org> - 4:5.28.1-434 | ||||
| - Add explicit Requires: libxcrypt-devel to devel sub-package (bug #1666098) | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user