3.0.x HEAD crashing

[Phil Mayers]

On 17/06/2014 20:02, Arran Cudbard-Bell wrote:
>
> On 17 Jun 2014, at 19:32, Phil Mayers <p.mayers at IMPERIAL.AC.UK>
> wrote:
>
>> On 17/06/2014 18:05, Arran Cudbard-Bell wrote:
>>
>>> What modules are you using?
>>
>> Man, what are we not...
>>
>> The main things are:
> ...
>
> I figured the cache module would be involved in there somewhere. It's
> really the only thing that does anything major with control
> attributes...
>
> That's interesting... well I noticed a check was missing for the
> parenting of VALUE_PAIR data buffers. If one wasn't being copied
> somewhere in the cache module, and being freed due to incorrect
> parenting, it'd result in heap corruption.
>
> I've added the check, could you run up v3.0.x HEAD and see if it
> fails?

Yeah, looks like that's it, it bombs out almost immediately with:

CONSISTENCY CHECK FAILED src/lib/valuepair.c[2350]: VALUE_PAIR ICzone 
uint8_t data buffer is not parented by VALUE_PAIR 0xa08820, instead 
parented by 0x892f10 (value_pair_tmpl_t)
SOFT ASSERT FAILED src/lib/debug.c[871]: 0

"ICzone" is populated like this:

policy {
         lookup_mac_zone {
                 # just look the zone for a mac
                 # designed for things where the vlan it not useful
                 # or static i.e. not a switch cached into HDB

                 # hdb lookup, via cache
                 hdbmaczone

                 if (control:Tmp-String-0 =~ /^(.+),(.+)$/) {
                         update request {
                                 Calling-Station-Id := "%{1}"
                                 ICzone = "%{2}"
                         }
                         updated
                 }
                 else {
                         update request {
                                 ICzone = "SUPERBANNED"
                         }
                         notfound
                 }
         }
}

...and the "hdbmaczone" is:

cache hdbmaczone {
         key = "%{Calling-Station-Id}"
         ttl = 15

         # LEAVE THIS AT 0 - it's intended for use internally
         epoch = 0
         add-stats = no
         update {
                 control:Tmp-String-0 := "%{newsql:select ...}"
         }
}

The assert is triggered on a comparison immediately after the "policy" 
module has run:

(0)  hdbmaczone : EXPAND %{newsql:select mac||','||zone from 
lookup_mac_zone('%{Calling-Station-Id}') order by prio desc limit 1}
(0)  hdbmaczone :    --> 02:00:00:00:00:01,PREREG
(0)  hdbmaczone : Adding to cache entry:
(0)  hdbmaczone :       control:Tmp-String-0 := "02:00:00:00:00:01,PREREG"
(0)  hdbmaczone : Adding to request:
(0)  hdbmaczone :       control:Tmp-String-0 := "02:00:00:00:00:01,PREREG"
(0)  hdbmaczone : Inserted entry, TTL 15 seconds
(0)    [hdbmaczone] = updated
(0)     if (control:Tmp-String-0 =~ /^(.+),(.+)$/)
(0)     if (control:Tmp-String-0 =~ /^(.+),(.+)$/)  -> TRUE
(0)    if (control:Tmp-String-0 =~ /^(.+),(.+)$/)  {
(0)     update request {
(0) EXPAND %{1}
(0)    --> 02:00:00:00:00:01
(0)     Calling-Station-Id := "02:00:00:00:00:01"
(0) EXPAND %{2}
(0)    --> PREREG
(0)     ICzone = "PREREG"
(0)     } # update request = noop
(0)     [updated] = updated
(0)    } # if (control:Tmp-String-0 =~ /^(.+),(.+)$/)  = updated
(0)     ... skipping else for request 0: Preceding "if" was taken
(0)   } # lookup_mac_zone lookup_mac_zone = updated
(0)    if (ICzone == "SUPERBANNED")
CONSISTENCY CHECK FAILED src/lib/valuepair.c[2350]: VALUE_PAIR ICzone 
uint8_t data buffer is not parented by VALUE_PAIR 0xa08820, instead 
parented by 0x892f10 (value_pair_tmpl_t)