You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Yann Ylavic <yl...@gmail.com> on 2017/02/16 22:48:01 UTC
Re: svn commit: r1783317 - /httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
On Thu, Feb 16, 2017 at 11:27 PM, <wr...@apache.org> wrote:
> Author: wrowe
> Date: Thu Feb 16 22:27:24 2017
> New Revision: 1783317
>
> URL: http://svn.apache.org/viewvc?rev=1783317&view=rev
> Log:
> Avoid unnecessary code (the deprecation macro wrapper itself emits unused args
> warnings) in OpenSSL 1.1.0 and avoid _free()ing NULL references.
>
>
> Modified:
> httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
[]
> #endif
> }
> - EC_KEY_free(eckey);
> - EC_GROUP_free(ecparams);
> +#endif
> + if (eckey)
> + EC_KEY_free(eckey);
> + if (ecparams)
> + EC_GROUP_free(ecparams);
ISTM that these are NULL safe already.
Re: svn commit: r1783317 - /httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Feb 16, 2017 17:33, "Jacob Champion" <ch...@gmail.com> wrote:
On 02/16/2017 03:16 PM, William A Rowe Jr wrote:
> With no docs to that effect, and trying to predict what 1.2.0 might do
> to us, the explicit avoidance seems safer, no?
>
There are docs to that effect for 1.1.0.
https://www.openssl.org/docs/man1.1.0/crypto/EC_GROUP_free.html
https://www.openssl.org/docs/man1.1.0/crypto/EC_KEY_free.html
"If [group, key] is NULL nothing is done."
Is there somewhere it is documented what openssl's NULL-safe policy is?
>
Closest I can find quickly is a recent discussion:
https://github.com/openssl/openssl/issues/2271
From Rich Salz: "All free functions, like C free will be a no-op for NULL.
No other changes are planned at this time. Having said that, I am only one
member of the team, and I may leave or get out-voted by others. What I can
promise is that we are going to try to avoid semantic changes before 1.2,
which is unlikely to happen this year."
And from Kurt Roeckx: "Calling free() with NULL is expected and documented
behaviour, we actually changed it in all functions to work. It seems very
unlikely that we will change that."
Excellent, I'll back out that change and the needless _internal() case
tomorrow if nobody beats me to it.
Thanks for the citations!
Re: svn commit: r1783317 -
/httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
Posted by Jacob Champion <ch...@gmail.com>.
On 02/16/2017 03:16 PM, William A Rowe Jr wrote:
> With no docs to that effect, and trying to predict what 1.2.0 might do
> to us, the explicit avoidance seems safer, no?
There are docs to that effect for 1.1.0.
https://www.openssl.org/docs/man1.1.0/crypto/EC_GROUP_free.html
https://www.openssl.org/docs/man1.1.0/crypto/EC_KEY_free.html
"If [group, key] is NULL nothing is done."
> Is there somewhere it is documented what openssl's NULL-safe policy is?
Closest I can find quickly is a recent discussion:
https://github.com/openssl/openssl/issues/2271
From Rich Salz: "All free functions, like C free will be a no-op for
NULL. No other changes are planned at this time. Having said that, I am
only one member of the team, and I may leave or get out-voted by others.
What I can promise is that we are going to try to avoid semantic changes
before 1.2, which is unlikely to happen this year."
And from Kurt Roeckx: "Calling free() with NULL is expected and
documented behaviour, we actually changed it in all functions to work.
It seems very unlikely that we will change that."
--Jacob
Re: svn commit: r1783317 - /httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Thu, Feb 16, 2017 at 4:48 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Thu, Feb 16, 2017 at 11:27 PM, <wr...@apache.org> wrote:
>> Author: wrowe
>> Date: Thu Feb 16 22:27:24 2017
>> New Revision: 1783317
>>
>> URL: http://svn.apache.org/viewvc?rev=1783317&view=rev
>> Log:
>> Avoid unnecessary code (the deprecation macro wrapper itself emits unused args
>> warnings) in OpenSSL 1.1.0 and avoid _free()ing NULL references.
>>
>>
>> Modified:
>> httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
> []
>> #endif
>> }
>> - EC_KEY_free(eckey);
>> - EC_GROUP_free(ecparams);
>> +#endif
>> + if (eckey)
>> + EC_KEY_free(eckey);
>> + if (ecparams)
>> + EC_GROUP_free(ecparams);
>
> ISTM that these are NULL safe already.
With no docs to that effect, and trying to predict what 1.2.0 might do
to us, the
explicit avoidance seems safer, no?
Is there somewhere it is documented what openssl's NULL-safe policy is?