Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safe stencils aren't completely safe #109

Closed
lehins opened this issue Dec 13, 2020 · 1 comment
Closed

Safe stencils aren't completely safe #109

lehins opened this issue Dec 13, 2020 · 1 comment

Comments

@lehins
Copy link
Owner

lehins commented Dec 13, 2020

It was brought to my attention on gitter that invalid stencils can cause reading memory out of bounds.

updateStencil :: Massiv.Stencil Ix2 Position Position
updateStencil =
  Massiv.makeStencilDef Floor (Sz (3 :. 3)) (0 :. 0) $ \ get ->
    let
      chair = get (0 :. 0)
      isOcc c = fromEnum . (== Occupied) <$> (get c)
      occ :: Massiv.Value Int
      occ =
        (  isOcc (-1 :. -1) + isOcc (-1 :. 0) + isOcc (-1 :. 1) +
           isOcc ( 0 :. -1)                   + isOcc ( 0 :. 1) +
           isOcc ( 1 :. -1) + isOcc ( 1 :. 0) + isOcc ( 1 :. 1)
        )
    in
      liftA2 threshold chair occ
{-# inline updateStencil #-}


threshold :: Position -> Int -> Position
threshold p n | p == Floor = p
              | n == 0     = Occupied
              | n >= 4     = Empty
              | otherwise  = p

The important part for validation is that indexing with stencil does not depend on values contained in the array stencil being applied to, which is the whole reason for existence of Value type in the first place. But the threshold function does just that: when chair == Floor then occ does not get evaluated during validation and invalid index that goes outside the stencil window is not used, thus bypassing validation. I long suspected it, but this example really proved to me that this problem is undecidable.

The only sensible solution I see is making:

  1. applyStencil is replaced with partial function applyStencil' that does no unsafe indexing, thus is slow, but safe
  2. Add unsafeApplyStencil
  3. remove validation completely and thus Value newtype
lehins added a commit that referenced this issue Dec 14, 2020
lehins added a commit that referenced this issue Dec 26, 2020
lehins added a commit that referenced this issue Jan 14, 2021
lehins added a commit that referenced this issue Jan 18, 2021
@lehins
Copy link
Owner Author

lehins commented Jan 22, 2021

This is now fixed in massiv-0.6.0.0

@lehins lehins closed this as completed Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant