Message ID | 20210114081349.3179376-1-shorne@gmail.com |
---|---|
State | Not applicable |
Headers |
Return-Path: <libc-alpha-bounces@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 54E9E3857C48; Thu, 14 Jan 2021 08:14:00 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 54E9E3857C48 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1610612040; bh=m18M0b+1sS8mXGwmLqzrFN+bNxNgnpdPfWcy481hook=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=lpOmPmipahysbgLxShQK+2inKI3dDYHu69EiGoA9b/R9YRLljG858BQCnLLxJlE0q iNXDJ53SgtVWphz4Q8LWkCKEFP49XyEg0lyCdjQNVn8WXaFXYYYS+vqfOcGYyOag16 U6ugHenwp4ajAN/O4XlD7eZjpmwoYOafJw+2+JQ4= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-pl1-x633.google.com (mail-pl1-x633.google.com [IPv6:2607:f8b0:4864:20::633]) by sourceware.org (Postfix) with ESMTPS id DADF7385800E for <libc-alpha@sourceware.org>; Thu, 14 Jan 2021 08:13:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DADF7385800E Received: by mail-pl1-x633.google.com with SMTP id g3so2536499plp.2 for <libc-alpha@sourceware.org>; Thu, 14 Jan 2021 00:13:56 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=m18M0b+1sS8mXGwmLqzrFN+bNxNgnpdPfWcy481hook=; b=cLaml7InTaGWBs9YARfWvJx+ptEKOkhTK4Ta1LPWOfYnOOQ6uLF0ZrjNtrAV7DByyd 5sAyfT6iN/F/BxTSH7Aq0MQRMzvGXk6cHBkYhZIasRnriK+xHR6yh1UOA66jWP2IffRd YGPO76cHiqy2oLzVlZrwb/qeeINSuyj2Ot0ZnIQ+DKt6yrgpgczO3qrpUhta8jlkBG1R ZW7RsT5A8iHVsFnLmqqboY03eTi7Ia4fGlkEA50D+6hOi0wT1VvWBZ0uZ1rtFpvJTAwQ 2FwvP2svYTFOlQrNNX8rzPupeyTVzhro/rX5nlrWC3kirBbiEjohDMQ82Yb/R19Chgt1 4kmA== X-Gm-Message-State: AOAM530FD6zo9ENukrCb3oncwOLSkep+jrnCU7dmUdPIM+adkmKHVvK5 K+NmY1r8uaPkTpT5pBtBs7ARWKBo070= X-Google-Smtp-Source: ABdhPJxgFszvnTnmbNySY5kFTy3652CS2pRMYYiUuNGPGZBQZM0kcTlUgQ2t4s/vYGIW05TyKjIjkA== X-Received: by 2002:a17:90a:8508:: with SMTP id l8mr3652536pjn.131.1610612035602; Thu, 14 Jan 2021 00:13:55 -0800 (PST) Received: from localhost (g238.115-65-210.ppp.wakwak.ne.jp. [115.65.210.238]) by smtp.gmail.com with ESMTPSA id h11sm4526553pjg.46.2021.01.14.00.13.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Jan 2021 00:13:54 -0800 (PST) To: GLIBC patches <libc-alpha@sourceware.org> Subject: [PATCH] linux: Use stat_overflow to check overflow in fstatat Date: Thu, 14 Jan 2021 17:13:49 +0900 Message-Id: <20210114081349.3179376-1-shorne@gmail.com> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> From: Stafford Horne via Libc-alpha <libc-alpha@sourceware.org> Reply-To: Stafford Horne <shorne@gmail.com> Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
linux: Use stat_overflow to check overflow in fstatat
|
|
Commit Message
Stafford Horne
Jan. 14, 2021, 8:13 a.m. UTC
After a recent rebase of the OpenRISC port I am working on the build started to fail with: ../sysdeps/unix/sysv/linux/fstatat.c: In function '__fstatat': ../sysdeps/unix/sysv/linux/fstatat.c:35:21: error: 'struct stat' has no member named '__st_ino_pad' 35 | if (r == 0 && (buf->__st_ino_pad != 0 | ^~ ../sysdeps/unix/sysv/linux/fstatat.c:36:24: error: 'struct stat' has no member named '__st_size_pad' 36 | || buf->__st_size_pad != 0 | ^~ ../sysdeps/unix/sysv/linux/fstatat.c:37:24: error: 'struct stat' has no member named '__st_blocks_pad' 37 | || buf->__st_blocks_pad != 0)) | ^~ This seems to be caused by 6073bae64c ("linux: Disentangle fstatat from fxstatat"). Which introduced the checks for overflow using the stat->__st_*_pad fields. These do not exist on all architectures, I cannot even find any that do exist by grepping code. The fix is to use the stat_overflow function which only runs the overflow checks if the pad fields are available. Note, This is not xstat but I use the xstatover.h header. I hope that is not an issue. Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org> --- sysdeps/unix/sysv/linux/fstatat.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Comments
On Jan 14 2021, Stafford Horne via Libc-alpha wrote: > This seems to be caused by 6073bae64c ("linux: Disentangle fstatat from > fxstatat"). Which introduced the checks for overflow using the > stat->__st_*_pad fields. These do not exist on all architectures, I > cannot even find any that do exist by grepping code. See __field64 in sysdeps/unix/sysv/linux/generic/bits/struct_stat.h. Andreas.
On Thu, Jan 14, 2021 at 10:29:17AM +0100, Andreas Schwab wrote: > On Jan 14 2021, Stafford Horne via Libc-alpha wrote: > > > This seems to be caused by 6073bae64c ("linux: Disentangle fstatat from > > fxstatat"). Which introduced the checks for overflow using the > > stat->__st_*_pad fields. These do not exist on all architectures, I > > cannot even find any that do exist by grepping code. > > See __field64 in sysdeps/unix/sysv/linux/generic/bits/struct_stat.h. Thank's I was only finding kernel_stat.h, this makes sense now. I think the patch is still valid, though. -Stafford
Ping Adhemerval on this one. I still thing the below is needed for the STAT_IS_KERNEL_STAT cases. On Thu, Jan 14, 2021 at 05:13:49PM +0900, Stafford Horne wrote: > After a recent rebase of the OpenRISC port I am working on the build > started to fail with: > > ../sysdeps/unix/sysv/linux/fstatat.c: In function '__fstatat': > ../sysdeps/unix/sysv/linux/fstatat.c:35:21: error: 'struct stat' has no member named '__st_ino_pad' > 35 | if (r == 0 && (buf->__st_ino_pad != 0 > | ^~ > ../sysdeps/unix/sysv/linux/fstatat.c:36:24: error: 'struct stat' has no member named '__st_size_pad' > 36 | || buf->__st_size_pad != 0 > | ^~ > ../sysdeps/unix/sysv/linux/fstatat.c:37:24: error: 'struct stat' has no member named '__st_blocks_pad' > 37 | || buf->__st_blocks_pad != 0)) > | ^~ > > This seems to be caused by 6073bae64c ("linux: Disentangle fstatat from > fxstatat"). Which introduced the checks for overflow using the > stat->__st_*_pad fields. These do not exist on all architectures, I > cannot even find any that do exist by grepping code. > > The fix is to use the stat_overflow function which only runs the > overflow checks if the pad fields are available. > > Note, This is not xstat but I use the xstatover.h header. I hope that > is not an issue. > > Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > sysdeps/unix/sysv/linux/fstatat.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/fstatat.c b/sysdeps/unix/sysv/linux/fstatat.c > index 59efff615f..5bff931183 100644 > --- a/sysdeps/unix/sysv/linux/fstatat.c > +++ b/sysdeps/unix/sysv/linux/fstatat.c > @@ -22,6 +22,7 @@ > > #if !XSTAT_IS_XSTAT64 > # include <kstat_cp.h> > +# include <xstatover.h> > > int > __fstatat (int fd, const char *file, struct stat *buf, int flag) > @@ -32,10 +33,7 @@ __fstatat (int fd, const char *file, struct stat *buf, int flag) > /* New kABIs which uses generic pre 64-bit time Linux ABI, e.g. > csky, nios2 */ > r = INTERNAL_SYSCALL_CALL (fstatat64, fd, file, buf, flag); > - if (r == 0 && (buf->__st_ino_pad != 0 > - || buf->__st_size_pad != 0 > - || buf->__st_blocks_pad != 0)) > - return INLINE_SYSCALL_ERROR_RETURN_VALUE (EOVERFLOW); > + return r ?: stat_overflow (buf); > # else > # ifdef __NR_fstatat64 > /* Old KABIs with old non-LFS support, e.g. arm, i386, hppa, m68k, mips32, > -- > 2.26.2 >
On 27/01/2021 09:40, Stafford Horne wrote: > Ping Adhemerval on this one. > > I still thing the below is needed for the STAT_IS_KERNEL_STAT > cases. > Why OpenRISC is not setting XSTAT_IS_XSTAT64 ? As a possible newer ABI, I see no pointing in support old non-LFS ABI.
On Wed, Jan 27, 2021 at 10:28:58AM -0300, Adhemerval Zanella wrote: > > > On 27/01/2021 09:40, Stafford Horne wrote: > > Ping Adhemerval on this one. > > > > I still thing the below is needed for the STAT_IS_KERNEL_STAT > > cases. > > > > Why OpenRISC is not setting XSTAT_IS_XSTAT64 ? As a possible newer > ABI, I see no pointing in support old non-LFS ABI. OK, I got it. I'll try to turn that on. I was not so clear on what all of the *STAT_IS_* macros defined, so I was trying to just go with the generic defaults. On OpenRISC which is only 32-bit XSTAT_IS_XSTAT64 is set to zero. As per: sysdeps/unix/sysv/linux/generic/kernel_stat.h #if __WORDSIZE == 64 # define XSTAT_IS_XSTAT64 1 #else # define XSTAT_IS_XSTAT64 0 #endif Again, I'll see what I can do. -Stafford
On Wed, Jan 27, 2021 at 10:28:58AM -0300, Adhemerval Zanella wrote: > > > On 27/01/2021 09:40, Stafford Horne wrote: > > Ping Adhemerval on this one. > > > > I still thing the below is needed for the STAT_IS_KERNEL_STAT > > cases. > > > > Why OpenRISC is not setting XSTAT_IS_XSTAT64 ? As a possible newer > ABI, I see no pointing in support old non-LFS ABI. I got it all working without this patch after: - Setting XSTAT_IS_XSTAT64 and a few other kernel_stats.h - Updating my shlib-version to 2.33 Note to myself in glibc LFS stands for Large File System. I see that in many parts of the stats code but couldn't find the actual acronym explained, I found it in the manual, it was hard to search for on the internet. Thanks. -Stafford
On 29/01/2021 19:25, Stafford Horne wrote: > On Wed, Jan 27, 2021 at 10:28:58AM -0300, Adhemerval Zanella wrote: >> >> >> On 27/01/2021 09:40, Stafford Horne wrote: >>> Ping Adhemerval on this one. >>> >>> I still thing the below is needed for the STAT_IS_KERNEL_STAT >>> cases. >>> >> >> Why OpenRISC is not setting XSTAT_IS_XSTAT64 ? As a possible newer >> ABI, I see no pointing in support old non-LFS ABI. > > I got it all working without this patch after: > > - Setting XSTAT_IS_XSTAT64 and a few other kernel_stats.h > - Updating my shlib-version to 2.33 > > Note to myself in glibc LFS stands for Large File System. I see that in many > parts of the stats code but couldn't find the actual acronym explained, I found > it in the manual, it was hard to search for on the internet. > > Thanks. > > -Stafford > I think I will need to revise the internal stat defaults, the whole idea of my stat consolidation is to avoid this very of issue for newer ports. Ideally newer ports should not have to support non-LFS calls, which means that sysdeps/unix/sysv/linux/[f,l]stat[at].c should be empty TU. However it seems that the default XSTAT_IS_XSTAT64 is still 0 on both the Linux and generic one (the generic defines it to 1 for __WORDSIZE == 64 at least). For 2.34 I will consolidate the kernel_stat.h, so the default values will be: #define STAT_IS_KERNEL_STAT 1 #define XSTAT_IS_XSTAT64 1 #define STATFS_IS_STATFS64 1 The STAT_IS_KERNEL_STAT is only used for non-LFS call and old xstat support so it wouldn't matter for newer ABIs. The only important flag here is XSTAT_IS_XSTAT64 (which is currently misleading since xstat is the compat symbols now). It would probably need to consolidate the statfs code, which is another messy one.
diff --git a/sysdeps/unix/sysv/linux/fstatat.c b/sysdeps/unix/sysv/linux/fstatat.c index 59efff615f..5bff931183 100644 --- a/sysdeps/unix/sysv/linux/fstatat.c +++ b/sysdeps/unix/sysv/linux/fstatat.c @@ -22,6 +22,7 @@ #if !XSTAT_IS_XSTAT64 # include <kstat_cp.h> +# include <xstatover.h> int __fstatat (int fd, const char *file, struct stat *buf, int flag) @@ -32,10 +33,7 @@ __fstatat (int fd, const char *file, struct stat *buf, int flag) /* New kABIs which uses generic pre 64-bit time Linux ABI, e.g. csky, nios2 */ r = INTERNAL_SYSCALL_CALL (fstatat64, fd, file, buf, flag); - if (r == 0 && (buf->__st_ino_pad != 0 - || buf->__st_size_pad != 0 - || buf->__st_blocks_pad != 0)) - return INLINE_SYSCALL_ERROR_RETURN_VALUE (EOVERFLOW); + return r ?: stat_overflow (buf); # else # ifdef __NR_fstatat64 /* Old KABIs with old non-LFS support, e.g. arm, i386, hppa, m68k, mips32,