frodo icon indicating copy to clipboard operation
frodo copied to clipboard

incorrect passing of Go pointers to C code

Open pdeva opened this issue 4 years ago • 3 comments

The golang doc says

C code may store Go pointers in C memory, subject to the rule above: it must stop storing the Go pointer when the C function returns.

however in this line https://github.com/agnivade/frodo/blob/55a39bb84f2ef483b1513665e4ee13924aa01575/frodo.c#L105 you store the pointer from the golang buffer in a struct that is passed to io_uring systerm calls which will access the data buffer long after the push_write_request function returns.

int push_write_request(int file_fd, void *data, off_t file_sz) {
    struct file_info *fi = malloc(sizeof(*fi) + (sizeof(struct iovec) * 1));
...
    fi->iovecs[0].iov_base = data;
...
    io_uring_sqe_set_data(sqe, fi);
    return 0;
}

while this may work in some cases where the gc doesnt run till the file is written to, it is very possible that the gc runs after this call to push_write_request and relocates the data buffer to a whole different address. in fact for the zeroBytes buffer case https://github.com/agnivade/frodo/blob/55a39bb84f2ef483b1513665e4ee13924aa01575/frodo.go#L173 the gc may completely destroy slice since there are no references to it past the push_write_request call

pdeva avatar Jul 21 '21 19:07 pdeva

Good catch there, and thanks for reading through the code!

I wrote this mainly as a POC, and just to understand io_uring better. So there may definitely be bugs like this. I am not sure what the fix here would be though since io_uring is asynchronous by design.

agnivade avatar Jul 22 '21 04:07 agnivade

the only correct way would be to copy the contents of 'data' into a new C allocated buffer. the copying of course would lessen the benefits of using io_uring in the first place, which is why it's difficult to implement in garbage collected languages

pdeva avatar Jul 22 '21 05:07 pdeva

Yep, fair point.

agnivade avatar Jul 22 '21 05:07 agnivade