[PATCH] Fixes for winbindd

Andrew Bartlett abartlet at samba.org
Thu Jul 31 02:28:32 MDT 2014


On Sat, 2014-07-26 at 09:44 +1200, Andrew Bartlett wrote:
> 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. 

Can we please push these patches?  I still want metze's view, but I
think the patches are as correct as things can be right now, and so
should go in while we restructure the pidl generator.

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