From debbugs-submit-bounces@debbugs.gnu.org Sun Aug 15 16:16:11 2021 Received: (at submit) by debbugs.gnu.org; 15 Aug 2021 20:16:11 +0000 Received: from localhost ([127.0.0.1]:47824 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mFMYE-0005aD-Sw for submit@debbugs.gnu.org; Sun, 15 Aug 2021 16:16:11 -0400 Received: from lists.gnu.org ([209.51.188.17]:41172) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mFMYE-0005a7-3w for submit@debbugs.gnu.org; Sun, 15 Aug 2021 16:16:10 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:45916) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mFMYD-0006zY-SL for bug-coreutils@gnu.org; Sun, 15 Aug 2021 16:16:09 -0400 Received: from mail-ej1-x62e.google.com ([2a00:1450:4864:20::62e]:36695) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mFMYA-0004BL-9b for bug-coreutils@gnu.org; Sun, 15 Aug 2021 16:16:09 -0400 Received: by mail-ej1-x62e.google.com with SMTP id bt14so12360335ejb.3 for ; Sun, 15 Aug 2021 13:16:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:date:mime-version:user-agent:from:subject:to :content-transfer-encoding; bh=WuYhmvbLQZIY8g6XJzHdeB93hXABHVlgoJ1BS2XoZHQ=; b=FoP4oR5aNdAaVh3wBKHcg4w/lrEezRVDydF4gwpv7zXoOcOD4vXx+i+HRc1hBhwvjd kb0hDxJq++tGhte/nnfGqDuUhwqp/W8aQSc1LbLgmXoZMDGRHJvrj+M5O9EnPMJ9vxzn szS2oJXKb8YlsELcbtmAEQj97pqC4oSRjH+cm3JUJK2o/2Pt8i2NrHVKKlzskeduYCRm M01/WQ4/lOFrklAWn+u9og+hisem7ts3gYgWn5IC+3R2VdbTgUhWE1iu/TXsj6woJH+y nf6bWeAw7AI7TivNNX/EiSJyHZBOt5JVl4reA8M4hkRE2qe/QEH9KsktyxrWWfpa26ux HcVw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:date:mime-version:user-agent:from :subject:to:content-transfer-encoding; bh=WuYhmvbLQZIY8g6XJzHdeB93hXABHVlgoJ1BS2XoZHQ=; b=suYcKIktmZjEUai5c2b2Tgy6ZpzJgcyt4f6MvMEoAS0aijsK4IM6sMtkVQO7R/OtlN ywTVmlXdQEM7cpHLRBXVxSL/yD2pvHKccjWmCwkBWHduZVIphae+KVAp/ZgXIZQtsg// ST2PKFjUe5yBFooEtxq1J6T5l+ksKODhR5dEv/puuhqWcPdTpXGKl2y1kmN3gRyWxT1D +Gw7dcOUPIvpAMg0Br0bKm0o7F6lWuJ6av1DA5W8XlVYcah44b4RFtivCeoXs/kBu+2n jNUWAwR/IGgRVlSK3Px3ZZI2DWDsc8yU2X+Nz/UhD3ORwcnWMQKE7QXNkwai0bEMohVZ itig== X-Gm-Message-State: AOAM531TETsJAOTB0ubh79MJa8d0qzcUDPZqrJI5zn4JG9APclT6IUcC DYFB3s0VosEsSOISbKjMN4/R0P4ywu4= X-Google-Smtp-Source: ABdhPJyyIzm54AQ8t4gpAFeCKoR/vrqsu92yVciZnO1FJNwZWAHGWKE+vU7j1V78pULLitIdRO1q+g== X-Received: by 2002:a17:906:c246:: with SMTP id bl6mr7242636ejb.80.1629058560408; Sun, 15 Aug 2021 13:16:00 -0700 (PDT) Received: from [192.168.174.179] ([151.68.102.165]) by smtp.gmail.com with UTF8SMTPSA id x17sm1080455ejj.58.2021.08.15.13.15.59 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 15 Aug 2021 13:16:00 -0700 (PDT) Message-ID: Date: Sun, 15 Aug 2021 22:15:58 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:93.0) Gecko/20100101 Thunderbird/93.0a1 From: Michael Debertol Subject: chmod reads uninitialized variables when fts_info is an error and -v is set, leading to random error messages To: bug-coreutils@gnu.org Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=2a00:1450:4864:20::62e; envelope-from=michael.debertol@gmail.com; helo=mail-ej1-x62e.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: -1.3 (-) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.3 (--) Hi, I noticed that when running chmod with -v on a dangling symlink the error message is somewhat random: > ln -s file-that-does-not-exist lnk > chmod -w lnk -v > chmod: cannot operate on dangling symlink 'lnk' > failed to change mode of 'lnk' from 0000 (---------) to 7777 (rwsrwsrwt) > chmod -w lnk -v > chmod: cannot operate on dangling symlink 'lnk' > failed to change mode of 'lnk' from 0000 (---------) to 7775 (rwsrwsr-t) > chmod -w lnk -v > chmod: cannot operate on dangling symlink 'lnk' > failed to change mode of 'lnk' from 0000 (---------) to 7774 (rwsrwsr-T) (note that the value for "to" is different) This appears to be because in the process_file function, old_mode and new_mode are only set when ok is true. When chmod encounters a dangling symlink, ok will be false and old_mode/new_mode won't be set. However, old_mode and new_mode are still accessed for the invocation of describe_change if verbosity is set to high, leading to incorrect error messages. I think a better behavior could be to not print the "from mode to mode" part of the error message if the file could not be accessed, since it is meaningless anyways. Have a nice day, Michael From debbugs-submit-bounces@debbugs.gnu.org Mon Aug 16 00:33:40 2021 Received: (at 50070-done) by debbugs.gnu.org; 16 Aug 2021 04:33:40 +0000 Received: from localhost ([127.0.0.1]:48170 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mFUJf-0002z2-Is for submit@debbugs.gnu.org; Mon, 16 Aug 2021 00:33:40 -0400 Received: from zimbra.cs.ucla.edu ([131.179.128.68]:33654) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mFUJd-0002yk-9E for 50070-done@debbugs.gnu.org; Mon, 16 Aug 2021 00:33:38 -0400 Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 50E74160051; Sun, 15 Aug 2021 21:33:31 -0700 (PDT) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id uGr-qFEgsv4G; Sun, 15 Aug 2021 21:33:26 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 9180316005D; Sun, 15 Aug 2021 21:33:26 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id 9aVH0bxrqniu; Sun, 15 Aug 2021 21:33:26 -0700 (PDT) Received: from [192.168.1.9] (cpe-172-91-119-151.socal.res.rr.com [172.91.119.151]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 623B4160051; Sun, 15 Aug 2021 21:33:26 -0700 (PDT) Subject: Re: bug#50070: chmod reads uninitialized variables when fts_info is an error and -v is set, leading to random error messages To: Michael Debertol References: From: Paul Eggert Organization: UCLA Computer Science Department Message-ID: <6eabb0d5-0ce2-a52e-3404-4feff67c3b9a@cs.ucla.edu> Date: Sun, 15 Aug 2021 21:33:26 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------7AB6E7B76E4FE349C0E24E5C" Content-Language: en-US X-Spam-Score: -2.4 (--) X-Debbugs-Envelope-To: 50070-done Cc: 50070-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.4 (---) This is a multi-part message in MIME format. --------------7AB6E7B76E4FE349C0E24E5C Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Thanks for reporting the bug. I installed the attached patch to fix it. --------------7AB6E7B76E4FE349C0E24E5C Content-Type: text/x-patch; charset=UTF-8; name="0001-chmod-fix-use-of-uninitialized-var-if-v.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-chmod-fix-use-of-uninitialized-var-if-v.patch" >From af4711d5dc15f0c014bfd9c92f4eadedb89f732f Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sun, 15 Aug 2021 21:29:38 -0700 Subject: [PATCH] chmod: fix use of uninitialized var if -v Problem reported by Michael Debertol (Bug#50070). * NEWS: Mention the fix. * src/chmod.c (struct change_status): New struct, replacing the old enum Change_status. All uses changed. (describe_change): Distinguish between cases depending on whether 'stat' or its equivalent succeeded. Report a line of output even if 'stat' failed, as that matches the documentation. Rework to avoid casts. (process_file): Do not output nonsense modes computed from uninitialized storage, removing a couple of IF_LINTs. Simplify by defaulting to CH_NO_STAT. --- NEWS | 3 ++ src/chmod.c | 127 ++++++++++++++++++++++++++-------------------------- 2 files changed, 67 insertions(+), 63 deletions(-) diff --git a/NEWS b/NEWS index e14de1397..ddec56bdf 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,9 @@ GNU coreutils NEWS -*- outline -*- ** Bug fixes + chmod -v no longer misreports modes of dangling symlinks. + [bug introduced in coreutils-5.3.0] + cp -a --attributes-only now never removes destination files, even if the destination files are hardlinked, or the source is a non regular file. diff --git a/src/chmod.c b/src/chmod.c index 160a0c537..4447e784e 100644 --- a/src/chmod.c +++ b/src/chmod.c @@ -39,12 +39,19 @@ proper_name ("David MacKenzie"), \ proper_name ("Jim Meyering") -enum Change_status +struct change_status { - CH_NOT_APPLIED, - CH_SUCCEEDED, - CH_FAILED, - CH_NO_CHANGE_REQUESTED + enum + { + CH_NO_STAT, + CH_NOT_APPLIED, + CH_FAILED, + CH_NO_CHANGE_REQUESTED, + CH_SUCCEEDED + } + status; + mode_t old_mode; + mode_t new_mode; }; enum Verbosity @@ -136,30 +143,42 @@ mode_changed (int dir_fd, char const *file, char const *file_full_name, } /* Tell the user how/if the MODE of FILE has been changed. - CHANGED describes what (if anything) has happened. */ + CH describes what (if anything) has happened. */ static void -describe_change (char const *file, mode_t old_mode, mode_t mode, - enum Change_status changed) +describe_change (char const *file, struct change_status const *ch) { char perms[12]; /* "-rwxrwxrwx" ls-style modes. */ char old_perms[12]; char const *fmt; + char const *quoted_file = quoteaf (file); - if (changed == CH_NOT_APPLIED) + switch (ch->status) { + case CH_NOT_APPLIED: printf (_("neither symbolic link %s nor referent has been changed\n"), - quoteaf (file)); + quoted_file); return; - } - strmode (mode, perms); + case CH_NO_STAT: + printf (_("%s could not be accessed\n"), quoted_file); + return; + + default: + break; + } + + unsigned long int + old_m = ch->old_mode & CHMOD_MODE_BITS, + m = ch->new_mode & CHMOD_MODE_BITS; + + strmode (ch->new_mode, perms); perms[10] = '\0'; /* Remove trailing space. */ - strmode (old_mode, old_perms); + strmode (ch->old_mode, old_perms); old_perms[10] = '\0'; /* Remove trailing space. */ - switch (changed) + switch (ch->status) { case CH_SUCCEEDED: fmt = _("mode of %s changed from %04lo (%s) to %04lo (%s)\n"); @@ -169,15 +188,12 @@ describe_change (char const *file, mode_t old_mode, mode_t mode, break; case CH_NO_CHANGE_REQUESTED: fmt = _("mode of %s retained as %04lo (%s)\n"); - printf (fmt, quoteaf (file), - (unsigned long int) (mode & CHMOD_MODE_BITS), &perms[1]); + printf (fmt, quoted_file, m, &perms[1]); return; default: abort (); } - printf (fmt, quoteaf (file), - (unsigned long int) (old_mode & CHMOD_MODE_BITS), &old_perms[1], - (unsigned long int) (mode & CHMOD_MODE_BITS), &perms[1]); + printf (fmt, quoted_file, old_m, &old_perms[1], m, &perms[1]); } /* Change the mode of FILE. @@ -190,10 +206,8 @@ process_file (FTS *fts, FTSENT *ent) char const *file_full_name = ent->fts_path; char const *file = ent->fts_accpath; const struct stat *file_stats = ent->fts_statp; - mode_t old_mode IF_LINT ( = 0); - mode_t new_mode IF_LINT ( = 0); - bool ok = true; - bool chmod_succeeded = false; + struct change_status ch; + ch.status = CH_NO_STAT; switch (ent->fts_info) { @@ -217,27 +231,23 @@ process_file (FTS *fts, FTSENT *ent) if (! force_silent) error (0, ent->fts_errno, _("cannot access %s"), quoteaf (file_full_name)); - ok = false; break; case FTS_ERR: if (! force_silent) error (0, ent->fts_errno, "%s", quotef (file_full_name)); - ok = false; break; case FTS_DNR: if (! force_silent) error (0, ent->fts_errno, _("cannot read directory %s"), quoteaf (file_full_name)); - ok = false; break; case FTS_SLNONE: if (! force_silent) error (0, 0, _("cannot operate on dangling symlink %s"), quoteaf (file_full_name)); - ok = false; break; case FTS_DC: /* directory that causes cycles */ @@ -246,13 +256,14 @@ process_file (FTS *fts, FTSENT *ent) emit_cycle_warning (file_full_name); return false; } - break; - + FALLTHROUGH; default: + ch.status = CH_NOT_APPLIED; break; } - if (ok && ROOT_DEV_INO_CHECK (root_dev_ino, file_stats)) + if (ch.status == CH_NOT_APPLIED + && ROOT_DEV_INO_CHECK (root_dev_ino, file_stats)) { ROOT_DEV_INO_WARN (file_full_name); /* Tell fts not to traverse into this hierarchy. */ @@ -262,66 +273,56 @@ process_file (FTS *fts, FTSENT *ent) return false; } - if (ok) + if (ch.status == CH_NOT_APPLIED && ! S_ISLNK (file_stats->st_mode)) { - old_mode = file_stats->st_mode; - new_mode = mode_adjust (old_mode, S_ISDIR (old_mode) != 0, umask_value, - change, NULL); - - if (! S_ISLNK (old_mode)) + ch.old_mode = file_stats->st_mode; + ch.new_mode = mode_adjust (ch.old_mode, S_ISDIR (ch.old_mode) != 0, + umask_value, change, NULL); + if (chmodat (fts->fts_cwd_fd, file, ch.new_mode) == 0) + ch.status = CH_SUCCEEDED; + else { - if (chmodat (fts->fts_cwd_fd, file, new_mode) == 0) - chmod_succeeded = true; - else - { - if (! force_silent) - error (0, errno, _("changing permissions of %s"), - quoteaf (file_full_name)); - ok = false; - } + if (! force_silent) + error (0, errno, _("changing permissions of %s"), + quoteaf (file_full_name)); + ch.status = CH_FAILED; } } if (verbosity != V_off) { - bool changed = (chmod_succeeded - && mode_changed (fts->fts_cwd_fd, file, file_full_name, - old_mode, new_mode)); + if (ch.status == CH_SUCCEEDED + && !mode_changed (fts->fts_cwd_fd, file, file_full_name, + ch.old_mode, ch.new_mode)) + ch.status = CH_NO_CHANGE_REQUESTED; - if (changed || verbosity == V_high) - { - enum Change_status ch_status = - (!ok ? CH_FAILED - : !chmod_succeeded ? CH_NOT_APPLIED - : !changed ? CH_NO_CHANGE_REQUESTED - : CH_SUCCEEDED); - describe_change (file_full_name, old_mode, new_mode, ch_status); - } + if (ch.status == CH_SUCCEEDED || verbosity == V_high) + describe_change (file_full_name, &ch); } - if (chmod_succeeded && diagnose_surprises) + if (CH_NO_CHANGE_REQUESTED <= ch.status && diagnose_surprises) { mode_t naively_expected_mode = - mode_adjust (old_mode, S_ISDIR (old_mode) != 0, 0, change, NULL); - if (new_mode & ~naively_expected_mode) + mode_adjust (ch.old_mode, S_ISDIR (ch.old_mode) != 0, 0, change, NULL); + if (ch.new_mode & ~naively_expected_mode) { char new_perms[12]; char naively_expected_perms[12]; - strmode (new_mode, new_perms); + strmode (ch.new_mode, new_perms); strmode (naively_expected_mode, naively_expected_perms); new_perms[10] = naively_expected_perms[10] = '\0'; error (0, 0, _("%s: new permissions are %s, not %s"), quotef (file_full_name), new_perms + 1, naively_expected_perms + 1); - ok = false; + ch.status = CH_FAILED; } } if ( ! recurse) fts_set (fts, ent, FTS_SKIP); - return ok; + return CH_NO_CHANGE_REQUESTED <= ch.status; } /* Recursively change the modes of the specified FILES (the last entry -- 2.30.2 --------------7AB6E7B76E4FE349C0E24E5C-- From unknown Sat Jun 14 05:31:18 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Mon, 13 Sep 2021 11:24:08 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator