On Mon, Jul 4, 2016 at 10:01 AM, Lukas Slebodnik <lsleb...@redhat.com> wrote:
> ehlo,
>
> attached patch fixes coverity issue.
>
> src/providers/ipa/ipa_auth.c:
> 311static void ipa_pam_auth_handler_connect_done(struct tevent_req *subreq)
> 312{
> 313 struct ipa_pam_auth_handler_state *state;
> 314 struct tevent_req *req;
> 1. var_decl: Declaring variable sh without initializer.
> 315 struct sdap_handle *sh;
> 316 const char *attrs[] = {SYSDB_ORIG_DN, NULL};
> 317 struct ldb_message *msg;
> 318 const char *dn;
> 319 int timeout;
> 320 errno_t ret;
> 321
> 322 req = tevent_req_callback_data(subreq, struct tevent_req);
> 323 state = tevent_req_data(req, struct ipa_pam_auth_handler_state);
> 324
> 325 state->pd->pam_status = PAM_SYSTEM_ERR;
> 326
> CID 13699 (#1 of 1): Uninitialized pointer read (UNINIT)2.
> uninit_use_in_call: Using uninitialized value sh when calling
> sdap_cli_connect_recv. [show details]
> 327 ret = sdap_cli_connect_recv(subreq, state, NULL, &sh, NULL);
>
> src/providers/ldap/sdap_async_connection.c:
> 2026int sdap_cli_connect_recv(struct tevent_req *req,
> 2027 TALLOC_CTX *memctx,
> 2028 bool *can_retry,
> 2029 struct sdap_handle **gsh,
> 2030 struct sdap_server_opts **srv_opts)
> 2031{
> 2032 struct sdap_cli_connect_state *state = tevent_req_data(req,
> 2033 struct
> sdap_cli_connect_state);
> 2034 enum tevent_req_state tstate;
> 2035 uint64_t err_uint64;
> 2036 int err;
> 2037
> 1. Condition can_retry, taking true branch
> 2038 if (can_retry) {
> 2039 *can_retry = true;
> 2040 }
> 2. Condition tevent_req_is_error(req, &tstate, &err_uint64), taking false
> branch
> 2041 if (tevent_req_is_error(req, &tstate, &err_uint64)) {
> 2042 /* mark the server as bad if connection failed */
> 2043 if (state->srv) {
> 2044 DEBUG(SSSDBG_OP_FAILURE, "Unable to establish connection "
> 2045 "[%"PRIu64"]: %s\n", err_uint64,
> sss_strerror(err_uint64));
> 2046
> 2047 be_fo_set_port_status(state->be, state->service->name,
> 2048 state->srv, PORT_NOT_WORKING);
> 2049 } else {
> 2050 if (can_retry) {
> 2051 *can_retry = false;
> 2052 }
> 2053 }
> 2054
> 2055 if (tstate == TEVENT_REQ_USER_ERROR) {
> 2056 err = (int)err_uint64;
> 2057 if (err == EOK) {
> 2058 return EINVAL;
> 2059 }
> 2060 return err;
> 2061 }
> 2062 return EIO;
> 3. Condition state->srv, taking true branch
> 2063 } else if (state->srv) {
> 4. Condition debug_level & __debug_macro_level, taking true branch
> 2064 DEBUG(SSSDBG_TRACE_FUNC, "Connection established.\n");
> 2065
> 2066 be_fo_set_port_status(state->be, state->service->name,
> 2067 state->srv, PORT_WORKING);
> 2068 }
> 2069
> 5. Condition gsh, taking true branch
> 2070 if (gsh) {
> 6. read_parm: Reading a parameter value.
> 2071 if (*gsh) {
> 2072 talloc_zfree(*gsh);
> 2073 }
> 2074 *gsh = talloc_steal(memctx, state->sh);
> 2075 if (!*gsh) {
> 2076 return ENOMEM;
> 2077 }
>
>
> LS
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>
I'd just mention in the commit log that the issue was caught by coverity.
Anyways, patch looks good to me.
Reviewed-by: Fabiano Fidêncio <fiden...@redhat.com?
--
Fabiano Fidêncio
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
