[URGENT][PATCH] Re: Possible issue in AD DC LSA server in master

[Andrew Bartlett]

On Thu, 2018-05-03 at 09:43 +0200, Stefan Metzmacher wrote:
> Am 03.05.2018 um 06:29 schrieb Andrew Bartlett via samba-technical:
> > On Sat, 2018-04-14 at 07:07 +1200, Andrew Bartlett via samba-technical
> > wrote:
> > > So, that autobuild failed with:
> > > 
> > > > [133(711)/525 at 10m56s] samba3.rpc.lsa.lookupsids(ad_dc)
> > > > smbtorture 4.9.0pre1-DEVELOPERBUILD
> > > > Using seed 1523610756
> > > > UNEXPECTED(failure): samba3.rpc.lsa.lookupsids.lsa.LookupSidsReply(ad_dc)
> > > > REASON: Exception: Exception: ../source4/torture/rpc/lsa_lookup.c:400: names.names[0].name.string was , expected S-1-5-21-1111111111-2222222222-3333333333-512: unexpected names[0].string
> > > > 
> > > > FAILED (1 failures, 0 errors and 0 unexpected successes in 0 testsuites)
> > > 
> > > And I mentioned before that I got one LSA failure on the branch up to:
> > > 
> > > commit cb607346d3c7c662343b0eae69e43eaa6358c188
> > > Author: Gary Lockyer <gary at catalyst.net.nz>
> > > Date:   Tue Mar 13 16:43:54 2018 +1300
> > > 
> > >     ldb-samba: require pid match for cached ldb
> > >     
> > >     Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
> > >     Reviewed-by: Andrew Bartlett <abartlet at samba.org>
> > > 
> > > > Testing OpenPolicy2
> > > > UNEXPECTED(failure): samba4.rpc.altercontext on ncalrpc with seal,padcheck.altercontext(ad_dc_ntvfs:local)
> > > > REASON: Exception: Exception: ../source4/torture/rpc/lsa.c:188: status was NT_STATUS_CONNECTION_RESET, expected NT_STATUS_CONNECTION_DISCONNECTED: OpenPolicy2 failed
> > > 
> > > (but wrote it off as I also got about 10 successes on branches with
> > > that series in it). 
> > > 
> > > This is fishy, as Joe yesterday got this in travis CI on master:
> > > 
> > > > Testing LookupSids
> > > > ndr_pull_error(1): Bad array size - got 0 expected 8
> > > > 
> > > > UNEXPECTED(failure): samba3.rpc.lsa.privileges.lsa.Privileges(ad_dc)
> > > > REASON: Exception: Exception: ../source4/torture/rpc/lsa.c:774: dcerpc_lsa_LookupSids_r(b, tctx, &r) was NT_STATUS_ARRAY_BOUNDS_EXCEEDED, expected NT_STATUS_OK: LookupSids failed
> > > > 
> > > > FAILED (1 failures, 0 errors and 0 unexpected successes in 0 testsuites)
> > > 
> > > If anybody has any insights or suggestions please don't hesitate to
> > > investigate.
> > 
> > It turned out to be an unrelated use-after-free in the LSA server after
> > the trusts changes recently. 
> > 
> > The issue was found fairly easily with address-sanitizer and is fixed
> > in the attached.  This needs to be in 4.8.2 the regression was shipped
> > with 4.8.0.
> 
> Do you have more detailed information on what memory is used after free?

It shows up pretty fast under address sanitizer with the other patches
posted today.  It seemed so ovbious that I didn't go much further in
making notes, but here is the backtrace.

https://attachments.samba.org/attachment.cgi?id=14174

> I'd prefer to do the correct talloc_move() calls in order to get a sane
> memory tree instead of being lazy.

Have a look at:
dcesrv_lsa_LookupSids()
	state->r.out.names = talloc_zero(state, struct
lsa_TransNameArray2);

Thanks!

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