Re: ssh_set_callbacks bug ?

[Yanis Kurganov <yanis.kurganov@xxxxxxxxx>]

Yes, It's a bug.
I reported this in 2014 (search my message with subject "[PATCH] crash with
callbacks").
Migrate to new libssh log API or use my patch (attached).

2016-02-14 13:21 GMT+03:00 Giovanni Panozzo <giovanni@xxxxxxxxxx>:

>
> Hi to all. I just subscribed to the list... but I think I have been
> subscribed in the past :)
>
> While developing with libssh I have found the following problem:
>
> The lines at the end of ssh_set_callbacks() are:
>
> /* LEGACY */
> if (ssh_get_log_callback() == NULL && cb->log_function) {
>     ssh_set_log_callback(ssh_legacy_log_callback);
>     ssh_set_log_userdata(session);
> }
>
> the last line store "session" (a short life variable) in a global variable.
> That pointer to "sesson" may become later no longer available during a
> program run. For example when you close/destroy your first session and then
> reopen a second and enable logging callback.
> You can try to setup again callbacks to the second session, but
>  if (ssh_get_log_callback() == NULL
> will prevent the call to ssh_set_log_userdata().
>
> Crashing example here:
>
> --------------
>
> #include <stdio.h>
> #include <malloc.h>
> #include <libssh/libssh.h>
> #include <libssh/callbacks.h>
> #include <stdlib.h>
>
> void mylogfunc(ssh_session session, int priority, const char *message,
> void *userdata)
> {
>         printf("LOG MESSAGE: userdata=%p message=%s\n", userdata,message);
> }
>
> void onerun()
> {
>
>         ssh_session sshs;
>         struct ssh_callbacks_struct *cb;
>         int rc;
>         int verbosity = SSH_LOG_PROTOCOL;
>
>         sshs = ssh_new();
>         if (sshs == NULL) {
>                 printf("ssh_new() failed\n");
>                 return;
>         }
>
>         ssh_options_set(sshs, SSH_OPTIONS_HOST, "localhost");
>         ssh_options_set(sshs, SSH_OPTIONS_LOG_VERBOSITY, &verbosity);
>
>         cb = malloc(sizeof(struct ssh_callbacks_struct));
>         memset(cb, 0, sizeof(struct ssh_callbacks_struct));
>         ssh_callbacks_init(cb);
>         cb->log_function = mylogfunc;
>         cb->userdata = NULL;
>         ssh_set_callbacks(sshs, cb);
>
>         rc = ssh_connect(sshs);
>         if (rc != SSH_OK) {
>                 printf("Unable to connect to server\n");
>         }
>         ssh_disconnect(sshs);
>
>         ssh_free(sshs);
>         free(cb);
>
> }
>
> int main()
> {
>         onerun();
>         // Allocate a new block of memory so ssh_new() will not reuse the
> old memory,
>         // making the pointer saved in ssh_set_callbacks() with
> ssh_set_log_userdata()
>         // erroneously available again
>         malloc(2000);
>         onerun();
> }
>
>
> -----
> As a workaround in user code, ssh_set_log_userdata(sshs) can be called
> just after a successful call to ssh_new.
>
> --------------
>
> Is this a bug or I'm doing something wrong ?
>
>
> Thank you
>
> Giovanni
>
>
>

Attachment: 0001-fix-crash-with-callbacks.patch
Description: Binary data