From libssh2-devel-bounces@cool.haxx.se Fri Nov 15 23:27:05 2019 Return-Path: Received: from www.haxx.se (mail [127.0.0.1]) by giant.haxx.se (8.15.2/8.15.2/Debian-4) with ESMTP id xAFMQRkm030241; Fri, 15 Nov 2019 23:26:53 +0100 Received: from mail-io1-xd36.google.com (mail-io1-xd36.google.com [IPv6:2607:f8b0:4864:20:0:0:0:d36]) by giant.haxx.se (8.15.2/8.15.2/Debian-4) with ESMTPS id xAFMQOtq030167 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Fri, 15 Nov 2019 23:26:25 +0100 Received: by mail-io1-xd36.google.com with SMTP id i13so12166026ioj.5 for ; Fri, 15 Nov 2019 14:26:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=safe-com.20150623.gappssmtp.com; s=20150623; h=mime-version:from:date:message-id:subject:to; bh=/pmLr0EIveZUzIh8pn4LVEK5WPxTUuEVcoS5JDi8oJA=; b=KxKdI7ws+uxPf6KD8Hx9nRXRkfOg2q+ZyQrxg9/G5ym3gfPs4ErZfPwH9tApbzEiwk nj62SVcUyQrfLQ4Rf1mXBnLyPfJCmq5dbT1dimqoslLiiJLu4rdn20BLB300olBEWHAB /uWC9Rs+YtK6VTbJxeAPr8GqIsZ7p5xCFxNRbcxmtsV/+iX/AmV0czbIpNwtl6sSlu5z VE2oXoNLS9Kk0C+zWle+3FXgWxAm87RiwiOp7Ivn1Pv/Vs2GHN3y2iq/y4dD/ocTNzv1 xC2mvEdyEsarGcg5CBfaOOiDZIEJN5n+0uTDtCTYpBSOl146CQZC0GX1qokvzExVsgon Fzrg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=/pmLr0EIveZUzIh8pn4LVEK5WPxTUuEVcoS5JDi8oJA=; b=t99qkHPCK2KgkC54nWEeQX61Irwd5zCb0Xu0P4oMxZ6nmbLkOMs1Rh+0CVm5S3t70h xFpcYHaXVx4CFb0CaZpG1hbGm+Z61hDBc90/i/2XoAwvAvoZgaSv+sRx5OrnR8qoakkW dcXD6pTwZ0tccWvjYHXZZxawFPCYMg/y/guX2zsP4YXfN9wwHxSZV7MN2yjYN8NNXCGo QsggMfqyBTSJC/mQTD4LLFlj0GEiuIzP0pNQ9EtHBX72R6tipSNuYdYZu8QGDwVlFbGu C/sWYoV+0bs1TIzMYxj/qUUpd2JgCSqFwz10OmIQu26TLG7fGhQVmgYSeTfZ+XcCidp8 24eA== X-Gm-Message-State: APjAAAXWNALnjnRRqRXJMu50tmJyyHULv0I8LmCWDqDMrRJm5qT0xqau CBLeV/FmQoBn6XUbKqMTVS88p4m6jUR3bAuPylrvxHpQTC7Gcw== X-Google-Smtp-Source: APXvYqxJvXLwOercAdfnI2i4d7I2oUVftGCDOqeI+iMDBUtQhaEIYO4DWjdgSxWeVDTUO5NKPPj7zlL674M0wPhCeUU= X-Received: by 2002:a02:6591:: with SMTP id u139mr3084655jab.110.1573856777397; Fri, 15 Nov 2019 14:26:17 -0800 (PST) MIME-Version: 1.0 From: Joel DePooter Date: Fri, 15 Nov 2019 14:25:33 -0800 Message-ID: Subject: Re: 1.9.1 release + call for maintainers To: "libssh2-devel@cool.haxx.se" X-BeenThere: libssh2-devel@cool.haxx.se X-Mailman-Version: 2.1.22 Precedence: list List-Id: libssh2 development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: libssh2 development Content-Type: multipart/mixed; boundary="===============1274395961==" Errors-To: libssh2-devel-bounces@cool.haxx.se Sender: "libssh2-devel" --===============1274395961== Content-Type: multipart/alternative; boundary="0000000000002503b705976a167e" --0000000000002503b705976a167e Content-Type: text/plain; charset="UTF-8" I would like to see pull request 420 (or something similar) merged into the next release. It's been fairly complicated to trace through the various changes to this section of code, but I believe I have figured out the sequence of changes. From what I can tell, there was a memory leak with the OpenSSL AES-CTR ciphers. These cipher structs were created in _libssh2_openssl_crypto_init(), but never cleaned up. The leak was fixed in pull request 244, which added the _libssh2_openssl_crypto_exit() function. This change was included in the 1.9.0 release. However, that change also introduced a use-after-free bug. The function scoped static pointers in the _libssh2_EVP_aes_XXX_ctr() functions would never be reset to NULL when _libssh2_openssl_crypto_exit() is called, and therefore pointers to already freed structs would be returned from the the _libssh2_EVP_aes_XXX_ctr() functions. This leads to crashes during repeated init/cleanup cycles. The use-after-free problem was fixed in pull request 387, which was merged into master after the 1.9.0 release, so would be included in a 1.9.1 release. A crash in our application due to the use-after free bug is what made me look into this in the first place. However, that change re-introduced a similar memory leak as existed originally. The AES-CTR ciphers can now be created without being cleaned up, if the _libssh2_EVP_aes_XXX_ctr() functions are ever called outside the _libssh2_openssl_crypto_init() function. Previously, callers of these functions did not need to worry about freeing the result, as it was intended that there would only ever be one instance of each of the ciphers, and they would be cleaned up in the library shutdown. I believe that pull request 420 reinstates this behaviour. https://github.com/libssh2/libssh2/pull/244 https://github.com/libssh2/libssh2/pull/387 https://github.com/libssh2/libssh2/pull/42 Thanks, Joel --0000000000002503b705976a167e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
I would like to see pull request 420 (or something si= milar) merged into the next release. It's been fairly complicated to tr= ace through the various changes to this section of code, but I believe I ha= ve figured out the sequence of changes.

