From 8f7fb199074ac0dcdb9eb2645621c9b30928d2ab Mon Sep 17 00:00:00 2001 From: Benoit Jacob Date: Sun, 16 Oct 2011 16:12:19 -0400 Subject: [PATCH] bug #363 - check for integer overflow in size computations --- Eigen/Core | 2 +- Eigen/src/Core/PlainObjectBase.h | 19 ++++++++ Eigen/src/Core/util/Memory.h | 53 +++++++++++++++------ test/CMakeLists.txt | 2 +- test/sizeoverflow.cpp | 81 ++++++++++++++++++++++++++++++++ 5 files changed, 140 insertions(+), 17 deletions(-) create mode 100644 test/sizeoverflow.cpp diff --git a/Eigen/Core b/Eigen/Core index 6e855427c..a5025e37e 100644 --- a/Eigen/Core +++ b/Eigen/Core @@ -167,7 +167,7 @@ #include #endif -#if (defined(_CPPUNWIND) || defined(__EXCEPTIONS)) && !defined(EIGEN_NO_EXCEPTIONS) +#if defined(_CPPUNWIND) || defined(__EXCEPTIONS) #define EIGEN_EXCEPTIONS #endif diff --git a/Eigen/src/Core/PlainObjectBase.h b/Eigen/src/Core/PlainObjectBase.h index c70db9247..63dd8b767 100644 --- a/Eigen/src/Core/PlainObjectBase.h +++ b/Eigen/src/Core/PlainObjectBase.h @@ -34,6 +34,19 @@ namespace internal { +template +inline void check_rows_cols_for_overflow(Index rows, Index cols) +{ + // http://hg.mozilla.org/mozilla-central/file/6c8a909977d3/xpcom/ds/CheckedInt.h#l242 + // we assume Index is signed + Index max_index = (size_t(1) << (8 * sizeof(Index) - 1)) - 1; // assume Index is signed + bool error = (rows < 0 || cols < 0) ? true + : (rows == 0 || cols == 0) ? false + : (rows > max_index / cols); + if (error) + throw_std_bad_alloc(); +} + template (Derived::IsVectorAtCompileTime)> struct conservative_resize_like_impl; template struct matrix_swap_impl; @@ -200,11 +213,13 @@ class PlainObjectBase : public internal::dense_xpr_base::type EIGEN_STRONG_INLINE void resize(Index rows, Index cols) { #ifdef EIGEN_INITIALIZE_MATRICES_BY_ZERO + internal::check_rows_cols_for_overflow(rows, cols); Index size = rows*cols; bool size_changed = size != this->size(); m_storage.resize(size, rows, cols); if(size_changed) EIGEN_INITIALIZE_BY_ZERO_IF_THAT_OPTION_IS_ENABLED #else + internal::check_rows_cols_for_overflow(rows, cols); m_storage.resize(rows*cols, rows, cols); #endif } @@ -273,6 +288,7 @@ class PlainObjectBase : public internal::dense_xpr_base::type EIGEN_STRONG_INLINE void resizeLike(const EigenBase& _other) { const OtherDerived& other = _other.derived(); + internal::check_rows_cols_for_overflow(other.rows(), other.cols()); const Index othersize = other.rows()*other.cols(); if(RowsAtCompileTime == 1) { @@ -417,6 +433,7 @@ class PlainObjectBase : public internal::dense_xpr_base::type : m_storage(other.derived().rows() * other.derived().cols(), other.derived().rows(), other.derived().cols()) { _check_template_params(); + internal::check_rows_cols_for_overflow(other.derived().rows(), other.derived().cols()); Base::operator=(other.derived()); } @@ -581,6 +598,7 @@ class PlainObjectBase : public internal::dense_xpr_base::type { eigen_assert(rows >= 0 && (RowsAtCompileTime == Dynamic || RowsAtCompileTime == rows) && cols >= 0 && (ColsAtCompileTime == Dynamic || ColsAtCompileTime == cols)); + internal::check_rows_cols_for_overflow(rows, cols); m_storage.resize(rows*cols,rows,cols); EIGEN_INITIALIZE_BY_ZERO_IF_THAT_OPTION_IS_ENABLED } @@ -638,6 +656,7 @@ struct internal::conservative_resize_like_impl if ( ( Derived::IsRowMajor && _this.cols() == cols) || // row-major and we change only the number of rows (!Derived::IsRowMajor && _this.rows() == rows) ) // column-major and we change only the number of columns { + internal::check_rows_cols_for_overflow(rows, cols); _this.derived().m_storage.conservativeResize(rows*cols,rows,cols); } else diff --git a/Eigen/src/Core/util/Memory.h b/Eigen/src/Core/util/Memory.h index a580b95ad..18c421449 100644 --- a/Eigen/src/Core/util/Memory.h +++ b/Eigen/src/Core/util/Memory.h @@ -82,6 +82,15 @@ namespace internal { +inline void throw_std_bad_alloc() +{ + #ifdef EIGEN_EXCEPTIONS + throw std::bad_alloc(); + #else + new int[size_t(-1)]; + #endif +} + /***************************************************************************** *** Implementation of handmade aligned functions *** *****************************************************************************/ @@ -192,7 +201,7 @@ inline void check_that_malloc_is_allowed() #endif /** \internal Allocates \a size bytes. The returned pointer is guaranteed to have 16 bytes alignment. - * On allocation error, the returned pointer is null, and if exceptions are enabled then a std::bad_alloc is thrown. + * On allocation error, the returned pointer is null, and std::bad_alloc is thrown. */ inline void* aligned_malloc(size_t size) { @@ -213,10 +222,9 @@ inline void* aligned_malloc(size_t size) result = handmade_aligned_malloc(size); #endif - #ifdef EIGEN_EXCEPTIONS - if(result == 0) - throw std::bad_alloc(); - #endif + if(!result && size) + throw_std_bad_alloc(); + return result; } @@ -241,7 +249,7 @@ inline void aligned_free(void *ptr) /** * \internal * \brief Reallocates an aligned block of memory. -* \throws std::bad_alloc if EIGEN_EXCEPTIONS are defined. +* \throws std::bad_alloc on allocation failure **/ inline void* aligned_realloc(void *ptr, size_t new_size, size_t old_size) { @@ -269,10 +277,9 @@ inline void* aligned_realloc(void *ptr, size_t new_size, size_t old_size) result = handmade_aligned_realloc(ptr,new_size,old_size); #endif -#ifdef EIGEN_EXCEPTIONS - if (result==0 && new_size!=0) - throw std::bad_alloc(); -#endif + if (!result && new_size) + throw_std_bad_alloc(); + return result; } @@ -281,7 +288,7 @@ inline void* aligned_realloc(void *ptr, size_t new_size, size_t old_size) *****************************************************************************/ /** \internal Allocates \a size bytes. If Align is true, then the returned ptr is 16-byte-aligned. - * On allocation error, the returned pointer is null, and if exceptions are enabled then a std::bad_alloc is thrown. + * On allocation error, the returned pointer is null, and a std::bad_alloc is thrown. */ template inline void* conditional_aligned_malloc(size_t size) { @@ -293,9 +300,8 @@ template<> inline void* conditional_aligned_malloc(size_t size) check_that_malloc_is_allowed(); void *result = std::malloc(size); - #ifdef EIGEN_EXCEPTIONS - if(!result) throw std::bad_alloc(); - #endif + if(!result && size) + throw_std_bad_alloc(); return result; } @@ -347,18 +353,27 @@ template inline void destruct_elements_of_array(T *ptr, size_t size) *** Implementation of aligned new/delete-like functions *** *****************************************************************************/ +template +inline void check_size_for_overflow(size_t size) +{ + if(size > size_t(-1) / sizeof(T)) + throw_std_bad_alloc(); +} + /** \internal Allocates \a size objects of type T. The returned pointer is guaranteed to have 16 bytes alignment. - * On allocation error, the returned pointer is undefined, but if exceptions are enabled then a std::bad_alloc is thrown. + * On allocation error, the returned pointer is undefined, but a std::bad_alloc is thrown. * The default constructor of T is called. */ template inline T* aligned_new(size_t size) { + check_size_for_overflow(size); T *result = reinterpret_cast(aligned_malloc(sizeof(T)*size)); return construct_elements_of_array(result, size); } template inline T* conditional_aligned_new(size_t size) { + check_size_for_overflow(size); T *result = reinterpret_cast(conditional_aligned_malloc(sizeof(T)*size)); return construct_elements_of_array(result, size); } @@ -383,6 +398,8 @@ template inline void conditional_aligned_delete(T *ptr, template inline T* conditional_aligned_realloc_new(T* pts, size_t new_size, size_t old_size) { + check_size_for_overflow(new_size); + check_size_for_overflow(old_size); if(new_size < old_size) destruct_elements_of_array(pts+new_size, old_size-new_size); T *result = reinterpret_cast(conditional_aligned_realloc(reinterpret_cast(pts), sizeof(T)*new_size, sizeof(T)*old_size)); @@ -394,6 +411,7 @@ template inline T* conditional_aligned_realloc_new(T* pt template inline T* conditional_aligned_new_auto(size_t size) { + check_size_for_overflow(size); T *result = reinterpret_cast(conditional_aligned_malloc(sizeof(T)*size)); if(NumTraits::RequireInitialization) construct_elements_of_array(result, size); @@ -402,6 +420,8 @@ template inline T* conditional_aligned_new_auto(size_t s template inline T* conditional_aligned_realloc_new_auto(T* pts, size_t new_size, size_t old_size) { + check_size_for_overflow(new_size); + check_size_for_overflow(old_size); if(NumTraits::RequireInitialization && (new_size < old_size)) destruct_elements_of_array(pts+new_size, old_size-new_size); T *result = reinterpret_cast(conditional_aligned_realloc(reinterpret_cast(pts), sizeof(T)*new_size, sizeof(T)*old_size)); @@ -536,6 +556,7 @@ template class aligned_stack_memory_handler #endif #define ei_declare_aligned_stack_constructed_variable(TYPE,NAME,SIZE,BUFFER) \ + Eigen::internal::check_size_for_overflow(SIZE); \ TYPE* NAME = (BUFFER)!=0 ? (BUFFER) \ : reinterpret_cast( \ (sizeof(TYPE)*SIZE<=EIGEN_STACK_ALLOCATION_LIMIT) ? EIGEN_ALIGNED_ALLOCA(sizeof(TYPE)*SIZE) \ @@ -545,6 +566,7 @@ template class aligned_stack_memory_handler #else #define ei_declare_aligned_stack_constructed_variable(TYPE,NAME,SIZE,BUFFER) \ + Eigen::internal::check_size_for_overflow(SIZE); \ TYPE* NAME = (BUFFER)!=0 ? BUFFER : reinterpret_cast(Eigen::internal::aligned_malloc(sizeof(TYPE)*SIZE)); \ Eigen::internal::aligned_stack_memory_handler EIGEN_CAT(NAME,_stack_memory_destructor)((BUFFER)==0 ? NAME : 0,SIZE,true) @@ -669,6 +691,7 @@ public: pointer allocate( size_type num, const void* hint = 0 ) { EIGEN_UNUSED_VARIABLE(hint); + internal::check_size_for_overflow(num); return static_cast( internal::aligned_malloc( num * sizeof(T) ) ); } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index adaecb5f8..fab7de0d4 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -121,7 +121,7 @@ ei_add_test(nullary) ei_add_test(nesting_ops "${CMAKE_CXX_FLAGS_DEBUG}") ei_add_test(zerosized) ei_add_test(dontalign) - +ei_add_test(sizeoverflow) ei_add_test(prec_inverse_4x4) string(TOLOWER "${CMAKE_CXX_COMPILER}" cmake_cxx_compiler_tolower) diff --git a/test/sizeoverflow.cpp b/test/sizeoverflow.cpp new file mode 100644 index 000000000..78ededbea --- /dev/null +++ b/test/sizeoverflow.cpp @@ -0,0 +1,81 @@ +// This file is part of Eigen, a lightweight C++ template library +// for linear algebra. +// +// Copyright (C) 2011 Benoit Jacob +// +// Eigen is free software; you can redistribute it and/or +// modify it under the terms of the GNU Lesser General Public +// License as published by the Free Software Foundation; either +// version 3 of the License, or (at your option) any later version. +// +// Alternatively, you can redistribute it and/or +// modify it under the terms of the GNU General Public License as +// published by the Free Software Foundation; either version 2 of +// the License, or (at your option) any later version. +// +// Eigen is distributed in the hope that it will be useful, but WITHOUT ANY +// WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +// FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License or the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License and a copy of the GNU General Public License along with +// Eigen. If not, see . + +#include "main.h" + +#define VERIFY_THROWS_BADALLOC(a) { \ + bool threw = false; \ + try { \ + a; \ + } \ + catch (std::bad_alloc&) { threw = true; } \ + VERIFY(threw && "should have thrown bad_alloc: " #a); \ + } + +typedef DenseIndex Index; + +template +void triggerMatrixBadAlloc(Index rows, Index cols) +{ + VERIFY_THROWS_BADALLOC( MatrixType m(rows, cols) ); + VERIFY_THROWS_BADALLOC( MatrixType m; m.resize(rows, cols) ); + VERIFY_THROWS_BADALLOC( MatrixType m; m.conservativeResize(rows, cols) ); +} + +template +void triggerVectorBadAlloc(Index size) +{ + VERIFY_THROWS_BADALLOC( VectorType v(size) ); + VERIFY_THROWS_BADALLOC( VectorType v; v.resize(size) ); + VERIFY_THROWS_BADALLOC( VectorType v; v.conservativeResize(size) ); +} + +void test_sizeoverflow() +{ + // there are 2 levels of overflow checking. first in PlainObjectBase.h we check for overflow in rows*cols computations. + // this is tested in tests of the form times_itself_gives_0 * times_itself_gives_0 + // Then in Memory.h we check for overflow in size * sizeof(T) computations. + // this is tested in tests of the form times_4_gives_0 * sizeof(float) + + size_t times_itself_gives_0 = size_t(1) << (8 * sizeof(Index) / 2); + VERIFY(times_itself_gives_0 * times_itself_gives_0 == 0); + + size_t times_4_gives_0 = size_t(1) << (8 * sizeof(Index) - 2); + VERIFY(times_4_gives_0 * 4 == 0); + + size_t times_8_gives_0 = size_t(1) << (8 * sizeof(Index) - 3); + VERIFY(times_8_gives_0 * 8 == 0); + + triggerMatrixBadAlloc(times_itself_gives_0, times_itself_gives_0); + triggerMatrixBadAlloc(times_itself_gives_0 / 4, times_itself_gives_0); + triggerMatrixBadAlloc(times_4_gives_0, 1); + + triggerMatrixBadAlloc(times_itself_gives_0, times_itself_gives_0); + triggerMatrixBadAlloc(times_itself_gives_0 / 8, times_itself_gives_0); + triggerMatrixBadAlloc(times_8_gives_0, 1); + + triggerVectorBadAlloc(times_4_gives_0); + + triggerVectorBadAlloc(times_8_gives_0); +}