From ef50a3c0f0686d708f08ee4754e80d68569831b7 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Wed, 30 Jul 2025 11:18:45 +0200 Subject: [PATCH] tests: Remove tests of operations on freed channels These tests are flaky because even though the care was taken to guess if the ssh_channel_free() really freed the channel, it might not always be correct and call to operation on the freed channel results in use after free. Generally, no operation should be called after the channel is freed by the user. Signed-off-by: Jakub Jelen Reviewed-by: Andreas Schneider --- tests/client/torture_session.c | 97 ---------------------------------- 1 file changed, 97 deletions(-) diff --git a/tests/client/torture_session.c b/tests/client/torture_session.c index cc83578f..673eae8b 100644 --- a/tests/client/torture_session.c +++ b/tests/client/torture_session.c @@ -321,51 +321,6 @@ static void torture_freed_channel_poll(void **state) assert_int_equal(rc, SSH_ERROR); } -/* Ensure that calling 'ssh_channel_poll_timeout' on a freed channel does not - * lead to segmentation faults. */ -static void torture_freed_channel_poll_timeout(void **state) -{ - struct torture_state *s = *state; - ssh_session session = s->ssh.session; - ssh_channel channel; - bool channel_freed = false; - char request[256]; - char buff[256] = {0}; - int rc; - - snprintf(request, 256, - "dd if=/dev/urandom of=/tmp/file bs=64000 count=2; hexdump -C /tmp/file"); - - channel = ssh_channel_new(session); - assert_non_null(channel); - - rc = ssh_channel_open_session(channel); - assert_ssh_return_code(session, rc); - - /* Make the request, read parts with close */ - rc = ssh_channel_request_exec(channel, request); - assert_ssh_return_code(session, rc); - - do { - rc = ssh_channel_read(channel, buff, 256, 0); - } while(rc > 0); - assert_ssh_return_code(session, rc); - - /* when either of these conditions is met the call to ssh_channel_free will - * actually free the channel so calling poll on that channel will be - * use-after-free */ - if ((channel->flags & SSH_CHANNEL_FLAG_CLOSED_REMOTE) || - (channel->flags & SSH_CHANNEL_FLAG_NOT_BOUND)) { - channel_freed = true; - } - ssh_channel_free(channel); - - if (!channel_freed) { - rc = ssh_channel_poll_timeout(channel, 500, 0); - assert_int_equal(rc, SSH_ERROR); - } -} - /* Ensure that calling 'ssh_channel_read_nonblocking' on a freed channel does * not lead to segmentation faults. */ static void torture_freed_channel_read_nonblocking(void **state) @@ -461,52 +416,6 @@ static void torture_channel_exit_signal(void **state) SAFE_FREE(exit_signal); } - -/* Ensure that calling 'ssh_channel_get_exit_status' on a freed channel does not - * lead to segmentation faults. */ -static void torture_freed_channel_get_exit_status(void **state) -{ - struct torture_state *s = *state; - ssh_session session = s->ssh.session; - ssh_channel channel; - bool channel_freed = false; - char request[256]; - char buff[256] = {0}; - int rc; - - snprintf(request, 256, - "dd if=/dev/urandom of=/tmp/file bs=64000 count=2; hexdump -C /tmp/file"); - - channel = ssh_channel_new(session); - assert_non_null(channel); - - rc = ssh_channel_open_session(channel); - assert_ssh_return_code(session, rc); - - /* Make the request, read parts with close */ - rc = ssh_channel_request_exec(channel, request); - assert_ssh_return_code(session, rc); - - do { - rc = ssh_channel_read(channel, buff, 256, 0); - } while(rc > 0); - assert_ssh_return_code(session, rc); - - /* when either of these conditions is met the call to ssh_channel_free will - * actually free the channel so calling poll on that channel will be - * use-after-free */ - if ((channel->flags & SSH_CHANNEL_FLAG_CLOSED_REMOTE) || - (channel->flags & SSH_CHANNEL_FLAG_NOT_BOUND)) { - channel_freed = true; - } - SSH_CHANNEL_FREE(channel); - - if (!channel_freed) { - rc = ssh_channel_get_exit_status(channel); - assert_ssh_return_code_equal(session, rc, SSH_ERROR); - } -} - static void torture_channel_read_stderr(void **state) { @@ -611,9 +520,6 @@ int torture_run_tests(void) { cmocka_unit_test_setup_teardown(torture_freed_channel_poll, session_setup, session_teardown), - cmocka_unit_test_setup_teardown(torture_freed_channel_poll_timeout, - session_setup, - session_teardown), cmocka_unit_test_setup_teardown(torture_freed_channel_read_nonblocking, session_setup, session_teardown), @@ -623,9 +529,6 @@ int torture_run_tests(void) { cmocka_unit_test_setup_teardown(torture_channel_exit_signal, session_setup, session_teardown), - cmocka_unit_test_setup_teardown(torture_freed_channel_get_exit_status, - session_setup, - session_teardown), cmocka_unit_test_setup_teardown(torture_channel_read_stderr, session_setup, session_teardown),