[PATCH] smbd: Fix snapshot query on shares with DFS enabled

Uri Simchoni uri at samba.org
Mon Aug 22 09:50:45 UTC 2016


On 08/20/2016 06:37 AM, Jeremy Allison wrote:
> On Fri, Aug 19, 2016 at 11:22:54AM +0300, Uri Simchoni wrote:
>> On 08/19/2016 12:51 AM, Jeremy Allison wrote:
>>>
>>> Once this is in I'll do the same fix for accessing shadow
>>> copies over SMB2.
>>>
>>> Cheers,
>>>
>>> 	Jeremy.
>>>
>>
>> Pushed with a small space-before-tab correction in "libsmb: Add uint16_t
>> addtional_flags2 to cli_trans_send()"
> 
> Thanks for the fix.
> 
> Here is the follow-up patchset to fix getting shadow
> copies using the 'allinfo' command over SMB2+.
> 
> Passes local make test.
> 
> Patch #1 is a leftover I forgot from the previous
> patchset.
> 
> Patch #2 took FAR TOO LONG to figure out. 6 hours
> of staring at smbclient SMB2create calls returning
> NT_STATUS_INVALID_PARAMETER until I finally started
> looking at padding :-).
> 
> Please review and push !
> 
> Cheers,
> 
> 	Jeremy.
> 

I didn't understand why the following check is made in
cli_smb2_shadow_copy_data_fnum_recv():

> +       if (state->out_output_buffer.length +
> +                       (2 * sizeof(SHADOW_COPY_LABEL)) <
> +                               state->out_output_buffer.length) {
> +               return NT_STATUS_INVALID_NETWORK_RESPONSE;
> +       }
> +

(besides not understanding its purpose, we should also keep in mind that
smb2cli_ioctl_recv() has made certain that the output buffer is not
larger than what we've asked for. The move to pointer arithmetic does
not prevent overflow in 32-bit platforms, but the check that the
response is < 64K does)

Also if you happen to touch the code, perhaps it's worth commenting that
dlength + 12 > state->out_output_buffer.length *is* a valid response (it
means there are more snapshots than our buffer allows), but we currently
don't support that.

Otherwise RB+ me.

Thanks,
Uri



More information about the samba-technical mailing list