Re: [patch] SIGSEGV when adding Connector to Event
On 11.02.19 09:04, g4-l...@tonarchiv.ch wrote: > Hi there > > Could someone reproduce this? You need something else to accept the patch? > Ignore this mail... I should check Git more often :-))
Re: [patch] SIGSEGV when adding Connector to Event
On 30.01.19 15:33, g4-l...@tonarchiv.ch wrote: > Hi all, > > You finally find the test code attached. It's a bit complicated, but > it can reproduce the issue: > > 1. Connect to an ssh server which supports direct socket tunnelling: > > > ./multichannelfw root myserver.com 22 > > This opens a local listening port at 20021 > > 2. Open two or more simultaneous ssh sessions via tunnel: > > > ssh root@localhost -p20021 > > 3. Close one session > > 4. Open a new one (will stall) > > 5. Trigger some traffic by typing something in an already open session* > > 6. Segmentation fault > Hi there Could someone reproduce this? You need something else to accept the patch? Cheers, Till
Re: [patch] SIGSEGV when adding Connector to Event
Hi Andreas, did you test it? It would be great if this patch could go to the lib. At the moment I have to statically link my ssh app with my own libssh version... Cheers, Till On 30.01.19 15:33, g4-l...@tonarchiv.ch wrote: > > Hi all, > > You finally find the test code attached. It's a bit complicated, but > it can reproduce the issue: > > 1. Connect to an ssh server which supports direct socket tunnelling: > > > ./multichannelfw root myserver.com 22 > > This opens a local listening port at 20021 > > 2. Open two or more simultaneous ssh sessions via tunnel: > > > ssh root@localhost -p20021 > > 3. Close one session > > 4. Open a new one (will stall) > > 5. Trigger some traffic by typing something in an already open session* > > 6. Segmentation fault > > * This is also affected by my other patch > "ssh_handle_packets_termination() ignores timeout=0". With this patch, > there's no need to trigger traffic to unblock receving messages) > > Regards, > Till > > On 28.01.19 23:10, g4-l...@tonarchiv.ch wrote: >> >> Hi all >> >> There's a bug in the connector API when subsequently adding to and >> removing connectors from an event loop. >> >> Here's some dummy code to reproduce it (I will add real code later): >> >> event = ssh_event_new(); >> >> /* ADD FIRST connector pair (in/out) */ >> ssh_connector in1 = ssh_connector_new(session); >> ssh_connector_set_out_channel(...); >> ssh_connector_set_in_fd(...); >> ssh_event_add_connector(event, in1); >> >> >> ssh_connector out1 = ssh_connector_new(session); >> ssh_connector_set_out_fd(...); >> >> >> ssh_event_dopoll(event, 2000); >> >> /* ADD SECOND connector pair */ >> ssh_connector in2 = ssh_connector_new(session); >> ssh_connector_set_out_channel(...); >> ssh_connector_set_in_fd(...); >> ssh_event_add_connector(event, in2); >> >> >> ssh_connector out2 = ssh_connector_new(session); >> ssh_connector_set_out_fd(...); >> >> >> ssh_event_dopoll(event, 2000); >> >> /* work done with connector pair 1 */ >> >> /* REMOVE FIRST connector pair */ >> ssh_event_remove_connector(event, in1); >> ssh_event_remove_connector(event, out1); >> ssh_connector_free(in1); >> ssh_connector_free(in1); >> >> >> ssh_event_dopoll(event, 2000); >> >> /* ADD THIRD connector pair */ >> ssh_connector in3 = ssh_connector_new(session); >> ssh_connector_set_out_channel(...); >> ssh_connector_set_in_fd(...); >> ssh_event_add_connector(event, in3); >> ***SIGSEGV >> >> The underlying problem is ssh_connector_remove_event() where >> connector->in_channel and ->out_channel is NULLed. >> >> The subsequent call to ssh_connector_free() does not remove the >> in_channel and out_channel callbacks, because in_ and out_channel is >> NULL. >> So we have some sticky callbacks attached to the channel, and >> ssh_connector_remove_event() has removed the poll objects... >> >> Later on, when adding a new connector, ssh_channel_poll_timeout() is >> called at a certain point, which calls the CB function with the >> polling objects removed. >> >> And voila: >> >> Thread 3 "sshfwd" received signal SIGSEGV, Segmentation fault. >> [Switching to Thread 0x74574700 (LWP 15333)] >> 0x77b76a04 in ssh_poll_get_events (p=0x0) at >> /home/till/libssh/src/poll.c:403 >> 403 return p->events; >> (gdb) backtrace >> #0 0x77b76a04 in ssh_poll_get_events (p=0x0) at >> /home/till/libssh/src/poll.c:403 >> #1 0x77b76ac3 in ssh_poll_add_events (p=0x0, events=4) at >> /home/till/libssh/src/poll.c:443 >> #2 0x77b56cd1 in ssh_connector_reset_pollevents >> (connector=0x7fffec0045b0) at /home/till/libssh/src/connector.c:235 >> #3 0x77b5757a in ssh_connector_channel_data_cb >> (session=0x7fffec0008c0, channel=0x7fffec006cc0, data=0x7fffec009940, >> len=120, >> is_stderr=0, userdata=0x7fffec0045b0) at >> /home/till/libssh/src/connector.c:489 >> #4 0x77b4cf50 in channel_rcv_data (session=0x7fffec0008c0, type=94 >> '^', packet=0x7fffec001450, user=0x7fffec0008c0) >> at /home/till/libssh/src/channels.c:563 >> #5 0x77b6dfb5 in ssh_packet_process (session=0x7fffec0008c0, >> type=94 '^') at /home/till/libssh/src/packet.c:1421 >> #6 0x77b6d9aa in ssh_packet_socket_callback (data=0x7fffec008d40, >> receivedlen=176, user=0x7fffec0008c0) >> at /home/till/libssh/src/packet.c:1275 >> #7 0x77b7ba1d in ssh_socket_pollcallback (p=0x7fffec004cc0, fd=6, >> revents=1, v_s=0x7fffec001280) >> at /home/till/libssh/src/socket.c:302 >> #8 0x77b771ad in ssh_poll_ctx_dopoll (ctx=0x7fffec006900, >> timeout=-1) at /home/till/libssh/src/poll.c:702 >> #9 0x77b789eb in ssh_handle_packets (session=0x7fffec0008c0, >> timeout=-1) at /home/till/libssh/src/session.c:643 >> #10 0x77b78af1 in ssh_handle_packets_termination >> (session=0x7fffec0008c0, timeout=0, >> fct=0x77b502c1 , user=0x7456fb40) >> at /home/till/libssh/src/session.c:711 >> #11 0x77b50867 in
Re: [patch] SIGSEGV when adding Connector to Event
Hi all, You finally find the test code attached. It's a bit complicated, but it can reproduce the issue: 1. Connect to an ssh server which supports direct socket tunnelling: > ./multichannelfw root myserver.com 22 This opens a local listening port at 20021 2. Open two or more simultaneous ssh sessions via tunnel: > ssh root@localhost -p20021 3. Close one session 4. Open a new one (will stall) 5. Trigger some traffic by typing something in an already open session* 6. Segmentation fault * This is also affected by my other patch "ssh_handle_packets_termination() ignores timeout=0". With this patch, there's no need to trigger traffic to unblock receving messages) Regards, Till On 28.01.19 23:10, g4-l...@tonarchiv.ch wrote: > > Hi all > > There's a bug in the connector API when subsequently adding to and > removing connectors from an event loop. > > Here's some dummy code to reproduce it (I will add real code later): > > event = ssh_event_new(); > > /* ADD FIRST connector pair (in/out) */ > ssh_connector in1 = ssh_connector_new(session); > ssh_connector_set_out_channel(...); > ssh_connector_set_in_fd(...); > ssh_event_add_connector(event, in1); > > > ssh_connector out1 = ssh_connector_new(session); > ssh_connector_set_out_fd(...); > > > ssh_event_dopoll(event, 2000); > > /* ADD SECOND connector pair */ > ssh_connector in2 = ssh_connector_new(session); > ssh_connector_set_out_channel(...); > ssh_connector_set_in_fd(...); > ssh_event_add_connector(event, in2); > > > ssh_connector out2 = ssh_connector_new(session); > ssh_connector_set_out_fd(...); > > > ssh_event_dopoll(event, 2000); > > /* work done with connector pair 1 */ > > /* REMOVE FIRST connector pair */ > ssh_event_remove_connector(event, in1); > ssh_event_remove_connector(event, out1); > ssh_connector_free(in1); > ssh_connector_free(in1); > > > ssh_event_dopoll(event, 2000); > > /* ADD THIRD connector pair */ > ssh_connector in3 = ssh_connector_new(session); > ssh_connector_set_out_channel(...); > ssh_connector_set_in_fd(...); > ssh_event_add_connector(event, in3); > ***SIGSEGV > > The underlying problem is ssh_connector_remove_event() where > connector->in_channel and ->out_channel is NULLed. > > The subsequent call to ssh_connector_free() does not remove the > in_channel and out_channel callbacks, because in_ and out_channel is NULL. > So we have some sticky callbacks attached to the channel, and > ssh_connector_remove_event() has removed the poll objects... > > Later on, when adding a new connector, ssh_channel_poll_timeout() is > called at a certain point, which calls the CB function with the > polling objects removed. > > And voila: > > Thread 3 "sshfwd" received signal SIGSEGV, Segmentation fault. > [Switching to Thread 0x74574700 (LWP 15333)] > 0x77b76a04 in ssh_poll_get_events (p=0x0) at > /home/till/libssh/src/poll.c:403 > 403 return p->events; > (gdb) backtrace > #0 0x77b76a04 in ssh_poll_get_events (p=0x0) at > /home/till/libssh/src/poll.c:403 > #1 0x77b76ac3 in ssh_poll_add_events (p=0x0, events=4) at > /home/till/libssh/src/poll.c:443 > #2 0x77b56cd1 in ssh_connector_reset_pollevents > (connector=0x7fffec0045b0) at /home/till/libssh/src/connector.c:235 > #3 0x77b5757a in ssh_connector_channel_data_cb > (session=0x7fffec0008c0, channel=0x7fffec006cc0, data=0x7fffec009940, > len=120, > is_stderr=0, userdata=0x7fffec0045b0) at > /home/till/libssh/src/connector.c:489 > #4 0x77b4cf50 in channel_rcv_data (session=0x7fffec0008c0, type=94 > '^', packet=0x7fffec001450, user=0x7fffec0008c0) > at /home/till/libssh/src/channels.c:563 > #5 0x77b6dfb5 in ssh_packet_process (session=0x7fffec0008c0, type=94 > '^') at /home/till/libssh/src/packet.c:1421 > #6 0x77b6d9aa in ssh_packet_socket_callback (data=0x7fffec008d40, > receivedlen=176, user=0x7fffec0008c0) > at /home/till/libssh/src/packet.c:1275 > #7 0x77b7ba1d in ssh_socket_pollcallback (p=0x7fffec004cc0, fd=6, > revents=1, v_s=0x7fffec001280) > at /home/till/libssh/src/socket.c:302 > #8 0x77b771ad in ssh_poll_ctx_dopoll (ctx=0x7fffec006900, > timeout=-1) at /home/till/libssh/src/poll.c:702 > #9 0x77b789eb in ssh_handle_packets (session=0x7fffec0008c0, > timeout=-1) at /home/till/libssh/src/session.c:643 > #10 0x77b78af1 in ssh_handle_packets_termination > (session=0x7fffec0008c0, timeout=0, > fct=0x77b502c1 , user=0x7456fb40) > at /home/till/libssh/src/session.c:711 > #11 0x77b50867 in ssh_channel_poll_timeout (channel=0x7fffec006cc0, > timeout=0, is_stderr=0) > at /home/till/libssh/src/channels.c:2974 > #12 0x77b5798c in ssh_connector_set_event (connector=0x7fffec007bb0, > event=0x7fffec0047f0) at /home/till/libssh/src/connector.c:607 > #13 0x77b7762d in ssh_event_add_connector (event=0x7fffec0047f0, > connector=0x7fffec007bb0) at
Re: [patch] SIGSEGV when adding Connector to Event
On Tuesday, 29 January 2019 12:05:28 CET g4-l...@tonarchiv.ch wrote: > On 29.01.19 09:49, Andreas Schneider wrote: > > On Monday, 28 January 2019 23:10:09 CET g4-l...@tonarchiv.ch wrote: > >> Hi all > >> > >> There's a bug in the connector API when subsequently adding to and > >> removing connectors from an event loop. > > > >> Here's some dummy code to reproduce it (I will add real code later): > > Hi Till, > > > > thank you very much for your contribution. The e-mail the patch has been > > created with looks like a company address. Could you please send the > > Certificate of Origin from that address? > > Hi Andreas, > > oh I see... This was not on purpose. My contributions can be considered > as private work. I attached a new patch. Then please resend the doc fixes too :-) -- Andreas Schneider a...@cryptomilk.org GPG-ID: 8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D
Re: [patch] SIGSEGV when adding Connector to Event
On 29.01.19 09:49, Andreas Schneider wrote:
> On Monday, 28 January 2019 23:10:09 CET g4-l...@tonarchiv.ch wrote:
>> Hi all
>>
>> There's a bug in the connector API when subsequently adding to and
>> removing connectors from an event loop.
>>
>> Here's some dummy code to reproduce it (I will add real code later):
> Hi Till,
>
> thank you very much for your contribution. The e-mail the patch has been
> created with looks like a company address. Could you please send the
> Certificate of Origin from that address?
Hi Andreas,
oh I see... This was not on purpose. My contributions can be considered
as private work. I attached a new patch.
Regards,
Till
>From 80d13178376fef3fa897b390cc1aad6830090712 Mon Sep 17 00:00:00 2001
From: Till Wimmer
Date: Tue, 29 Jan 2019 12:01:31 +0100
Subject: [PATCH] Don't NULL connector->channels on event remove
Signed-off-by: Till Wimmer
---
src/connector.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/connector.c b/src/connector.c
index c4f6af5..0062785 100644
--- a/src/connector.c
+++ b/src/connector.c
@@ -641,14 +641,12 @@ int ssh_connector_remove_event(ssh_connector connector) {
session = ssh_channel_get_session(connector->in_channel);
ssh_event_remove_session(connector->event, session);
-connector->in_channel = NULL;
}
if (connector->out_channel != NULL) {
session = ssh_channel_get_session(connector->out_channel);
ssh_event_remove_session(connector->event, session);
-connector->out_channel = NULL;
}
connector->event = NULL;
--
2.7.4
