Skip to content

s4:torture/smb2: test FSCTL_QUERY_ALLOCATED_RANGES truncation

David Disseldorp requested to merge samba-team/devel/samba:ddiss/qar_rsp_truncation_v2 into master

v2 follow-up from my previous MR via !3775 (closed) ... Changes since v1, hopefully addressing Noel's feedback:

  • drop unused old_len test variable
  • reorder commits, with test first (and knownfail)
  • put s4 cli fix (needed for working tests) with server change
  • rebase against current master

This attempts to address https://bugzilla.samba.org/show_bug.cgi?id=15699

raised by David Howells on the upstream Samba / cifs.ko mailing lists. Tom Talpey and Ronnie clarified the expected behaviour:

On 5/23/2024 2:36 AM, David Howells wrote:
> ronnie sahlberg <ronniesahlberg@gmail.com> wrote:
>   
>>> The problem is that it essentially renders SEEK_DATA/SEEK_HOLE unusable for
>>> applications on cifs.  If there's more than one extent above the starting
>>> position, they'll fail with EIO.  The only way to do it is to provide for a
>>> sufficiently large buffer to accommodate however many extents that there are
>>> (and there could be millions, in theory) in order to get just the first one.  
>>
>> Wait, I didn't read all the text in the initial posts correctly.
>> Do you mean if you ask for "max x bytes of response, enough for n
>> entries" then if there
>> are > n entries on the server you get nothing back?
>>
>> I am pretty sure Windows will return as many entries as fits in the
>> reponse out-data-size
>> nad some error code.
>> But you are saying that instead of returning a truncated out-blob that > If OutputBufferSize < ((OutputBufferIndex + 1) *   
sizeof(FILE_ALLOCATED_RANGE_BUFFER)) then:
> 
>     Set Status to STATUS_BUFFER_OVERFLOW.  

>> samba will return nothing?  
> 
> It returns a STATUS_BUFFER_TOO_SMALL error if there's more than one extent
> record to return.  

Yeah, I think this is a Samba server issue. Ronnie is right that it
should return a partial response and a STATUS_BUFFER_OVERFLOW error
indicating that it's partial. It's not supposed to return
STATUS_BUFFER_TOO_SMALL unless the entire buffer is less than one
entry.

MS-FSA section 2.5.10.22

https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-fsa/385dec98-90fe-477f-9789-20a47a7b8467

Tom.

Checklist

  • Commits have Signed-off-by: with name/author being identical to the commit author
  • (optional) This MR is just one part towards a larger feature.
  • (optional, if backport required) Bugzilla bug filed and BUG: tag added
  • Test suite updated with functionality tests
  • Test suite updated with negative tests
  • Documentation updated
  • CI timeout is 3h or higher (see Settings/CICD/General pipelines/ Timeout)

Merge request reports