From patchwork Tue Feb 11 15:27:14 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 37968 Received: (qmail 88313 invoked by alias); 11 Feb 2020 15:27:40 -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 88229 invoked by uid 89); 11 Feb 2020 15:27:39 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=mount, privileges, reveal, 020 X-HELO: us-smtp-1.mimecast.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1581434855; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=N8tLk2XM+zBEkSu8jfgS6ytBd0WoFp/HSTe/F0nH2pg=; b=eZFmNbIazbRw9RcQrNIZ19Pwk2zD62+lDTmnsqR+SgI6qv47v9G1xeqydD8SBta55bFfNe s3qoMFlmhy9MJ3BpSQ2msaokIOsh7I5RHWdEsfS1Sqvib0j4ojbfUNb5FbIvnElfhtYgyQ GAXWjURmSAA4Flw8klpAZ7Psn69wxDs= From: Florian Weimer To: libc-alpha@sourceware.org Subject: Re: [PATCH 4/5] io: Add io/tst-lchmod covering lchmod and fchmodat References: Date: Tue, 11 Feb 2020 16:27:14 +0100 In-Reply-To: (Florian Weimer's message of "Wed, 22 Jan 2020 21:03:47 +0100") Message-ID: <87a75pjga5.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com * Florian Weimer: > + /* Without AT_SYMLINK_NOFOLLOW, changing the permissions should > + work as before. */ > + arg = select_path (do_relative_path, path_file, "file"); > + TEST_COMPARE (chmod_func (fd, arg, 3, 0), 0); > + xstat (path_file, &st); > + TEST_COMPARE (st.st_mode & 0777, 3); > + /* But with AT_SYMLINK_NOFOLLOW, even if we originally had > + support, we may have lost it. */ > + ret = chmod_func (fd, arg, 2, 0); The last line misses AT_SYMLINK_NOFOLLOW. The version below (which I'm going to commit) fixes this and adds another check. Thanks, Florian 8<------------------------------------------------------------------8< ----- io/Makefile | 2 +- io/tst-lchmod.c | 314 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 315 insertions(+), 1 deletion(-) diff --git a/io/Makefile b/io/Makefile index d9a1da4566..fe2f8c5065 100644 --- a/io/Makefile +++ b/io/Makefile @@ -74,7 +74,7 @@ tests := test-utime test-stat test-stat2 test-lfs tst-getcwd \ tst-posix_fallocate tst-posix_fallocate64 \ tst-fts tst-fts-lfs tst-open-tmpfile \ tst-copy_file_range tst-getcwd-abspath tst-lockf \ - tst-ftw-lnk + tst-ftw-lnk tst-lchmod # Likewise for statx, but we do not need static linking here. tests-internal += tst-statx diff --git a/io/tst-lchmod.c b/io/tst-lchmod.c new file mode 100644 index 0000000000..73e45549af --- /dev/null +++ b/io/tst-lchmod.c @@ -0,0 +1,314 @@ +/* Tests for lchmod and fchmodat with AT_SYMLINK_NOFOLLOW. + Copyright (C) 2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#if __has_include () +# include +#endif + +/* Array of file descriptors. */ +#define DYNARRAY_STRUCT fd_list +#define DYNARRAY_ELEMENT int +#define DYNARRAY_INITIAL_SIZE 0 +#define DYNARRAY_PREFIX fd_list_ +#include + +static int +fchmodat_with_lchmod (int fd, const char *path, mode_t mode, int flags) +{ + TEST_COMPARE (fd, AT_FDCWD); + if (flags == 0) + return chmod (path, mode); + else + { + TEST_COMPARE (flags, AT_SYMLINK_NOFOLLOW); + return lchmod (path, mode); + } +} + +/* Chose the appropriate path to pass as the path argument to the *at + functions. */ +static const char * +select_path (bool do_relative_path, const char *full_path, const char *relative_path) +{ + if (do_relative_path) + return relative_path; + else + return full_path; +} + +static void +test_1 (bool do_relative_path, int (*chmod_func) (int fd, const char *, mode_t, int)) +{ + char *tempdir = support_create_temp_directory ("tst-lchmod-"); + + char *path_dangling = xasprintf ("%s/dangling", tempdir); + char *path_file = xasprintf ("%s/file", tempdir); + char *path_loop = xasprintf ("%s/loop", tempdir); + char *path_missing = xasprintf ("%s/missing", tempdir); + char *path_to_file = xasprintf ("%s/to-file", tempdir); + + int fd; + if (do_relative_path) + fd = xopen (tempdir, O_DIRECTORY | O_RDONLY, 0); + else + fd = AT_FDCWD; + + add_temp_file (path_dangling); + add_temp_file (path_loop); + add_temp_file (path_file); + add_temp_file (path_to_file); + + support_write_file_string (path_file, ""); + xsymlink ("file", path_to_file); + xsymlink ("loop", path_loop); + xsymlink ("target-does-not-exist", path_dangling); + + /* Check that the modes do not collide with what we will use in the + test. */ + struct stat64 st; + xstat (path_file, &st); + TEST_VERIFY ((st.st_mode & 0777) != 1); + xlstat (path_to_file, &st); + TEST_VERIFY ((st.st_mode & 0777) != 2); + mode_t original_symlink_mode = st.st_mode; + + /* Set to true if AT_SYMLINK_NOFOLLOW is supported. */ + bool nofollow; + + /* We should be able to change the mode of a file, including through + the symbolic link to-file. */ + const char *arg = select_path (do_relative_path, path_file, "file"); + TEST_COMPARE (chmod_func (fd, arg, 1, 0), 0); + xstat (path_file, &st); + TEST_COMPARE (st.st_mode & 0777, 1); + int ret = chmod_func (fd, path_file, 2, AT_SYMLINK_NOFOLLOW); + if (ret == 0) + { + printf ("info: AT_SYMLINK_NOFOLLOW support in %s\n", tempdir); + nofollow = true; + } + else + { + printf ("info: no AT_SYMLINK_NOFOLLOW support in %s\n", tempdir); + nofollow = false; + + /* Set up things for the code below. */ + TEST_COMPARE (chmod_func (fd, path_file, 2, 0), 0); + } + xstat (path_file, &st); + TEST_COMPARE (st.st_mode & 0777, 2); + arg = select_path (do_relative_path, path_to_file, "to-file"); + TEST_COMPARE (chmod_func (fd, path_to_file, 1, 0), 0); + xstat (path_file, &st); + TEST_COMPARE (st.st_mode & 0777, 1); + xlstat (path_to_file, &st); + TEST_COMPARE (original_symlink_mode, st.st_mode); + + /* Changing the mode of a symbolic link may fail. */ + arg = select_path (do_relative_path, path_to_file, "to-file"); + ret = chmod_func (fd, arg, 2, AT_SYMLINK_NOFOLLOW); + if (nofollow) + { + TEST_COMPARE (ret, 0); + + /* The mode of the link changed. */ + xlstat (path_to_file, &st); + TEST_COMPARE (st.st_mode & 0777, 2); + + /* But the mode of the file is unchanged. */ + xstat (path_file, &st); + TEST_COMPARE (st.st_mode & 0777, 1); + + } + else + { + TEST_COMPARE (ret, -1); + TEST_COMPARE (errno, EOPNOTSUPP); + + /* The modes should remain unchanged. */ + xstat (path_file, &st); + TEST_COMPARE (st.st_mode & 0777, 1); + xlstat (path_to_file, &st); + TEST_COMPARE (original_symlink_mode, st.st_mode); + } + + /* If we have NOFOLLOW support, we should be able to change the mode + of a dangling symbolic link or a symbolic link loop. */ + const char *paths[] = { path_dangling, path_loop }; + for (size_t i = 0; i < array_length (paths); ++i) + { + const char *path = paths[i]; + const char *filename = strrchr (path, '/'); + TEST_VERIFY_EXIT (filename != NULL); + ++filename; + mode_t new_mode = 010 + i; + + xlstat (path, &st); + TEST_VERIFY ((st.st_mode & 0777) != new_mode); + original_symlink_mode = st.st_mode; + arg = select_path (do_relative_path, path, filename); + ret = chmod_func (fd, arg, new_mode, AT_SYMLINK_NOFOLLOW); + if (nofollow) + { + TEST_COMPARE (ret, 0); + xlstat (path, &st); + TEST_COMPARE (st.st_mode & 0777, new_mode); + } + else /* !nofollow. */ + { + TEST_COMPARE (ret, -1); + TEST_COMPARE (errno, EOPNOTSUPP); + xlstat (path, &st); + TEST_COMPARE (st.st_mode, original_symlink_mode); + } + } + + /* A missing file should always result in ENOENT. The presence of + /proc does not matter. */ + arg = select_path (do_relative_path, path_missing, "missing"); + TEST_COMPARE (chmod_func (fd, arg, 020, 0), -1); + TEST_COMPARE (errno, ENOENT); + TEST_COMPARE (chmod_func (fd, arg, 020, AT_SYMLINK_NOFOLLOW), -1); + TEST_COMPARE (errno, ENOENT); + + /* Test without available file descriptors. */ + { + struct fd_list fd_list; + fd_list_init (&fd_list); + while (true) + { + int ret = dup (STDOUT_FILENO); + if (ret == -1) + { + if (errno == ENFILE || errno == EMFILE) + break; + FAIL_EXIT1 ("dup: %m"); + } + fd_list_add (&fd_list, ret); + TEST_VERIFY_EXIT (!fd_list_has_failed (&fd_list)); + } + /* Without AT_SYMLINK_NOFOLLOW, changing the permissions should + work as before. */ + arg = select_path (do_relative_path, path_file, "file"); + TEST_COMPARE (chmod_func (fd, arg, 3, 0), 0); + xstat (path_file, &st); + TEST_COMPARE (st.st_mode & 0777, 3); + /* But with AT_SYMLINK_NOFOLLOW, even if we originally had + support, we may have lost it. */ + ret = chmod_func (fd, arg, 2, AT_SYMLINK_NOFOLLOW); + if (ret == 0) + { + xstat (path_file, &st); + TEST_COMPARE (st.st_mode & 0777, 2); + } + else + { + TEST_COMPARE (ret, -1); + /* The error code from the openat fallback leaks out. */ + if (errno != ENFILE && errno != EMFILE) + TEST_COMPARE (errno, EOPNOTSUPP); + } + xstat (path_file, &st); + TEST_COMPARE (st.st_mode & 0777, 3); + + /* Close the descriptors. */ + for (int *pfd = fd_list_begin (&fd_list); pfd < fd_list_end (&fd_list); + ++pfd) + xclose (*pfd); + fd_list_free (&fd_list); + } + + if (do_relative_path) + xclose (fd); + + free (path_dangling); + free (path_file); + free (path_loop); + free (path_missing); + free (path_to_file); + + free (tempdir); +} + +static void +test_3 (void) +{ + puts ("info: testing lchmod"); + test_1 (false, fchmodat_with_lchmod); + puts ("info: testing fchmodat with AT_FDCWD"); + test_1 (false, fchmodat); + puts ("info: testing fchmodat with relative path"); + test_1 (true, fchmodat); +} + +static int +do_test (void) +{ + struct support_descriptors *descriptors = support_descriptors_list (); + + /* Run the three tests in the default environment. */ + test_3 (); + + /* Try to set up a /proc-less environment and re-test. */ +#if __has_include () + if (!support_become_root ()) + puts ("warning: could not obtain root-like privileges"); + if (!support_enter_mount_namespace ()) + puts ("warning: could enter a mount namespace"); + else + { + /* Attempt to mount an empty directory over /proc. */ + char *tempdir = support_create_temp_directory ("tst-lchmod-"); + bool proc_emptied + = mount (tempdir, "/proc", "none", MS_BIND, NULL) == 0; + if (!proc_emptied) + printf ("warning: bind-mounting /proc failed: %m"); + free (tempdir); + + puts ("info: re-running tests (after trying to empty /proc)"); + test_3 (); + + if (proc_emptied) + /* Reveal the original /proc, which is needed by the + descriptors check below. */ + TEST_COMPARE (umount ("/proc"), 0); + } +#endif /* . */ + + support_descriptors_check (descriptors); + support_descriptors_free (descriptors); + + return 0; +} + +#include