[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