[squid-dev] [PATCH] Uninitialised errors during Squid startup

[Alex Rousskov]

On 06/08/2016 10:27 PM, Amos Jeffries wrote:
> On 9/06/2016 12:52 a.m., Eduard Bagdasaryan wrote:
>> Hello,
>>
>> This patch fixes valgrind-discovered trunk errors.
>>
>> During start-up, Valgrind reported many errors with a similar message:
>> "Use of uninitialised value of size 8...". These errors were caused by
>> HttpRequestMethod& parameter during private key generation:
>> it was used as a raw void* memory there. HttpRequestMethod is a non-POD
>> type and has some "padding" bytes, which are not initialized by the
>> constructor.
>>
>> The patch simplifies private key generation by using a counter for this,
>> which is sufficient to create a unique key within a single Squid process.
>> Except fixing Valgrind errors, this solution saves many CPU cycles
>> wasted on
>> generating MD5 hashes just to "guarantee" uniqueness within a single
>> process.

> How does this interact with objects that begin their life as private
> then get converted to public keys?

This change is not meant to affect public keys.


> * the comment about kidID and PID in the new private key blob as being
> just an optimization I dont think is correct. Without them I think we
> could have ID collisions in collapsed_forwarding, shared cache_mem, or
> rock across workers.

I believe the comment is correct because private keys are not shared
among kids. Being unknown to others is the essence of being a private
key. The only entities that are supposed to know about a private key is
the entity holding the corresponding StoreEntry lock. One cannot find a
StoreEntry with a private key in Store.

If you know of any exceptions to that rule, please let me know.


> That would make them required for proper operations in SMP.

Caching (SMP-aware and not) should be done using public, not private keys.

It may help to look at this bug from a different angle: After we stopped
zeroing memory and before this fix, all private keys were essentially
random values -- calling private key generation function with the same
set of arguments would produce a different value. And yet, everything
"worked" because private keys are compatible with random values by their
nature/definition. The fix takes advantage of that understanding to
simplify and optimize private key generation.


HTH,

Alex.