* [PATCH v2] ksmbd: decode and honor DOS attribute validity flags
@ 2026-11-14 1:27 koraynilay
0 siblings, 0 replies; only message in thread
From: koraynilay @ 2026-11-14 1:27 UTC (permalink / raw)
To: Namjae Jeon
Cc: Sergey Senozhatsky, Tom Talpey, ChenXiaoSong, Hyunchul Lee,
linux-cifs, linux-kernel, 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
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2026-11-14 1:27 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-14 1:27 [PATCH v2] ksmbd: decode and honor DOS attribute validity flags koraynilay
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®