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

Missing Close() in gvisor-tap-vsock, or tcpproxy bug? #387

Open
cfergeau opened this issue Aug 21, 2024 · 3 comments
Open

Missing Close() in gvisor-tap-vsock, or tcpproxy bug? #387

cfergeau opened this issue Aug 21, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@cfergeau
Copy link
Collaborator

We cannot use the latest github.com/inetaf/tcpproxy commit because of 61dc4e1 which causes a regression in podman containers/podman#23616

My feeling is that this revert is either hiding a gvisor-tap-sock bug which has been present for a long time, or it's avoiding a newly introduced bug in inetaf/tcpproxy. In either cases ,it would be good to understand better what caused the issue leading to the revert. The podman bug has a reproducer for it.

@cfergeau
Copy link
Collaborator Author

cfergeau commented Sep 19, 2024

This can be reproduced with podman machine by following these steps (I tested on a mac):

  • if podman was installed with their installer, replace /opt/podman/bin/gvproxy with the binary you want to test
  • podman machine stop && podman machine start
  • podman run -it --rm -p 8111:8111 ubi9
  • in the container, yum install -y nmap-ncat followed by nc -l -k -v 8111
  • in another terminal on the mac, echo hello | nc localhost 8111
  • if this command does not return without ctrl+c, then you are seeing the bug
  • you can also run it multiple times in a row, and then check lsof -nP -iTCP | grep 8111, there will be multiple lines when the bug occurs

@evidolob
Copy link
Contributor

evidolob commented Oct 8, 2024

@cfergeau I make some investigation on this, it look like that problem is in recent version of /inetaf/tcpproxy
The problem is in
https://github.com/inetaf/tcpproxy/blob/3ce58045626c8bc343a593c90354975e61b1817a/tcpproxy.go#L404-L408

If you look on 61dc4e1 it is removing second <-errc, which is added in newer version. For me, that second channel receive is never resolve, so function HandleConn is never return and connections is never closed.
So if I remove that second <-errc in newest version it fix the issue.
Look like one of go proxyCopy calls is never returns(put anything in the errc channel), going to look deeper in to the code

@cfergeau
Copy link
Collaborator Author

cfergeau commented Oct 8, 2024

If you look on 61dc4e1 it is removing second <-errc, which is added in newer version. For me, that second channel receive is never resolve, so function HandleConn is never return and connections is never closed.
So if I remove that second <-errc in newest version it fix the issue.

Thanks for the research! Looks consistent with the investigation which was done in containers/podman#23616

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants