From patchwork Mon Feb 26 23:38:16 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nix X-Patchwork-Id: 26098 Received: (qmail 79253 invoked by alias); 26 Feb 2018 23:40:52 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 78800 invoked by uid 89); 26 Feb 2018 23:40:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, UNPARSEABLE_RELAY autolearn=ham version=3.3.2 spammy= X-HELO: aserp2120.oracle.com From: Nick Alcock To: Adhemerval Zanella Cc: libc-alpha@sourceware.org Subject: Re: [PATCH v2] Fix 32-bit getcwd() on filesystems with 64-bit inodes. References: <87h8q47e5p.fsf@esperi.org.uk> <2a84a9a8-c838-af48-e549-bb7394e92287@linaro.org> Date: Mon, 26 Feb 2018 23:38:16 +0000 In-Reply-To: <2a84a9a8-c838-af48-e549-bb7394e92287@linaro.org> (Adhemerval Zanella's message of "Mon, 26 Feb 2018 10:15:48 -0300") Message-ID: <87606j5nkn.fsf_-_@esperi.org.uk> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=8816 signatures=668680 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1802260296 Here's the latest patch round, with your review comments incorporated, and retested on trunk. Thanks for the review! On 26 Feb 2018, Adhemerval Zanella said: > Thanks for checking on it and I see no reason to continue using non-LFS > calls on loader for 32-bits architectures. I'm sure there are more. I might do a big audit for all remaining external references to non-LFS stuff from stuff that is not itself a non-LFS implementation, and squash them all at once. (But maybe we should do this differently, by internally defining __readdir() etc to be __readdir64() as is true when _FILE_OFFSET_BITS=64, and adding an internal __readdir32() for the non-LFS version? That way you'd have to do something unusual to call the non-LFS variant, which is surely the right way around.) I audited all other readdir64() implementations: no others define any non-default symbol versions except by #including the i386 one, so I think we're fine on other arches. The Hurd version calls __dir_readdir() which is not in glibc so I just have to hope that it doesn't have the same limitation as Linux getdents(). :) (I hope scissors lines work here: git am --scissors.) 8<---------------------->8 From: Nick Alcock When compiling a 32-bit glibc on a filesystem with 64-bit inodes, we observe failures of io/tst-getcwd-abspath: tst-getcwd-abspath.c:42: numeric comparison failure left: 75 (0x4b); from: errno right: 2 (0x2); from: ENOENT tst-getcwd-abspath.c:47: numeric comparison failure left: 75 (0x4b); from: errno right: 2 (0x2); from: ENOENT error: 2 test failures Errno 75 is EOVERFLOW, which is most unexpected from getcwd()! Having had experience with this class of pain in zic before (a patch which I should perhaps resubmit or combine with this one), the cause is clear: the getcwd() implementation was calling readdir() because it was in a disconnected subtree, and in glibc that is the non-LFS implementation so it ends up calling getdents(), which falls over with just that error if it sees a single inode with a value nonrepresentable in 32 bits in the directories it fetches, and it scans all parents of the current directory so it has a lot of opportunities to hit such an inode. getcwd() is used in the dynamic linker as part of $ORIGIN support, so the usual SHLIB_COMPAT dance is needed there to prevent versioned symbols getting into it and causing disaster. [BZ #22899] * include/dirent.h: Make __readdir64 hidden in rtld too. * sysdeps/unix/sysv/linux/i386/readdir64.c: Mark SHLIB_COMPAT in libc only. * posix/getcwd.c [__GNU_LIBRARY__] (__readdir64): Define. (__getcwd): Use dirent64 and __readdir64. Reviewed-by: Adhemerval Zanella --- include/dirent.h | 4 +++- sysdeps/posix/getcwd.c | 5 +++-- sysdeps/unix/sysv/linux/i386/readdir64.c | 14 ++++++++------ 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/include/dirent.h b/include/dirent.h index caaeb0be85..9ca588a4e3 100644 --- a/include/dirent.h +++ b/include/dirent.h @@ -21,7 +21,9 @@ extern DIR *__fdopendir (int __fd) attribute_hidden; extern int __closedir (DIR *__dirp) attribute_hidden; extern struct dirent *__readdir (DIR *__dirp) attribute_hidden; extern struct dirent64 *__readdir64 (DIR *__dirp); -libc_hidden_proto (__readdir64) +# if IS_IN (libc) || (IS_IN (rtld) && !defined NO_RTLD_HIDDEN) + hidden_proto (__readdir64) +# endif extern int __readdir_r (DIR *__dirp, struct dirent *__entry, struct dirent **__result); extern int __readdir64_r (DIR *__dirp, struct dirent64 *__entry, diff --git a/sysdeps/posix/getcwd.c b/sysdeps/posix/getcwd.c index b53433a2dc..f547cd6612 100644 --- a/sysdeps/posix/getcwd.c +++ b/sysdeps/posix/getcwd.c @@ -194,6 +194,7 @@ extern char *alloca (); #ifndef __GNU_LIBRARY__ # define __lstat64 stat64 +# define __readdir64 readdir64 #endif #ifndef _LIBC @@ -366,14 +367,14 @@ __getcwd (char *buf, size_t size) goto lose; fd_needs_closing = false; - struct dirent *d; + struct dirent64 *d; bool use_d_ino = true; while (1) { /* Clear errno to distinguish EOF from error if readdir returns NULL. */ __set_errno (0); - d = __readdir (dirstream); + d = __readdir64 (dirstream); if (d == NULL) { if (errno == 0) diff --git a/sysdeps/unix/sysv/linux/i386/readdir64.c b/sysdeps/unix/sysv/linux/i386/readdir64.c index 42b73023e0..e2981c7f9c 100644 --- a/sysdeps/unix/sysv/linux/i386/readdir64.c +++ b/sysdeps/unix/sysv/linux/i386/readdir64.c @@ -28,19 +28,21 @@ #undef DIRENT_TYPE libc_hidden_def (__readdir64) +#if IS_IN (libc) versioned_symbol (libc, __readdir64, readdir64, GLIBC_2_2); -#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2) +# if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2) -#include +# include -#define __READDIR attribute_compat_text_section __old_readdir64 -#define __GETDENTS __old_getdents64 -#define DIRENT_TYPE struct __old_dirent64 +# define __READDIR attribute_compat_text_section __old_readdir64 +# define __GETDENTS __old_getdents64 +# define DIRENT_TYPE struct __old_dirent64 -#include +# include libc_hidden_def (__old_readdir64) compat_symbol (libc, __old_readdir64, readdir64, GLIBC_2_1); #endif +#endif