Re: Bug in Samba's implementation of FSCTL_QUERY_ALLOCATED_RANGES?

[David Disseldorp <ddiss@samba.org>]

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