Skip to content

Commit

Permalink
handle allocations for problematic structs to avoid sharing pointers-…
Browse files Browse the repository at this point in the history
…to-pointers with C (from Go) (filecoin-project#82)

* alternative verify post

* manually allocate proxy object to prevent sharing Go heap with C

* Revert "alternative verify post"

This reverts commit 841ff78.

* manually allocate proxy object to prevent sharing Go heap with C

* pass flattened messages to fil_hash_verify to prevent pointer-to-pointer issues with CGO

* rename symbols within custom allocate methods

* run go test with cgocheck=2

* delete temporary branches from circleci config
  • Loading branch information
laser authored and gracenoah committed Dec 20, 2020
1 parent f15d06a commit 93d341c
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 92 deletions.
5 changes: 2 additions & 3 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ workflows:
branches:
only:
- master
- feat/lint-during-build
- build_darwin_cgo_bindings
- publish_darwin_staticlib:
filters:
Expand Down Expand Up @@ -320,13 +319,13 @@ commands:
no_output_timeout: 60m
- run:
name: Test project
command: RUST_LOG=info go test -p 1 -timeout 60m
command: GODEBUG=cgocheck=2 RUST_LOG=info go test -p 1 -timeout 60m
no_output_timeout: 60m
compile_tests:
steps:
- run:
name: Build project and tests, but don't actually run the tests (used to verify that build/link works with Darwin)
command: RUST_LOG=info go test -run=^$
command: GODEBUG=cgocheck=2 RUST_LOG=info go test -run=^$
restore_parameter_cache:
steps:
- restore_cache:
Expand Down
48 changes: 0 additions & 48 deletions generated/cgo_helpers.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

54 changes: 54 additions & 0 deletions generated/customallocs.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package generated

/*
#cgo LDFLAGS: -L${SRCDIR}/.. -lfilcrypto
#cgo pkg-config: ${SRCDIR}/../filcrypto.pc
#include "../filcrypto.h"
#include <stdlib.h>
#include "cgo_helpers.h"
*/
import "C"
import (
"unsafe"
)

// AllocateProxy allocates a FilPrivateReplicaInfo proxy object in the C heap,
// returning a function which, when called, frees the allocated memory. This
// method exists because the default c-for-go allocation strategy allocates a
// C struct with a field whose values is a pointer into the Go heap, which is
// not permitted by the most strict CGO check (cgocheck=2).
func (x *FilPrivateReplicaInfo) AllocateProxy() func() {
mem := allocFilPrivateReplicaInfoMemory(1)
proxy := (*C.fil_PrivateReplicaInfo)(mem)
proxy.cache_dir_path = C.CString(x.CacheDirPath)
proxy.comm_r = *(*[32]C.uint8_t)(unsafe.Pointer(&x.CommR))
proxy.registered_proof = (C.fil_RegisteredPoStProof)(x.RegisteredProof)
proxy.replica_path = C.CString(x.ReplicaPath)
proxy.sector_id = (C.uint64_t)(x.SectorId)

x.ref81a31e9b = proxy

return func() {
C.free(unsafe.Pointer(proxy.cache_dir_path))
C.free(unsafe.Pointer(proxy.replica_path))
C.free(unsafe.Pointer(proxy))
}
}

// AllocateProxy allocates a FilPoStProof proxy object in the C heap,
// returning a function which, when called, frees the allocated memory.
func (x *FilPoStProof) AllocateProxy() func() {
mem := allocFilPoStProofMemory(1)
proxy := (*C.fil_PoStProof)(mem)

proxy.registered_proof = (C.fil_RegisteredPoStProof)(x.RegisteredProof)
proxy.proof_len = (C.size_t)(x.ProofLen)
proxy.proof_ptr = (*C.uchar)(unsafe.Pointer(C.CString(x.ProofPtr)))

x.ref3451bfa = proxy

return func() {
C.free(unsafe.Pointer(proxy.proof_ptr))
C.free(unsafe.Pointer(proxy))
}
}
55 changes: 29 additions & 26 deletions generated/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 21 additions & 15 deletions rust/src/bls/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,23 +188,28 @@ pub unsafe extern "C" fn fil_verify(
#[no_mangle]
pub unsafe extern "C" fn fil_hash_verify(
signature_ptr: *const u8,
messages_ptr: *const *const u8,
messages_sizes_ptr: *const libc::size_t,
messages_len: libc::size_t,
flattened_messages_ptr: *const u8,
flattened_messages_len: libc::size_t,
message_sizes_ptr: *const libc::size_t,
message_sizes_len: libc::size_t,
flattened_public_keys_ptr: *const u8,
flattened_public_keys_len: libc::size_t,
) -> libc::c_int {
// prep request
let raw_signature = from_raw_parts(signature_ptr, SIGNATURE_BYTES);
let signature = try_ffi!(Signature::from_bytes(raw_signature), 0);

let raw_messages_sizes: &[usize] = from_raw_parts(messages_sizes_ptr, messages_len);
let raw_messages: &[*const u8] = from_raw_parts(messages_ptr, messages_len);
let messages: Vec<&[u8]> = raw_messages
.iter()
.zip(raw_messages_sizes.iter())
.map(|(ptr, size)| from_raw_parts(*ptr, *size))
.collect();
let flattened = from_raw_parts(flattened_messages_ptr, flattened_messages_len);
let chunk_sizes = from_raw_parts(message_sizes_ptr, message_sizes_len);

// split the flattened message array into slices of individual messages to
// be hashed
let mut messages: Vec<&[u8]> = Vec::with_capacity(message_sizes_len);
let mut offset = 0;
for chunk_size in chunk_sizes.iter() {
messages.push(&flattened[offset..offset + *chunk_size]);
offset += *chunk_size
}

let raw_public_keys = from_raw_parts(flattened_public_keys_ptr, flattened_public_keys_len);

Expand Down Expand Up @@ -369,13 +374,14 @@ mod tests {

assert_eq!(1, verified);

let messages = [message.as_ptr()];
let messages_sizes = [message.len()];
let flattened_messages = message;
let message_sizes = [message.len()];
let verified = fil_hash_verify(
signature.as_ptr(),
messages.as_ptr(),
messages_sizes.as_ptr(),
1,
flattened_messages.as_ptr(),
flattened_messages.len(),
message_sizes.as_ptr(),
message_sizes.len(),
public_key.as_ptr(),
public_key.len(),
);
Expand Down

0 comments on commit 93d341c

Please sign in to comment.