Jigsaw-Code / outline-go-tun2socks

Apache License 2.0
222 stars 94 forks source link

No re-initialization of TCP stack after Close() #114

Open floryst opened 1 year ago

floryst commented 1 year ago

I'm pretty sure this is related to https://github.com/Jigsaw-Code/outline-client/issues/882 and https://github.com/Jigsaw-Code/outline-client/issues/1476.

When testing the Outline VPN Android client (v1.10.0), I encountered the same logcat output from the linked issues when transitioning from WiFi to LTE while actively connected to an Outline server. I cannot reliably reproduce the issue; it occurs at random.

I did notice that the issue would arise when logcat (on VpnTunnelService) reported that UDP support transitioned from true to false. Looking through tun2socks, I think the issue is the t.lwipStack.Close() call in UpdateUDPSupport(). Close() gets called when UDP support changes, matching the logcat output. Since t.lwipStack is never re-initialized, any call to tunnel.Disconnect() afterwards will call t.lwipStack.Close() on an already closed t.lwipStack, resulting in a double-free. (I tested the double Close() call and got a double free.)

I suppose UpdateUDPSupport should call t.Disconnect() and then re-create a new LWIPStack and tunnel. I haven't had a chance to build the android client to verify the issue and test such a potential fix, so I'm writing up my analysis first to start the discussion.

fortuna commented 1 year ago

@jyyi1 FYI, since you are working on that code. We should make sure we don't have that problem in the new version of the code.