From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS22989 209.51.188.0/24 X-Spam-Status: No, score=-4.0 required=3.0 tests=AWL,BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id A390F1F45E for ; Sun, 16 Feb 2020 21:39:13 +0000 (UTC) Received: from localhost ([::1]:36758 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1j3RdA-0003uJ-Qe for normalperson@yhbt.net; Sun, 16 Feb 2020 16:39:12 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:47322) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1j3Rd5-0003qP-7F for bug-gnulib@gnu.org; Sun, 16 Feb 2020 16:39:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1j3Rd2-0000Dh-Qb for bug-gnulib@gnu.org; Sun, 16 Feb 2020 16:39:07 -0500 Received: from mo6-p00-ob.smtp.rzone.de ([2a01:238:20a:202:5300::6]:13425) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1j3Rd1-00007m-U5 for bug-gnulib@gnu.org; Sun, 16 Feb 2020 16:39:04 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1581889140; s=strato-dkim-0002; d=clisp.org; h=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From: X-RZG-CLASS-ID:X-RZG-AUTH:From:Subject:Sender; bh=hlAnVmKokTEzej+jluIV2qsNQ9LUbvYRtm/HQbt08KI=; b=mjj7gdwxR2W6wDbmn/fSkOJl9XmPbgA82qMbynHWgRTD04V0sczCnelNQ5fzoxV3VO tkvrYAHpzdqi/WZUx5lnEWCcxxVsOMps56Jgxl5Yly51wTPe34cxV9q7a56yomYgIBkp 2ETV66BMIZjlWDMO+ZVGBZ9pf9Yd3pACODZjMsnHhodRwmqcAULd8ij7K2Uq3Dzk39eD dtVlqHfKPH6q2weALMYx4QOdblBnKLlvWqfwnWdPzXrf/nb5iPqTxp59E36wMX7rXJ3J 6LI/tvZwhkGIbTWpFBMkq+anoAkJmxatHnhGm/zvJqYq6k1iHBDl01RwZaX+lZWaG8ck F6xw== X-RZG-AUTH: ":Ln4Re0+Ic/6oZXR1YgKryK8brlshOcZlIWs+iCP5vnk6shH+AHjwLuWOH6fzxfs=" X-RZG-CLASS-ID: mo00 Received: from bruno.haible.de by smtp.strato.de (RZmta 46.1.12 DYNA|AUTH) with ESMTPSA id g00701w1GLcxDaD (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (curve X9_62_prime256v1 with 256 ECDH bits, eq. 3072 bits RSA)) (Client did not present a certificate); Sun, 16 Feb 2020 22:38:59 +0100 (CET) From: Bruno Haible To: Paul Eggert Subject: Re: [PATCH] fchmodat, lchmod: port to buggy Linux filesystems Date: Sun, 16 Feb 2020 22:38:58 +0100 Message-ID: <3279179.fFYm18bb12@omega> User-Agent: KMail/5.1.3 (Linux/4.4.0-171-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: References: <20200213184209.34020-1-eggert@cs.ucla.edu> <3278688.kFbZAF1gG3@omega> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart2955332.bGUug7GkUl" Content-Transfer-Encoding: 7Bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a01:238:20a:202:5300::6 X-BeenThere: bug-gnulib@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Gnulib discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: bug-gnulib@gnu.org Errors-To: bug-gnulib-bounces+normalperson=yhbt.net@gnu.org Sender: "bug-gnulib" This is a multi-part message in MIME format. --nextPart2955332.bGUug7GkUl Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Paul Eggert wrote: > > 2) Also the discussion what is the "right" behaviour is specific to Linux. > > The code in the '#else' case > > > > if (S_ISLNK (st.st_mode)) > > { > > close (fd); > > errno = EOPNOTSUPP; > > return -1; > > } > > > > will surely upset users on BSD systems, where symlinks are intended to have > > permission bits. > > Because of the Autoconf tests, that code should be executed only on > platforms where lchmod fails (or does not exist), whould shouldn't occur > on BSD systems. Unfortunately, it doesn't: The line in the module description lib_SOURCES += lchmod.c causes lchmod.c to be compiled (to a .o file that defines 'lchmod', not even 'rpl_lchmod'!) on macOS, FreeBSD, NetBSD, etc. > Still, I take your point that the code is confusing. > Perhaps lchmod.m4 and fchmodat.m4 should define a symbol > HAVE_LCHMOD_BUG_WITH_NON_SYMLINKS and the C code could refer to that. > The resulting machine code would be the same as now, but the > cause-and-effect would be clearer. Yes, I agree. Let me do this. Also I'm adding more comments. This patch is tested on all platforms that have an lchown() function. Before: HAVE_LCHMOD REPLACE_LCHMOD nm lchmod.o nm fchmodat.o glibc/Linux 0 0 rpl_fchmodat fchmodat,openat,chmod,... macOS 1 0 rpl_lstat,chmod save_cwd,chmod,lchmod,... HP-UX 1 0 rpl_lstat,chmod save_cwd,chmod,lchmod,... musl/Linux 1 0 fchmodat -- glibc/Hurd 1 0 fchmodat -- glibc/kFreeBSD 1 0 fchmodat -- FreeBSD 1 0 fchmodat -- NetBSD 1 0 fchmodat -- After: HAVE_LCHMOD REPLACE_LCHMOD nm lchmod.o nm fchmodat.o glibc/Linux 0 0 rpl_fchmodat fchmodat,openat,chmod,... macOS 1 0 -- save_cwd,chmod,lchmod,... HP-UX 1 0 -- save_cwd,chmod,lchmod,... musl/Linux 1 0 -- -- glibc/Hurd 1 0 -- -- glibc/kFreeBSD 1 0 -- -- FreeBSD 1 0 -- -- NetBSD 1 0 -- -- 2020-02-16 Bruno Haible lchmod: Make more future-proof. * m4/lchmod.m4 (gl_FUNC_LCHMOD): Define NEED_LCHMOD_NONSYMLINK_FIX. (gl_PREREQ_LCHMOD): New macro. * lib/lchmod.c (orig_lchmod): New function. (lchmod): Test NEED_LCHMOD_NONSYMLINK_FIX. Access /proc only on Linux. Return EOPNOTSUPP only on Linux and on platforms without lchmod function. * modules/lchmod (configure.ac): Invoke gl_PREREQ_LCHMOD. lchmod: Fix buggy override on macOS, HP-UX (regression from 2020-02-08). * modules/lchmod (Makefile.am): Don't add lchmod.c to lib_SOURCES. --nextPart2955332.bGUug7GkUl Content-Disposition: attachment; filename="0001-lchmod-Fix-buggy-override-on-macOS-HP-UX-regression-.patch" Content-Transfer-Encoding: 7Bit Content-Type: text/x-patch; charset="UTF-8"; name="0001-lchmod-Fix-buggy-override-on-macOS-HP-UX-regression-.patch" >From 4efc18c4a2927074bf0653ba8bd79230661b8238 Mon Sep 17 00:00:00 2001 From: Bruno Haible Date: Sun, 16 Feb 2020 22:31:34 +0100 Subject: [PATCH 1/2] lchmod: Fix buggy override on macOS, HP-UX (regression from 2020-02-08). * modules/lchmod (Makefile.am): Don't add lchmod.c to lib_SOURCES. --- ChangeLog | 5 +++++ modules/lchmod | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index beacc09..f7a3959 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2020-02-16 Bruno Haible + lchmod: Fix buggy override on macOS, HP-UX (regression from 2020-02-08). + * modules/lchmod (Makefile.am): Don't add lchmod.c to lib_SOURCES. + +2020-02-16 Bruno Haible + lchmod: Improve cross-compilation guess. * m4/lchmod.m4 (gl_FUNC_LCHMOD): Require AC_CANONICAL_HOST. When cross-compiling, guess depending on the platform. diff --git a/modules/lchmod b/modules/lchmod index e1054f6..c829910 100644 --- a/modules/lchmod +++ b/modules/lchmod @@ -23,7 +23,6 @@ fi gl_SYS_STAT_MODULE_INDICATOR([lchmod]) Makefile.am: -lib_SOURCES += lchmod.c Include: -- 2.7.4 --nextPart2955332.bGUug7GkUl Content-Disposition: attachment; filename="0002-lchmod-Make-more-future-proof.patch" Content-Transfer-Encoding: 7Bit Content-Type: text/x-patch; charset="UTF-8"; name="0002-lchmod-Make-more-future-proof.patch" >From ea91b32b0719d46354c1d995c1d1b1c36842a2eb Mon Sep 17 00:00:00 2001 From: Bruno Haible Date: Sun, 16 Feb 2020 22:37:28 +0100 Subject: [PATCH 2/2] lchmod: Make more future-proof. * m4/lchmod.m4 (gl_FUNC_LCHMOD): Define NEED_LCHMOD_NONSYMLINK_FIX. (gl_PREREQ_LCHMOD): New macro. * lib/lchmod.c (orig_lchmod): New function. (lchmod): Test NEED_LCHMOD_NONSYMLINK_FIX. Access /proc only on Linux. Return EOPNOTSUPP only on Linux and on platforms without lchmod function. * modules/lchmod (configure.ac): Invoke gl_PREREQ_LCHMOD. --- ChangeLog | 9 +++++++++ lib/lchmod.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++------- m4/lchmod.m4 | 15 +++++++++++++-- modules/lchmod | 1 + 4 files changed, 77 insertions(+), 9 deletions(-) diff --git a/ChangeLog b/ChangeLog index f7a3959..0705107 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,14 @@ 2020-02-16 Bruno Haible + lchmod: Make more future-proof. + * m4/lchmod.m4 (gl_FUNC_LCHMOD): Define NEED_LCHMOD_NONSYMLINK_FIX. + (gl_PREREQ_LCHMOD): New macro. + * lib/lchmod.c (orig_lchmod): New function. + (lchmod): Test NEED_LCHMOD_NONSYMLINK_FIX. Access /proc only on Linux. + Return EOPNOTSUPP only on Linux and on platforms without lchmod + function. + * modules/lchmod (configure.ac): Invoke gl_PREREQ_LCHMOD. + lchmod: Fix buggy override on macOS, HP-UX (regression from 2020-02-08). * modules/lchmod (Makefile.am): Don't add lchmod.c to lib_SOURCES. diff --git a/lib/lchmod.c b/lib/lchmod.c index c7191c0..57f75da 100644 --- a/lib/lchmod.c +++ b/lib/lchmod.c @@ -19,30 +19,68 @@ #include +/* If the user's config.h happens to include , let it include only + the system's here, so that orig_fchmodat doesn't recurse to + rpl_fchmodat. */ +#define __need_system_sys_stat_h +#include + +/* Specification. */ #include +#undef __need_system_sys_stat_h + +#if HAVE_LCHMOD +static inline int +orig_lchmod (char const *file, mode_t mode) +{ + return lchmod (file, mode); +} +#endif #include #include #include #include +#ifdef __osf__ +/* Write "sys/stat.h" here, not , otherwise OSF/1 5.1 DTK cc + eliminates this include because of the preliminary #include + above. */ +# include "sys/stat.h" +#else +# include +#endif + #include -/* Work like lchmod, except when FILE is a symbolic link. - In that case, set errno to EOPNOTSUPP and return -1. */ +/* Work like chmod, except when FILE is a symbolic link. + In that case, on systems where permissions on symbolic links are unsupported + (such as Linux), set errno to EOPNOTSUPP and return -1. */ int lchmod (char const *file, mode_t mode) { #if HAVE_FCHMODAT + /* Gnulib's fchmodat contains the workaround. No need to duplicate it + here. */ return fchmodat (AT_FDCWD, file, mode, AT_SYMLINK_NOFOLLOW); -#else -# if defined AT_FDCWD && defined O_PATH && defined AT_EMPTY_PATH +#elif NEED_LCHMOD_NONSYMLINK_FIX +# if defined AT_FDCWD && defined O_PATH && defined AT_EMPTY_PATH \ + && (defined __linux__ || defined __ANDROID__) + /* Open a file descriptor with O_NOFOLLOW, to make sure we don't + follow symbolic links, if /proc is mounted. O_PATH is used to + avoid a failure if the file is not readable. + Cf. */ int fd = openat (AT_FDCWD, file, O_PATH | O_NOFOLLOW | O_CLOEXEC); if (fd < 0) return fd; - /* Use fstatat because fstat does not work on O_PATH descriptors + /* Up to Linux 5.3 at least, when FILE refers to a symbolic link, the + chmod call below will change the permissions of the symbolic link + - which is undersired - and on many file systems (ext4, btrfs, jfs, + xfs, ..., but not reiserfs) fail with error EOPNOTSUPP - which is + misleading. Therefore test for a symbolic link explicitly. + Use fstatat because fstat does not work on O_PATH descriptors before Linux 3.6. */ struct stat st; if (fstatat (fd, "", &st, AT_EMPTY_PATH) != 0) @@ -73,7 +111,10 @@ lchmod (char const *file, mode_t mode) return chmod_result; } /* /proc is not mounted. */ + /* Fall back on chmod, despite the race. */ + return chmod (file, mode); # elif HAVE_LSTAT +# if (defined __linux__ || defined __ANDROID__) || !HAVE_LCHMOD struct stat st; int lstat_result = lstat (file, &st); if (lstat_result != 0) @@ -83,9 +124,15 @@ lchmod (char const *file, mode_t mode) errno = EOPNOTSUPP; return -1; } -# endif - /* Fall back on chmod, despite the race. */ return chmod (file, mode); +# else /* GNU/kFreeBSD, GNU/Hurd, macOS, FreeBSD, NetBSD, HP-UX */ + return orig_lchmod (file, mode); +# endif +# else /* native Windows */ + return chmod (file, mode); +# endif +#else + return orig_lchmod (file, mode); #endif } diff --git a/m4/lchmod.m4 b/m4/lchmod.m4 index 1646bd7..61e3f11 100644 --- a/m4/lchmod.m4 +++ b/m4/lchmod.m4 @@ -1,4 +1,4 @@ -#serial 5 +#serial 6 dnl Copyright (C) 2005-2006, 2008-2020 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation @@ -67,7 +67,18 @@ AC_DEFUN([gl_FUNC_LCHMOD], rm -f conftest.lchmod]) case $gl_cv_func_lchmod_works in *yes) ;; - *) REPLACE_LCHMOD=1;; + *) + AC_DEFINE([NEED_LCHMOD_NONSYMLINK_FIX], [1], + [Define to 1 if lchmod does not work right on non-symlinks.]) + REPLACE_LCHMOD=1 + ;; esac fi ]) + +# Prerequisites of lib/lchmod.c. +AC_DEFUN([gl_PREREQ_LCHMOD], +[ + AC_REQUIRE([AC_C_INLINE]) + : +]) diff --git a/modules/lchmod b/modules/lchmod index c829910..c3e0a16 100644 --- a/modules/lchmod +++ b/modules/lchmod @@ -19,6 +19,7 @@ configure.ac: gl_FUNC_LCHMOD if test $HAVE_LCHMOD = 0 || test $REPLACE_LCHMOD = 1; then AC_LIBOBJ([lchmod]) + gl_PREREQ_LCHMOD fi gl_SYS_STAT_MODULE_INDICATOR([lchmod]) -- 2.7.4 --nextPart2955332.bGUug7GkUl--