Out-of-bounds read in Kokkos_Layout.hpp

[ibaned]

Moving this from trilinos/Trilinos#1516 for @CamelliaDPG.

He wrote the following:

The order_dimensions() method implemented in Kokkos_Layout.hpp is as follows:

template< typename iTypeOrder , typename iTypeDimen >
KOKKOS_INLINE_FUNCTION static
LayoutStride order_dimensions( int const rank
                             , iTypeOrder const * const order
                             , iTypeDimen const * const dimen )
{
  LayoutStride tmp ;
  // Verify valid rank order:
  int check_input = ARRAY_LAYOUT_MAX_RANK < rank ? 0 : int( 1 << rank ) - 1 ;
  for ( int r = 0 ; r < ARRAY_LAYOUT_MAX_RANK ; ++r ) {
    tmp.dimension[r] = 0 ;
    tmp.stride[r]    = 0 ;
    check_input &= ~int( 1 << order[r] );
  }
  if ( 0 == check_input ) {
    size_t n = 1 ;
    for ( int r = 0 ; r < rank ; ++r ) {
      tmp.stride[ order[r] ] = n ;
      n *= ( dimen[order[r]] );
      tmp.dimension[r] = dimen[r];
    }
  }
  return tmp ;
}

Whenever rank != ARRAY_LAYOUT_MAX_RANK, that first for loop will read beyond the end of the order array. Proposed fix is as follows:

template< typename iTypeOrder , typename iTypeDimen >
  KOKKOS_INLINE_FUNCTION static
  LayoutStride order_dimensions( int const rank
                               , iTypeOrder const * const order
                               , iTypeDimen const * const dimen )
    {
      LayoutStride tmp ;
      // Verify valid rank order:
      int check_input = ARRAY_LAYOUT_MAX_RANK < rank ? 0 : int( 1 << rank ) - 1 ;
      for ( int r = 0 ; r < ARRAY_LAYOUT_MAX_RANK ; ++r ) {
        tmp.dimension[r] = 0 ;
        tmp.stride[r]    = 0 ;
      }
      for ( int r = 0; r < rank; ++r ) {
        check_input &= ~int( 1 << order[r] );
      }
      if ( 0 == check_input ) {
        size_t n = 1 ;
        for ( int r = 0 ; r < rank ; ++r ) {
          tmp.stride[ order[r] ] = n ;
          n *= ( dimen[order[r]] );
          tmp.dimension[r] = dimen[r];
        }
      }
      return tmp ;
    }

[ibaned]

My guess is no one calls this with an array shorter than the max right now, so I'll relabel this an enhancement to allow the function to work with smaller arrays. If you know of a case where code inside Kokkos or Trilinos calls it with a shorter array, then its back to being a bug.

[ibaned]

Also renamed because buffer overflow usually refers to an out-of-bounds write, not a read.

[crtrott]

While nobody might be calling it, it is technically allowed so this is a bug (with maybe zero impact right now). Though I actually believe I do use it in LAMMPS with less than that. Most likely I am just lucky that the rest of the cache line is zeroed out.