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

[Mario Castelán Castro]

URL:
  <http://savannah.gnu.org/bugs/?27923>

                 Summary: rm -f gives an error when trying to delete a
non-existent file on a read-only filesystem
                 Project: GNU Core Utilities
            Submitted by: sbd
            Submitted on: Tue 03 Nov 2009 10:32:18 PM NZDT
                Category: None
                Severity: 3 - Normal
              Item Group: None
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any

    _______________________________________________________

Details:

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.)

(coreutils version 7.4)




    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?27923>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/

[Jim Meyering]

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.

From 77b6a86cf1cc87635793202ec869528f07f5fba3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@...>
Date: Tue, 3 Nov 2009 12:01:40 +0100
Subject: [PATCH] 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 ++++++
 src/remove.c |   12 ++++++++++++
 2 files changed, 18 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/src/remove.c b/src/remove.c
index 87fb32b..a234829 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 ( ! (fstatat (fts->fts_cwd_fd, ent->fts_accpath, &st,
+                       AT_SYMLINK_NOFOLLOW) && errno == ENOENT))
+        errno = EROFS;
+    }
+
   if (ignorable_missing (x, errno))
     return RM_OK;

--
1.6.5.2.292.g1cda2

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Jim Meyering on 11/3/2009 4:04 AM:
> +  if (errno == EROFS)
> +    {
> +      struct stat st;
> +      if ( ! (fstatat (fts->fts_cwd_fd, ent->fts_accpath, &st,
> +                       AT_SYMLINK_NOFOLLOW) && errno == ENOENT))

Why not use lstatat, rather than fstatat(,AT_SYMLINK_NOFOLLOW)?

- --
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9@...
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkrwKRcACgkQ84KuGfSFAYCyCACfcO+1LhRbfVFWPkAMAN05Xo9E
kbwAn1Ck3ShDKV9SB/svSDK7j52fsFXP
=vzWQ
-----END PGP SIGNATURE-----

[Jim Meyering]

Eric Blake wrote:
> According to Jim Meyering on 11/3/2009 4:04 AM:
>> +  if (errno == EROFS)
>> +    {
>> +      struct stat st;
>> +      if ( ! (fstatat (fts->fts_cwd_fd, ent->fts_accpath, &st,
>> +                       AT_SYMLINK_NOFOLLOW) && errno == ENOENT))
>
> Why not use lstatat, rather than fstatat(,AT_SYMLINK_NOFOLLOW)?

It's equivalent and more readable, but not standard (albeit guaranteed
by gnulib), and there is no other use in coreutils proper.

To be honest, I didn't even think about it.

But I do prefer it, so have adjusted.
Thanks.

Here's the incremental, from my git diff HEAD@{5} HEAD@{4}

diff --git a/src/remove.c b/src/remove.c
index a234829..c4b93fe 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -444,8 +444,8 @@ excise (FTS *fts, FTSENT *ent, struct rm_options const *x, bool is_dir)
   if (errno == EROFS)
     {
       struct stat st;
-      if ( ! (fstatat (fts->fts_cwd_fd, ent->fts_accpath, &st,
-                       AT_SYMLINK_NOFOLLOW) && errno == ENOENT))
+      if ( ! (lstatat (fts->fts_cwd_fd, ent->fts_accpath, &st)
+                       && errno == ENOENT))
         errno = EROFS;
     }

[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 <meyering@...>
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                     smooge@...
 Steve McConnel                      steve@...
 Steve McIntyre                      steve@...
 Steve Ward                          planet36@...
+Steven Drake                        sbd@...
 Steven G. Johnson                   stevenj@...
 Steven Mocking                      ufo@...
 Steven Parkes                       smparkes@...
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 <meyering@...>
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

[Mario Castelán Castro]

Update of bug #27923 (project coreutils):

                  Status:                    None => Fixed                  
             Open/Closed:                    Open => Closed                

    _______________________________________________________

Follow-up Comment #1:

Thanks again.
Fixed for upcoming coreutils-8.1

http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/18786/focus=18798

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?27923>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/