Bug 12926 - A GetNCChanges can be triggered while waiting for a DsReplicaSync
Summary: A GetNCChanges can be triggered while waiting for a DsReplicaSync
Status: ASSIGNED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.7.0rc2
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Andrew Bartlett
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-22 10:05 UTC by Andrew Bartlett
Modified: 2017-07-24 06:41 UTC (History)
3 users (show)

See Also:


Attachments
patch for master (1.16 KB, patch)
2017-07-22 10:16 UTC, Andrew Bartlett
no flags Details
the problem stream (partial pcapng) (2.40 MB, application/octet-stream)
2017-07-23 01:38 UTC, Andrew Bartlett
no flags Details
subunit logs from the issue matching the capture (83.91 KB, text/plain)
2017-07-23 01:58 UTC, Andrew Bartlett
no flags Details
patch for 4.7 cherry-picked from master (1.40 KB, patch)
2017-07-24 06:40 UTC, Andrew Bartlett
abartlet: review? (garming)
abartlet: review? (metze)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Bartlett 2017-07-22 10:05:17 UTC
The drepl server does not currently have a mutual exclusion between GetNCChanges and DsReplicaSync, meaning that we can get replication errors triggered by an RPC fault if we end up with both outstanding at the same time.
Comment 1 Andrew Bartlett 2017-07-22 10:16:50 UTC
Created attachment 13411 [details]
patch for master

Attached is a possible patch, under test.
Comment 2 Andrew Bartlett 2017-07-23 01:38:48 UTC
Created attachment 13415 [details]
the problem stream (partial pcapng)
Comment 3 Andrew Bartlett 2017-07-23 01:58:05 UTC
Created attachment 13416 [details]
subunit logs from the issue matching the capture
Comment 4 Stefan Metzmacher 2017-07-23 06:43:01 UTC
(In reply to Andrew Bartlett from comment #2)

Ok, I think we hit this in the server,
dcesrv_process_ncacn_packet:

      if (dce_conn->pending_call_list != NULL) {
             /*
              * concurrent requests are only allowed
              * if DCERPC_PFC_FLAG_CONC_MPX was negotiated.
              */
             if (!(dce_conn->state_flags & DCESRV_CALL_STATE_FLAG_MULTIPLEXED)) {
                     dcesrv_call_disconnect_after(call,
                                   "dcesrv_auth_request - "
                                   "existing pending call without CONN_MPX");
                     return dcesrv_fault(call,
                                         DCERPC_NCA_S_PROTO_ERROR);
             }
      }

So the fix is correct for now. We should really only do one outgoing operation
at a time.
Comment 5 Andrew Bartlett 2017-07-23 07:23:05 UTC
(In reply to Stefan Metzmacher from comment #4)
That makes a lot of sense. 

Thanks!
Comment 6 Andrew Bartlett 2017-07-24 06:40:44 UTC
Created attachment 13421 [details]
patch for 4.7 cherry-picked from master