[SR-Users] [sr-dev] kamailio core at qm_status

[Andrei Pelinescu-Onciul]

On Sep 19, 2012 at 10:54, Jijo <realjijo at gmail.com> wrote:
> Hi All,
> 
> Finally i found the issue,
> 
> Here is one of the bad trace for SUBSCRIBE(722bytes) and NOTIFY(1282bytes)
> which corrupted the memory. The messages came in the order NOTIFY and
> SUBSCRIBE. The core is generated in a different place but I believe this
> could be the reason for memory corruption.
> 
> Here is the trace UDP Process 27294 processing NOTIFY and Process
> 27303 processing
> SUBSCRIBE .
> 
[...]

> 
> Process 27294(NOTIFY) created the TCP connection structure for destination
> IP and just before calling wbufq_insert(), context switch happened and
> process 27303(SUBSCRIBE) got the cpu. Since the connection structure is
> already available process 27303 add the SUBSCRIBE message(722 bytes) to
> wbufq. Afterwards process 27294 got the CPU and invoked wbufq_insert()
> which basically corrupted the memory due to an overflow with the existing
> implementation.
> 
> 

Thanks a lot, it was indeed a problem in _wbufq_insert, however the fix
is slightly different (see below).
> 

[...]

> I think we don?t need memove, so we can change the code as below

We need the memmove, because the memory areas might overlap
(e.g we are moving a 1000 bytes message  only 512 bytes further in the
buffer).
> 
> 
> 
>        if ((q->first==q->last) && ((q->last->b_size-q->last_used)>=size)){
> 
>                 /* one block with enough space in it for size bytes */
> 
>                 //memmove(q->first->buf+size, q->first->buf, size);
> 
>                 memcpy(q->first->buf+q->last_used, data, size);

The problem here was that the wrong size was used for the memmove.
The fixed version is:
                  memmove(q->first->buf+size, q->first->buf, q->last_used);
> 
>                 q->last_used+=size;
> 
>         }
[...]


Andrei