All of lore.kernel.org
 help / color / mirror / Atom feed
From: "koraynilay" <koray.fra@gmail.com>
To: "Namjae Jeon" <linkinjeon@kernel.org>,
	"koraynilay" <koray.fra@gmail.com>
Cc: "Sergey Senozhatsky" <senozhatsky@chromium.org>,
	"Tom Talpey" <tom@talpey.com>,
	"ChenXiaoSong" <chenxiaosong@chenxiaosong.com>,
	"Hyunchul Lee" <hyc.lee@gmail.com>, <linux-cifs@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ksmbd: decode and honor DOS attribute validity flags
Date: Sat, 14 Nov 2026 01:58:45 +0100	[thread overview]
Message-ID: <DMUIYLA1N1WI.58082D8BPB0T@gmail.com> (raw)
In-Reply-To: <CAKYAXd-X_u1CapKjpLPJF+ecUo5LN15Zj9T8bLguhyYWY7YrWQ@mail.gmail.com>

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

On Sat Nov 14, 2026 at 1:26 AM CET, Namjae Jeon wrote:
> On Sat, Nov 14, 2026 at 7:16 AM koraynilay <koray.fra@gmail.com> wrote:
>>
>> 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>
> Can you check Sashiko's review comments ?
>
> https://sashiko.dev/#/patchset/20261113221610.392113-1-koray.fra%40gmail.com

Yes thanks, I am able to reproduce what sashiko said, I'll send a v2.

Best,
koraynilay

>
> Thanks.



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

      reply	other threads:[~2026-11-14  0:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-13 22:16 [PATCH] ksmbd: decode and honor DOS attribute validity flags koraynilay
2026-11-14  0:26 ` Namjae Jeon
2026-11-14  0:58   ` koraynilay [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DMUIYLA1N1WI.58082D8BPB0T@gmail.com \
    --to=koray.fra@gmail.com \
    --cc=chenxiaosong@chenxiaosong.com \
    --cc=hyc.lee@gmail.com \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=senozhatsky@chromium.org \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.