{"action":"closed","issue":{"url":"https://api.github.com/repos/ofiwg/libfabric/issues/3459","repository_url":"https://api.github.com/repos/ofiwg/libfabric","labels_url":"https://api.github.com/repos/ofiwg/libfabric/issues/3459/labels{/name}","comments_url":"https://api.github.com/repos/ofiwg/libfabric/issues/3459/comments","events_url":"https://api.github.com/repos/ofiwg/libfabric/issues/3459/events","html_url":"https://github.com/ofiwg/libfabric/issues/3459","id":268175151,"number":3459,"title":"Coverity issues","user":{"login":"gladkovdmitry17","id":11650339,"avatar_url":"https://avatars1.githubusercontent.com/u/11650339?v=4","gravatar_id":"","url":"https://api.github.com/users/gladkovdmitry17","html_url":"https://github.com/gladkovdmitry17","followers_url":"https://api.github.com/users/gladkovdmitry17/followers","following_url":"https://api.github.com/users/gladkovdmitry17/following{/other_user}","gists_url":"https://api.github.com/users/gladkovdmitry17/gists{/gist_id}","starred_url":"https://api.github.com/users/gladkovdmitry17/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gladkovdmitry17/subscriptions","organizations_url":"https://api.github.com/users/gladkovdmitry17/orgs","repos_url":"https://api.github.com/users/gladkovdmitry17/repos","events_url":"https://api.github.com/users/gladkovdmitry17/events{/privacy}","received_events_url":"https://api.github.com/users/gladkovdmitry17/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2017-10-24T20:30:26Z","updated_at":"2017-11-29T21:37:44Z","closed_at":"2017-11-29T21:37:44Z","author_association":"MEMBER","body":"1. Don't free resources `ofi_domain_init`. I guess more proper way is to free memory when it was allocated. Let's remove `free(util_domain)` from `ofi_domain_init`\r\n```\r\n72int smr_domain_open(struct fid_fabric *fabric, struct fi_info *info,\r\n73                struct fid_domain **domain, void *context)\r\n74{\r\n75        int ret;\r\n76        struct util_domain *util_domain;\r\n77\r\n78        ret = smr_check_info(info);\r\n79        if (ret)\r\n80                return ret;\r\n81\r\n82        util_domain = calloc(1, sizeof(*util_domain));\r\n83        if (!util_domain)\r\n84                return -FI_ENOMEM;\r\n85\r\n86        ret = ofi_domain_init(fabric, info, util_domain, context);\r\n87        if (ret) {\r\n88                free(util_domain);    <=============== DOUBLE FREE (already freed in the ofi_domain_util)\r\n89                return ret;\r\n90        }\r\n```\r\n\r\n2. Buffer not null terminated:\r\n```\r\n179 int smr_map_add(const struct fi_provider *prov, struct smr_map *map,\r\n180                const char *name, int *id)\r\n181{\r\n...\r\n189                strncpy(map->peer_addr[*id].name, name, SMR_NAME_SIZE);\r\n```\r\nShould we add something like `map->peer_addr[*id].name[SMR_NAME_SIZE - 1] = '\\0`' or copy only `SMR_NAME_SIZE - 1` elements\r\n\r\n3. Possible deadlock\r\n\r\n```\r\n252 void smr_progress_cmd(struct smr_ep *ep)\r\n```\r\nAcquires locks in the following sequence:\r\n- smr_region.lock\r\n- util_cq.cq_lock\r\n\r\n```\r\n430 ssize_t smr_sendmsg(struct fid_ep *ep_fid, const struct fi_msg *msg,\r\n431                uint64_t flags)\r\n```\r\nAcquires locks in the following sequence:\r\n- util_cq.cq_lock\r\n- smr_region.lock\r\n\r\nSeems we should acquire locks in the similar sequence (i.e. acquires all or nothing).\r\n\r\n\r\n4. Possible null-dereferencing (if `flag & FI_SOUCE`, `service != NULL`, but `node == NULL`)\r\n```\r\n141i nt util_getinfo(const struct util_prov *util_prov, uint32_t version,\r\n142                 const char *node, const char *service, uint64_t flags,\r\n143                 const struct fi_info *hints, struct fi_info **info)\r\n144 {\r\n...\r\n155        if ((flags & FI_SOURCE) && !node && !service) {\r\n```\r\nShoudl it be `if ((flags & FI_SOURCE) && !node || !service) {` ? Seems we should have both `node` and `service` not NULL in case of `FI_SOURCE`"}}
{"actor":{"display_login":"shefty"}}