Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

segfault in blake2_update #193

Open
domenkozar opened this issue Oct 1, 2017 · 3 comments
Open

segfault in blake2_update #193

domenkozar opened this issue Oct 1, 2017 · 3 comments

Comments

@domenkozar
Copy link
Contributor

We found a segfault in blake2_update function and @vincenthz provided a workaround patch:

diff --git a/Crypto/Hash/Blake2b.hs b/Crypto/Hash/Blake2b.hs
index c22c284..19c68ba 100644
--- a/Crypto/Hash/Blake2b.hs
+++ b/Crypto/Hash/Blake2b.hs
@@ -85,7 +85,7 @@ instance HashAlgorithm Blake2b_512 where
 foreign import ccall unsafe "cryptonite_blake2b_init"
     c_blake2b_init :: Ptr (Context a) -> Word32 -> IO ()

-foreign import ccall "cryptonite_blake2b_update"
+foreign import ccall unsafe "cryptonite_blake2b_update"
     c_blake2b_update :: Ptr (Context a) -> Ptr Word8 -> Word32 -> IO ()

 foreign import ccall unsafe "cryptonite_blake2b_finalize"
domenkozar added a commit to input-output-hk/cardano-sl that referenced this issue Oct 1, 2017
domenkozar added a commit to input-output-hk/cardano-sl that referenced this issue Oct 1, 2017
domenkozar added a commit to input-output-hk/cardano-sl that referenced this issue Oct 1, 2017
@dcoutts
Copy link

dcoutts commented Oct 1, 2017

I'd like to understand this. On the face of it, it's deeply suspicious. If the thing can be moved during the FFI call, it can be moved before or after too. It's not a ByteArray# being passed to the FFI call. The B.withByteArray in hashUpdates should guarantee that we have a stable pointer for the duration. So what is going on?

domenkozar added a commit to input-output-hk/cardano-sl that referenced this issue Oct 1, 2017
domenkozar added a commit to input-output-hk/cardano-sl that referenced this issue Oct 1, 2017
@njd42
Copy link

njd42 commented Oct 1, 2017

Note that the segfault appears to be being raised in a part of libc where the actual code sequences are processor capability dependent.

domenkozar added a commit to input-output-hk/cardano-sl that referenced this issue Oct 1, 2017
@vincenthz
Copy link
Member

From the coredump I had direct access to, we've entered a code path that shouldn't exist of trying to copy > 0x80 bytes with memcpy when we should by design only copy <= 0x80 bytes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants