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

Violations of pointer passing rules #584

Open
ansiwen opened this issue Oct 1, 2021 · 6 comments
Open

Violations of pointer passing rules #584

ansiwen opened this issue Oct 1, 2021 · 6 comments
Assignees
Labels

Comments

@ansiwen
Copy link
Collaborator

ansiwen commented Oct 1, 2021

At least at one place we hand over Go pointers to the Ceph API that gonna get stored in a handler registry after the call returns, which violates the pointer passing rules:

go-ceph/rados/read_op.go

Lines 80 to 81 in 312e4cf

&gos.more,
&gos.rval,

So there is no guarantee that the pointers remain valid.

With the iterator right before:

&gos.iter,

we are safe, since - as far as I can tell - it doesn't get stored anywhere.

@phlogistonjohn
Copy link
Collaborator

Fixing this should largely just be a matter of allocating C-memory and storing C-pointers into our structs right?

@phlogistonjohn
Copy link
Collaborator

Also, do you have any thoughts on why this wasn't caught by the stricter checking we enable with the env var?

@ansiwen
Copy link
Collaborator Author

ansiwen commented Oct 1, 2021

Fixing this should largely just be a matter of allocating C-memory and storing C-pointers into our structs right?

Right. Or allocating the whole struct in C memory, if that is easier.

Also, do you have any thoughts on why this wasn't caught by the stricter checking we enable with the env var?

I think the checker can not do anything in this case, because it only can observe, what is passed to C, which is legal in this case. But it can't really check, what the C code is doing with the pointers. So we always have to be aware of that, to make sure, no ceph API is storing some pointers we are passing to it.

@gman0
Copy link
Contributor

gman0 commented Oct 5, 2021

Isn't the byte buffer writeStep.cBuffer in write ops susceptible to the same problem as well and might need to be malloc'd?

func newWriteStep(b []byte, writeLen, offset uint64) *writeStep {
return &writeStep{
b: b,
cBuffer: (*C.char)(unsafe.Pointer(&b[0])),

@ansiwen
Copy link
Collaborator Author

ansiwen commented Oct 6, 2021

Isn't the byte buffer writeStep.cBuffer in write ops susceptible to the same problem as well and might need to be malloc'd?

func newWriteStep(b []byte, writeLen, offset uint64) *writeStep {
return &writeStep{
b: b,
cBuffer: (*C.char)(unsafe.Pointer(&b[0])),

Yes, indeed. Thanks! Here we might use PtrGuard to break the rules. Else we need to copy the data.

@gman0
Copy link
Contributor

gman0 commented Oct 29, 2021

Do you have any ETA on this please, so that I can continue with PRs?

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

No branches or pull requests

3 participants