[PATCH] Fixes for winbindd
Andrew Bartlett
abartlet at samba.org
Fri Jul 25 15:44:05 MDT 2014
On Fri, 2014-07-25 at 22:59 +0200, Volker Lendecke wrote:
> On Sat, Jul 26, 2014 at 08:26:18AM +1200, Andrew Bartlett wrote:
> > netlogon_creds_cli_DsrUpdateReadOnlyServerDnsRecords_send() creates
> > state on mem_ctx, which is frame in
> > netlogon_creds_cli_DsrUpdateReadOnlyServerDnsRecords()
> >
> > struct netlogon_creds_cli_DsrUpdateReadOnlyServerDnsRecords_state *state;
> > struct tevent_req *subreq;
> >
> > req = tevent_req_create(mem_ctx, &state,
> > struct netlogon_creds_cli_DsrUpdateReadOnlyServerDnsRecords_state);
> > if (req == NULL) {
> > return NULL;
> > }
> >
> > The state->dns_names is an in/out parameter as dns_names to
> > netlogon_creds_cli_DsrUpdateReadOnlyServerDnsRecords() and has a longer
> > life time.
> >
> > The issue isn't the async, it is that the memory needs to be around
> > longer than the state variable.
> >
> > The typical approach of a talloc_steal won't work, due to the way the
> > ref variable is overwritten. The struct NL_DNS_NAME_INFO_ARRAY
> > dns_names is the original input array, but the elements are overwritten
> > in dcerpc_netr_DsrUpdateReadOnlyServerDnsRecords_done()
>
> Ok, understood. To be honest, I'd call this a bug in the way
> pidl generates code. I'm not sure there's a better way to do
> it, but this is really, really obscure. I'd like to hear
> what metze has to say about this before it goes in.
>
> The pure fact that this crash initially went in shows that
> something is wrong.
I do feel the same. It seems to me that in/out variables used this way
should be the talloc children of what we pass in, and/or that the
structure (rather than argument based) calling convention could have
made this clearer.
Andrew Bartlett
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Catalyst IT http://catalyst.net.nz/services/samba
More information about the samba-technical
mailing list