[SCM] Samba Shared Repository - branch master updated

[Stefan (metze) Metzmacher]

Jeremy Allison schrieb:
> On Tue, Apr 27, 2010 at 09:18:54AM +0200, Stefan (metze) Metzmacher wrote:
>> I'm not sure this is correct.
>>
>> I haven't looked in detail, but this looks like just hiding the real
>> problem.
>>
>> The real problem is likely that we're abusing the tevent_req guidelines.
>>
>> I think 8f67f873ace91964da066c421986e260aceba75b is maybe ok, for
>> getting stuff working, but I'd like to see the design changed.
>>
>> smb2_deferred_open_timer() should not call smbd_smb2_request_dispatch().
>>
>> The re-entrant should happen inside the smbd_smb2_create_* code,
>> the place were it decides to go async, instead of two layers above.
>>
>> I think the smbd_smb2_create_* should setup a
>> smb2req->retry_callback(struct tevent_req *) function pointer.
>> smb2_deferred_open_timer() would then just call it should just call it.
>>
>> I'd like to have something similar for smb1 (I know it would be a lot of
>> work), but the layer violation is really confusing.
>>
>> The top level smb1/2 server code should not see any of this retry logic,
>> it should just do a foo_send() call set it's callback
>> on the returned tevent_req and get the final result with foo_recv().
>> All magic should be in one spot in the lower level.
> 
> I disagree with this. Mainly because it will lead to a lot of
> code duplication, in potentially security-sensitive paths.
> 
> The great thing about the way I have it working now, is that
> it works identically to the SMB1 code paths in terms of reentrancy,
> and allows us to re-use all the parameter parsing and setting of
> the security context that we naturally have to have to make
> non-reentrant code work.
> 
> If we break the smbd_smb2_request_dispatch() path we will see
> the problem immediately. If we add specialized retry logic to
> individual reentrant code paths it will be a source of logic
> and possibly security bugs for some time to come.
> 
> The method I'm using, as it's identical to the SMB1 logic,
> also allows me to hook in the backends in exactly the same
> way that SMB1 uses them.
> 
> I'm planning to finish implementing blocking locks (the
> remaining reentrant call) in the same way.
> 
> Once this gets done we'll have a working system, and if you
> want to look at refactoring at that point then I'd be happy
> to help.
> 
> But I want to get to a fully featured system quickly, and
> that currently means re-implementing the SMB1 logic. Remember,
> Volker also understands how this works, so that's at least
> two people who can fix this :-).

Sure, getting it working first is a good plan:-)

Thanks for your hard work on it!

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 260 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100427/7e43cd94/attachment.pgp>