sr-dev July 2013

sr-dev@lists.kamailio.org

21 participants
246 discussions

[tracker] Comment added: bugs in uac_redirect module

by sip-router

THIS IS AN AUTOMATED MESSAGE, DO NOT REPLY. The following task has a new comment added: FS#327 - bugs in uac_redirect module User who did this - Federico Cabiddu (lester) ---------- Hi Daniel, the problem of the table's name resides in the way the connection ids are created in db_flatstore. struct flat_id* new_flat_id(char* dir, char* table) { struct flat_id* ptr; .................. ptr = (struct flat_id*)pkg_malloc(sizeof(struct flat_id)); if (!ptr) { LM_ERR("no pkg memory left\n"); return 0; } memset(ptr, 0, sizeof(struct flat_id)); ................ ptr->table.s = table; ptr->table.len = strlen(table); return ptr; } Here table is the static buffer db_table_name_buf, everytime the buffer is changed all the tables' name in the pools' ids are changed (but not the length). As said, this can cause a request to be written to the wrong file. I think that the table name should be copied into the id instead of assigning the pointer. I attach a patch (against master branch), I hope that the code will explain what I mean better than I do. ---------- More information can be found at the following URL: http://sip-router.org/tracker/index.php?do=details&task_id=327#comment1036 You are receiving this message because you have requested it from the Flyspray bugtracking system. If you did not expect this message or don't want to receive mails in future, you can change your notification settings at the URL shown above.

10 years, 9 months

[tracker] Comment added: bugs in uac_redirect module

by sip-router

THIS IS AN AUTOMATED MESSAGE, DO NOT REPLY. The following task has a new comment added: FS#327 - bugs in uac_redirect module User who did this - Federico Cabiddu (lester) ---------- Hi Daniel, I'll try to explain it better (sorry if I haven't been clear enough). The problem is that db_table_name_buf overwrite the table name of all the ids of the pools. Then when an accounting request is done, the check to find the right id is performed, in db_flatstore module, by cmp_flat_id function. This function compare the table name (s and len part of the str). We already know that the s part will be matched (overwritten), if the table name length is the same for two tables, it can happens that a request is written to the wrong file, depending on which id is first found in the pools list. I hope I've been I tried the committed patches and everything works fine, thanks. ---------- More information can be found at the following URL: https://sip-router.org/tracker/index.php?do=details&task_id=327#comment1035 You are receiving this message because you have requested it from the Flyspray bugtracking system. If you did not expect this message or don't want to receive mails in future, you can change your notification settings at the URL shown above.

10 years, 9 months

[tracker] Comment added: Possible memory leak in srdb1

by sip-router

THIS IS AN AUTOMATED MESSAGE, DO NOT REPLY. The following task has a new comment added: FS#319 - Possible memory leak in srdb1 User who did this - Daniel-Constantin Mierla (miconda) ---------- As said in previous comment and visible in the code, the sname->s is not allocated, but it is pointed inside a larger chunk that was allocated for sname. Your patch will free sname->s, which is incorrect and will result in a crash. ---------- More information can be found at the following URL: https://sip-router.org/tracker/index.php?do=details&task_id=319#comment1034 You are receiving this message because you have requested it from the Flyspray bugtracking system. If you did not expect this message or don't want to receive mails in future, you can change your notification settings at the URL shown above.

10 years, 9 months

[tracker] Comment added: bugs in uac_redirect module

by sip-router

THIS IS AN AUTOMATED MESSAGE, DO NOT REPLY. The following task has a new comment added: FS#327 - bugs in uac_redirect module User who did this - Daniel-Constantin Mierla (miconda) ---------- I don't understand why two tables having the same length can lead to a problem. Where is the condition on the length that does not perform update of the name itself? Also, the function get_str_fparam() may use internally the PV static buffers, that expose the same potential issue, if I understood what you mean. Maybe giving an example, pointing in the code where is affecting. I applied the other parts of the patches for now, with some changes to get rid of some compile warnings and extending the check for the multi-leg accounting. Can you try the patches as committed (you can cherry pick in branch 4.0) -- if they work ok, I'll backport. ---------- More information can be found at the following URL: https://sip-router.org/tracker/index.php?do=details&task_id=327#comment1033 You are receiving this message because you have requested it from the Flyspray bugtracking system. If you did not expect this message or don't want to receive mails in future, you can change your notification settings at the URL shown above.

