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

main: fix Array out of bounds access #1184

Closed
wants to merge 5 commits into from
Closed

Conversation

jobo-zt
Copy link
Contributor

@jobo-zt jobo-zt commented Sep 4, 2024

No description provided.

@jobo-zt
Copy link
Contributor Author

jobo-zt commented Sep 4, 2024

maby this right, in multi thread.

if (!fhs || !fhs->fh)
	continue;
++cfds;

@sreimers sreimers marked this pull request as draft September 7, 2024 09:03
@jobo-zt jobo-zt marked this pull request as ready for review September 8, 2024 02:32
@sreimers sreimers marked this pull request as draft September 21, 2024 09:11
@sreimers
Copy link
Member

I think this is hiding the underlying problem, there should never more active file descriptors than maxfds. I have to investigate, we should first add a test case to reproduce the problem. I can have a look next week.

@jobo-zt
Copy link
Contributor Author

jobo-zt commented Sep 24, 2024

udp_thread_detach needs to end in the thread where fd_sten is located, otherwise (! Fhs | |! Fhs ->fh) holds and continues to loop.

Can the above description provide some clues for testing work. thanks

@cspiel1 cspiel1 added this to the v3.17.0 milestone Oct 2, 2024
@alfredh
Copy link
Contributor

alfredh commented Oct 10, 2024

I dont think this change is needed.

It would be nice to have a testcase first to demonstrate the problem...

@alfredh alfredh removed this from the v3.17.0 milestone Oct 10, 2024
@jobo-zt
Copy link
Contributor Author

jobo-zt commented Oct 17, 2024

In the multi thread of restund, it is not in the same thread, so there will be problems when ending the UDP connection.

  1. The purpose of this change is to increase boundary security.
  2. As a result, some efficiency will be lost in the main loop.

@jobo-zt jobo-zt closed this Oct 17, 2024
@jobo-zt jobo-zt deleted the fixbug branch October 17, 2024 12:35
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.

4 participants