From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 113241 invoked by alias); 27 Feb 2018 13:13:34 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 113231 invoked by uid 89); 27 Feb 2018 13:13:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-qk0-f195.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=K+i8Te444Cexwrv3OQ/K4Yl4VAm0BTc+p/OOeN4fl54=; b=M+k6Yqblo0TkcJxMxcvHxpPTx23lX6n36WkBDC1e6uoTk4cIZWkcgcUvztewZbBskN NAZZkS4sdQSkzRiDdK7Y+QRQXaQizEH5u9oxAuSqZNgS8QbiVkFsos/6eBwcyg4B2AZg 5+zSgmrderVcC6B8sgvlBe4Bxr9rqPXD2h+HC31IoUAQoMJtEUTGCDhmMXfJIF4rkLLC hfnRY7sZoEwEPVfwM2yDPAIIKze4YS/jlrtVC486IZ0TD8s2qqbYDfbJJ/Iby3DgqpHB iLcOYtvBmV+crhCb8QJ6ACac4kzDwlORReGRfsXJbrej+f96nxKGsfShpu1S3/bEMYUP OBXg== X-Gm-Message-State: APf1xPD4RBQ7BioVmfCxuQRoquSL5AmOUUY4MKt8s/gcfbrAHnM/chd9 cGA4ChS2F3v4thIZvgk7ScE9bg2uNi0= X-Google-Smtp-Source: AG47ELsIGSl9E/SNx2CpC65DKjVpkzJ+F9bhil66AjkZZR1cUanL317aBQv/C+0LIZsCj1Pcm5Y1Kw== X-Received: by 10.55.180.133 with SMTP id d127mr21788596qkf.242.1519737209627; Tue, 27 Feb 2018 05:13:29 -0800 (PST) Subject: Re: [PATCH] Fix 32-bit getcwd() on filesystems with 64-bit inodes. To: Nix Cc: libc-alpha@sourceware.org References: <87h8q47e5p.fsf@esperi.org.uk> <2a84a9a8-c838-af48-e549-bb7394e92287@linaro.org> <87efl75wlj.fsf@esperi.org.uk> From: Adhemerval Zanella Message-ID: <4c0e2ce2-68f1-b1f3-8b6b-42af0c13b99b@linaro.org> Date: Tue, 27 Feb 2018 16:32:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <87efl75wlj.fsf@esperi.org.uk> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-02/txt/msg00805.txt.bz2 On 26/02/2018 17:23, Nix wrote: > On 26 Feb 2018, Adhemerval Zanella told this: > >> On 25/02/2018 22:06, Nick Alcock wrote: >>> 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(), and in glibc that is >>> the non-LFS implementation so it falls over with just that error if it >>> sees a single 64-bit inode. >> >> Thanks for checking on it and I see no reason to continue using non-LFS >> calls on loader for 32-bits architectures. > > Neither do I. There are quite a lot of non-LFS calls elsewhere (I have > another patch that purges a bunch of them from other tests, for > instance, but they're even less related to this bug than the ttyname > stuff in here was). > >> I don't see this regression with a i686-linux-gnu build running on a >> x86_64 kernel, so it seems the case where the system configuration is >> interfering on the testcases. It would be good if we could isolate such >> issues, so do you have any information why exactly getdents is failing? > > You need a filesystem on which inode numbers are routinely > 2^32, for > instance a >1TiB XFS filesystem mounted without the > (performance-destroying) inode32 option. On such filesystems, getdents() > (but not getdents64()) will return -EOVERFLOW if any inode number in the > set to be returned will not fit in the 32-bit space of an ino_t. (See > the various references to -EOVERFLOW in fs/readdir.c, which handle one > instance or another of this.) > > This happens on xfs because it uses physical disk block index as an > inode number: ext4 etc, which use more conventional inode tables, always > has sub-32-bit inode numbers except in the largest of filesystems and so > will not reveal this problem. But XFS is getting more common these days, > and big filesystems are too... you'll also see it on NFSv4 mounts to > such a kernel (and if youre using NFS, you'll be using NFSv4: NFSv3 > mounts will generally not work right because the protocol cannot handle > 64-bit inode numbers). Thanks for thoroughly explanation, this explain why I am not seeing this issue in any of environments I have access. > >>> 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. >>> >>> While we're at it, fix likely similar errors in ttyname() (determined >>> by code inspection, since my /dev is a tmpfs with 32-bit indoes: but >>> one could run glibc on a system with a 64-bit-inode filesystem and >>> a static /dev, and then this would probably fail too, the same way.) >> >> I will recommend to spit the ttyname fix on its own patch (and Linux has >> its own versio which already uses LFS calls already). > > Oh, does it? I missed that, The ttyname part may be entirely unnecessary > then. I'll just drop it for now: if it belongs anywhere it's not here, > and I have no proof it causes any trouble at all (unlike getcwd). Linux uses sysdeps/unix/sysv/linux/ttyname.c and it does not use any fallback implementation as getcwd. > >>> >>> * include/dirent.h: Make __readdir64 hidden in rtld too. >>> * sysdeps/unix/sysv/linux/i386/readdir64.c: Mark SHLIB_COMPAT >>> in libc only. >> >> Indentation seems a bit off here with extra spaces. > > That's what happens when you forget an M-x tabify. :/ > >>> It is quite possible I'm being unidiomatic in include/dirent.h and >>> sysdeps/unix/sysv/linux/i386/readdir64.c: I'd be happy to switch to >>> the idiomatic approach if someone could tell me what it is. :) >> >> The readdir LFS consolidation to avoid the multiple version for the >> different architecture we have is on my backlog for some time. I think >> current approach is fine. > > Thanks! > >>> diff --git a/include/dirent.h b/include/dirent.h >>> index caaeb0be85..c1d3c6b53f 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) >>> + hidden_proto (__readdir64) >>> +# endif >> >> I think you will need a '&& !defined NO_RTLD_HIDDEN' for Hurd (I can't confirm >> even with build-many-glibc.py Hurd is missing the thread configuration to >> complete the build). > > It does look like it, from other uses. I'll add that, drop the ttyname > stuff for now, open a bug, and resubmit. >