[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] channels: Fix segfaults when the channel data is freed


On 8/15/21 10:01 PM, Artyom V. Poptsov wrote:
Hello,

I've found that some channel procedures in libssh 0.9.3 on Ubuntu GNU/Linux 20.04 (and the latest versions from Git) fails with SEGFAULT when a parent session of a channel is freed.

I investigated the errors and found that some "channels.h" procedures (namely 'ssh_channel_is_open' and 'ssh_channel_is_closed') try to determine whether the parent session is alive by checking 'channel->session->alive' variable.  But when a channel is freed by 'ssh_channel_do_free' procedure, its 'session' field is set to NULL.  In that situation calling e.g. 'ssh_channel_is_open' on a channel will lead to a SEGFAULT.

The patch to fix that behavior is attached to the email.

I ran "make test" on the libssh sources with my changes and all tests have passed successfully.

Thank you for the patch!

It looks fine to me technically, but can you adjust the formatting to match the coding style? Mostly minor details as spaces after "if" and aligning opening braces of multiline conditions (with moving the && to the end of previous lines) and replacing "! channel->session" with explicit check for NULL.

Do you have a simple way how to reproduce the behavior you encountered inside of the testsuite to make sure we will not reintroduce the issue in the future?

You can also submit a merge request in gitlab, which will run all testes on different configurations:

https://gitlab.com/libssh/libssh-mirror/

Thanks,
--
Jakub Jelen
Crypto Team, Security Engineering
Red Hat, Inc.


Follow-Ups:
Re: [PATCH] channels: Fix segfaults when the channel data is freedpoptsov.artyom@xxxxxxxxx (Artyom V. Poptsov)
Re: [PATCH] channels: Fix segfaults when the channel data is freedpoptsov.artyom@xxxxxxxxx (Artyom V. Poptsov)
References:
[PATCH] channels: Fix segfaults when the channel data is freed"Artyom V. Poptsov" <poptsov.artyom@xxxxxxxxx>
Archive administrator: postmaster@lists.cynapses.org