13 Jul
2018
13 Jul
'18
9:09 a.m.
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/1a20bcaa35db4aa80d6460dfb0fb9c7…
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(a)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