Hi David,
On Wed, 22 May 2024 00:53:59 +0100, David Howells via samba-technical wrote:
> I've been debugging a failure in xfstests generic/286 whereby llseek() return
> -EIO and am thinking that the bug is in Samba. The test can be distilled to:
>
> mount //my/share /mnt -ooptions
> truncate -s 100M /mnt/a
> for i in $(seq 0 5 100); do xfs_io -c "w ${i}M 1M" /mnt/a; done
> xfstests-dev/src/seek_copy_test /mnt/a /mnt/b
Thanks for providing the reproducer and detailed analysis...
> This creates a big sparse file, makes 21 1MiB extents at 5MiB intervals and
> then tries to copy them one at a time, using SEEK_DATA/SEEK_HOLE to enumerate
> each extent.
>
> I can see the error in the protocol being returned from the server:
>
> 16 2.353013398 192.168.6.2 → 192.168.6.1 SMB2 206 Ioctl Request FSCTL_QUERY_ALLOCATED_RANGES File: a
> 17 2.353220936 192.168.6.1 → 192.168.6.2 SMB2 143 Ioctl Response, Error: STATUS_BUFFER_TOO_SMALL
>
> Stracing Samba shows:
>
> lseek(30, 94371840, SEEK_HOLE) = 95420416
> lseek(30, 95420416, SEEK_DATA) = 99614720
> lseek(30, 99614720, SEEK_HOLE) = 100663296
> lseek(30, 100663296, SEEK_DATA) = 104857600
> lseek(30, 104857600, SEEK_HOLE) = 105906176
> sendmsg(35, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="\0\0\0I", iov_len=4}, {iov_base=NULL, iov_len=0}, {iov_base="\376SMB@\0\1\0#\0\0\300\v\0\n\0\1\0\0\0\0\0\0\0\265\2\0\0\0\0\0\0"..., iov_len=64}, {iov_base="\t\0\0\0\0\0\0\0", iov_len=8}, {iov_base="\0", iov_len=1}], msg_iovlen=5, msg_controllen=0, msg_flags=0}, MSG_DONTWAIT|MSG_NOSIGNAL) = 77
>
> You can see the error code in the sendmsg() as "...#\0\0\300...".
>
> Turning debugging on on Samba shows:
>
> [2024/05/21 23:56:58.727547, 2] ../../source3/smbd/smb2_ioctl_filesys.c:707(fsctl_qar)
> QAR output len 336 exceeds max 16
> [2024/05/21 23:56:58.727652, 3] ../../source3/smbd/smb2_server.c:4031(smbd_smb2_request_error_ex)
> smbd_smb2_request_error_ex: smbd_smb2_request_error_ex: idx[1] status[NT_STATUS_BUFFER_TOO_SMALL] || at ../../source3/smbd/smb2_ioctl.c:353
>
> The "exceeds max 16" indicates the "Max Ioctl Out Size" setting passed in the
> Ioctl Request frame (which can be seen as 16 in the full packet trace). 336
> is 21 * 16.
>
> This stems from the smb2_llseek() function in the cifs filesystem in the Linux
> kernel calling:
>
> rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid,
> FSCTL_QUERY_ALLOCATED_RANGES,
> (char *)&in_data, sizeof(in_data),
> sizeof(struct file_allocated_range_buffer),
> (char **)&out_data, &out_data_len);
>
> where the max_out_data_len parameter is sizeof() you can see, allowing for
> just a single element to be returned. The file_allocated_range_buffer struct
> is just:
>
> struct file_allocated_range_buffer {
> __le64 file_offset;
> __le64 length;
> } __packed;
>
> which is 16 bytes - hence the maximum output data seen by Samba.
>
>
> Now, cifs only wants the next extent. It fills in the input data with the
> base file position and the EOF:
>
> in_data.file_offset = cpu_to_le64(offset);
> in_data.length = cpu_to_le64(i_size_read(inode));
>
> and asks the server to retrieve the first allocated extent within this range.
>
> In Samba, however, fsctl_qar() does not pass in_max_output to
> fsctl_qar_seek_fill() and therefore doesn't limit the amount returned. The
> fill loop seems only to be limited by the maximum file offset and not the max
> buffer size.
>
> The:
>
> if (out_output->length > in_max_output) {
> DEBUG(2, ("QAR output len %lu exceeds max %lu\n",
> (unsigned long)out_output->length,
> (unsigned long)in_max_output));
> data_blob_free(out_output);
> return NT_STATUS_BUFFER_TOO_SMALL;
> }
>
> check at the end of fsctl_qar() generates the complaint.
>
> I think that fsctl_qar() should probably just discard the excess records.
>
> Looking at:
>
> https://learn.microsoft.com/en-us/windows/win32/api/winioctl/ni-winioctl-fsctl_query_allocated_ranges
>
> it's not obvious what should be done if the available records won't fit.
MS-FSCC from https://winprotocoldoc.blob.core.windows.net/productionwindowsarchives/MS-FSCC/%5bMS-FSCC%5d.pdf
is a more protocol-specific reference here, but it's still unclear
regarding partial / truncated responses.
We do have an existing test for the STATUS_BUFFER_TOO_SMALL response in
test_ioctl_sparse_qar_malformed(), which would have been run against a
Windows server to confirm matching protocol behaviour. But it doesn't
cover partial responses.
I think the best way to proceed here would be to capture traffic for the
same workload against a Windows SMB server. This could be don't by using
your cifs.ko workload or extending test_ioctl_sparse_qar_malformed().
Cheers, David
