From 6a620bdc9d4b87705db6804cb8dc377da4853529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= Date: Wed, 10 Jan 2018 13:15:23 +0100 Subject: [PATCH] Avoid undefined behavior when copying memory in Glob and pp_caller (RT#131746) --- ...void-undefined-behaviour-in-Copy-etc.patch | 107 ++++++++++++++++++ ...s-of-.-will-always-evaluate-as-.-war.patch | 60 ++++++++++ perl.spec | 9 ++ 3 files changed, 176 insertions(+) create mode 100644 perl-5.26.1-perl-131746-avoid-undefined-behaviour-in-Copy-etc.patch create mode 100644 perl-5.27.3-avoid-the-address-of-.-will-always-evaluate-as-.-war.patch diff --git a/perl-5.26.1-perl-131746-avoid-undefined-behaviour-in-Copy-etc.patch b/perl-5.26.1-perl-131746-avoid-undefined-behaviour-in-Copy-etc.patch new file mode 100644 index 0000000..d6471a8 --- /dev/null +++ b/perl-5.26.1-perl-131746-avoid-undefined-behaviour-in-Copy-etc.patch @@ -0,0 +1,107 @@ +From 7a962424149cc60f3a187d0213a12689dd5e806b Mon Sep 17 00:00:00 2001 +From: Tony Cook +Date: Mon, 14 Aug 2017 11:52:39 +1000 +Subject: [PATCH] (perl #131746) avoid undefined behaviour in Copy() etc +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +These functions depend on C library functions which have undefined +behaviour when passed NULL pointers, even when passed a zero 'n' value. + +Some compilers use this information, ie. assume the pointers are +non-NULL when optimizing any following code, so we do need to +prevent such unguarded calls. + +My initial thought was to add conditionals to each macro to skip the +call to the library function when n is zero, but this adds a cost to +every use of these macros, even when the n value is always true. + +So instead I added asserts() which will give us a much more visible +indicator of such broken code and revealed the pp_caller and Glob.xs +issues also patched here. + +Petr Písař: Ported to 5.26.1 from +f14cf3632059d421de83cf901c7e849adc1fcd03. + +Signed-off-by: Petr Písař +--- + ext/File-Glob/Glob.xs | 2 +- + handy.h | 14 +++++++------- + pp_ctl.c | 3 ++- + pp_hot.c | 3 ++- + 4 files changed, 12 insertions(+), 10 deletions(-) + +diff --git a/ext/File-Glob/Glob.xs b/ext/File-Glob/Glob.xs +index e0a3681..9779d54 100644 +--- a/ext/File-Glob/Glob.xs ++++ b/ext/File-Glob/Glob.xs +@@ -121,7 +121,7 @@ iterate(pTHX_ bool(*globber)(pTHX_ AV *entries, const char *pat, STRLEN len, boo + + /* chuck it all out, quick or slow */ + if (gimme == G_ARRAY) { +- if (!on_stack) { ++ if (!on_stack && AvFILLp(entries) + 1) { + EXTEND(SP, AvFILLp(entries)+1); + Copy(AvARRAY(entries), SP+1, AvFILLp(entries)+1, SV *); + SP += AvFILLp(entries)+1; +diff --git a/handy.h b/handy.h +index 80f9cf4..88b5b55 100644 +--- a/handy.h ++++ b/handy.h +@@ -2409,17 +2409,17 @@ void Perl_mem_log_del_sv(const SV *sv, const char *filename, const int linenumbe + #define Safefree(d) safefree(MEM_LOG_FREE((Malloc_t)(d))) + #endif + +-#define Move(s,d,n,t) (MEM_WRAP_CHECK_(n,t) (void)memmove((char*)(d),(const char*)(s), (n) * sizeof(t))) +-#define Copy(s,d,n,t) (MEM_WRAP_CHECK_(n,t) (void)memcpy((char*)(d),(const char*)(s), (n) * sizeof(t))) +-#define Zero(d,n,t) (MEM_WRAP_CHECK_(n,t) (void)memzero((char*)(d), (n) * sizeof(t))) ++#define Move(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), (void)memmove((char*)(d),(const char*)(s), (n) * sizeof(t))) ++#define Copy(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), (void)memcpy((char*)(d),(const char*)(s), (n) * sizeof(t))) ++#define Zero(d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), (void)memzero((char*)(d), (n) * sizeof(t))) + +-#define MoveD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) memmove((char*)(d),(const char*)(s), (n) * sizeof(t))) +-#define CopyD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) memcpy((char*)(d),(const char*)(s), (n) * sizeof(t))) ++#define MoveD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), memmove((char*)(d),(const char*)(s), (n) * sizeof(t))) ++#define CopyD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), memcpy((char*)(d),(const char*)(s), (n) * sizeof(t))) + #ifdef HAS_MEMSET +-#define ZeroD(d,n,t) (MEM_WRAP_CHECK_(n,t) memzero((char*)(d), (n) * sizeof(t))) ++#define ZeroD(d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), memzero((char*)(d), (n) * sizeof(t))) + #else + /* Using bzero(), which returns void. */ +-#define ZeroD(d,n,t) (MEM_WRAP_CHECK_(n,t) memzero((char*)(d), (n) * sizeof(t)),d) ++#define ZeroD(d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), memzero((char*)(d), (n) * sizeof(t)),d) + #endif + + #define PoisonWith(d,n,t,b) (MEM_WRAP_CHECK_(n,t) (void)memset((char*)(d), (U8)(b), (n) * sizeof(t))) +diff --git a/pp_ctl.c b/pp_ctl.c +index 15c193b..f1c57bc 100644 +--- a/pp_ctl.c ++++ b/pp_ctl.c +@@ -1971,7 +1971,8 @@ PP(pp_caller) + + if (AvMAX(PL_dbargs) < AvFILLp(ary) + off) + av_extend(PL_dbargs, AvFILLp(ary) + off); +- Copy(AvALLOC(ary), AvARRAY(PL_dbargs), AvFILLp(ary) + 1 + off, SV*); ++ if (AvFILLp(ary) + 1 + off) ++ Copy(AvALLOC(ary), AvARRAY(PL_dbargs), AvFILLp(ary) + 1 + off, SV*); + AvFILLp(PL_dbargs) = AvFILLp(ary) + off; + } + mPUSHi(CopHINTS_get(cx->blk_oldcop)); +diff --git a/pp_hot.c b/pp_hot.c +index 5899413..66b79ea 100644 +--- a/pp_hot.c ++++ b/pp_hot.c +@@ -4138,7 +4138,8 @@ PP(pp_entersub) + AvARRAY(av) = ary; + } + +- Copy(MARK+1,AvARRAY(av),items,SV*); ++ if (items) ++ Copy(MARK+1,AvARRAY(av),items,SV*); + AvFILLp(av) = items - 1; + } + if (UNLIKELY((cx->blk_u16 & OPpENTERSUB_LVAL_MASK) == OPpLVAL_INTRO && +-- +2.13.6 + diff --git a/perl-5.27.3-avoid-the-address-of-.-will-always-evaluate-as-.-war.patch b/perl-5.27.3-avoid-the-address-of-.-will-always-evaluate-as-.-war.patch new file mode 100644 index 0000000..7f2a9f4 --- /dev/null +++ b/perl-5.27.3-avoid-the-address-of-.-will-always-evaluate-as-.-war.patch @@ -0,0 +1,60 @@ +From 45908e4d120d33a558a8b052036c56cd0c90b898 Mon Sep 17 00:00:00 2001 +From: Yves Orton +Date: Wed, 13 Sep 2017 13:30:25 +0200 +Subject: [PATCH] avoid 'the address of ... will always evaluate as ...' warns + in mem macros +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +In f14cf363205 we added asserts to our memory macros (Copy(), Zero() etc) +to ensure that the target is non-null. These asserts throw warnings like + + perl.c: In function ‘Perl_eval_sv’: + perl.c:2976:264: warning: the address of ‘myop’ will always evaluate + as ‘true’ [-Waddress] + Zero(&myop, 1, UNOP); + +which is annoying. This patch changes how these asserts are coded so +we avoid the warning. Thanks to Zefram for the fix. + +Signed-off-by: Petr Písař +--- + handy.h | 17 ++++++++++------- + 1 file changed, 10 insertions(+), 7 deletions(-) + +diff --git a/handy.h b/handy.h +index 31afaae65e..85e8f70721 100644 +--- a/handy.h ++++ b/handy.h +@@ -2409,17 +2409,20 @@ void Perl_mem_log_del_sv(const SV *sv, const char *filename, const int linenumbe + #define Safefree(d) safefree(MEM_LOG_FREE((Malloc_t)(d))) + #endif + +-#define Move(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), (void)memmove((char*)(d),(const char*)(s), (n) * sizeof(t))) +-#define Copy(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), (void)memcpy((char*)(d),(const char*)(s), (n) * sizeof(t))) +-#define Zero(d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), (void)memzero((char*)(d), (n) * sizeof(t))) ++#define perl_assert_ptr(p) assert( ((void*)(p)) != 0 ) + +-#define MoveD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), memmove((char*)(d),(const char*)(s), (n) * sizeof(t))) +-#define CopyD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), memcpy((char*)(d),(const char*)(s), (n) * sizeof(t))) ++ ++#define Move(s,d,n,t) (MEM_WRAP_CHECK_(n,t) perl_assert_ptr(d), perl_assert_ptr(s), (void)memmove((char*)(d),(const char*)(s), (n) * sizeof(t))) ++#define Copy(s,d,n,t) (MEM_WRAP_CHECK_(n,t) perl_assert_ptr(d), perl_assert_ptr(s), (void)memcpy((char*)(d),(const char*)(s), (n) * sizeof(t))) ++#define Zero(d,n,t) (MEM_WRAP_CHECK_(n,t) perl_assert_ptr(d), (void)memzero((char*)(d), (n) * sizeof(t))) ++ ++#define MoveD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) perl_assert_ptr(d), perl_assert_ptr(s), memmove((char*)(d),(const char*)(s), (n) * sizeof(t))) ++#define CopyD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) perl_assert_ptr(d), perl_assert_ptr(s), memcpy((char*)(d),(const char*)(s), (n) * sizeof(t))) + #ifdef HAS_MEMSET +-#define ZeroD(d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), memzero((char*)(d), (n) * sizeof(t))) ++#define ZeroD(d,n,t) (MEM_WRAP_CHECK_(n,t) perl_assert_ptr(d), memzero((char*)(d), (n) * sizeof(t))) + #else + /* Using bzero(), which returns void. */ +-#define ZeroD(d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), memzero((char*)(d), (n) * sizeof(t)),d) ++#define ZeroD(d,n,t) (MEM_WRAP_CHECK_(n,t) perl_assert_ptr(d), memzero((char*)(d), (n) * sizeof(t)),d) + #endif + + #define PoisonWith(d,n,t,b) (MEM_WRAP_CHECK_(n,t) (void)memset((char*)(d), (U8)(b), (n) * sizeof(t))) +-- +2.13.6 + diff --git a/perl.spec b/perl.spec index b9d9ea4..3068293 100644 --- a/perl.spec +++ b/perl.spec @@ -252,6 +252,11 @@ Patch73: perl-5.26.1-perform-system-arg-processing-before-fork.patch Patch74: perl-5.27.7-preserve-numericness-of-system-args-on-Win32.patch Patch75: perl-5.27.7-Reenable-numeric-first-argument-of-system-on-VMS.patch +# Avoid undefined behavior when copying memory in Glob and pp_caller, +# RT#131746, in upstream after 5.27.3 +Patch76: perl-5.26.1-perl-131746-avoid-undefined-behaviour-in-Copy-etc.patch +Patch77: perl-5.27.3-avoid-the-address-of-.-will-always-evaluate-as-.-war.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 @@ -2842,6 +2847,8 @@ Perl extension for Version Objects %patch73 -p1 %patch74 -p1 %patch75 -p1 +%patch76 -p1 +%patch77 -p1 %patch200 -p1 %patch201 -p1 @@ -2889,6 +2896,7 @@ perl -x patchlevel.h \ 'Fedora Patch71: Fix setting $! when statting a closed filehandle (RT#108288)' \ 'Fedora Patch72: Fix tainting of s/// with overloaded replacement (RT#115266)' \ 'Fedora Patch73: Expand system() arguments before a fork (RT#121105)' \ + 'Fedora Patch76: Avoid undefined behavior when copying memory in Glob and pp_caller (RT#131746)' \ '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} @@ -5190,6 +5198,7 @@ popd - Fix setting $! when statting a closed filehandle (RT#108288) - Fix tainting of s/// with overloaded replacement (RT#115266) - Expand system() arguments before a fork (RT#121105) +- Avoid undefined behavior when copying memory in Glob and pp_caller (RT#131746) * Mon Sep 25 2017 Jitka Plesnikova - 4:5.26.1-401 - Update perl(:MODULE_COMPAT)