Re: [openssl-dev] [openssl.org #4151] [PATCH] Function pop_info in crypto/mem_dbg.c returns a dangling pointer

2015-11-22 Thread Salz, Rich via RT
We have another internal cleanup in-progress that will fix this in a different 
way.



___
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev


[openssl-dev] [openssl.org #4151] [PATCH] Function pop_info in crypto/mem_dbg.c returns a dangling pointer

2015-11-22 Thread Pascal Cuoq via RT
In crypto/mem_dbg.c, the type of static function pop_info() is to return an 
APP_INFO*.
This pointer can be a dangling pointer, as can be seen in the following link as 
long as one assumes that line 382 is reachable (it is).

https://github.com/openssl/openssl/blob/90945fa31a42dcf3beb90540c618e4d627c595ea/crypto/mem_dbg.c#L382-L386

Returning a dangling pointer from a function is not terribly good style. Using 
dangling pointers for anything is technically undefined behavior, so there is 
nothing in theory that a caller can do with the function's result that will not 
be illegal when a dangling pointer is returned.

(It is well-known that dereferencing dangling pointers is undefined behavior, 
but examples at http://blog.regehr.org/archives/767#comment-4717 and at 
http://trust-in-soft.com/dangling-pointer-indeterminate/ show that the behavior 
of a program the *only* fault of which is to compare a dangling pointer to 
something else can be surprising too).

The function pop_info() being static, no-one may use it outside mem_dbg.c and 
the callers inside mem_dbg.c do not dereference the returned pointer. They do 
use it in a comparison though, which should ideally be avoided because of the 
examples above.

The attached patch does three small things:

- change the type of the function to return an int: 1 if the pointer ret was 
non-null before being possibly freed, 0 otherwise, and adjust the two call 
sites correspondingly;
- add a one-line comment describing the behavior of the function;
- rename the local variable ret, the name of which no longer makes sense, into 
"current".

None of these three things should change the behavior in practice of OpenSSL. 
As compiled with current C compilers, the tests "pop_info() != NULL" evaluated 
to 1 when a dangling pointer was returned. These tests were replaced with 
"pop_info()" which now evaluates to 1 under the circumstances in which a 
dangling pointer was returned.





pop_info.patch
Description: Binary data
___
openssl-bugs-mod mailing list
openssl-bugs-...@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-bugs-mod___
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev