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

["koraynilay" <koray.fra@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3215 bytes --]

On Sun Nov 15, 2026 at 1:22 AM CET, Namjae Jeon wrote:
> On Sun, Nov 15, 2026 at 1:03 AM Ian Bridges <icb@fastmail.org> wrote:
>>
>> 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?
> Agreed. separate patch in the same series is fine, but the
> compatibility handling should land together with the read-side checks
> to avoid changing the creation time reported for existing files.

Hello, so I should include the legacy handling in this same patch and
separate the bugfix for sparse/compression in another one?

The problem I had found when trying this approach (I originally wanted
to send PATCH 1/2 to fix sparse/compression and 2/2 for the decode/read
fixes) is that with only the sparse/compression patch applied, ksmbd
clobbers the time on reading by reporting 0, since it doesn't check
the flags and reads the timestamp anyway.

For example:

file created on host directly
sparse through cifs/ksmbd
file's xattr's creation time is 0 (because sparse is now fixed)
allinfo reports NTTIME(0) because the reader doesn't check the flags

I also tried reversing them (so first fix decode and then
sparse/compression) but I couldn't find a good reproducer for the first
patch, as it's not really visible.

Please tell me if I wasn't clear, this patch/bug is quite confusing for
me since it touches a few bugs and they all depend on each other.
(And one fix introduces another bug fixed by the next fix).

Thanks,
koraynilay

> Thanks!
>>
>> Thanks,
>> Ian



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]