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

[Namjae Jeon]

On Sun, Nov 15, 2026 at 12:07 PM Ian Bridges <icb@xxxxxxxxxxxx> wrote:
>
> On Sun Nov 15, 2026 at 2:34 AM CET, koraynilay wrote:
> > Hello, so I should include the legacy handling in this same patch and
> > separate the bugfix for sparse/compression in another one?
>
> One option might be to keep it all in this patch, with a few changes
> so that it stays compatible with the xattrs older ksmbd has already
> stored.
>
> When sparse or compression updates an existing xattr, it keeps the
> creation time, but ndr_decode_dos_attr() discards the flags. The
> writer then stores the time with only XATTR_DOSINFO_ATTRIB set.
> Checking CREATE_TIME on read would hide creation times that ksmbd
> reports today.
>
> Your changes to decode the flags and keep the existing fields are
> useful. On top of those, something like this might work.
>
> - Read the stored attributes unconditionally, including a value of 0.
>   Samba also reads the attributes without checking ATTRIB.
>
> - Use a non-zero stored creation time even when CREATE_TIME is clear,
>   and fall back to the filesystem timestamp when it is 0. A comment
>   could explain that older ksmbd writers lost the flag.
>
> - When rewriting an xattr, preserve the other fields and flags, set
>   ATTRIB, and restore CREATE_TIME if the time being written is non-zero.
>
> > 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.
>
> With the reader change in the same patch, a creation time of 0 would
> select the filesystem timestamp. That handles attribute-only xattrs,
> but it would not make the writer change safe to apply on its own.
>
> > 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).
>
> Keeping the current no-xattr initialization would avoid that ordering
> problem altogether. We could fix preservation of existing xattrs here
> and consider changing newly written xattrs separately.
>
> It might also be best to leave changes to ITIME validity policy out of
> this fix. I don't think ksmbd uses the itime for anything it reports, so
> those changes would not affect this bug.
>
> Namjae, would accepting a non-zero creation time despite a missing
> flag be a reasonable compatibility rule for existing ksmbd xattrs?
Yes, that is reasonable for compatibility.
Thanks.