From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16664 invoked by alias); 26 Feb 2013 19:55:41 -0000 Received: (qmail 16644 invoked by uid 22791); 26 Feb 2013 19:55:39 -0000 X-SWARE-Spam-Status: No, hits=-7.4 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 26 Feb 2013 19:55:34 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r1QJtYxH021209 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 26 Feb 2013 14:55:34 -0500 Received: from [10.3.113.137] (ovpn-113-137.phx2.redhat.com [10.3.113.137]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r1QJtXKh014933; Tue, 26 Feb 2013 14:55:33 -0500 Message-ID: <512D1335.1020704@redhat.com> Date: Tue, 26 Feb 2013 19:55:00 -0000 From: "Carlos O'Donell" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: GNU C Library , libc-ports@sourceware.org Subject: [RFC] Copy as much as you can during a 32-bit stat before returning EOVERFLOW? Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact libc-ports-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: libc-ports-owner@sourceware.org X-SW-Source: 2013-02/txt/msg00063.txt.bz2 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 * 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 * 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; ---