Re: [bug #27923] rm -f gives an error when trying to delete a non-existent file on a read-only filesystem

[Jim Meyering]

Jim Meyering wrote:

> Steven Drake wrote:
>>  Summary: rm -f gives an error when trying to delete a non-existent
>>           file on a read-only filesystem
>>
>> Eg:
>> /bin/rm: cannot remove `non-existent-file': Read-only file system.
>>
>> On a linux/glibc system the unlinkat syscall will set errno to EROFS and
>> nonexistent_file_errno() does not perform any checks
>> for this case.  (Another rm does an fts_open/fts_read and ignores
>> the file if fts_info is FST_NS before trying to unlink it.)
>
> Thanks for reporting that.
> It also affected the very latest versions.
>
> I expect to fix it with this patch once I've added a test.

Here's the change+test pair I'll push shortly:

>From 7bf2e3db23bd0a9ed59d95a683edd188fa52a033 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 3 Nov 2009 12:01:40 +0100
Subject: [PATCH 1/2] rm -f: ignore EROFS when it's really ENOENT

rm -f must not print a diagnostic for a nonexistent file.  However,
most linux-based kernel unlinkat functions set errno to EROFS when
the named file (regardless of whether it exists) would lie on a
read-only file system.  remove.c now performs an extra fstatat call
in that case, to determine whether the file exists.
* src/remove.c (excise): Map EROFS to ENOENT, if a file is nonexistent.
Reported by Steven Drake in <http://savannah.gnu.org/bugs/?27923>.
* NEWS (Changes in behavior): Mention it.
---
 NEWS         |    6 ++++++
 THANKS       |    1 +
 src/remove.c |   12 ++++++++++++
 3 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/NEWS b/NEWS
index 0760775..03ed83f 100644
--- a/NEWS
+++ b/NEWS
@@ -49,6 +49,12 @@ GNU coreutils NEWS                                    -*- 
outline -*-

   echo and printf now interpret \e as the Escape character (0x1B).

+  rm -f /read-only-fs/nonexistent now succeeds and prints no diagnostic
+  on systems with an unlinkat syscall that sets errno to EROFS in that case.
+  Before, it would fail with a "Read-only file system" diagnostic.
+  Also, "rm /read-only-fs/nonexistent" now reports "file not found" rather
+  than the less precise "Read-only file system" error.
+
 ** New features

   env and printenv now accept the option --null (-0), as a means to
diff --git a/THANKS b/THANKS
index 5efe2fa..c583e2a 100644
--- a/THANKS
+++ b/THANKS
@@ -541,6 +541,7 @@ Stephen Smoogen                     address@hidden
 Steve McConnel                      address@hidden
 Steve McIntyre                      address@hidden
 Steve Ward                          address@hidden
+Steven Drake                        address@hidden
 Steven G. Johnson                   address@hidden
 Steven Mocking                      address@hidden
 Steven Parkes                       address@hidden
diff --git a/src/remove.c b/src/remove.c
index 87fb32b..c4b93fe 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -437,6 +437,18 @@ excise (FTS *fts, FTSENT *ent, struct rm_options const *x, 
bool is_dir)
       return RM_OK;
     }

+  /* The unlinkat from kernels like linux-2.6.32 reports EROFS even for
+     nonexistent files.  When the file is indeed missing, map that to ENOENT,
+     so that rm -f ignores it, as required.  Even without -f, this is useful
+     because it makes rm print the more precise diagnostic.  */
+  if (errno == EROFS)
+    {
+      struct stat st;
+      if ( ! (lstatat (fts->fts_cwd_fd, ent->fts_accpath, &st)
+                       && errno == ENOENT))
+        errno = EROFS;
+    }
+
   if (ignorable_missing (x, errno))
     return RM_OK;

--
1.6.5.2.292.g1cda2


>From d4b96c26ce0e79cb64fc99dda56d795765d5656d Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 3 Nov 2009 14:30:56 +0100
Subject: [PATCH 2/2] tests: rm: add test for today's change in behavior

* tests/Makefile.am (root_tests): Add rm/read-only to the list.
* tests/rm/read-only: New file.
---
 tests/Makefile.am  |    1 +
 tests/rm/read-only |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 0 deletions(-)
 create mode 100755 tests/rm/read-only

diff --git a/tests/Makefile.am b/tests/Makefile.am
index eec31ae..f67b796 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -37,6 +37,7 @@ root_tests =                                  \
   rm/fail-2eperm                               \
   rm/no-give-up                                        \
   rm/one-file-system                           \
+  rm/read-only                                 \
   tail-2/append-only                           \
   touch/now-owned-by-other

diff --git a/tests/rm/read-only b/tests/rm/read-only
new file mode 100755
index 0000000..3c83aad
--- /dev/null
+++ b/tests/rm/read-only
@@ -0,0 +1,56 @@
+#!/bin/sh
+# Ensure that rm -f nonexistent-file-on-read-only-fs succeeds.
+
+# Copyright (C) 2009 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program 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 General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  rm --version
+fi
+
+. $srcdir/test-lib.sh
+require_root_
+
+cwd=`pwd`
+cleanup_() { cd /; umount "$cwd/mnt"; }
+
+skip=0
+# Create a file system, then mount it.
+dd if=/dev/zero of=blob bs=8192 count=200 > /dev/null 2>&1 \
+                                             || skip=1
+mkdir mnt                                    || skip=1
+mkfs -t ext2 -F blob \
+  || skip_test_ "failed to create ext2 file system"
+
+mount -oloop blob mnt                        || skip=1
+echo test > mnt/f                            || skip=1
+test -s mnt/f                                || skip=1
+mount -o remount,loop,ro mnt                 || skip=1
+
+test $skip = 1 \
+  && skip_test_ "insufficient mount/ext2 support"
+
+# Applying rm -f to a nonexistent file on a read-only file system must succeed.
+rm -f mnt/no-such > out 2>&1 || fail=1
+# It must produce no diagnostic.
+test -s out && fail=1
+
+# However, trying to remove an existing file must fail.
+rm -f mnt/f > out 2>&1 && fail=1
+# with a diagnostic.
+test -s out || fail=1
+
+Exit $fail
--
1.6.5.2.292.g1cda2