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

incorrect passing of Go pointers to C code #2

Open
pdeva opened this issue Jul 21, 2021 · 3 comments
Open

incorrect passing of Go pointers to C code #2

pdeva opened this issue Jul 21, 2021 · 3 comments

Comments

@pdeva
Copy link

pdeva commented Jul 21, 2021

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

frodo/frodo.c

Line 105 in 55a39bb

fi->iovecs[0].iov_base = data;
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

frodo/frodo.go

Line 173 in 55a39bb

zeroBytes := []byte("")
the gc may completely destroy slice since there are no references to it past the push_write_request call

@agnivade
Copy link
Owner

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.

@pdeva
Copy link
Author

pdeva commented Jul 22, 2021

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

@agnivade
Copy link
Owner

Yep, fair point.

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

2 participants