23 Jul
2014
23 Jul
'14
8:27 a.m.
Hello,
At 1&1 we have spotted an issue related to the cdr_extra parameters: for
more than 10 string cdr_extra parameters, the addresses used by the new
parameters overwrite the previous ones (this did not happen in 3.1, but
is reproducible since at least 3.3).
I attached a patch that implements a solution where we allocate memory
for the cdr extra params with pkg_malloc() and free it once it is no
longer needed.
Daniel, if there is no comment related to this solution, I will commit
the patch.
Thank you,
Lucian Balaceanu
23 Jul
23 Jul
12:45 p.m.
Hello,
this is probably related to the value of pv buffer slots:
http://www.kamailio.org/wiki/cookbooks/devel/core#pv_buffer_slots
Having a way to allocate dynamically could be a good alternative. I will
check the patch and come back with the remarks.
Cheers,
Daniel
On 23/07/14 08:27, Lucian Balaceanu wrote:
Hello,
At 1&1 we have spotted an issue related to the cdr_extra parameters:
for more than 10 string cdr_extra parameters, the addresses used by
the new parameters overwrite the previous ones (this did not happen in
3.1, but is reproducible since at least 3.3).
I attached a patch that implements a solution where we allocate memory
for the cdr extra params with pkg_malloc() and free it once it is no
longer needed.
Daniel, if there is no comment related to this solution, I will commit
the patch.
Thank you,
Lucian Balaceanu
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
28 Jul
28 Jul
3:30 p.m.
Hello,
I think the cleanup of the array with allocated values is not safe
enough, if there is an error.
I thought of this situation:
- acc went fine for several iterations and the allocated values are freed
- on current iteration it fails populating properly the values and the
execution jumps to free the array. .s field can be a pointer from
previous iteration
I think that .s has to be made NULL after it is freed and checked to not
be null before freeing it.
Let me know if I overlooked something there.
On 23/07/14 08:27, Lucian Balaceanu wrote:
Hello,
At 1&1 we have spotted an issue related to the cdr_extra parameters:
for more than 10 string cdr_extra parameters, the addresses used by
the new parameters overwrite the previous ones (this did not happen in
3.1, but is reproducible since at least 3.3).
I attached a patch that implements a solution where we allocate memory
for the cdr extra params with pkg_malloc() and free it once it is no
longer needed.
Daniel, if there is no comment related to this solution, I will commit
the patch.
Thank you,
Lucian Balaceanu
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
6:35 p.m.
Hi Daniel,
Thank you for your reply. I did not fully understand exactly where this
free error occurs in our code, but I agree that setting pointer to NULL
after free is a good defensive programming practice ( and would take
care of such a faulty case as you described). I attach a patch where the
freeing is done as per your indications.
Thank you,
Lucian Balaceanu
On 07/28/2014 04:30 PM, Daniel-Constantin Mierla wrote:
Hello,
I think the cleanup of the array with allocated values is not safe
enough, if there is an error.
I thought of this situation:
- acc went fine for several iterations and the allocated values are freed
- on current iteration it fails populating properly the values and the
execution jumps to free the array. .s field can be a pointer from
previous iteration
I think that .s has to be made NULL after it is freed and checked to
not be null before freeing it.
Let me know if I overlooked something there.
On 23/07/14 08:27, Lucian Balaceanu wrote:
Hello,
At 1&1 we have spotted an issue related to the cdr_extra parameters:
for more than 10 string cdr_extra parameters, the addresses used by
the new parameters overwrite the previous ones (this did not happen
in 3.1, but is reproducible since at least 3.3).
I attached a patch that implements a solution where we allocate
memory for the cdr extra params with pkg_malloc() and free it once it
is no longer needed.
Daniel, if there is no comment related to this solution, I will
commit the patch.
Thank you,
Lucian Balaceanu
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
--
Daniel-Constantin Mierla -http://www.asipto.com
http://twitter.com/#!/miconda -http://www.linkedin.com/in/miconda
29 Jul
29 Jul
2:36 p.m.
Hello,
On 28/07/14 18:35, Lucian Balaceanu wrote:
Hi Daniel,
Thank you for your reply. I did not fully understand exactly where
this free error occurs in our code,
I didn't applied the patch to see all the cases when the 'goto error'
can occur. Maybe it was safe with previous patch as well.
My concern was that working with a global array, via some error
situation, the free array happens and the array still has the valuee of
the pointers from the previous execution and another free on such value
will cause a crash.
but I agree that setting pointer to NULL after free is
a good
defensive programming practice ( and would take care of such a faulty
case as you described). I attach a patch where the freeing is done as
per your indications.
You can commit it.
Thanks,
Daniel
Thank you,
Lucian Balaceanu
On 07/28/2014 04:30 PM, Daniel-Constantin Mierla wrote:
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Hello,
I think the cleanup of the array with allocated values is not safe
enough, if there is an error.
I thought of this situation:
- acc went fine for several iterations and the allocated values are freed
- on current iteration it fails populating properly the values and
the execution jumps to free the array. .s field can be a pointer from
previous iteration
I think that .s has to be made NULL after it is freed and checked to
not be null before freeing it.
Let me know if I overlooked something there.
On 23/07/14 08:27, Lucian Balaceanu wrote:
Hello,
At 1&1 we have spotted an issue related to the cdr_extra parameters:
for more than 10 string cdr_extra parameters, the addresses used by
the new parameters overwrite the previous ones (this did not happen
in 3.1, but is reproducible since at least 3.3).
I attached a patch that implements a solution where we allocate
memory for the cdr extra params with pkg_malloc() and free it once
it is no longer needed.
Daniel, if there is no comment related to this solution, I will
commit the patch.
Thank you,
Lucian Balaceanu
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
--
Daniel-Constantin Mierla -http://www.asipto.com
http://twitter.com/#!/miconda -http://www.linkedin.com/in/miconda
3553
days inactive
3559
days old
4 comments
2 participants
participants (2)
-
Daniel-Constantin Mierla -
Lucian Balaceanu