Question about sshbuf

Ron Frederick ronf at
Tue May 24 09:18:52 AEST 2022

On May 23, 2022, at 3:55 PM, Damien Miller <djm at> wrote:
> On Mon, 23 May 2022, Ron Frederick wrote:
>>>>       ...snip...
>>>> out:
>>>>       sshbuf_free(server_host_key_blob);
>>>> I believe the problem here is that the call to sshpkt_getb_froms() is
>>>> returning an sshbuf in server_host_key_blob which is a reference to
>>>> the string being consumed from the packet being read, setting that
>>>> original packet as its parent. As a result, the “ssh” buffer now has
>>>> a refcount of 2, and when it returns to the top of the do {...} while
>>>> and tries to read another packet into “ssh”, it gets the error about
>>>> the sshbuf being “read-only” (for good reason).
>>> IMO the best fix for this would be to put a sshbuf_free() at the
>>> start of the do {} loop (it will handle a NULL argument fine).
>> Thanks Damien for the quick response.
>> Given that “ssh” is actually a “struct ssh *” and not a “struct sshbuf *”, what would be the right argument to pass into sshbuf_free()? It looks like it would need to be something like ssh->state->incoming_packet, but this calling code probably shouldn’t be poking into the internals like that. I don’t see a wrapper in packet.c which would do a free of that internal buffer.
> I'd do this:
>>>>                       /* If we've sent them data, they should reply */
>>>>                       do {
>                                    sshbuf_free(server_host_key_blob);
>                                    server_host_key_blob = NULL;
>>>>                               type = ssh_packet_read(ssh);
>>>>                               if (type == SSH2_MSG_KEXGSS_HOSTKEY) {
>>>>                                       debug("Received KEXGSS_HOSTKEY");
>>>>                                       if (server_host_key_blob)
>>>>                                               fatal("Server host key received more than once");
>>>>                                       if ((r = sshpkt_getb_froms(ssh, &server_host_key_blob)) != 0)
>>>>                                               fatal("Failed to read server host key: %s", ssh_err(r));
>>>>                               }
>>>>                       } while (type == SSH2_MSG_KEXGSS_HOSTKEY);

I don’t think that will work, as server_host_key_blob is used outside of the do..while loop. It needs to survive for the lifetime of the call to kexgss_client(). See line 282 for where it is used. 
Ron Frederick
ronf at

More information about the openssh-unix-dev mailing list