Invalid sizes on XFS backed share

[mmakassikis]

Hello,

When sharing a directory that is on an XFS filesystem, and a Linux client mounts it (using cifs.ko client), some applications such as "du" report invalid sizes.

Example:

$ ls -lth 10M.bin 
-rwxr-xr-x 1 user user 10M Feb 12 19:33 10M.bin
$ du -csh 10M.bin
0       10M.bin
0       total

10M.bin is a 10MB file created using dd if=/dev/urandom of=10M.bin bs=1M count=10, so it is effectively 10MB rather than a sparse file.

I did some wireshark captures, and it appears to be caused by the AllocationSize field set to 0 in GetInfo responses. In smb2pdu.c, this field is filled using the inode->i_blocks:

sinfo->AllocationSize = cpu_to_le64(inode->i_blocks << 9); 

This works for ext4, but XFS stopped filling ->i_blocks forever ago [0], so this is always 0. Any ideas on how to fix this ?

[0] https://www.mail-archive.com/git-commits-head@vger.kernel.org/msg39878.html

[namjaejeon]

Can you check if problem do not happen with samba & xfs ?

[mmakassikis]

No issues on samba (using version 4.19.4 from debian repositories), and the same client (kernel 6.6.13 cifs.ko).

When doing a stat() call on a file on a XFS partition, st_blocks is also non-zero. If I read the code correctly, when a userspace application calls stat(), the call is handled by vfs_getattr_nosec https://github.com/torvalds/linux/blob/master/fs/stat.c#L134

Since XFS defines a ->getattr op, blocks is filled in the kstat struct : https://github.com/torvalds/linux/blob/master/fs/xfs/xfs_iops.c#L578

Perhaps generic_fillattr() calls in ksmbd code should be replaced with vfs_getattr(). Do you think there is a downside to this ?

Edit: Actually, there is a ksmbd_vfs_getattr() helper, but it's only called in smb2_open(). Not sure if it's intentional to use generic_fillattr() elsewhere.

[namjaejeon]

We need to change all generic_fillattr with vfs_getattr(). Hyunchul tried to change it before. He said it is hard to change it in ksmbd_vfs_fill_dentry_attrs(). So he changed only smb2_open. Can you check if it is true ? And If it is really hard to change it in ksmbd_vfs_fill_dentry_attrs(), we can replace all except ksmbd_vfs_fill_dentry_attrs() for now.

[mmakassikis]

I sent two patches on the linux-cifs mailing list. If they're OK, I'll also do the changes to replace generic_fillattr with vfs_getattr in smb1pdu.c.

[namjaejeon]

I will check them:) Thanks for your work!

[namjaejeon]

Applied them to both mainline and github ksmbd. Please send the patch for SMB1. Thanks for your work!

[namjaejeon]

Applied, Thanks for your work!