10 years, 9 months

git:master: acc: safety check for accounted values not to be NULL

by Daniel-Constantin Mierla

Module: sip-router Branch: master Commit: fb3a5f7022ebd20fc516b5b8303a0274bda18258 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=fb3a5f7… Author: Daniel-Constantin Mierla <miconda(a)gmail.com> Committer: Daniel-Constantin Mierla <miconda(a)gmail.com> Date: Fri Jul 26 14:55:38 2013 +0200 acc: safety check for accounted values not to be NULL - based on a patch by Federico Cabiddu, FS#327 --- modules/acc/acc.c | 16 ++++++++++------ modules/acc/acc_logic.c | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/modules/acc/acc.c b/modules/acc/acc.c index d4fd89c..0a563d6 100644 --- a/modules/acc/acc.c +++ b/modules/acc/acc.c @@ -241,13 +241,15 @@ int acc_log_request( struct sip_msg *rq) memcpy(p, log_attrs[i].s, log_attrs[i].len); p += log_attrs[i].len; *(p++) = A_EQ_CHR; - memcpy(p, val_arr[i].s, val_arr[i].len); - p += val_arr[i].len; + if (val_arr[i].s != NULL) { + memcpy(p, val_arr[i].s, val_arr[i].len); + p += val_arr[i].len; + } } /* get per leg attributes */ if ( leg_info ) { - n = legs2strar(leg_info,rq,val_arr+m,int_arr+m,type_arr+m, 1); + n = legs2strar(leg_info,rq,val_arr+m,int_arr+m,type_arr+m, 1); do { for (i=m; i<m+n; i++) { if (p+1+log_attrs[i].len+1+val_arr[i].len >= log_msg_end) { @@ -259,10 +261,12 @@ int acc_log_request( struct sip_msg *rq) memcpy(p, log_attrs[i].s, log_attrs[i].len); p += log_attrs[i].len; *(p++) = A_EQ_CHR; - memcpy(p, val_arr[i].s, val_arr[i].len); - p += val_arr[i].len; + if (val_arr[i].s != NULL) { + memcpy(p, val_arr[i].s, val_arr[i].len); + p += val_arr[i].len; + } } - }while (p!=log_msg_end && (n=legs2strar(leg_info,rq,val_arr+m, + } while (p!=log_msg_end && (n=legs2strar(leg_info,rq,val_arr+m, int_arr+m,type_arr+m, 0))!=0); } diff --git a/modules/acc/acc_logic.c b/modules/acc/acc_logic.c index f2efaa7..55b4196 100644 --- a/modules/acc/acc_logic.c +++ b/modules/acc/acc_logic.c @@ -220,7 +220,7 @@ int w_acc_db_request(struct sip_msg *rq, char *comment, char *table) if(acc_db_set_table_name(rq, (void*)table, NULL)<0) { LM_ERR("cannot set table name\n"); return -1; - } + } env_set_to( rq->to ); env_set_comment((struct acc_param*)comment); return acc_db_request(rq);

10 years, 9 months

git:master: uac_redirect: fixup for using acc table parameter

by Daniel-Constantin Mierla

