[PATCH] Add pidhigh in the common response header

[Per Förlin]

Thanks Jeremy and Alexander for reviewing and considering the patch,

Regards,
Per Forlin

On Fri, Apr 15, 2016 at 7:03 AM, Jeremy Allison <jra at samba.org> wrote:
> On Thu, Apr 14, 2016 at 09:58:59PM -0700, Jeremy Allison wrote:
>> On Fri, Apr 15, 2016 at 07:36:10AM +0300, Alexander Bokovoy wrote:
>> > On Thu, 14 Apr 2016, Jeremy Allison wrote:
>> > > On Thu, Apr 14, 2016 at 10:41:56PM +0200, per.forlin at gmail.com wrote:
>> > > > From: Per Forlin <per.forlin at gmail.com>
>> > > >
>> > > > The pidhigh field is required to specify larger PID than 0xFFFF.
>> > > > A client sending a samba negotiation request may rely on a correct
>> > > > PID in the returning response header. The client may consider the
>> > > > response as invalid if the PID in the request doesn't match with
>> > > > the reply. Until now only PIDs up to 0xFFFF are supported since
>> > > > pidhigh has been set to 0 in the response. To resolve this issue the
>> > > > PID high is added to the response header.
>> > > >
>> > > > Signed-off-by: Per Forlin <per.forlin at gmail.com>
>> > >
>> > > Reviewed-by: Jeremy Allison <jra at samba.org>
>> > >
>> > > Can I get a second Team reviewer ? Once I do I'll
>> > > log a bug and push with the correct BUG: url, as
>> > > I think we'll want this fix in 4.4.next, 4.3.next,
>> > > 4.2.next.
>> > A torture test test_pidhigh needs to be fixed as well as we currently
>> > expect to fail if high pid is in use on the server.
>> >
>> > See source4/torture/raw/lock.c:470 or around.
>>
>> Actually, I'm not sure it does (I did look at this).
>> That test is testing the interaction between pid sent in
>> the session field in the header, and the pid used
>> in the SMB1 byte range locking.
>>
>> The pid sent and stored in the SMB1 byte range
>> lock requests is limited to 16-bits by definition.
>>
>> This means it doesn't look at pidhigh (only pidlow)
>> when checking for blocking_smblctx conflicts.
>>
>> Actually, even though I think this change is correct,
>> what I should really do is add another varient
>> of the source4/torture/raw/lock.c that sets a >16-bit
>> pid on the initial lock, and see if the MOD(>16-bit)
>> pid context conflicts.
>
> Ah. Just realized that can't be done, as that
> field (pid in the lock context) is only 16-bits.
>
> Essentially this means the use of the pid in
> SMB1 lock context is inherently broken and can't
> conflict correctly as designed. So the additional
> test I need to do is ensure that a Windows server
> reflects highpid on reply in the same way that
> this patch would make us do.