ims_charging unlink_unsafe_ro_session

[thulasizwe-n]

Apologies if i have not kept with the reporting standard.

Insertion attempts of out-of-credit replies from resume_on_initial_ccr callback, are corrupting the ro_session list. i.e. they nullify the first and last fields, which under certain conditions the first entry gets restored but not the last.This then crashes the process at ro_session_hash.c (link_ro_session), where the first entry has a valid address but the last does not.
Solution: ro_session_hash.h - unlink_unsafe_ro_session - check if the session to be linked is actually part of list i.e

    if ((ro_session->next == 0x00) &&
    (ro_session->prev == 0x00) &&
    (ro_session != ro_session_entry->first) )
{
             ro_session, &(ro_session->ro_tl),
             ro_session->callid.len,
             ro_session->callid.len,
             ro_session->callid.s);
    return;
}

rest of the code ....

[thulasizwe-n]

Sorry, previous supposed be as below.This must be first check

 if ((ro_session->next == 0x00) &&
    (ro_session->prev == 0x00) &&
    (ro_session != ro_session_entry->first) )
{
    return;
}

rest of the code ....

[miconda]

Can you provide a patch with the changes? Eventually make it a pull request, to be easy to review and then just merge from the web interface if all ok.

[miconda]

Also, maybe @ngvoice or @christoph-v can comment on this issue...

[christoph-v]

Hi @miconda . So sorry, no experience with Ro Interface / charging yet on my side.

[thulasizwe-n]

Ok I'll do the pull request etc.
Please forgive my unfamiliarity with kamailio patch standards...am in process of learning.

Below is from modules/ims_charging/ro_session_hash.h function, I've just indicated patch changes as "+"

static inline void unlink_unsafe_ro_session(struct ro_session_entry *ro_session_entry, struct ro_session *ro_session) {
+    if ((ro_session->next == 0x00) &&   
+       (ro_session->prev == 0x00) &&
+        (ro_session != ro_session_entry->first) )
+    {
+       return;
+  }

    if (ro_session->next)
        ro_session->next->prev = ro_session->prev;
    else
        ro_session_entry->last = ro_session->prev;
    if (ro_session->prev)
        ro_session->prev->next = ro_session->next;
    else
        ro_session_entry->first = ro_session->next;

    ro_session->next = ro_session->prev = 0;

    counter_add(ims_charging_cnts_h.active_ro_sessions, -1);

    return;
}

[miconda]

@christoph-v - no worries, I tried to approach those that I saw having activity with ims modules, thanks anyhow.

@thulasizwe-n - I pushed the patch from your comment.