Module: sip-router Branch: master Commit: 3d33733f299dda595704108aef73a2912dc3069f URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=3d33733… Author: Daniel-Constantin Mierla <miconda(a)gmail.com> Committer: Daniel-Constantin Mierla <miconda(a)gmail.com> Date: Fri Jul 26 14:52:21 2013 +0200 uac_redirect: fixup for using acc table parameter - set reason parameter for acc function - based on a patch by Federico Cabiddu, FS#327 --- modules/uac_redirect/rd_funcs.c | 4 +++ modules/uac_redirect/redirect.c | 40 +++++++++++++++++++++++--------------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/modules/uac_redirect/rd_funcs.c b/modules/uac_redirect/rd_funcs.c index 89917f7..ca34e0c 100644 --- a/modules/uac_redirect/rd_funcs.c +++ b/modules/uac_redirect/rd_funcs.c @@ -57,6 +57,7 @@ int get_redirect( struct sip_msg *msg , int maxt, int maxb, int n; int i; int first_branch; + char code_buf[INT2STR_MAX_LEN]; /* get transaction */ t = rd_tmb.t_gett(); @@ -95,6 +96,9 @@ int get_redirect( struct sip_msg *msg , int maxt, int maxb, } if (max==0) continue; + /* put the response code into the acc_param reason struct */ + reason->code = t->uac[i].last_received; + reason->code_s.s = int2bstr((unsigned long)reason->code, code_buf, &reason->code_s.len); /* get the contact from it */ n = shmcontact2dset( msg, t->uac[i].reply, max, reason, bflags); if ( n<0 ) { diff --git a/modules/uac_redirect/redirect.c b/modules/uac_redirect/redirect.c index b4e55d1..48dec84 100644 --- a/modules/uac_redirect/redirect.c +++ b/modules/uac_redirect/redirect.c @@ -164,25 +164,24 @@ static int get_redirect_fixup(void** param, int param_no) pkg_free(*param); *param=(void*)(long)( (((unsigned short)maxt)<<8) | maxb); - } else if (param_no==2) { /* acc function loaded? */ - if (rd_acc_fct!=0) - return 0; - /* must import the acc stuff */ - if (acc_fct_s==0 || acc_fct_s[0]==0) { - LM_ERR("acc support enabled, but no acc function defined\n"); - return E_UNSPEC; - } - fct = find_export(acc_fct_s, 2, REQUEST_ROUTE); - if ( fct==0 ) - fct = find_export(acc_fct_s, 1, REQUEST_ROUTE); - if ( fct==0 ) { - LM_ERR("cannot import %s function; is acc loaded and proper " - "compiled?\n", acc_fct_s); - return E_UNSPEC; + if (rd_acc_fct==0) { + /* must import the acc stuff */ + if (acc_fct_s==0 || acc_fct_s[0]==0) { + LM_ERR("acc support enabled, but no acc function defined\n"); + return E_UNSPEC; + } + fct = find_export(acc_fct_s, 2, REQUEST_ROUTE); + if ( fct==0 ) + fct = find_export(acc_fct_s, 1, REQUEST_ROUTE); + if ( fct==0 ) { + LM_ERR("cannot import %s function; is acc loaded and proper " + "compiled?\n", acc_fct_s); + return E_UNSPEC; + } + rd_acc_fct = fct; } - rd_acc_fct = fct; /* set the reason str */ accp = (struct acc_param*)pkg_malloc(sizeof(struct acc_param)); if (accp==0) { @@ -264,6 +263,7 @@ static int regexp_compile(char *re_s, regex_t **re) static int redirect_init(void) { regex_t *filter; + void *p; /* load the TM API */ if (load_tm_api(&rd_tmb)!=0) { @@ -271,6 +271,14 @@ static int redirect_init(void) goto error; } + p = (void*)acc_db_table; + /* fixup table name */ + if(fixup_var_pve_str_12(&p, 1)<0) { + LM_ERR("failed to fixup acc db table\n"); + goto error; + } + acc_db_table = p; + /* init filter */ init_filters();

10 years, 9 months

[tracker] Comment added: Possible memory leak in srdb1

by sip-router

