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

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

[koraynilay]

Currently, when setting file attributes via FSCTL_SET_SPARSE,
FSCTL_SET_COMPRESSION or FileBasicInformation, ksmbd hardcodes
flags = 0x51 (XATTR_DOSINFO_ATTRIB | XATTR_DOSINFO_CREATE_TIME |
XATTR_DOSINFO_ITIME). This injects unrequested creation time and itime
into user.DOSATTRIB on disk, even though the client only requested an
attribute modification.

Furthermore, ndr_decode_dos_attr() discards the validity flags on read
by reading them into NULL. When reading a file with an attribute-only
xattr (flags = 0x01, where create_time on disk is 0), ksmbd treats the 0
as a valid timestamp and reports NTTIME(0) instead of falling back to
the underlying filesystem's btime. Modifying any attribute via
set_file_basic_info() then clobbers the xattr back to 0x51.

Fix this by:
1. decoding da->flags in ndr_decode_dos_attr() and only updating
   create_time and itime if their validity flags are set (in
   smb2_update_xattrs() and ksmbd_vfs_fill_dentry_attrs());
2. reading existing attributes and only setting XATTR_DOSINFO_ATTRIB in
   fsctl_set_sparse(), __ksmbd_vfs_set_compression(), and
   set_file_basic_info() so unrequested timestamps are not injected.

Reproducer, with /tmp as the local filesystem folder exported as [test]
in ksmbd.conf and /mnt as the smb folder mounted with mount -t cifs:

  mount -t cifs //127.0.0.1/test /mnt -o username=user,password=pass

test with sparse (for compression, use chattr +c instead of fallocate):

  echo data > /tmp/sparse.txt
  chmod 777 /tmp/sparse.txt

  stat -c 'Local btime (%n): %w' /tmp/sparse.txt
  getfattr -n user.DOSATTRIB -e hex /tmp/sparse.txt

  # punching hole via CIFS triggers FSCTL_SET_SPARSE
  fallocate -p -o 0 -l 4 /mnt/sparse.txt
  getfattr -n user.DOSATTRIB -e hex /tmp/sparse.txt

  stat -c 'CIFS btime (%n):  %w' /mnt/sparse.txt
  smbclient //127.0.0.1/test -U user%pass -c 'allinfo sparse.txt'

  # evict cache so btime is read fresh again
  umount /mnt
  mount -t cifs //127.0.0.1/test /mnt -o username=user,password=pass
  stat -c 'CIFS btime (%n):  %w' /mnt/sparse.txt

which prints:

  Local btime (/tmp/sparse.txt): 2026-11-13 21:13:27.167926440 +0000
  /tmp/sparse.txt: user.DOSATTRIB: No such attribute

  # file: tmp/sparse.txt
  user.DOSATTRIB=0x000004000400000051000000200200000000000000000000209542d3b373dd01
     0x51 = 0x1 | 0x10 | 0x40 -----^^         |      unrequested --^^^^^^^^^^^^^^^^
   (ATTRIB | CREATE_TIME | ITIME)             |   creation time = btime
                                     SPARSE --^
  CIFS btime (/mnt/sparse.txt):  2026-11-13 21:13:27.167926400 +0000
  [...]
  create_time:    Fri Nov 13 21:13:27 2026 UTC
  [...]
  CIFS btime (/mnt/sparse.txt):  2026-11-13 21:13:27.167926400 +0000

test with setmode:

  echo data > /tmp/attrib.txt
  chmod 777 /tmp/attrib.txt

  stat -c 'Local btime (%n): %w' /tmp/attrib.txt
  getfattr -n user.DOSATTRIB -e hex /tmp/attrib.txt

  # modify attribute via CIFS
  smbclient //127.0.0.1/test -U user%pass -c 'setmode attrib.txt +r'
  getfattr -n user.DOSATTRIB -e hex /tmp/attrib.txt

  stat -c 'CIFS btime (%n):  %w' /mnt/attrib.txt
  smbclient //127.0.0.1/test -U user%pass -c 'allinfo attrib.txt'

  # evict cache so btime is read fresh again
  umount /mnt
  mount -t cifs //127.0.0.1/test /mnt -o username=user,password=pass
  stat -c 'CIFS btime (%n):  %w' /mnt/attrib.txt

which prints:

  Local btime (/tmp/attrib.txt): 2026-11-13 21:13:27.195647069 +0000
  /tmp/attrib.txt: user.DOSATTRIB: No such attribute

  # file: tmp/attrib.txt
  user.DOSATTRIB=0x000004000400000051000000210000000000000000000000f6cf46d3b373dd01
     0x51 = 0x1 | 0x10 | 0x40 -----^^       |        unrequested --^^^^^^^^^^^^^^^^
   (ATTRIB | CREATE_TIME | ITIME)           |     creation time = btime
                                 READONLY --^
  CIFS btime (/mnt/attrib.txt):  2026-11-13 21:13:27.195647000 +0000
  [...]
  create_time:    Fri Nov 13 21:13:27 2026 UTC
  [...]
  CIFS btime (/mnt/attrib.txt):  2026-11-13 21:13:27.195647000 +0000

