Bug 397354 - utimensat should ignore timespec tv_sec when tv_nsec is UTIME_NOW or UTIME_OMIT
Summary: utimensat should ignore timespec tv_sec when tv_nsec is UTIME_NOW or UTIME_OMIT
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (other bugs)
Version First Reported In: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-10 19:04 UTC by Mark Wielaard
Modified: 2018-09-03 09:59 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Wielaard 2018-08-10 19:04:54 UTC
$ valgrind -q tar --xattrs -xf ~/test-ltar--1.20-5.3.ch4.2.tar
==8939== Syscall param utimensat(tvp) points to uninitialised byte(s)
==8939==    at 0x535D0DF: utimensat (in /usr/lib64/libc-2.27.so)
==8939==    by 0x1455D3: fdutimensat (fdutimensat.c:50)
==8939==    by 0x11E40F: set_stat (extract.c:353)
==8939==    by 0x11E995: apply_nonancestor_delayed_set_stat (extract.c:923)
==8939==    by 0x1205D4: extract_archive (extract.c:1686)
==8939==    by 0x129EAF: read_and (list.c:230)
==8939==    by 0x1128B9: main (tar.c:2729)
==8939==  Address 0x1ffefff1c0 is on thread 1's stack
==8939==  in frame #2, created by set_stat (extract.c:340)
==8939== 
==8939== Syscall param utimensat(tvp) points to uninitialised byte(s)
==8939==    at 0x535D133: futimens (in /usr/lib64/libc-2.27.so)
==8939==    by 0x1455EF: fdutimensat (fdutimensat.c:48)
==8939==    by 0x11E40F: set_stat (extract.c:353)
==8939==    by 0x12015F: extract_file (extract.c:1238)
==8939==    by 0x120440: extract_archive (extract.c:1709)
==8939==    by 0x129EAF: read_and (list.c:230)
==8939==    by 0x1128B9: main (tar.c:2729)
==8939==  Address 0x1ffefff360 is on thread 1's stack
==8939==  in frame #2, created by set_stat (extract.c:340)
==8939== 

The code uses UTIME_OMIT to not set the time.

As the utimensat manpage says:

       If the tv_nsec field of one of the timespec structures has the  special
       value  UTIME_NOW,  then  the corresponding file timestamp is set to the
       current time.  If the tv_nsec field of one of the  timespec  structures
       has the special value UTIME_OMIT, then the corresponding file timestamp
       is left unchanged.  In both of these cases, the  value  of  the  corre‐
       sponding tv_sec field is ignored.

Proposed patch:

diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index 2336c29..0f8e07a 100644
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -5277,8 +5277,25 @@ PRE(sys_utimensat)
                  int, dfd, char *, filename, struct timespec *, utimes, int, flags);
    if (ARG2 != 0)
       PRE_MEM_RASCIIZ( "utimensat(filename)", ARG2 );
-   if (ARG3 != 0)
-      PRE_MEM_READ( "utimensat(tvp)", ARG3, 2 * sizeof(struct vki_timespec) );
+   if (ARG3 != 0) {
+      /* If timespec.tv_nsec has the special value UTIME_NOW or UTIME_OMIT
+         then the tv_sec field is ignored.  */
+      struct vki_timespec *times = (struct vki_timespec *)(Addr)ARG3;
+      PRE_MEM_READ( "utimensat(times[0].tv_nsec)",
+                    (Addr)&times[0].tv_nsec, sizeof(times[0].tv_nsec));
+      PRE_MEM_READ( "utimensat(times[1].tv_nsec)",
+                    (Addr)&times[1].tv_nsec, sizeof(times[1].tv_nsec));
+      if (ML_(safe_to_deref)(times, 2 * sizeof(struct vki_timespec))) {
+         if (times[0].tv_nsec != VKI_UTIME_NOW
+             && times[0].tv_nsec != VKI_UTIME_OMIT)
+            PRE_MEM_READ( "utimensat(times[0].tv_sec)",
+                          (Addr)&times[0].tv_sec, sizeof(times[0].tv_sec));
+         if (times[1].tv_nsec != VKI_UTIME_NOW
+             && times[1].tv_nsec != VKI_UTIME_OMIT)
+            PRE_MEM_READ( "utimensat(times[1].tv_sec)",
+                          (Addr)&times[1].tv_sec, sizeof(times[1].tv_sec));
+      }
+   }
 }
 
 PRE(sys_newfstatat)
diff --git a/include/vki/vki-linux.h b/include/vki/vki-linux.h
index 7072080..bf0c1aa 100644
--- a/include/vki/vki-linux.h
+++ b/include/vki/vki-linux.h
@@ -283,6 +283,10 @@ struct vki_timespec {
 	long		tv_nsec;	/* nanoseconds */
 };
 
+/* Special values for vki_timespec.tv_nsec when used with utimensat.  */
+#define VKI_UTIME_NOW  ((1l << 30) - 1l)
+#define VKI_UTIME_OMIT ((1l << 30) - 2l)
+
 struct vki_timeval {
 	vki_time_t	tv_sec;		/* seconds */
 	vki_suseconds_t	tv_usec;	/* microseconds */
Comment 1 Julian Seward 2018-09-03 09:46:55 UTC
Patch seems reasonable to me; commit?
Comment 2 Mark Wielaard 2018-09-03 09:59:49 UTC
commit 790f5f3018f807153339e441e7aea1414f4b5c8d
Author: Mark Wielaard <mark@klomp.org>
Date:   Mon Sep 3 11:54:38 2018 +0200

    Bug 397354 utimensat should ignore tv_sec if tv_nsec is UTIME_NOW/OMIT.
    
    When code uses utimensat with UTIME_NOW or UTIME_OMIT valgrind memcheck
    would generate a warning. But as the utimensat manpage says:
    
      If the tv_nsec field of one of the timespec structures has the  special
      value  UTIME_NOW,  then  the corresponding file timestamp is set to the
      current time.  If the tv_nsec field of one of the  timespec  structures
      has the special value UTIME_OMIT, then the corresponding file timestamp
      is left unchanged.  In both of these cases, the  value  of  the  corre‐
      sponding tv_sec field is ignored.
    
    So ignore the timespec tv_sec when tv_nsec is set to UTIME_NOW or
    UTIME_OMIT.