#4036 PROXY: Return data in output parameter if everything is OK
Merged 4 years ago by jhrozek. Opened 4 years ago by lslebodn.
SSSD/ lslebodn/sssd proxy_crash  into  master

@@ -698,10 +698,12 @@ 

      }

      grp->gr_mem[i] = NULL;

  

-     *_grp = talloc_steal(mem_ctx, grp);

      ret = EOK;

  

  done:

+     if (ret == EOK) {

+         *_grp = talloc_steal(mem_ctx, grp);

+     }

      talloc_zfree(tmp_ctx);

  

      return ret;

The function remove_duplicate_group_members might return EOK also in the middle
of function but return parameter was not set with right data.
Processing continued in the function save_group but there was a
dereference of NULL pointer.

Introduced in: https://pagure.io/SSSD/sssd/issue/3931

Crash:

  (gdb) bt
  #0  0x00007fb4ce4a9ac5 in save_group (sysdb=sysdb@entry=0x55c9a0efb230, dom=dom@entry=0x55c9a0efb420, grp=grp@entry=0x55c9a0f370f0, real_name=0x55c9a0f47340 "nobody@ldap",
      alias=alias@entry=0x0) at src/providers/proxy/proxy_id.c:748
  #1  0x00007fb4ce4aa600 in get_gr_gid (mem_ctx=mem_ctx@entry=0x55c9a0f38be0, sysdb=sysdb@entry=0x55c9a0efb230, dom=dom@entry=0x55c9a0efb420, gid=99, now=<optimized out>,
      ctx=<optimized out>) at src/providers/proxy/proxy_id.c:1160
  #2  0x00007fb4ce4ac9e5 in get_initgr_groups_process (pwd=0x55c9a0f384a0, pwd=0x55c9a0f384a0, dom=0x55c9a0efb420, sysdb=0x55c9a0efb230, ctx=0x55c9a0f048e0, memctx=0x55c9a0f38be0)
      at src/providers/proxy/proxy_id.c:1553
  #3  get_initgr (i_name=<optimized out>, dom=0x55c9a0efb420, sysdb=<optimized out>, ctx=0x55c9a0f048e0, mem_ctx=0x55c9a0f38b70) at src/providers/proxy/proxy_id.c:1461
  #4  proxy_account_info (domain=0x55c9a0efb420, be_ctx=<optimized out>, data=<optimized out>, ctx=0x55c9a0f048e0, mem_ctx=0x55c9a0f38b70) at src/providers/proxy/proxy_id.c:1659
  #5  proxy_account_info_handler_send (mem_ctx=<optimized out>, id_ctx=0x55c9a0f048e0, data=<optimized out>, params=0x55c9a0f39790) at src/providers/proxy/proxy_id.c:1758
  #6  0x000055c99fc67677 in file_dp_request (_dp_req=<synthetic pointer>, req=0x55c9a0f39470, request_data=<optimized out>, dp_flags=1, method=DPM_ACCOUNT_HANDLER, target=DPT_ID,
      name=<optimized out>, domainname=0x55c9a0f39190 "LDAP", provider=0x55c9a0efe0e0, mem_ctx=<optimized out>) at src/providers/data_provider/dp_request.c:250
  #7  dp_req_send (mem_ctx=0x55c9a0f37b60, provider=provider@entry=0x55c9a0efe0e0, domain=domain@entry=0x55c9a0f39190 "LDAP", name=<optimized out>, target=target@entry=DPT_ID,
      method=method@entry=DPM_ACCOUNT_HANDLER, dp_flags=dp_flags@entry=1, request_data=0x55c9a0f37c00, _request_name=0x55c9a0f37b60) at src/providers/data_provider/dp_request.c:295
  #8  0x000055c99fc6a132 in dp_get_account_info_send (mem_ctx=<optimized out>, ev=0x55c9a0eddbc0, sbus_req=<optimized out>, provider=0x55c9a0efe0e0, dp_flags=1,
      entry_type=<optimized out>, filter=0x55c9a0f358d0 "name=nobody@ldap", domain=0x55c9a0f39190 "LDAP", extra=0x55c9a0f354a0 "") at src/providers/data_provider/dp_target_id.c:528
  #9  0x00007fb4da35265b in _sbus_sss_invoke_in_uusss_out_qus_step (ev=0x55c9a0eddbc0, te=<optimized out>, tv=..., private_data=<optimized out>) at src/sss_iface/sbus_sss_invokers.c:2847
  #10 0x00007fb4d9cfb1cf in tevent_common_invoke_timer_handler () from /lib64/libtevent.so.0
  #11 0x00007fb4d9cfb339 in tevent_common_loop_timer_delay () from /lib64/libtevent.so.0
  #12 0x00007fb4d9cfc2f9 in epoll_event_loop_once () from /lib64/libtevent.so.0
  #13 0x00007fb4d9cfa7b7 in std_event_loop_once () from /lib64/libtevent.so.0
  #14 0x00007fb4d9cf5b5d in _tevent_loop_once () from /lib64/libtevent.so.0
  #15 0x00007fb4d9cf5d8b in tevent_common_loop_wait () from /lib64/libtevent.so.0
  #16 0x00007fb4d9cfa757 in std_event_loop_wait () from /lib64/libtevent.so.0
  #17 0x00007fb4dd955ac3 in server_loop (main_ctx=0x55c9a0edf090) at src/util/server.c:724
  #18 0x000055c99fc59760 in main (argc=8, argv=<optimized out>) at src/providers/data_provider_be.c:747
  (gdb) l
  (gdb) bt
  #0  0x00007fb4ce4a9ac5 in save_group (sysdb=sysdb@entry=0x55c9a0efb230, dom=dom@entry=0x55c9a0efb420, grp=grp@entry=0x55c9a0f370f0, real_name=0x55c9a0f47340 "nobody@ldap",
      alias=alias@entry=0x0) at src/providers/proxy/proxy_id.c:748
  #1  0x00007fb4ce4aa600 in get_gr_gid (mem_ctx=mem_ctx@entry=0x55c9a0f38be0, sysdb=sysdb@entry=0x55c9a0efb230, dom=dom@entry=0x55c9a0efb420, gid=99, now=<optimized out>,
      ctx=<optimized out>) at src/providers/proxy/proxy_id.c:1160
  #2  0x00007fb4ce4ac9e5 in get_initgr_groups_process (pwd=0x55c9a0f384a0, pwd=0x55c9a0f384a0, dom=0x55c9a0efb420, sysdb=0x55c9a0efb230, ctx=0x55c9a0f048e0, memctx=0x55c9a0f38be0)
      at src/providers/proxy/proxy_id.c:1553
  #3  get_initgr (i_name=<optimized out>, dom=0x55c9a0efb420, sysdb=<optimized out>, ctx=0x55c9a0f048e0, mem_ctx=0x55c9a0f38b70) at src/providers/proxy/proxy_id.c:1461
  #4  proxy_account_info (domain=0x55c9a0efb420, be_ctx=<optimized out>, data=<optimized out>, ctx=0x55c9a0f048e0, mem_ctx=0x55c9a0f38b70) at src/providers/proxy/proxy_id.c:1659
  #5  proxy_account_info_handler_send (mem_ctx=<optimized out>, id_ctx=0x55c9a0f048e0, data=<optimized out>, params=0x55c9a0f39790) at src/providers/proxy/proxy_id.c:1758
  #6  0x000055c99fc67677 in file_dp_request (_dp_req=<synthetic pointer>, req=0x55c9a0f39470, request_data=<optimized out>, dp_flags=1, method=DPM_ACCOUNT_HANDLER, target=DPT_ID,
      name=<optimized out>, domainname=0x55c9a0f39190 "LDAP", provider=0x55c9a0efe0e0, mem_ctx=<optimized out>) at src/providers/data_provider/dp_request.c:250
  #7  dp_req_send (mem_ctx=0x55c9a0f37b60, provider=provider@entry=0x55c9a0efe0e0, domain=domain@entry=0x55c9a0f39190 "LDAP", name=<optimized out>, target=target@entry=DPT_ID,
      method=method@entry=DPM_ACCOUNT_HANDLER, dp_flags=dp_flags@entry=1, request_data=0x55c9a0f37c00, _request_name=0x55c9a0f37b60) at src/providers/data_provider/dp_request.c:295
  #8  0x000055c99fc6a132 in dp_get_account_info_send (mem_ctx=<optimized out>, ev=0x55c9a0eddbc0, sbus_req=<optimized out>, provider=0x55c9a0efe0e0, dp_flags=1,
      entry_type=<optimized out>, filter=0x55c9a0f358d0 "name=nobody@ldap", domain=0x55c9a0f39190 "LDAP", extra=0x55c9a0f354a0 "") at src/providers/data_provider/dp_target_id.c:528
  #9  0x00007fb4da35265b in _sbus_sss_invoke_in_uusss_out_qus_step (ev=0x55c9a0eddbc0, te=<optimized out>, tv=..., private_data=<optimized out>) at src/sss_iface/sbus_sss_invokers.c:2847
  #10 0x00007fb4d9cfb1cf in tevent_common_invoke_timer_handler () from /lib64/libtevent.so.0
  #11 0x00007fb4d9cfb339 in tevent_common_loop_timer_delay () from /lib64/libtevent.so.0
  #12 0x00007fb4d9cfc2f9 in epoll_event_loop_once () from /lib64/libtevent.so.0
  #13 0x00007fb4d9cfa7b7 in std_event_loop_once () from /lib64/libtevent.so.0
  #14 0x00007fb4d9cf5b5d in _tevent_loop_once () from /lib64/libtevent.so.0
  #15 0x00007fb4d9cf5d8b in tevent_common_loop_wait () from /lib64/libtevent.so.0
  #16 0x00007fb4d9cfa757 in std_event_loop_wait () from /lib64/libtevent.so.0
  #17 0x00007fb4dd955ac3 in server_loop (main_ctx=0x55c9a0edf090) at src/util/server.c:724
  #18 0x000055c99fc59760 in main (argc=8, argv=<optimized out>) at src/providers/data_provider_be.c:747
  (gdb) l
  733         ret = remove_duplicate_group_members(tmp_ctx, grp, &ngroup);
  734         if (ret != EOK) {
  735             DEBUG(SSSDBG_CRIT_FAILURE, "Failed to remove duplicate group member     s\n");
  736             goto done;
  737         }
  738
  739         DEBUG_GR_MEM(SSSDBG_TRACE_LIBS, ngroup);
  740
  741         ret = sysdb_transaction_start(sysdb);
  742         if (ret != EOK) {
  743             DEBUG(SSSDBG_CRIT_FAILURE, "Failed to start transaction\n");
  744             goto done;
  745         }
  746         in_transaction = true;
  747
  748         if (ngroup->gr_mem && ngroup->gr_mem[0]) {
  749             attrs = sysdb_new_attrs(tmp_ctx);
  750             if (!attrs) {
  751                 DEBUG(SSSDBG_CRIT_FAILURE, "Allocation error?!\n");
  752                 ret = ENOMEM;
  (gdb) p ngroup
  $1 = (struct group *) 0x0
  743             DEBUG(SSSDBG_CRIT_FAILURE, "Failed to start transaction\n");
  744             goto done;
  745         }
  746         in_transaction = true;
  747
  748         if (ngroup->gr_mem && ngroup->gr_mem[0]) {
  749             attrs = sysdb_new_attrs(tmp_ctx);
  750             if (!attrs) {
  751                 DEBUG(SSSDBG_CRIT_FAILURE, "Allocation error?!\n");
  752                 ret = ENOMEM;
  (gdb) p ngroup
  $1 = (struct group *) 0x0

Resolves:
https://pagure.io/SSSD/sssd/issue/XXXX

rebased onto b7d2468

4 years ago

thanks, this is the same patch I have locally, so ack.

Just please in the future say you are working on some issue, we could have saved some duplicated time.

Commit e1b678c fixes this pull-request

Pull-Request has been merged by jhrozek

4 years ago

I could not see issue in pagure because there was not any at that time :-)
And I wanted to provide scratch build in fedora BZ.

Thank you very much for fast review

Metadata