[RFC] Copy as much as you can during a 32-bit stat before returningEOVERFLOW?

["Carlos O'Donell" <carlos at redhat dot com>]

Community,

There are several filesystems which are going to be switching over
or already have switched over to unconditional 64-bit inodes and 
file sizes. This means that the current crop of 32-bit applications
may not be able to run reliably on these filesystems since stat 
may return EOVERFLOW for large values.

The correct solution is to switch to the 64-bit function variants
and avoid the overflow when converting from the kernel stat 
structure to the userspace version.

The correct solution is going to take time to implement and I am 
being asked if the glibc community can help lessen the burden of
the transition. I know that this has been coming now for over
a decade, but we should not ignore the request for assistance.

For example in the scenario where the application does not use 
any field which would overflow the user could ignore EOVERFLOW
from stat, mark the offending code as needing an audit and move
on.

The guarantee from glibc would be that on EOVERFLOW and -1 returned
that as much of the user stat structure will be filled as possible.
In the current glibc code we bail out at any point when we detect
an overflow during field conversion. The following patch shows how
the new behaviour would be implemented. I saw only two places 
where we quit early without copying other fields.

I liked the way the Hurd code in sysdeps/mach/hurd/xstatconv.c
does the overflow check last for all fields that might overflow.

The only argument against such a change is that glibc would like
to have a fail fast policy, which we don't today. The current
library policy is that failures are considered slow paths and
do not need to fail fast.

Comments?

Cheers,
Carlos.

ports/ChangeLog.mips

2012-02-26  Carlos O'Donell  <carlos@redhat.com>

	* sysdeps/unix/sysv/linux/mips/xstatconv.c (__xstat_conv):
	Convert all fields first. Set errno if any field had an overflow.

ChangeLog

2013-02-26  Carlos O'Donell  <carlos@redhat.com>

	* sysdeps/unix/sysv/linux/xstatconv.c (__xstat32_conv):
	Convert all fields first. Set errno if any field had an overflow.


diff --git a/ports/sysdeps/unix/sysv/linux/mips/xstatconv.c b/ports/sysdeps/unix/sysv/linux/mips/xstatconv.c
index 3fe65b1..891a98e 100644
--- a/ports/sysdeps/unix/sysv/linux/mips/xstatconv.c
+++ b/ports/sysdeps/unix/sysv/linux/mips/xstatconv.c
@@ -37,6 +37,7 @@ __xstat_conv (int vers, struct kernel_stat *kbuf, void *ubuf)
     case _STAT_VER_LINUX:
       {
 	struct stat *buf = ubuf;
+	int err = 0;
 
 	/* Convert to current kernel version of `struct stat'.  */
 	buf->st_dev = kbuf->st_dev;
@@ -44,10 +45,7 @@ __xstat_conv (int vers, struct kernel_stat *kbuf, void *ubuf)
 	buf->st_ino = kbuf->st_ino;
 	/* Check for overflow.  */
 	if (buf->st_ino != kbuf->st_ino)
-	  {
-	    __set_errno (EOVERFLOW);
-	    return -1;
-	  }
+	  err = EOVERFLOW;
 	buf->st_mode = kbuf->st_mode;
 	buf->st_nlink = kbuf->st_nlink;
 	buf->st_uid = kbuf->st_uid;
@@ -57,10 +55,7 @@ __xstat_conv (int vers, struct kernel_stat *kbuf, void *ubuf)
 	buf->st_size = kbuf->st_size;
 	/* Check for overflow.  */
 	if (buf->st_size != kbuf->st_size)
-	  {
-	    __set_errno (EOVERFLOW);
-	    return -1;
-	  }
+	  err = EOVERFLOW;
 	buf->st_pad3 = 0;
 	buf->st_atim.tv_sec = kbuf->st_atime_sec;
 	buf->st_atim.tv_nsec = kbuf->st_atime_nsec;
@@ -72,11 +67,13 @@ __xstat_conv (int vers, struct kernel_stat *kbuf, void *ubuf)
 	buf->st_blocks = kbuf->st_blocks;
 	/* Check for overflow.  */
 	if (buf->st_blocks != kbuf->st_blocks)
+	  err = EOVERFLOW;
+	memset (&buf->st_pad5, 0, sizeof (buf->st_pad5));
+	if (err)
 	  {
-	    __set_errno (EOVERFLOW);
+	    __set_errno (err);
 	    return -1;
 	  }
-	memset (&buf->st_pad5, 0, sizeof (buf->st_pad5));
       }
       break;
 
diff --git a/sysdeps/unix/sysv/linux/xstatconv.c b/sysdeps/unix/sysv/linux/xstatconv.c
index 858b911..68995df 100644
--- a/sysdeps/unix/sysv/linux/xstatconv.c
+++ b/sysdeps/unix/sysv/linux/xstatconv.c
@@ -185,6 +185,7 @@ __xstat32_conv (int vers, struct stat64 *kbuf, struct stat *buf)
     {
     case _STAT_VER_LINUX:
       {
+	int err = 0;
 	/* Convert current kernel version of `struct stat64' to
            `struct stat'.  */
 	buf->st_dev = kbuf->st_dev;
@@ -201,19 +202,13 @@ __xstat32_conv (int vers, struct stat64 *kbuf, struct stat *buf)
 	    buf->st_ino = kbuf->st_ino;
 	    if (sizeof (buf->st_ino) != sizeof (kbuf->st_ino)
 		&& buf->st_ino != kbuf->st_ino)
-	      {
-		__set_errno (EOVERFLOW);
-		return -1;
-	      }
+	      err = EOVERFLOW;
 	  }
 #else
 	buf->st_ino = kbuf->st_ino;
 	if (sizeof (buf->st_ino) != sizeof (kbuf->st_ino)
 	    && buf->st_ino != kbuf->st_ino)
-	  {
-	    __set_errno (EOVERFLOW);
-	    return -1;
-	  }
+	  err = EOVERFLOW;
 #endif
 	buf->st_mode = kbuf->st_mode;
 	buf->st_nlink = kbuf->st_nlink;
@@ -227,19 +222,13 @@ __xstat32_conv (int vers, struct stat64 *kbuf, struct stat *buf)
 	/* Check for overflow.  */
 	if (sizeof (buf->st_size) != sizeof (kbuf->st_size)
 	    && buf->st_size != kbuf->st_size)
-	  {
-	    __set_errno (EOVERFLOW);
-	    return -1;
-	  }
+	  err = EOVERFLOW;
 	buf->st_blksize = kbuf->st_blksize;
 	buf->st_blocks = kbuf->st_blocks;
 	/* Check for overflow.  */
 	if (sizeof (buf->st_blocks) != sizeof (kbuf->st_blocks)
 	    && buf->st_blocks != kbuf->st_blocks)
-	  {
-	    __set_errno (EOVERFLOW);
-	    return -1;
-	  }
+	  err = EOVERFLOW;
 #ifdef _HAVE_STAT_NSEC
 	buf->st_atim.tv_sec = kbuf->st_atim.tv_sec;
 	buf->st_atim.tv_nsec = kbuf->st_atim.tv_nsec;
@@ -268,6 +257,12 @@ __xstat32_conv (int vers, struct stat64 *kbuf, struct stat *buf)
 #ifdef _HAVE_STAT___UNUSED5
 	buf->__unused5 = 0;
 #endif
+
+	if (err)
+	  {
+	    __set_errno (err);
+	    return -1;
+	  }
       }
       break;
---