Recent Linux kernels have different clocks used for the time() system
call and stat() st_mtime of a file.  time() is set to use a lower
granularity clock.  This means that right around the change of
seconds, there can be an "X" millisecond gap where time() seconds is
behind st_mtime seconds.

The change has led to a bug in Mutt, where recording the stamp of an
attachment using time() sometimes has a "second" value earlier than
the stat() st_mtime of the file just modified (again, due to the
granularity difference of the clock used).

This caused a sporadic false warning before sending an email, that the
attachment has changed on disk since it was last checked.  For more
details, see:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1144613

The attachment->stamp is only used to compare against the stat
st_mtime of the file just before sending.  So it actually makes more
sense to just record the previous st_mtime of the file.  This occurs
during message composition, so an extra stat on a file that was just
read is not likely to cause a meaningful slow down.

Change mutt_stamp_attachment() to use st_mtime for the stamp.  Since
the function previously had no error case, use "time(NULL) + 1" as a
fallback behavior in case stat fails or (for some strange reason) it's
called for a receive-mode attachment.

Thanks to Vincent Lefèvre for reporting the issue and working on
finding out the source of the bug.  Thanks also to the other
contributors in the thread who helped with reproducing and diagnosing
the problem: Ian Collier, Reed Underwood, and Steffen Nurpmeso.
---

I've tried to make the commit message clearer.  Please let me know if
the explanation makes sense and my terminology is correct.  If not,
please do make suggestions.  Thank you.

 sendlib.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/sendlib.c b/sendlib.c
index 150bc72d..0e4a8146 100644
--- a/sendlib.c
+++ b/sendlib.c
@@ -1284,7 +1284,21 @@ static void mutt_set_encoding(BODY *b, CONTENT *info)
 
 void mutt_stamp_attachment(BODY *a)
 {
-  a->stamp = time(NULL);
+  struct stat sb;
+
+  if (a->filename && stat(a->filename, &sb) == 0)
+    a->stamp = sb.st_mtime;
+  else
+  {
+    /* Recent Linux kernels have an issue where time()'s clock has a
+     * lower granularity than the stat st_mtime clock.  This can
+     * result in time()'s second value being earlier than the st_mtime
+     * seconds of a file just modified.  See:
+     * https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1144613.  As
+     * an error fallback case, add 1 to work around the behavior.
+     */
+    a->stamp = time(NULL) + 1;
+  }
 }
 
 /* Get a body's character set */
-- 
2.55.0

Reply via email to