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

Need more configurability in fd handling behavior #644

Open
khuey opened this issue Nov 12, 2021 · 6 comments
Open

Need more configurability in fd handling behavior #644

khuey opened this issue Nov 12, 2021 · 6 comments

Comments

@khuey
Copy link
Contributor

khuey commented Nov 12, 2021

So I have a program that uses x11rb to implement xtrace-like functionality on copies of the data that flows over the x11 socket. When I parse messages I have copies of them but the fds don't actually exist in my program. RawFdContainer will try to close fds when it goes out of scope which could stomp on other fds in my process.

The most correct solution here is probably to replace RawFdContainer with a FdType parameter for all of the functions and structs that use it for encoding/decoding. (A dynamic trait probably won't work because there are structs that #[derive(Eq)] with RawFdContainer and Eq is not an object-safe trait). That's going to be rather invasive though. So I'd like to know if that's something you'd be open to before I do any work. If not, I'll just keep a local fork with the close in RawFdContainer patched out.

@eduardosm
Copy link
Collaborator

When I parse messages I have copies of them but the fds don't actually exist in my program. RawFdContainer will try to close fds when it goes out of scope which could stomp on other fds in my process.

If I understood correctly, RawFdContainers with dangling FDs are being created. How does this actually happen?

@khuey
Copy link
Contributor Author

khuey commented Nov 12, 2021

The problem is more that I can't (safely) create the RawFdContainers in the first place. I have a copy of the control messages from the recvmsg(2) but the corresponding fds don't actually exist in the kernel's fd table.

@psychon
Copy link
Owner

psychon commented Nov 13, 2021

but the corresponding fds don't actually exist in the kernel's fd table.

...why? How do you get the control messages without also getting the FDs that they refer to? And how are you creating the RawFdContainers currently for the dangling FDs? Couldn't you just patch that code to open /dev/null instead so that they have a FD to close?

I feel like I do not understand what is going on here. Do you have some code that I could look at?

@khuey
Copy link
Contributor Author

khuey commented Nov 18, 2021

I feel like I do not understand what is going on here. Do you have some code that I could look at?

I have an rr[0] recording of a process. That recording contains the results and memory written by each syscall, so from that I can reconstruct the traffic over the x11 socket. I have the complete struct msghdr including things like the control messages that tell which fds were transferred over the socket, but the socket doesn't actually exist in the process that's parsing the messages, nor do these transferred fds. I just have a copy of all the messages.

Right now I don't create any RawFdContainers so messages that expect to receive fds just fail to deserialize. I would like to fix that, hence this issue. Opening /dev/null to make fds is an option I guess but there's no guarantee I can get the precise fd number that was used in the original recording (since that fd number may be used for something different in the process that's processing this data) so it's not my preferred option.

[0] https://rr-project.org/

@psychon
Copy link
Owner

psychon commented Nov 18, 2021

Ah, thanks for that info. That clears things up a bit.

So... basically, it is impossible to get the FDs correct... I guess

but there's no guarantee I can get the precise fd number that was used in the original recording

Is that required for anything? Or do you want to avoid having to patch the msghdr instances to give them the right FDs?

Uhm... I'm not really sure how you are going about this, but why are the msghdrs important at all? Are you somehow replaying something with rr and using the RustConnection? Or which API of x11rb are you using?

Since this is just about parsing, I would expect that TryParseFd is the API to go to (plus some additional magic to figure out the right type for this call). If so, one can just provide a Vec containing a hundred FDs for /dev/null and the parsing code will fetch them from the Vec as needed.

@khuey
Copy link
Contributor Author

khuey commented Nov 29, 2021

Ah, thanks for that info. That clears things up a bit.

So... basically, it is impossible to get the FDs correct... I guess

but there's no guarantee I can get the precise fd number that was used in the original recording

Is that required for anything? Or do you want to avoid having to patch the msghdr instances to give them the right FDs?

It's not strictly required but I think it would be better to preserve the fd numbers if possible so that the output matches the original program precisely.

Uhm... I'm not really sure how you are going about this, but why are the msghdrs important at all? Are you somehow replaying something with rr and using the RustConnection? Or which API of x11rb are you using?

The msghdr contains the information needed to know if any fds were transferred with the message.

Since this is just about parsing, I would expect that TryParseFd is the API to go to (plus some additional magic to figure out the right type for this call). If so, one can just provide a Vec containing a hundred FDs for /dev/null and the parsing code will fetch them from the Vec as needed.

Yeah, TryParseFd is what I'm using.

For the time being I'm just carrying a fork with a changeset that comments out the parts of RawFdContainer that actually do kernel operations with the fds (e.g. dup/close). Supporting this behavior in mainline x11rb in addition to the current behavior may be too much work.

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

3 participants