After this patch, the outputs should be:

sparse:

  Local btime (/tmp/sparse.txt): 2026-11-13 21:20:25.856425751 +0000
  /tmp/sparse.txt: user.DOSATTRIB: No such attribute
  # file: tmp/sparse.txt
  user.DOSATTRIB=0x0000040004000000010000002002000000000000000000000000000000000000
          0x01 = 0x1 (ATTRIB) -----^^         | no creation time --^^^^^^^^^^^^^^^^
                                     SPARSE --^
  CIFS btime (/mnt/sparse.txt):  2026-11-13 21:20:25.856425700 +0000
  create_time:    Fri Nov 13 21:20:26 2026 UTC
  CIFS btime (/mnt/sparse.txt):  2026-11-13 21:20:25.856425700 +0000

setmode:

  Local btime (/tmp/attrib.txt): 2026-11-13 21:20:25.976415893 +0000
  /tmp/attrib.txt: user.DOSATTRIB: No such attribute
  # file: tmp/attrib.txt
  user.DOSATTRIB=0x0000040004000000010000002100000000000000000000000000000000000000
          0x01 = 0x1 (ATTRIB) -----^^       |   no creation time --^^^^^^^^^^^^^^^^
                                 READONLY --^
  CIFS btime (/mnt/attrib.txt):  2026-11-13 21:20:25.976415800 +0000
  create_time:    Fri Nov 13 21:20:26 2026 UTC
  CIFS btime (/mnt/attrib.txt):  2026-11-13 21:20:25.976415800 +0000

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") # ndr decode/basic info
Fixes: f44158485826 ("cifsd: add file operations") # ksmbd_vfs_fill_dentry_attrs
Fixes: 159e727f763d ("ksmbd: persist FSCTL_SET_SPARSE state") # sparse
Fixes: 0fb327626ad9 ("ksmbd: support file compression attributes") # compression
Cc: stable@vger.kernel.org
Assisted-by: Gemini:3.8-flash-high antigravity-cli-1.5.3
Signed-off-by: koraynilay <koray.fra@gmail.com>
---
v2 changes:
- Check XATTR_DOSINFO_ATTRIB on read in smb2_update_xattrs() and
  ksmbd_vfs_fill_dentry_attrs() to prevent overwriting in-memory attributes
  with 0 when an xattr lacks DOS attributes (found by sashiko[2]);
- unconditionally preserve current attributes and set XATTR_DOSINFO_ATTRIB
  in set_file_basic_info() so setting CreationTime alone does not write
  zeroed attributes to disk (found by sashiko[2]);
- decode version 3 fields (ea_size, size, alloc_size, change_time) in
  ndr_decode_dos_attr() instead of reading into NULL so preserving flags on
  version 3 xattrs retains their values (found by sashiko[2]).

This should be (hopefully) the last of the fixes needed to implement ptime[1].
Thanks for testing all of them and merging them <3

Best,
koraynilay

[1]: https://lore.kernel.org/all/20261101015551.786044-1-koray.fra@gmail.com/T/#t
[2]: https://sashiko.dev/#/patchset/20261113221610.392113-1-koray.fra%40gmail.com


 fs/smb/server/ndr.c     | 10 +++++-----
 fs/smb/server/smb2pdu.c | 34 ++++++++++++++++++++++------------
 fs/smb/server/vfs.c     | 11 +++++------
 3 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/fs/smb/server/ndr.c b/fs/smb/server/ndr.c
index 7e546c22e284..1376d5a59fc8 100644
--- a/fs/smb/server/ndr.c
+++ b/fs/smb/server/ndr.c
@@ -256,7 +256,7 @@ int ndr_decode_dos_attr(struct ndr *n, struct xattr_dos_attrib *da)
 		return -EINVAL;
 	}
 
-	ret = ndr_read_int32(n, NULL);
+	ret = ndr_read_int32(n, &da->flags);
 	if (ret)
 		return ret;
 
