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

[Jakub Jelen <jjelen@xxxxxxxxxx>]

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.