[WIP PATCH] DCE/RPC server merge

Samuel Cabrero scabrero at suse.de
Thu Feb 14 17:56:23 UTC 2019

On Fri, 2019-02-08 at 20:07 +0100, Andreas Schneider wrote:
> > * https://gitlab.com/scabrero/samba/tree/dcerpc-server-merge-s3-
> > prep-v2
> > - This is preparation for S3
> > - Rename functions to make the merge easier
> > - Remove the named_pipe_client struct to make use of general
> > ncacn_conn
> > - Add the posibility to attach private data to sockets in prefork
> > "process model", used later to associate the socket with the
> > dcesrv_endpoint.
> I've just reviewed this branch and had a few comments. However this
> already 
> looks quite good. Let me know if something is unclear.
> Will look at the others next week.

Thanks for the review, I will address your comments before pushing the
next update (v3).

In the meanwhile I would like to raise a question regarding the
differences in the NDR generated code between S3 and S4, as I have
found some tests which are failing because of this difference, either
crashing or because the "out." fields in the "r struct" are NULL-
checked and not written when the S3 RPC service call implementation is
called from the S4 server loop.

I will try to explain myself taking winreg EnumKey as an example:

[public] WERROR inreg_EnumKey(
    [in,ref]        policy_handle    *handle,
    [in]            uint32           enum_index,                    
    [in,out,ref]    winreg_StringBuf *name,                         
    [in,out,unique] winreg_StringBuf *keyclass,                     
    [in,out,unique] NTTIME           *last_changed_time             

The S3 generated "api_winreg_EnumKey" function initializes the "r.out"
members with "r.in" values for "[in,out]" arguments before dispatching
the call:

r->out.name = r->in.name;
r->out.keyclass = r->in.keyclass;
r->out.last_changed_time = r->in.last_changed_time;
r->out.result = _winreg_EnumKey(p, r);

The S3 service implementation assume that it is initialized and deref
the pointer:

r->out.keyclass->name = "";

On the other hand the S4 generated NDR code is common for all interface
calls (winreg__op_ndr_pull, winreg__op_ndr_push), and it does not
initialize the "out" members but the call handler does:

static WERROR dcesrv_winreg_EnumKey(struct dcesrv_call_state ...
r->out.keyclass = r->in.keyclass;

As a result the S3 implementation crashes when invoked from the S4
server loop:

status = call->context->iface->ndr_pull(call, call, pull, &call->r);
status = call->context->iface->dispatch(call, call, call->r);

I would like to ask in first place if initializing the "out" members
with the "in" members is correct to find the best way to solve this
issue, as I have manually fixed a few S3 call handlers triggered while
running the tests but probably there will be lot of them not invoked
which may crash or not work properly if this gets merged.


All tests are passing the merged build, I will push *-v3 branches to
gitlab once I have finished cleaning them up.



More information about the samba-technical mailing list