[squid-dev] Broken trunk after r14735, r14726

[Alex Rousskov]

On 07/18/2016 08:49 AM, Christos Tsantilas wrote:
> On 07/18/2016 02:12 PM, Christos Tsantilas wrote:
>> On 07/16/2016 03:56 PM, Amos Jeffries wrote:
>>> On 16/07/2016 7:02 a.m., Alex Rousskov wrote:
>>>> * After r14726 (GnuTLS: support for TLS session resume): Squid
>>>> segfaults
>>>> when attempting to connect to a Secure ICAP service. Official Squid
>>>> v4.0.12 suffers from this bug.

>> It is a strange crash. Is it a corrupted SSL object?

It is an already freed object (its references member is 0).


> The patch uses the following line:
>  Security::GetSessionResumeData(Security::SessionPointer(ssl),
> icapService->sslSession);
> 
> 
> The Security::SessionPointer, is a LockingPointer which in trunk-r14726,
> does not increase the references of the attached "SSL" object in
> constructor.
> So the SSL will be released after the Security::SessionPointer is
> destroyed, immediately after the above line executed.

... which would explain the zero "references" value. This raises a
question -- did the old code work because the old LockingPointer
constructor was locking? If the answer is yes, then the
invisible-in-callers change from locking to non-locking was a mistake.

I can only repeat my earlier suggestions to hide that dangerous
constructor behind an explicit static method like
LockingPointer::NewWithoutLocking() and to assert that
resetWithoutLocking() method is fed a previously locked object.


Thank you,

Alex.