valgrind error on LSA lookup

[Andrew Bartlett]

On Wed, 2011-07-27 at 17:20 +1000, Andrew Bartlett wrote:
> On Wed, 2011-07-13 at 02:51 +0400, Matthieu Patou wrote:
> > Hello Andrew,
> > 
> > I was trying to push the attached patch but I get errors, after a bit of 
> > hunting I found that the problem is the unit test that do async 
> > lsarlookupsid.
> > 
> > As I had no clue of why state->access_mask & LSA_POLICY_LOOKUP_NAMES 
> > returned 1 when the openpolicy didn't set this flag I started to suspect 
> > an error on state.
> > 
> > Running valgrind on samba4  + ./bin/smbtorture  
> > ncacn_np:172.16.100.1[bigendian] rpc.lsa  -U administrator%totoTATA122 
> > with my patches yield this:
> > ==2592== Conditional jump or move depends on uninitialised value(s)
> > ==2592==    at 0x125206CF: dcesrv_lsa_LookupSids2 (lsa_lookup.c:571)
> > ==2592==    by 0x12520D43: dcesrv_lsa_LookupSids (lsa_lookup.c:732)
> > ==2592==    by 0x12511762: lsarpc__op_dispatch (ndr_lsa_s.c:233)
> > ==2592==    by 0x124EA510: dcesrv_request (dcerpc_server.c:964)
> > ==2592==    by 0x124EAA7A: dcesrv_process_ncacn_packet 
> > (dcerpc_server.c:1109)
> > ==2592==    by 0x124EBABF: dcesrv_read_fragment_done (dcerpc_server.c:1487)
> > ==2592==    by 0x71B82DA: _tevent_req_notify_callback (tevent_req.c:101)
> > ==2592==    by 0x71B830C: tevent_req_finish (tevent_req.c:110)
> > ==2592==    by 0x71B8333: _tevent_req_done (tevent_req.c:116)
> > ==2592==    by 0xDB33E19: dcerpc_read_ncacn_packet_done (dcerpc_util.c:295)
> > ==2592==    by 0x71B82DA: _tevent_req_notify_callback (tevent_req.c:101)
> > ==2592==    by 0x71B830C: tevent_req_finish (tevent_req.c:110)
> > ==2592==
> > 
> > 
> > Obviously dcesrv_lsa_get_policy_state is not returning something correct 
> > ... can you have a look because this async madness is more than I can 
> > understand for the moment.
> 
> Don't think about the async, this isn't an async bug.  The async test
> just makes a lot of requests, or exercises this particular interface.
> 
> The issue is that LsaLookupSids2 does not emulate the Microsoft
> behaviour fully, because it does not do a full internal LsaOpenPolicy2.
> The difference is in exactly the access_mask that you de-reference.
> Only LsaOpenPolicy2 (but not  dcesrv_lsa_get_policy_state()) fills in
> state->access_mask. 
> 
> We need to either call the full LsaOpenPolicy2, or at least it's full
> equivalent.  We need to work out the access mask to use in the absence
> of a policy handle.  The Samba3 code did a lot of access mask work,
> perhaps it can help. 

The LsaOpenPolicy2 actually does take a policy handle, I just ignored it
when I re-factored this code in 2006:
459a2301a5d63f5a1a6b27996c8a0358b20f2ab2

I propose this patch.  

However, as regards your patch, there is no point testing for
state->access_mask & LSA_POLICY_LOOKUP_NAMES if nothing later denies
access on that basis.  

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s4-lsa-Use-the-supplied-handle-in-LsaLookupNames2.patch
Type: text/x-patch
Size: 1796 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20110728/5f95dce0/attachment.bin>