[PATCH] Fix futimesat with NULL second argument

[PATCH] Fix futimesat with NULL second argument

[Jakub Jelinek]

Hi!

According to http://docs.sun.com/app/docs/doc/816-5167/6mbb2jam2?a=view
futimesat (fd, NULL, tvp)
is supposed to set times on the file referenced by fd.

touch from coreutils uses this, so with current CVS glibc crashes
(see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=173581).

It is unclear what happens with
futimesat (AT_FDCWD, NULL, tvp)
though, my patch will just fail, other variant would be set times on
current working directory.

Also, in generic futimesat.c, the documentation above says that
for absolute paths fd argument is ignored.  Therefore we shouldn't
IMHO verify that FD is non-negative or AT_FDCWD in that case.

2005-11-18  Jakub Jelinek  <jakub@redhat.com>

	* sysdeps/unix/sysv/linux/futimesat.c (futimesat): If FILE is NULL,
	set access and modification times of the file referenced by FD.
	* sysdeps/generic/futimesat.c (futimesat): Don't return EINVAL if
	FILE is NULL.  Don't check FD if FILE is absolute path.

--- libc/sysdeps/unix/sysv/linux/futimesat.c.jj	2005-11-18 20:48:37.000000000 +0100
+++ libc/sysdeps/unix/sysv/linux/futimesat.c	2005-11-18 21:59:12.000000000 +0100
@@ -37,7 +37,22 @@ futimesat (fd, file, tvp)
 {
   char *buf = NULL;
 
-  if (fd != AT_FDCWD && file[0] != '/')
+  if (file == NULL)
+    {
+      static const char procfd[] = "/proc/self/fd/%d";
+      /* Buffer for the path name we are going to use.  It consists of
+	 - the string /proc/self/fd/
+	 - the file descriptor number.
+	 The final NUL is included in the sizeof.   A bit of overhead
+	 due to the format elements compensates for possible negative
+	 numbers.  */
+      size_t buflen = sizeof (procfd) + sizeof (int) * 3;
+      buf = alloca (buflen);
+
+      __snprintf (buf, buflen, procfd, fd);
+      file = buf;
+    }
+  else if (fd != AT_FDCWD && file[0] != '/')
     {
       size_t filelen = strlen (file);
       static const char procfd[] = "/proc/self/fd/%d/%s";
--- libc/sysdeps/generic/futimesat.c.jj	2005-11-11 20:02:44.000000000 +0100
+++ libc/sysdeps/generic/futimesat.c	2005-11-18 21:54:36.000000000 +0100
@@ -30,18 +30,14 @@ futimesat (fd, file, tvp)
      const char *file;
      const struct timeval tvp[2];
 {
-  if (fd < 0 && fd != AT_FDCWD)
+  if (fd < 0
+      && (file == NULL
+          || (fd != AT_FDCWD && file[0] != '/')))
     {
       __set_errno (EBADF);
       return -1;
     }
 
-  if (file == NULL)
-    {
-      __set_errno (EINVAL);
-      return -1;
-    }
-
   __set_errno (ENOSYS);
   return -1;
 }

	Jakub

Re: [PATCH] Fix futimesat with NULL second argument

[Jakub Jelinek]

On Fri, Nov 18, 2005 at 10:09:58PM +0100, Jakub Jelinek wrote:
> It is unclear what happens with
> futimesat (AT_FDCWD, NULL, tvp)
> though, my patch will just fail, other variant would be set times on
> current working directory.

Tried on Solaris 10, for:
  futimesat (AT_FDCWD, NULL, NULL);
truss says:
futimesat(-3041965, "", 0x00000000)             Err#14 EFAULT

/usr/include/sys/fcntl.h:#define        AT_FDCWD 0xffd19553

	Jakub

Re: [PATCH] Fix futimesat with NULL second argument

[Ulrich Drepper]

Applied.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

Re: [PATCH] Fix futimesat with NULL second argument

[Andreas Schwab]

Jakub Jelinek <jakub@redhat.com> writes:

> According to http://docs.sun.com/app/docs/doc/816-5167/6mbb2jam2?a=view
> futimesat (fd, NULL, tvp)
> is supposed to set times on the file referenced by fd.
>
> touch from coreutils uses this, so with current CVS glibc crashes
> (see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=173581).

It still crashes.

Andreas.

2005-11-24  Andreas Schwab  <schwab@suse.de>

	* time/sys/time.h: Remove nonnull attribute from futimesat.

--- time/sys/time.h.~1.36.~	2005-11-13 22:35:20.000000000 +0100
+++ time/sys/time.h	2005-11-24 15:29:18.000000000 +0100
@@ -153,7 +153,7 @@ extern int futimes (int __fd, __const st
    modification time of FILE to TVP[1].  If TVP is a null pointer, use
    the current time instead.  Returns 0 on success, -1 on errors.  */
 extern int futimesat (int __fd, __const char *__file,
-		      __const struct timeval __tvp[2]) __THROW __nonnull ((2));
+		      __const struct timeval __tvp[2]) __THROW;
 #endif
 
 

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Fix futimesat with NULL second argument

[Ulrich Drepper]

Applied.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