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

recvmsg: Check if CMSG buffer was too small and return an error #2413

Merged
merged 1 commit into from
May 22, 2024

Conversation

agrover
Copy link
Contributor

@agrover agrover commented May 21, 2024

If MSG_CTRUNC is set, it is not safe to iterate the cmsgs, since they could have been truncated. Change RecvMsg::cmsgs() to return a Result, and to check for this flag (an API change).

Update tests for API change. Add test for too-small buffer.

fixes #2400

Checklist:

  • I have read CONTRIBUTING.md
  • I have written necessary tests and rustdoc comments
  • A change log has been added if this PR modifies nix's API

@agrover agrover force-pushed the check-ctrunc branch 6 times, most recently from 53b4487 to f92993f Compare May 21, 2024 17:53
Copy link
Member

@SteveLauC SteveLauC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the fix! Generally looks good!

src/sys/socket/mod.rs Outdated Show resolved Hide resolved
test/sys/test_socket.rs Outdated Show resolved Hide resolved
@agrover
Copy link
Contributor Author

agrover commented May 22, 2024

Proposed fix accepted and squashed. Took a swag at updating docstring to mention the new error condition.

If MSG_CTRUNC is set, it is not safe to iterate the cmsgs, since they
could have been truncated. Change RecvMsg::cmsgs() to return a Result,
and to check for this flag (an API change).

Update tests for API change. Add test for too-small buffer.
@agrover agrover requested a review from SteveLauC May 22, 2024 16:37
Copy link
Member

@SteveLauC SteveLauC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@SteveLauC SteveLauC added this pull request to the merge queue May 22, 2024
Merged via the queue into nix-rust:master with commit 208b80b May 22, 2024
36 checks passed
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

Successfully merging this pull request may close these issues.

RecvMsg.cmsg() should check MSG_CTRUNC
2 participants