From w= hat I can tell, there was a memory leak with the OpenSSL AES-CTR ciphers. T= hese cipher structs were created in _libssh2_openssl_crypto_i= nit(), but never cleaned up. The leak was fixed in pull reque= st 244, which added the=20 _libssh2_openssl_crypto_exit() function. This change was incl= uded in the 1.9.0 release.

However, that change also introduced a use= -after-free bug. The function scoped static pointers in the=20 _libssh2_EVP_aes_XXX_ctr() functions would never be reset to = NULL when=20 _libssh2_openssl_crypto_exit() is called, and t= herefore pointers to already freed structs would be returned from the=C2=A0 the=20 _libssh2_EVP_aes_XXX_ctr() functions. This lead= s to crashes during repeated init/cleanup cycles. The use-aft= er-free problem was fixed in pull request 387, which was merged into master= after the 1.9.0 release, so would be included in a 1.9.1 release. A crash = in our application due to the use-after free bug is what made me look into = this in the first place.

However, that change re-introduced a sim= ilar memory leak as existed originally. The AES-CTR ciphers can now be crea= ted without being cleaned up, if the=20 _libssh2_EVP_aes_XXX_ctr() functions are ever called outside = the=20 _libssh2_openssl_crypto_init() function. Previo= usly, callers of these functions did not need to worry about freeing the re= sult, as it was intended that there would only ever be one instance of each= of the ciphers, and they would be cleaned up in the library shutdown. I be= lieve that pull request 420 reinstates this behaviour.

https://github.com= /libssh2/libssh2/pull/387

Thanks,
Joel<= br>
--0000000000002503b705976a167e-- --===============1274395961== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbGlic3NoMi1k ZXZlbCBodHRwczovL2Nvb2wuaGF4eC5zZS9jZ2ktYmluL21haWxtYW4vbGlzdGluZm8vbGlic3No Mi1kZXZlbAo= --===============1274395961==--