Bug 11226 - Fix terminate connection behavior for asynchronous endpoint with PUSH notification flavors
Summary: Fix terminate connection behavior for asynchronous endpoint with PUSH notific...
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: DCE-RPCs and pipes (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on: 11225 11236
Blocks:
  Show dependency treegraph
 
Reported: 2015-04-20 10:36 UTC by Julien Kerihuel (mail address dead)
Modified: 2015-05-04 18:19 UTC (History)
5 users (show)

See Also:
j.kerihuel: review? (metze)
jelmer: review+


Attachments
Patch to fix termination of asynchronous connections with pending calls (2.70 KB, patch)
2015-04-20 10:36 UTC, Julien Kerihuel (mail address dead)
no flags Details
patch for v4-1 series (3.05 KB, patch)
2015-04-20 14:34 UTC, Julien Kerihuel (mail address dead)
jra: review+
Details
git-am fix for 4.2.next. (2.99 KB, patch)
2015-04-21 22:28 UTC, Jeremy Allison
jra: review? (jelmer)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Kerihuel (mail address dead) 2015-04-20 10:36:14 UTC
Created attachment 10968 [details]
Patch to fix termination of asynchronous connections with pending calls

Description:
============
Attached is a patch to fix how the dcerpc server handles termination of
connection for dcerpc endpoints with PUSH notification like behaviors.

The patch adds a DCESRV_CALL_STATE_FLAG_PROCESS_PENDING_CALL and set it to
call->context->conn->state_flags during dcesrv_bind() and dcesrv_alter()
calls if it was available in dce_call->state_flags. This patch loops over all
contexts within dcesrv_cleanup_broken_connections to call the destructor and makes the unbind function for the endpoint responsible for managing/cleaning up context->conn->pending_call_list. 

Overview:
=========
When implementing an asynchronous dcerpc endpoint that behaves
as a PUSH notification service, the client sends a request for which the
server does not send the reply unless there is either pending
notifications to return or a timeout was reached.

If the client closes the connection before the server returns, the
connection is handled as a broken connection with pending calls on the
dcerpc server and the destructor dcesrv_connection_context_destructor is
never called. It also means that the unbind method on the interface is
never called. A sample output is available below:

ndr_pull_error(11): Pull bytes 4 (../librpc/ndr/ndr_basic.c:148)
dcesrv: terminating connection due to 'NT_STATUS_BUFFER_TOO_SMALL' defered due to pending calls
ndr_pull_error(11): Pull bytes 4 (../librpc/ndr/ndr_basic.c:148)
dcesrv: terminating connection due to 'NT_STATUS_BUFFER_TOO_SMALL' defered due to pending calls

With this patch applied, the unbind method of the interface is called and pending calls can be cleaned by the endpoint service:

ndr_pull_error(11): Pull bytes 4 (../librpc/ndr/ndr_basic.c:148)
Terminating connection - 'dcesrv: NT_STATUS_BUFFER_TOO_SMALL'
imessaging: cleaning up /var/lib/samba/private/smbd.tmp/msg/msg.12698.43
single_terminate: reason[dcesrv: NT_STATUS_BUFFER_TOO_SMALL]
mapiproxy/servers/default/asyncemsmdb/dcesrv_asyncemsmdb.c:339(dcerpc_server_asyncemsmdb_unbind): DISCONNECTION FROM CLIENT
Comment 1 Julien Kerihuel (mail address dead) 2015-04-20 10:37:31 UTC
Applied in master in fd90d270c7e97a639f42a96b674a674d1b51aa0d
Comment 2 Julien Kerihuel (mail address dead) 2015-04-20 14:34:34 UTC
Created attachment 10971 [details]
patch for v4-1 series
Comment 3 Jeremy Allison 2015-04-21 22:25:32 UTC
Comment on attachment 10971 [details]
patch for v4-1 series

LGTM.
Comment 4 Jeremy Allison 2015-04-21 22:28:02 UTC
Created attachment 10975 [details]
git-am fix for 4.2.next.

Jelmer, please +1 for 4.2.next.

Thanks,

Jeremy.
Comment 5 Jeremy Allison 2015-04-21 22:28:35 UTC
Comment on attachment 10975 [details]
git-am fix for 4.2.next.

git cherry-pick from master.
Comment 6 Jeremy Allison 2015-04-22 03:34:12 UTC
Re-assigning to Karolin for inclusion in 4.2.next, 4.1.next.

Karolin, Jelmer +1'ed the bug itself rather than the attachment, but it means the same thing :-).

Jeremy.
Comment 7 Julien Kerihuel (mail address dead) 2015-04-24 10:33:21 UTC
A regression/crash introduced by this patch has been identified when an unknown dcerpc endpoint is queried in dcesrv_alter. 

I have created a bug entry and attached a fix to address this problem:

https://bugzilla.samba.org/show_bug.cgi?id=11236
Comment 8 Karolin Seeger 2015-04-27 20:01:53 UTC
Pushed to autobuild-v4-[1|2]-test.
Comment 9 Karolin Seeger 2015-05-04 18:19:59 UTC
Pushed to both branches.
Closing out bug report.

Thanks!