[sr-dev] Remove check for msg->msg_flags inside corex module ?

Daniel-Constantin Mierla miconda at gmail.com
Fri Jul 13 14:09:33 CEST 2018


Hello,

well, not sure at this moment because would take some time to analyze
what execution paths can end up there. I did look and I saw that
path_vec field is cloned in shm, so doing pkg_free() on that value will
fail.

If it ends up to that code when the value is not the cloned one, then it
should be ok. On the other hand, the shm clone flag can be set even when
path_vec points to a pkg address to due execution on a faked request.

Because of these cases, I just added a helper function to check if a
pointer is inside shared memory, see:

  *
https://github.com/kamailio/kamailio/commit/1a20bcaa35db4aa80d6460dfb0fb9c70026248c9

Maybe this is safer overall and we can change in other parts where we
rely on some tricks to see if it is pkg or shm -- in many cases related
to processing in context of transaction/tm module, pkg needs to be free
and shm is cloned in a continuous block to be freed at once.

If you find this solution acceptable, then you can go ahead and push a
commit using shm_address_in(). That might be safe to be added in
reset_path_vector() as well, to avoid troubles if someone calls it for a
shm cloned shm structure.

Cheers,
Daniel

On 13.07.18 13:06, Lucian Balaceanu wrote:
> Hi guys,
>
> Seeing the https://github.com/kamailio/kamailio/pull/1144 about fixing
> a memory leak in the tm module due to incorrectly using FL_SHM_CLONE
> when freing path_vec memory, isn't there a case that a similar patch
> should be applied to corex/corex_lib.c ?
>
> Inside corex_append_branch(..) function:
>
> /* if this is a cloned message, don't free the path vector as it was
> copied into shm memory and will be freed as contiguous block*/
>         if (!(msg->msg_flags&FL_SHM_CLONE)) {
>             if(msg->path_vec.s!=0)
>                 pkg_free(msg->path_vec.s);
>             msg->path_vec.s = 0;
>             msg->path_vec.len = 0;
>         }
>
> Maybe we should switch to:
>
>         if(msg->path_vec.s!=0)
>             pkg_free(msg->path_vec.s);
>         msg->path_vec.s = 0;
>         msg->path_vec.len = 0;
>
> If you agree this is the case, I will  prepare a pull request.
>
> Thank you,
> Lucian
>
> _______________________________________________
> Kamailio (SER) - Development Mailing List
> sr-dev at lists.kamailio.org
> https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

-- 
Daniel-Constantin Mierla -- www.asipto.com
www.twitter.com/miconda -- www.linkedin.com/in/miconda
Kamailio World Conference -- www.kamailioworld.com




More information about the sr-dev mailing list