THIS IS AN AUTOMATED MESSAGE, DO NOT REPLY. The following task has a new comment added: FS#319 - Possible memory leak in srdb1 User who did this - Konstantin (Konstantin) ---------- Res.c (modules_k\db_oracle) line 111 code: text* name; str* sname; status = OCIAttrGet(param, OCI_DTYPE_PARAM, (dvoid**)(dvoid*)&name, &len, OCI_ATTR_NAME, con->errhp); if (status != OCI_SUCCESS) goto ora_err; --->>> sname = (str*)pkg_malloc(sizeof(str)+len+1); if (!sname) { db_free_columns(_r); LM_ERR("no private memory left\n"); return -5; } sname->len = len; sname->s = (char*)sname + sizeof(str); memcpy(sname->s, name, len); sname->s[len] = '\0'; RES_NAMES(_r)[i] = sname; ---------- More information can be found at the following URL: https://sip-router.org/tracker/index.php?do=details&task_id=319#comment1032 You are receiving this message because you have requested it from the Flyspray bugtracking system. If you did not expect this message or don't want to receive mails in future, you can change your notification settings at the URL shown above.

10 years, 9 months

[tracker] Comment added: bugs in uac_redirect module

by sip-router

THIS IS AN AUTOMATED MESSAGE, DO NOT REPLY. The following task has a new comment added: FS#327 - bugs in uac_redirect module User who did this - Federico Cabiddu (lester) ---------- You're right for the patch, it's broken. the strncpy(db_table_name_buf, dbtable.s, dbtable.len) line has to be removed, my mistake making the diff. I prefer the second patch (with the correction, because with the first one the table name of the id of the pool is overwritten at each db_table_name_buf change and can lead to the case in which the data for one table are written into another file because the two tables name have the same length. Best reagards, Federico ---------- More information can be found at the following URL: http://sip-router.org/tracker/index.php?do=details&task_id=327#comment1031 You are receiving this message because you have requested it from the Flyspray bugtracking system. If you did not expect this message or don't want to receive mails in future, you can change your notification settings at the URL shown above.

10 years, 9 months

[tracker] Comment added: bugs in uac_redirect module

by sip-router

THIS IS AN AUTOMATED MESSAGE, DO NOT REPLY. The following task has a new comment added: FS#327 - bugs in uac_redirect module User who did this - Daniel-Constantin Mierla (miconda) ---------- The IF condition in the patch is always true, because the IF above makes the opposite test. The second patch seems broken, there is still a strncpy to a variable that you removed now from declaration. I will use the first patch. ---------- More information can be found at the following URL: https://sip-router.org/tracker/index.php?do=details&task_id=327#comment1030 You are receiving this message because you have requested it from the Flyspray bugtracking system. If you did not expect this message or don't want to receive mails in future, you can change your notification settings at the URL shown above.

10 years, 9 months

[tracker] Comment added: bugs in uac_redirect module (Attachment added)

by sip-router

THIS IS AN AUTOMATED MESSAGE, DO NOT REPLY. The following task has a new comment added: FS#327 - bugs in uac_redirect module User who did this - Federico Cabiddu (lester) ---------- Hi Daniel, this part is a workaround for when acc_db_set_table_name is called with tables which have different name length. The db_table_name_buf is static and it's copied (as a pointer) into acc_env.text which is then used to call use_table. In my case I am using db_flatstore so it's flat_use_table which is being called. I have db accounting enabled for calls and for missed calls, and I have as table names "completed_calls" and "acc" (default one for uac_redirect). When get_redirects is called for the first time the db accounting is called with "acc" as table name, db_table_name_buf contains "acc", the id for the pool is created with this table name and everything is fine. Then once the call is answered the db accounting is called again, this time with "completed_calls" as table name, db_table_name_buf contains the good value but, at the same time the previous id contains the new table value. Now at the next redirection, even if the table name is "acc" db_table_name_buf contains "accleted_calls" and all the id created using db_table_name_buf are changed and, due to the way the ids are created and compared, the CDRs is written in the "completed_calls" file. So, putting '\0' at the end of the buffer helps at least to have the correct table name and to write to the correct file. It's a rough solution. Maybe the static db_table_name_buf can be avoided, as shown in the new attached patch. Best regards, Federico Cabiddu ---------- One or more files have been attached. More information can be found at the following URL: https://sip-router.org/tracker/index.php?do=details&task_id=327#comment1029 You are receiving this message because you have requested it from the Flyspray bugtracking system. If you did not expect this message or don't want to receive mails in future, you can change your notification settings at the URL shown above.

10 years, 9 months

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

sr-dev July 2013