Project

General

Profile

Bug #13299

File mtime changes twice, when file is modified using SMB server

Added by Florian Manschwetus 20 days ago. Updated 6 days ago.

Status:
In Progress
Priority:
Normal
Assignee:
Category:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

When working on files located on smb share served by illumos smb server, editors monitoring for file changes detecting changes, where are no.
Editors like notepad++ or eclipse behave unexpected, also make utils are affected.
The mtime changes after some seconds again, even if the changing program has already terminated.

here a sample from cygwin, demonstrating the behavior:

$ echo test > z:\\testtime2.txt  && stat -c "after test program       : %y" z:\\testtime2.txt && sleep 20 &&  stat -c "after additional 20s     : %y" z:\\testtime2.txt
after test program       : 2020-11-09 16:10:08.482197400 +0100
after additional 20s     : 2020-11-09 16:10:08.484312500 +0100


Files

dtraceOutNP++.txt (37 KB) dtraceOutNP++.txt dtrace output showing second close with explicit mtime change Florian Manschwetus, 2020-11-11 03:38 PM
dtraceOutTestProg.txt (26 KB) dtraceOutTestProg.txt dtrace output showing the problem, driven by simpler program Florian Manschwetus, 2020-11-11 03:43 PM
#1

Updated by Florian Manschwetus 19 days ago

I managed to get some dtrace of smb_vop_ functions and I see some ops among them a setattr some time after the test program has ended.
I'll try to get into details of this ops.

#2

Updated by Florian Manschwetus 19 days ago

It looks a bit like the close operations happen twice

#3

Updated by Florian Manschwetus 18 days ago

I managed to put together some dtrace showing the unexpected explicit mtime change.
Also I used some test program, not doing that much after completing file op.

In summary, the programs alter and close the file, then they record the mtime.
Some seconds later a second smb close op happens, explicitly setting the mtime.

#4

Updated by Florian Manschwetus 18 days ago

I did some further dtrace research and was able to track it down to a specific code section, forcing mtime change on close in error here, I think.

https://github.com/illumos/illumos-gate/blob/27f3c5a8194b190aeea38638adf95bd93e61cef1/usr/src/uts/common/fs/smbsrv/smb_ofile.c#L508

#5

Updated by Florian Manschwetus 18 days ago

The problem is easy to reproduce with np++, open txt file, change sth, safe, change sth again, do not safe, switch window, maybe wait some/several seconds, switch back.

If interested I can share my dtrace stuff.

#6

Updated by Florian Manschwetus 13 days ago

Ok I disabled the code as Gordon has suggested, seems to solve the problem for now.
Would be good if someone can check in detail, if this code is more than really needed to meet the specs or if the f_written bit was set when it should not.
Additionally I'd like to mention, that I haven't found the initializer for this yet, maybe it is as simple as this.

Here the diff of the provisional fix:

diff --git a/usr/src/uts/common/fs/smbsrv/smb_ofile.c b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
index d5388037c3..6f9e839e24 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_ofile.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
@@ -505,12 +505,14 @@ smb_ofile_close(smb_ofile_t *of, int32_t mtime_sec)
                 * during the close.  Windows expects this.
                 * [ MS-FSA 2.1.5.4 "Update Timestamps" ]
                 */
+#if 0
                if (of->f_written &&
                    (pa->sa_mask & SMB_AT_MTIME) == 0) {
                        pa->sa_mask |= SMB_AT_MTIME;
                        gethrestime(&now);
                        pa->sa_vattr.va_mtime = now;
                }
+#endif

                if (of->f_flags & SMB_OFLAGS_SET_DELETE_ON_CLOSE) {
                        /* We delete using the on-disk name. */

#7

Updated by Gordon Ross 6 days ago

  • Status changed from New to In Progress
  • Assignee set to Gordon Ross

It looks like that update (during close) was from a possibly misunderstood section from a very old version of the MS-FSA spec.
The mtime update cases described in recent versions of MS-FSA appear to be handled well enough without this bit of code.

#8

Updated by Electric Monk 6 days ago

  • Gerrit CR set to 1076

Also available in: Atom PDF