1

i have this function in my code that seems to work just fine:

-- type declaration just for reference, i don't have it in my actual code
retrieveVulkanArray :: Storable a => (Ptr Word32 -> Ptr a -> IO b) -> IO (Ptr a, Int)
retrieveVulkanArray' f =
    alloca $ \arrCount -> do
        f arrCount vkNullPtr
        arrCount' <- fromIntegral <$> peek arrCount
        allocaArray arrCount' $ \resArray -> do
            f arrCount resArray
            pure (resArray, arrCount')

(for context this is a helper function to get FFI arrays from Vulkan API, f might be for example vkEnumeratePhysicalDevices)

As i was reviewing my code, i noticed that it returns resArray(which from description of allocaArray seems to only be valid within the inner lambda) to its caller. In C, a code like this would be undefined behaviour. Is my intuition correct here or is there something more going on? I haven't noticed any crashes yet after all :)

Pavel Beliy
  • 494
  • 1
  • 3
  • 14
  • I think this is UB. You are returning a pointer to feed memory (`resArray`). You could use a non-"alloca" allocation function and let the caller free that at the right time. (Or perhaps a foreign pointer) – chi May 09 '20 at 10:28
  • I thought so. I guess absence of Access Violation can be explained by GC not actually freeing the memory until some time later? – Pavel Beliy May 09 '20 at 10:51
  • It's hard to tell. It's likely that the memory isn't immediately returned to the OS when it is freed, so it keeps to be available but could be reused by other allocas. This also happens in many C implementations: if you free() a pointer and access it immediately, it might not cause a access violation since the memory isn't always returned to the OS, especially if it's a small memory area. – chi May 09 '20 at 12:14
  • You should always add a function type signature to your code for the sake of others who will ever read your code and for your own sake in the future when you read your own code. `-- type declaration just for reference, i don't have it in my actual code` – lehins May 09 '20 at 16:19

1 Answers1

2

The fact that it works certainly doesn't prove that it is correct, in fact this function is indeed very wrong.

alloca, as well as allocaArray, will allocate a Haskell MutableByteArray# convert it to a pointer. Operate on that pointer and then ensure that the array is still alive with a special touch# function. Problem is that once you loose reference to the actual MutableByteArray#, which is what happens when you exit alloca, GC will clean it up and the Ptr a that was pointing to that array will no longer be valid. So if you continue reading or writing into that pointer Ptr a after you return it from retrieveVulkanArray you are reading/writing into memory that can be used by something else and are now in real danger of a segfault and all sorts of other security vulnerabilities that come with it.

The proper way would be:

retrieveVulkanArray
  :: Storable a => (Ptr Word32 -> Ptr a -> IO b) -> IO (ForeignPtr a, Int)
retrieveVulkanArray' f =
    alloca $ \arrCount -> do
        f arrCount vkNullPtr
        arrCount' <- fromIntegral <$> peek arrCount
        resArray <- mallocForeignPtrArray arrCount'
        _ <- withForeignPtr resArray (f arrCount)
        pure (resArray, arrCount')

ForeignPtr a ensures that you can operate on the raw Ptr a when needed, without worrying that memory it points to is freed until ForeignPtr is no longer used.

lehins
  • 9,642
  • 2
  • 35
  • 49
  • thanks, i didn't notice `ForeignPtr` type before for some reason. what are functions ending on `#`? – Pavel Beliy May 10 '20 at 11:23
  • You are welcome. Functions and types ending with `MagicHash` are so called ghc primitive functions, eg `touch#`: https://downloads.haskell.org/~ghc/8.10.1/docs/html/libraries/base-4.14.0.0/GHC-Exts.html#v:touch-35- These are the core pieces, which are used to implement the rest of `base` and pretty much most of the functionality in Haskell – lehins May 10 '20 at 15:21