[squid-dev] Regression introduced in r14268

Amos Jeffries squid3 at treenet.co.nz
Sun Aug 30 19:50:04 UTC 2015


On 30/08/2015 6:51 p.m., Kinkie wrote:
> Hi all,
>   I suspect that  r14268 is causing a regression: running that revision of
> trunk or later on any URL causes:
> 
> ----
> 2015/08/30 08:12:59 kid1| SECURITY ERROR: ACL 0x7f96e1c2ea88 checked with
> nothing to match against!!
> ----
> 
> The user-visible result is ERR_ACCESS_DENIED and connection termination.
> I've tried checing the patch, and noticed nothing obvious in it that could
> cause the behavior.
> 
>    Kinkie
> 

Turns out to be the locking sequence was borked and releasing the one it
needed to hold.

Apologies to Alex for associating his name with this. The bug was
entirely my bad cut-n-paste'ing.


After fixing that I am now getting:

#3  0x083316dd in xassert (msg=0x8703069 "c->locks > 0", file=0x8702ed3
"../../src/cbdata.cc", line=425) at ../../src/debug.cc:556

Hey reproducible case for *that* long annoying bug !!

#4  0x082d7feb in cbdataReferenceValid (p=0x8c92e48) at
../../src/cbdata.cc:425
#5  0x082fe3f0 in ACLChecklist::changeAcl (this=0xbffff110, t=0x0) at
../../src/acl/Checklist.h:175
#6  0x084d972b in ACLChecklist::fastCheck (this=0xbffff110,
list=0x8c92e48) at ../../../src/acl/Checklist.cc:326
#7  0x0863b5a6 in accessLogLogTo (log=0x8a7c780, al=...,
checklist=0xbffff110) at ../../../src/log/access_log.cc:96


It looks like this is a result from the SquidConfig.accessList.*
pointers not having any cbdataReference locks. They and squid.conf
aprsing are the true "owner" of the CBDATA object so the only place
valid to set initial lock and call cbdataReferenceValidDone() from. But
none of that logic exists.

So the actual locking/unlocking happens properly symmetric within ACL
testing but the final validity test fails since its still a valid but
now fully unlocked CBDATA object. Yuck.

I'm reverting the validity test for now. But this kind of highlights the
need to fix/remove *ValidDone(). Removing all locks and leaving the
object still valid is very wrong for any lock-counted referencing model
whose own validity test requires locks to exist.

Amos



More information about the squid-dev mailing list