Skip to content

CmsgSpace::new unsafely creates uninitialized, arbitrary types #994

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.

Sign up for GitHub

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

Jump to bottom
Closed
goffrie opened this issue Dec 6, 2018 · 2 comments · Fixed by #1020
Closed

CmsgSpace::new unsafely creates uninitialized, arbitrary types #994

goffrie opened this issue Dec 6, 2018 · 2 comments · Fixed by #1020

Comments

@goffrie
Copy link

goffrie commented Dec 6, 2018

Even though the uninitialized field is private, it still gets dropped. For example nix::sys::socket::CmsgSpace::<String>::new(); easily causes a segfault.

The field should probably be ManuallyDrop<T>, or there should be a T: Copy bound.
@asomers
Copy link
Member

asomers commented Dec 13, 2018

Good catch. Unfortunately, we can't require Copy. If we did, our own tests wouldn't compile! That's because libc structs aren't Copy. Would you like to submit a PR that implements the ManuallyDrop solution?

@asomers
Copy link
Member

asomers commented Jan 15, 2019

Nevermind. I'm going to eliminate CmsgSpace instead.

asomers added a commit to asomers/nix that referenced this issue Jan 30, 2019
@asomers
Replace CmsgSpace with a macro
139846a 
CmsgSpace had three problems:
1) It would oversize buffers that expect multiple control messages
2) It didn't use the libc CMSG_SPACE(3) macro, so it might actually
   undersize a buffer for a single control message.
3) It could do bad things on drop, if you instantiate it with a type
   that implements Drop (which none of the currently supported
   ControlMessage types do).

Fixes nix-rust#994
@asomers asomers mentioned this issue Jan 30, 2019
Merged
asomers added a commit to asomers/nix that referenced this issue Jan 31, 2019
@asomers
Replace CmsgSpace with a macro
ab6166a 
CmsgSpace had three problems:
1) It would oversize buffers that expect multiple control messages
2) It didn't use the libc CMSG_SPACE(3) macro, so it might actually
   undersize a buffer for a single control message.
3) It could do bad things on drop, if you instantiate it with a type
   that implements Drop (which none of the currently supported
   ControlMessage types do).

Fixes nix-rust#994
asomers added a commit to asomers/nix that referenced this issue Feb 12, 2019
@asomers
Replace CmsgSpace with a macro
9f69c7d 
CmsgSpace had three problems:
1) It would oversize buffers that expect multiple control messages
2) It didn't use the libc CMSG_SPACE(3) macro, so it might actually
   undersize a buffer for a single control message.
3) It could do bad things on drop, if you instantiate it with a type
   that implements Drop (which none of the currently supported
   ControlMessage types do).

Fixes nix-rust#994
asomers added a commit to asomers/nix that referenced this issue Feb 14, 2019
@asomers
Replace CmsgSpace with a macro
e0f612d 
CmsgSpace had three problems:
1) It would oversize buffers that expect multiple control messages
2) It didn't use the libc CMSG_SPACE(3) macro, so it might actually
   undersize a buffer for a single control message.
3) It could do bad things on drop, if you instantiate it with a type
   that implements Drop (which none of the currently supported
   ControlMessage types do).

Fixes nix-rust#994
bors bot added a commit that referenced this issue Feb 14, 2019
@bors @asomers
Merge #1020
a2fa282 
1020: Major cmsg cleanup r=asomers a=asomers

This PR fixes multiple bugs in the cmsg code.
Fixes #994 
Fixes #999 
Fixes #1013 

Co-authored-by: Alan Somers <[email protected]>
@bors bors bot closed this as completed in #1020 Feb 14, 2019
vdagonneau pushed a commit to vdagonneau/nix that referenced this issue Feb 20, 2019
@asomers @vdagonneau
Replace CmsgSpace with a macro
b3a94a6 
CmsgSpace had three problems:
1) It would oversize buffers that expect multiple control messages
2) It didn't use the libc CMSG_SPACE(3) macro, so it might actually
   undersize a buffer for a single control message.
3) It could do bad things on drop, if you instantiate it with a type
   that implements Drop (which none of the currently supported
   ControlMessage types do).

Fixes nix-rust#994
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Major cmsg cleanup asomers/nix
2 participants
@asomers @goffrie