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

Panic when receiving too many fds? #14

Open
chadaustin opened this issue Aug 31, 2023 · 1 comment
Open

Panic when receiving too many fds? #14

chadaustin opened this issue Aug 31, 2023 · 1 comment

Comments

@chadaustin
Copy link

Regarding the comment and unreachable!() in

sendfd/src/lib.rs

Lines 155 to 164 in 69994c1

// This branch is unreachable. We allocate the ancillary data buffer just
// large enough to fit exactly the number of `RawFd`s that are in the `fds`
// buffer. It is not possible for the OS to return more of them.
//
// If this branch ended up being reachable for some reason, it would be
// necessary for this branch to close the file descriptors to avoid leaking
// resources.
//
// TODO: consider using unreachable_unchecked
unreachable!();

I am using sendfd in a new project. While researching the actual semantics of fd-passing, I found Kenton Varda's excellent summary of the subtleties and dangers: https://gist.github.com/kentonv/bc7592af98c68ba2738f4436920868dc

In particular:

The CMSG_SPACE() macro is intended to help you decide how much space to allocate to receive an ancillary message. It rounds up its calculation to the next word boundary. Unfortunately, on 64-bit systems, this means you will always end up with enough space for an even number of file descriptors. If you were expecting just one FD, you'll end up with enough buffer space to receive two. You MUST check whether you received two and close the second one, otherwise, again, an attacker can fill up your FD table.

That implies that calling recvmsg expecting one fd will panic if two are actually sent.

I have not tried to reproduce this issue in a test.

@nagisa
Copy link
Collaborator

nagisa commented Sep 1, 2023

I would accept a PR fixing this. It has been a while since I worked on this crate, so I may misremember some things, but IIRC one of the reasons we were careful around this area of the code was specifically because we didn't really want to make a decision that dropping unexpected file descriptors in the library is the right thing to do. In fact, if we cannot really receive exactly up-to the number of file descriptors as allowed by the file descriptor buffer, I would consider the API for this library to be wrong.

I wonder if it would be possible to set the msg_controllen to be exactly right despite what the CMSG_SPACE returns 🤔

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