Avoid undefined behavior when copying memory in Glob and pp_caller (RT#131746)

This commit is contained in:
Petr Písař 2018-01-10 13:15:23 +01:00
parent ef39cf486c
commit 6a620bdc9d
3 changed files with 176 additions and 0 deletions

View File

@ -0,0 +1,107 @@
From 7a962424149cc60f3a187d0213a12689dd5e806b Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
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ř <ppisar@redhat.com>
---
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

View File

@ -0,0 +1,60 @@
From 45908e4d120d33a558a8b052036c56cd0c90b898 Mon Sep 17 00:00:00 2001
From: Yves Orton <demerphq@gmail.com>
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ř <ppisar@redhat.com>
---
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

View File

@ -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 <jplesnik@redhat.com> - 4:5.26.1-401
- Update perl(:MODULE_COMPAT)