5 Jul
2013
5 Jul
'13
7:29 a.m.
On Fri, Jul 05, 2013 at 12:23:54AM +0200, Pablo Neira Ayuso wrote:
Hi,
I'll proceed to remove HSL support from
libosmo-abis as well.
thanks, I already removed it (as part of looking through the
coverity issues). If there was an issue with my removal please
poke me.
In coverity we have four more issues, it would be nice if you
could have a look at the following:
src/input/ipaccess.c:
static int ipaccess_bts_cb(struct ipa_client_conn *link, struct msgb *msg)
...
862 } else if (link->port == IPA_TCP_PORT_OML)
863 e1i_ts = &link->line->ts[0];
4. Condition "link->port == 3003", taking false branch
864 else if (link->port == IPA_TCP_PORT_RSL)
865 e1i_ts = &link->line->ts[1];
866
867 /* look up for some existing signaling link. */
CID 1042352 (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
5. var_deref_model: Passing null pointer "e1i_ts" to function
"e1inp_lookup_sign_link(struct e1inp_ts *, uint8_t, uint8_t)", which
dereferences it. [show details]
868 sign_link = e1inp_lookup_sign_link(e1i_ts, hh->proto, 0);
port can probably only be IPA_TCP_PORT_OML|IPA_TCP_PORT_RSL. Can
you confirm this is a false positive? Shall we add an else branch
and add a OSMO_ASSERT(false) there?
src/input/ipa.c:
static int ipa_server_fd_cb(struct osmo_fd *ofd, unsigned int what)
...
4. Condition "link->accept_cb", taking true branch
324 if (link->accept_cb)
325 link->accept_cb(link, ret);
326
CID 1040692 (#1 of 1): Resource leak (RESOURCE_LEAK)
5. leaked_handle: Handle variable "ret" going out of scope leaks the handle.
327 return 0;
Shall we have an else close(ret)? or should we enforce the presence
of a accept_cb when the socket is bound?
src/input/misdn.c:
static int _mi_e1_line_update(struct e1inp_line *line)
...
649 ret = mi_e1_setup(line, 1);
12. Condition "ret", taking false branch
650 if (ret)
CID 1040691 (#3 of 4): Resource leak (RESOURCE_LEAK) [select issue]
651 return ret;
652
CID 1040691 (#4 of 4): Resource leak (RESOURCE_LEAK)
13. leaked_handle: Handle variable "sk" going out of scope leaks the handle.
653 return 0;
I would add a close(sk); in the code but I was not sure if mISDN requires
this code to be open?
src/input/ipaccess.c:
static struct msgb *
713ipa_bts_id_resp(struct ipaccess_unit *dev, uint8_t *data, int len)
751 case IPAC_IDTAG_SWVERSION:
CID 1040690 (#4 of 5): Copy into fixed size buffer (STRING_OVERFLOW)
15. fixed_size_dest: You might overrun the 64 byte fixed-size string "str" by
copying "dev->swversion" without checking the length.
16. parameter_as_source: Note: This defect has an elevated risk because the source
argument is a parameter of the current function.
752 strcpy(str, dev->swversion);
753 break;
this appears to be a genuine issue.
cheers
holger