Re: [PATCH v2] ksmbd: decode and honor DOS attribute validity flags

[Ian Bridges <icb@fastmail.org>]

Hi,

There is an existing bug that matters when enabling these checks.

ksmbd stores DOS attributes and the client's creation time in
user.DOSATTRIB. The xattr also contains flags saying which fields
are valid. Currently, ndr_decode_dos_attr() reads the values but
discards those flags.

When the sparse or compression code updates an existing xattr,
it writes the creation time back but sets only
XATTR_DOSINFO_ATTRIB. The time is still on disk, but its validity
flag is missing. This has not affected the reported creation time
because ksmbd currently reads it without checking the flag.

Your patch preserves the flags on future updates. However, the new
read-side checks also apply to xattrs that ksmbd has already written.
Those files will stop reporting their stored creation time.

For example, with DOS attribute storage enabled:

1. On the unpatched server, create a file over SMB and set its
   creation time to a date different from its filesystem birth time.
2. Make the file sparse. Its xattr retains the requested time, but
   now has only XATTR_DOSINFO_ATTRIB set.
3. Upgrade to the patched server and reopen the file.

The server now reports the filesystem birth time instead of the
client's time. Directory listings change too. On a filesystem without
birth time support, the fallback is ctime.

The affected writers are fsctl_set_sparse() and
__ksmbd_vfs_set_compression(). Their existing-xattr path differs from
the no-xattr case in the commit message: it writes flags 0x01, not
0x51.

I am not sure whether handling these old xattrs belongs in this patch
or in a separate one. Namjae, what do you think?

Thanks,
Ian