From nobody Thu Jan 27 18:02:41 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id A2D84198EA9D; Thu, 27 Jan 2022 18:02:41 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Jl7jF41TQz3K0g; Thu, 27 Jan 2022 18:02:41 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1643306561; 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; bh=I63tpmmloW4miAIzEminSUWTZtw0lWe6dd1AV8MnbKI=; b=JLkWbiYcvCVWmKB0mKha9gbt4XmCei7U87gm/Atm/gJ/rw1RZ07WmyI00VRh4hDjpjLumL +bz+s2fbJNfaRM4En9mGe9oo28hWSYd0oVplsDVMF0aSqtb6dmT4XuLJfny7gofbuqiL5p W1JlrncEcHcAcd9ZefRZevQmZin+VA6ZIptPQ75kYacgXyOMFuH+48bu0xjUaGHoV3wp4D SAg8cptlf7yy9OzAx3VP/EsCyEgfTEFleKRrN4EkZqRL5+uI2wYnL+c2gWfQfARQzFjtSE C7A7ILKs1MDDAqv1Q4pPT+RAcV9lpBL/O+a8ppadPVfJP2SMER95UIZIoSPAOg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 696B61B4A1; Thu, 27 Jan 2022 18:02:41 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 20RI2fJo035361; Thu, 27 Jan 2022 18:02:41 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 20RI2frC035360; Thu, 27 Jan 2022 18:02:41 GMT (envelope-from git) Date: Thu, 27 Jan 2022 18:02:41 GMT Message-Id: <202201271802.20RI2frC035360@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kyle Evans Subject: git: 848263aad129 - main - cp: fix some cases with infinite recursion List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kevans X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 848263aad129c8f9de75b58a5ab9a010611b75ac Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1643306561; 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; bh=I63tpmmloW4miAIzEminSUWTZtw0lWe6dd1AV8MnbKI=; b=wK/+6W31SdZkh1NtWHuUAyltKBgYUnKlRexH6zc/sCo/CvSMA7H6nPoVwNkSDoyv9o4gtc mAFBD23TOQLBf1HX11x/tLp+jPXLLFYwMamGpQe1KFBUooqJ0eJVnhcwV9dTaRZt7PM2w0 jFGP+UzzYQLsE/6jeyNRoIbb994V0NdzkI8buba38QbBF7x37vHakxSJt97Af1UFOrPtJT 8fmeE46gMzc4nsk8dqN3s+zIKr3eZqEQyO1WaWZlFexRLb17GRvBN443b4AUqK09irDooL uW+NRRJTRqx/78FqbFe+ZuVXtnvvhTWLoxVG6QdWU+NH+Z2GYSaYlZXZeRjzpQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1643306561; a=rsa-sha256; cv=none; b=XRY9HQ1/KYkw2FqINPhVeNnbopE/Aql0dAgxrR2uRUo6W6zf26slDbje8Z34BXlflGeY5B SIAGB7KEnYd1FrsnhVL0pTOiAHh2EFdwB13/+FAhcps7TsJSehikwl6s2kEiMtvZ4o0yj1 GHTlrck/Su8dGokK3qiE9Jh8DcPeSusJ2z504x9yHfUEA/5tO4JWN3BN3VWsdORY3y5tRB p9BS5g9q/t7OMFBrhSMmxiWyO5WJ08maWlFWNSrTykQmc8MVM6geEV99XJZsalJnoPwnPE 8xAKbjwzDJOFHR7GZ3neVXnaA8WAV2fwwY1HnhxUiY7iJpaDaIzSMGcpRggY/Q== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=848263aad129c8f9de75b58a5ab9a010611b75ac commit 848263aad129c8f9de75b58a5ab9a010611b75ac Author: Kyle Evans AuthorDate: 2022-01-27 18:02:17 +0000 Commit: Kyle Evans CommitDate: 2022-01-27 18:02:17 +0000 cp: fix some cases with infinite recursion As noted in the PR, cp -R has some surprising behavior. Typically, when you `cp -R foo bar` where both foo and bar exist, foo is cleanly copied to foo/bar. When you `cp -R foo foo` (where foo clearly exists), cp(1) goes a little off the rails as it creates foo/foo, then discovers that and creates foo/foo/foo, so on and so forth, until it eventually fails. POSIX doesn't seem to disallow this behavior, but it isn't very useful. GNU cp(1) will detect the recursion and squash it, but emit a message in the process that it has done so. This change seemingly follows the GNU behavior, but it currently doesn't warn about the situation -- the author feels that the final product is about what one might expect from doing this and thus, doesn't need a warning. The author doesn't feel strongly about this. PR: 235438 Reviewed by: bapt Sponsored by: Klara, Inc. Differential Revision: https://reviews.freebsd.org/D33944 --- bin/cp/cp.c | 75 +++++++++++++++++++++++++++++++++++++++++++---- bin/cp/tests/cp_test.sh | 77 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 5 deletions(-) diff --git a/bin/cp/cp.c b/bin/cp/cp.c index 3a23394df35d..4328b89cfd7a 100644 --- a/bin/cp/cp.c +++ b/bin/cp/cp.c @@ -64,6 +64,7 @@ __FBSDID("$FreeBSD$"); #include #include +#include #include #include #include @@ -91,7 +92,7 @@ volatile sig_atomic_t info; enum op { FILE_TO_FILE, FILE_TO_DIR, DIR_TO_DNE }; -static int copy(char *[], enum op, int); +static int copy(char *[], enum op, int, struct stat *); static void siginfo(int __unused); int @@ -259,18 +260,26 @@ main(int argc, char *argv[]) */ type = FILE_TO_DIR; - exit (copy(argv, type, fts_options)); + /* + * For DIR_TO_DNE, we could provide copy() with the to_stat we've + * already allocated on the stack here that isn't being used for + * anything. Not doing so, though, simplifies later logic a little bit + * as we need to skip checking root_stat on the first iteration and + * ensure that we set it with the first mkdir(). + */ + exit (copy(argv, type, fts_options, (type == DIR_TO_DNE ? NULL : + &to_stat))); } static int -copy(char *argv[], enum op type, int fts_options) +copy(char *argv[], enum op type, int fts_options, struct stat *root_stat) { - struct stat to_stat; + struct stat created_root_stat, to_stat; FTS *ftsp; FTSENT *curr; int base = 0, dne, badcp, rval; size_t nlen; - char *p, *target_mid; + char *p, *recurse_path, *target_mid; mode_t mask, mode; /* @@ -280,6 +289,7 @@ copy(char *argv[], enum op type, int fts_options) mask = ~umask(0777); umask(~mask); + recurse_path = NULL; if ((ftsp = fts_open(argv, fts_options, NULL)) == NULL) err(1, "fts_open"); for (badcp = rval = 0; errno = 0, (curr = fts_read(ftsp)) != NULL; @@ -300,6 +310,47 @@ copy(char *argv[], enum op type, int fts_options) ; } + if (curr->fts_info == FTS_D && type != FILE_TO_FILE && + root_stat != NULL && + root_stat->st_dev == curr->fts_statp->st_dev && + root_stat->st_ino == curr->fts_statp->st_ino) { + assert(recurse_path == NULL); + if (curr->fts_level > FTS_ROOTLEVEL) { + /* + * If the recursion isn't at the immediate + * level, we can just not traverse into this + * directory. + */ + fts_set(ftsp, curr, FTS_SKIP); + continue; + } else { + const char *slash; + + /* + * Grab the last path component and double it, + * to make life easier later and ensure that + * we work even with fts_level == 0 is a couple + * of components deep in fts_path. No path + * separators are fine and expected in the + * common case, though. + */ + slash = strrchr(curr->fts_path, '/'); + if (slash != NULL) + slash++; + else + slash = curr->fts_path; + if (asprintf(&recurse_path, "%s/%s", + curr->fts_path, slash) == -1) + err(1, "asprintf"); + } + } + + if (recurse_path != NULL && + strcmp(curr->fts_path, recurse_path) == 0) { + fts_set(ftsp, curr, FTS_SKIP); + continue; + } + /* * If we are in case (2) or (3) above, we need to append the * source name to the target name. @@ -448,6 +499,19 @@ copy(char *argv[], enum op type, int fts_options) if (mkdir(to.p_path, curr->fts_statp->st_mode | S_IRWXU) < 0) err(1, "%s", to.p_path); + /* + * First DNE with a NULL root_stat is the root + * path, so set root_stat. We can't really + * tell in all cases if the target path is + * within the src path, so we just stat() the + * first directory we created and use that. + */ + if (root_stat == NULL && + stat(to.p_path, &created_root_stat) == -1) { + err(1, "stat"); + } else if (root_stat == NULL) { + root_stat = &created_root_stat; + } } else if (!S_ISDIR(to_stat.st_mode)) { errno = ENOTDIR; err(1, "%s", to.p_path); @@ -493,6 +557,7 @@ copy(char *argv[], enum op type, int fts_options) if (errno) err(1, "fts_read"); fts_close(ftsp); + free(recurse_path); return (rval); } diff --git a/bin/cp/tests/cp_test.sh b/bin/cp/tests/cp_test.sh index 753e69f2d619..3220b45b4438 100755 --- a/bin/cp/tests/cp_test.sh +++ b/bin/cp/tests/cp_test.sh @@ -57,8 +57,85 @@ chrdev_body() check_size trunc 0 } +atf_test_case matching_srctgt +matching_srctgt_body() +{ + + # PR235438: `cp -R foo foo` would previously infinitely recurse and + # eventually error out. + mkdir foo + echo "qux" > foo/bar + cp foo/bar foo/zoo + + atf_check cp -R foo foo + atf_check -o inline:"qux\n" cat foo/foo/bar + atf_check -o inline:"qux\n" cat foo/foo/zoo + atf_check -e not-empty -s not-exit:0 stat foo/foo/foo +} + +atf_test_case matching_srctgt_contained +matching_srctgt_contained_body() +{ + + # Let's do the same thing, except we'll try to recursively copy foo into + # one of its subdirectories. + mkdir foo + echo "qux" > foo/bar + mkdir foo/loo + mkdir foo/moo + mkdir foo/roo + cp foo/bar foo/zoo + + atf_check cp -R foo foo/moo + atf_check -o inline:"qux\n" cat foo/moo/foo/bar + atf_check -o inline:"qux\n" cat foo/moo/foo/zoo + atf_check -e not-empty -s not-exit:0 stat foo/moo/foo/moo +} + +atf_test_case matching_srctgt_link +matching_srctgt_link_body() +{ + + mkdir foo + echo "qux" > foo/bar + cp foo/bar foo/zoo + + atf_check ln -s foo roo + atf_check cp -RH roo foo + atf_check -o inline:"qux\n" cat foo/roo/bar + atf_check -o inline:"qux\n" cat foo/roo/zoo +} + +atf_test_case matching_srctgt_nonexistent +matching_srctgt_nonexistent_body() +{ + + # We'll copy foo to a nonexistent subdirectory; ideally, we would + # skip just the directory and end up with a layout like; + # + # foo/ + # bar + # dne/ + # bar + # zoo + # zoo + # + mkdir foo + echo "qux" > foo/bar + cp foo/bar foo/zoo + + atf_check cp -R foo foo/dne + atf_check -o inline:"qux\n" cat foo/dne/bar + atf_check -o inline:"qux\n" cat foo/dne/zoo + atf_check -e not-empty -s not-exit:0 stat foo/dne/foo +} + atf_init_test_cases() { atf_add_test_case basic atf_add_test_case chrdev + atf_add_test_case matching_srctgt + atf_add_test_case matching_srctgt_contained + atf_add_test_case matching_srctgt_link + atf_add_test_case matching_srctgt_nonexistent }