[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:

ZERO_STRUCT(r->out);
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.

-- SPOILER ALERT --

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

Cheers.

Samuel.




More information about the samba-technical mailing list