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

struct sss_cli_mc_ctx added fd_inode, ensure that open and close fd are #6987

Closed
wants to merge 1 commit into from
Closed

Conversation

answer9030
Copy link

Save the inode number while opening the file, and verify the inode number when closing the file to ensure that the same file is being opened and closed.

Used to solve the following issue:

@alexey-tikhonov
Copy link
Member

This approach won't solve your issue (#6986) because "for(int fd = 3; fd <= 32; ++fd) { close(fd);" will close a socket used for communication with sssd_nss as well. And next sss_client will try to write to fd opened by your code (tcp socket or whatever).

@answer9030
Copy link
Author

This approach won't solve your issue (#6986) because "for(int fd = 3; fd <= 32; ++fd) { close(fd);" will close a socket used for communication with sssd_nss as well. And next sss_client will try to write to fd opened by your code (tcp socket or whatever).

The socket will indeed be closed, and the corresponding fd may already be another file, but the purpose of my modification is to avoid libnss_ sss.so closes files opened by the process to avoid causing larger problems

@answer9030
Copy link
Author

And I have a question, why not close "/var/lib/sss/mc/*" 'fd after mmap?

@alexey-tikhonov
Copy link
Member

And I have a question, why not close "/var/lib/sss/mc/*" 'fd after mmap?

It's used in sss_nss_mc_validate()

@answer9030 answer9030 marked this pull request as draft December 4, 2023 08:40
@answer9030 answer9030 marked this pull request as ready for review December 4, 2023 08:44
@pbrezina
Copy link
Member

pbrezina commented Dec 4, 2023

Thank you for your contribution, we are certainly happy that people are willing to look into the code and submit a patch. However, I agree with Alexey, that this is not a solution for your case which needs to be fixed in your code. See: #6986 (comment) for details.

@pbrezina pbrezina closed this Dec 4, 2023
@answer9030
Copy link
Author

I think that libnss_ sss.so should adapt to various applications, rather than allowing applications to adapt to libnss_ sss.so. And SSSD's fds are invisible to applications, making it difficult for applications to adapt to libnss_ Sss.so.

@answer9030
Copy link
Author

Before closing Unix socket, the inode was verified. And I think the inode should also be verified here

@alexey-tikhonov
Copy link
Member

@answer9030 , is software in question - tigervnc?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport This should go to target branch only. Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants