[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