From: Paul Floyd Date: Wed, 20 Mar 2024 20:22:37 +0000 (+0100) Subject: Bug 484002 - Add suppression for invalid read in glibc's __wcpncpy_avx2() via wcsxfrm() X-Git-Tag: VALGRIND_3_23_0~98 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=49216fd60e3fcf0c7dad1dfc797662236255e18a;p=thirdparty%2Fvalgrind.git Bug 484002 - Add suppression for invalid read in glibc's __wcpncpy_avx2() via wcsxfrm() This needed a redirect, not a suppression. --- diff --git a/.gitignore b/.gitignore index 1c54be91e1..982305c355 100644 --- a/.gitignore +++ b/.gitignore @@ -855,9 +855,10 @@ /memcheck/tests/bug155125 /memcheck/tests/bug287260 /memcheck/tests/bug340392 +/memcheck/tests/bug401284 /memcheck/tests/bug464969_d_demangle /memcheck/tests/bug472219 -/memcheck/tests/bug401284 +/memcheck/tests/bug484002 /memcheck/tests/calloc-overflow /memcheck/tests/cdebug_zlib /memcheck/tests/cdebug_zlib_gnu @@ -1030,6 +1031,7 @@ /memcheck/tests/wcs /memcheck/tests/weirdioctl /memcheck/tests/with space +/memcheck/tests/wcpncpy /memcheck/tests/wcsncpy /memcheck/tests/wmemcmp /memcheck/tests/wrap1 diff --git a/NEWS b/NEWS index 20bb0e2d8d..d7686b80a9 100644 --- a/NEWS +++ b/NEWS @@ -70,6 +70,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 480706 Unhandled syscall 325 (mlock2) 481131 [PATCH] x86 regtest: fix clobber lists in generated asm statements 483786 Incorrect parameter indexing in FreeBSD clock_nanosleep syscall wrapper +484002 Add suppression for invalid read in glibc's __wcpncpy_avx2() via wcsxfrm() n-i-bz Add redirect for memccpy To see details of a given bug, visit diff --git a/configure.ac b/configure.ac index 858405ef67..8279683ac3 100755 --- a/configure.ac +++ b/configure.ac @@ -4969,7 +4969,9 @@ AC_CHECK_FUNCS([ \ strndup \ close_range \ wcsncpy \ - free_aligned_sized + free_aligned_sized \ + wcpncpy \ + wcsxfrm ]) # AC_CHECK_LIB adds any library found to the variable LIBS, and links these @@ -5009,6 +5011,10 @@ AM_CONDITIONAL([HAVE_STRLCPY], [test x$ac_cv_func_strlcpy = xyes]) AM_CONDITIONAL([HAVE_FREE_ALIGNED_SIZED], [test x$ac_cv_func_free_aligned_sized = xyes]) +AM_CONDITIONAL([HAVE_WCPNCPY], + [test x$ac_cv_func_wcpncpy = xyes]) +AM_CONDITIONAL([HAVE_WCSXFRM], + [test x$ac_cv_func_wcsxfrm = xyes]) if test x$VGCONF_PLATFORM_PRI_CAPS = xMIPS32_LINUX \ -o x$VGCONF_PLATFORM_PRI_CAPS = xMIPS64_LINUX \ diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am index 150e632ccd..76970529f2 100644 --- a/memcheck/tests/Makefile.am +++ b/memcheck/tests/Makefile.am @@ -121,6 +121,7 @@ EXTRA_DIST = \ bug464969_d_demangle.stderr.exp bug464969_d_demangle.vgtest \ bug464969_d_demangle.stdout.exp \ bug472219.stderr.exp bug472219.vgtest \ + bug484002.stderr.exp bug484002.stdout.exp bug484002.vgtest \ calloc-overflow.stderr.exp calloc-overflow.vgtest\ cdebug_zlib.stderr.exp cdebug_zlib.vgtest \ cdebug_zlib_gnu.stderr.exp cdebug_zlib_gnu.vgtest \ @@ -434,6 +435,7 @@ EXTRA_DIST = \ vcpu_fnfns.stdout.exp vcpu_fnfns.stdout.exp-glibc28-amd64 \ vcpu_fnfns.stdout.exp-darwin vcpu_fnfns.stdout.exp-solaris \ vcpu_fnfns.stderr.exp vcpu_fnfns.vgtest \ + wcpncpy.stderr.exp wcpncpy.vgtest \ wcs.vgtest wcs.stderr.exp wcs.stdout.exp \ wcsncpy.vgtest wcsncpy.stderr.exp \ wmemcmp.vgtest wmemcmp.stderr.exp \ @@ -559,6 +561,10 @@ endif bug464969_d_demangle_SOURCES = bug464969_d_demangle.cpp bug464969_d_demangle_CXXFLAGS = $(AM_CXXFLAGS) @FLAG_W_NO_UNINITIALIZED@ +if HAVE_WCSXFRM +check_PROGRAMS += bug484002 +endif + if GZ_ZLIB check_PROGRAMS += cdebug_zlib cdebug_zlib_SOURCES = cdebug.c @@ -602,6 +608,10 @@ if HAVE_PTHREAD_SETNAME_NP check_PROGRAMS += threadname endif +if HAVE_WCPNCPY +check_PROGRAMS += wcpncpy +endif + # are there still pre-C99 C compilers? if HAVE_WCSNCPY check_PROGRAMS += wcsncpy diff --git a/memcheck/tests/bug484002.c b/memcheck/tests/bug484002.c new file mode 100644 index 0000000000..c9d41f0dc3 --- /dev/null +++ b/memcheck/tests/bug484002.c @@ -0,0 +1,28 @@ +#include +#include +#include +#include + +int main() +{ + + const wchar_t in[] = {L'a', L'b', L'c', 0}; + wchar_t out[3 + 1] = { + 0, + }; + + size_t res = wcsxfrm(out, in, 3); + printf("%lu\n", res); + + wchar_t* in2 = malloc(sizeof(wchar_t) * 4); + memcpy(in2, in, sizeof(in)); + res = wcsxfrm(out, in2, 3); + printf("%lu\n", res); + free(in2); + + wchar_t* in3 = malloc(sizeof(wchar_t) * 4); + memcpy(in3, in, sizeof(in)); + res = wcsxfrm(out, in3, 3); + printf("%lu\n", res); + free(in3); +} diff --git a/memcheck/tests/bug484002.stderr.exp b/memcheck/tests/bug484002.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/memcheck/tests/bug484002.stdout.exp b/memcheck/tests/bug484002.stdout.exp new file mode 100644 index 0000000000..1f242fa6f0 --- /dev/null +++ b/memcheck/tests/bug484002.stdout.exp @@ -0,0 +1,3 @@ +3 +3 +3 diff --git a/memcheck/tests/bug484002.vgtest b/memcheck/tests/bug484002.vgtest new file mode 100644 index 0000000000..67fc503205 --- /dev/null +++ b/memcheck/tests/bug484002.vgtest @@ -0,0 +1,3 @@ +prereq: test -e ./bug484002 +prog: bug484002 +vgopts: -q diff --git a/memcheck/tests/wcpncpy.c b/memcheck/tests/wcpncpy.c new file mode 100644 index 0000000000..97d1a13bb8 --- /dev/null +++ b/memcheck/tests/wcpncpy.c @@ -0,0 +1,47 @@ +#include +#include +#include +#include + +int main(void) +{ + const wchar_t in[] = {L'H', L'e', L'l', L'l', L'o', 0}; + + wchar_t* dest1 = malloc(5*sizeof(wchar_t) + 2); + wchar_t* dest2 = malloc(11*sizeof(wchar_t)); + + // uninit read + wcpncpy(dest1, dest2, 3); + + wchar_t* end = wcpncpy(dest1, in, 3); + + assert(3 == end - dest1); + assert(0 == wmemcmp(in , dest1, 3)); + + end = wcpncpy(dest2, in, 10); + assert(5 == end - dest2); + assert(0 == wmemcmp(dest2 , in, 6)); + assert(0 == dest2[9]); + + // too small - invalid write + end = wcpncpy(dest1, in, 6); + + wcpncpy(dest2, in, 5); + wcpncpy(dest2+5, in, 6); + + // overlap + // sss + // ddd + wcpncpy(dest2, dest2+2, 3); + + wcpncpy(dest2, in, 5); + wcpncpy(dest2+5, in, 6); + + // overlap + // sss + // ddd + wcpncpy(dest2+2, dest2, 3); + + free(dest1); + free(dest2); +} diff --git a/memcheck/tests/wcpncpy.stderr.exp b/memcheck/tests/wcpncpy.stderr.exp new file mode 100644 index 0000000000..abe23b7730 --- /dev/null +++ b/memcheck/tests/wcpncpy.stderr.exp @@ -0,0 +1,19 @@ +Conditional jump or move depends on uninitialised value(s) + at 0x........: wcpncpy (vg_replace_strmem.c:2426) + by 0x........: main (wcpncpy.c:14) + +Invalid write of size 4 + at 0x........: wcpncpy (vg_replace_strmem.c:2426) + by 0x........: main (wcpncpy.c:27) + Address 0x........ is 20 bytes inside a block of size 22 alloc'd + at 0x........: malloc (vg_replace_malloc.c:...) + by 0x........: main (wcpncpy.c:10) + +Source and destination overlap in wcpncpy(0x........, 0x........) + at 0x........: wcpncpy (vg_replace_strmem.c:2426) + by 0x........: main (wcpncpy.c:35) + +Source and destination overlap in wcpncpy(0x........, 0x........) + at 0x........: wcpncpy (vg_replace_strmem.c:2426) + by 0x........: main (wcpncpy.c:43) + diff --git a/memcheck/tests/wcpncpy.vgtest b/memcheck/tests/wcpncpy.vgtest new file mode 100644 index 0000000000..f4edd4b8d5 --- /dev/null +++ b/memcheck/tests/wcpncpy.vgtest @@ -0,0 +1,3 @@ +prereq: test -e ./wcpncpy +prog: wcpncpy +vgopts: -q diff --git a/shared/vg_replace_strmem.c b/shared/vg_replace_strmem.c index 781dd784bf..3d88fb824b 100644 --- a/shared/vg_replace_strmem.c +++ b/shared/vg_replace_strmem.c @@ -107,6 +107,7 @@ 20470 WMEMCMP 20480 WCSNCPY 20490 MEMCCPY + 20500 WCPNCPY */ #if defined(VGO_solaris) @@ -2327,7 +2328,7 @@ static inline void my_exit ( int x ) \ /* This checks for overlap after copying, unavoidable without */ \ /* pre-counting length... should be ok */ \ - /* +4 because sizeof(wchar_t) == 4 */ \ + /* *4 because sizeof(wchar_t) == 4 */ \ SizeT srclen = ((m < n) ? m+1 : n)*4; \ RECORD_COPY(srclen); \ if (is_overlap(dst_orig, \ @@ -2382,6 +2383,50 @@ static inline void my_exit ( int x ) MEMCCPY(VG_Z_LIBC_SONAME, memccpy) #endif + /*---------------------- wcpncpy ----------------------*/ + + // This is a wchar_t equivalent to strncpy. We don't + // have wchar_t available here, but in the GNU C Library + // wchar_t is always 32 bits wide. + +#define WCPNCPY(soname, fnname) \ + Int* VG_REPLACE_FUNCTION_EZU(20500,soname,fnname) \ + ( Int* dst, const Int* src, SizeT n ); \ + Int* VG_REPLACE_FUNCTION_EZU(20500,soname,fnname) \ + ( Int* dst, const Int* src, SizeT n ) \ + { \ + const Int* src_orig = src; \ + Int* dst_orig = dst; \ + SizeT m = 0; \ + \ + while (m < n && *src) { \ + m++; \ + *dst++ = *src++; \ + } \ + \ + /* This checks for overlap after copying, unavoidable without */ \ + /* pre-counting length... should be ok */ \ + /* *4 because sizeof(wchar_t) == 4 */ \ + SizeT srclen = ((m < n) ? m+1 : n)*4; \ + RECORD_COPY(srclen); \ + if (is_overlap(dst_orig, \ + src_orig, \ + n*4, \ + srclen)) \ + RECORD_OVERLAP_ERROR("wcpncpy", dst_orig, src_orig, 0); \ + \ + while (m++ < n) { \ + *dst++ = 0; \ + } \ + \ + return dst_orig + (src - src_orig); \ + } + +#if defined(VGO_linux) || defined(VGO_freebsd) + WCPNCPY(VG_Z_LIBC_SONAME, wcpncpy) +#endif + + /*------------------------------------------------------------*/ /*--- Improve definedness checking of process environment ---*/ /*------------------------------------------------------------*/