@@ -271,15 +271,15 @@ int ndr_decode_dos_attr(struct ndr *n, struct xattr_dos_attrib *da)
 
 		ret = ndr_read_int64(n, &da->create_time);
 	} else {
-		ret = ndr_read_int32(n, NULL);
+		ret = ndr_read_int32(n, &da->ea_size);
 		if (ret)
 			return ret;
 
-		ret = ndr_read_int64(n, NULL);
+		ret = ndr_read_int64(n, &da->size);
 		if (ret)
 			return ret;
 
-		ret = ndr_read_int64(n, NULL);
+		ret = ndr_read_int64(n, &da->alloc_size);
 		if (ret)
 			return ret;
 
@@ -287,7 +287,7 @@ int ndr_decode_dos_attr(struct ndr *n, struct xattr_dos_attrib *da)
 		if (ret)
 			return ret;
 
-		ret = ndr_read_int64(n, NULL);
+		ret = ndr_read_int64(n, &da->change_time);
 	}
 
 	return ret;
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index dac7cd05e023..748061a0899d 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -3866,9 +3866,12 @@ static void smb2_update_xattrs(struct ksmbd_tree_connect *tcon,
 					    path->dentry, &da);
 	if (rc > 0) {
 		if (store_dos_attrs) {
-			fp->f_ci->m_fattr = cpu_to_le32(da.attr);
-			fp->create_time = da.create_time;
-			fp->itime = da.itime;
+			if (da.flags & XATTR_DOSINFO_ATTRIB)
+				fp->f_ci->m_fattr = cpu_to_le32(da.attr);
+			if (da.flags & XATTR_DOSINFO_CREATE_TIME)
+				fp->create_time = da.create_time;
+			if (da.flags & XATTR_DOSINFO_ITIME)
+				fp->itime = da.itime;
 		} else {
 			fp->f_ci->m_fattr &=
 				~(FILE_ATTRIBUTE_COMPRESSED_LE |
@@ -9039,12 +9042,22 @@ static int set_file_basic_info(struct ksmbd_file *fp,
 	    (crtime > 0 || file_info->Attributes)) {
 		struct xattr_dos_attrib da = {0};
 
-		da.version = 4;
-		da.itime = fp->itime;
-		da.create_time = fp->create_time;
+		rc = ksmbd_vfs_get_dos_attrib_xattr(idmap, fp->filp->f_path.dentry, &da);
+		if (rc <= 0) {
+			da.version = 4;
+			da.flags = 0;
+		}
+
+		/*
+		 * itime gets preserved when reading the xattr if already present
+		 * otherwise don't write it as it wasn't asked for by the client
+		 */
+		if (crtime > 0) {
+			da.create_time = fp->create_time;
+			da.flags |= XATTR_DOSINFO_CREATE_TIME;
+		}
 		da.attr = le32_to_cpu(fp->f_ci->m_fattr);
-		da.flags = XATTR_DOSINFO_ATTRIB | XATTR_DOSINFO_CREATE_TIME |
-			XATTR_DOSINFO_ITIME;
+		da.flags |= XATTR_DOSINFO_ATTRIB;
 
 		rc = ksmbd_vfs_set_dos_attrib_xattr(idmap, &filp->f_path, &da,
 				true);
@@ -11877,10 +11890,7 @@ static inline int fsctl_set_sparse(struct ksmbd_work *work, u64 id,
 						     fp->filp->f_path.dentry, &da);
 		if (ret <= 0) {
 			da.version = 4;
-			da.itime = fp->itime;
-			da.create_time = fp->create_time;
-			da.flags = XATTR_DOSINFO_CREATE_TIME |
-				XATTR_DOSINFO_ITIME;
+			da.flags = 0;
 		}
 
 		da.attr = le32_to_cpu(fp->f_ci->m_fattr);
diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index 0c95d4668370..40d86abaad7a 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -1935,8 +1935,10 @@ int ksmbd_vfs_fill_dentry_attrs(struct ksmbd_work *work,
 
 		rc = ksmbd_vfs_get_dos_attrib_xattr(idmap, dentry, &da);
 		if (rc > 0) {
-			ksmbd_kstat->file_attributes = cpu_to_le32(da.attr);
-			ksmbd_kstat->create_time = da.create_time;
+			if (da.flags & XATTR_DOSINFO_ATTRIB)
+				ksmbd_kstat->file_attributes = cpu_to_le32(da.attr);
+			if (da.flags & XATTR_DOSINFO_CREATE_TIME)
+				ksmbd_kstat->create_time = da.create_time;
 		} else {
 			ksmbd_debug(VFS, "fail to load dos attribute.\n");
 		}
@@ -2475,10 +2477,7 @@ static int __ksmbd_vfs_set_compression(struct ksmbd_work *work,
 		rc = ksmbd_vfs_get_dos_attrib_xattr(idmap, dentry, &da);
 		if (rc <= 0) {
 			da.version = 4;
-			da.itime = fp->itime;
-			da.create_time = fp->create_time;
-			da.flags = XATTR_DOSINFO_CREATE_TIME |
-				XATTR_DOSINFO_ITIME;
+			da.flags = 0;
 		}
 
 		da.attr = le32_to_cpu(fp->f_ci->m_fattr);

base-commit: 2cc92355992022c1c6edad59499c49814e17cac0
prerequisite-patch-id: 049ae0eba275b5be35a238833107e0b251a55aa2
-- 
2.56.0

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

[Ian Bridges]

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

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

[Namjae Jeon]

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.

Thanks!
>
> Thanks,
> Ian

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

[koraynilay]

[-- 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 --]