[Toybox] [PATCH 1/2] rm: Check file existence with lstat() explicitly if "-f" is specified

Yi-Yo Chiang yochiang at google.com
Mon Feb 1 11:22:02 PST 2021


Instead of unlink() && check errno, call lstat() explicitly to check
file existence if "-f" is specified.
There is a regression when if the path to be removed is nonexistence
and within a readonly filesystem, then unlink() could set the EROFS
errno instead of ENOENT, thus screwing up the output of `rm`.
---
 toys/posix/rm.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/toys/posix/rm.c b/toys/posix/rm.c
index eec586c5..b1fd0fee 100644
--- a/toys/posix/rm.c
+++ b/toys/posix/rm.c
@@ -90,6 +90,7 @@ nodelete:
 void rm_main(void)
 {
   char **s;
+  struct stat st;
 
   // Can't use <1 in optstring because zero arguments with -f isn't an error
   if (!toys.optc && !FLAG(f)) help_exit("Needs 1 argument");
@@ -105,9 +106,10 @@ void rm_main(void)
       continue;
     }
 
-    // Files that already don't exist aren't errors for -f, so try a quick
-    // unlink now to see if it succeeds or reports that it didn't exist.
-    if (FLAG(f) && (!unlink(*s) || errno == ENOENT)) continue;
+    // Files that already don't exist aren't errors for -f.
+    // We explicitly use lstat() but not faccessat() because Android bionic
+    // intentionally don't support AT_SYMLINK_NOFOLLOW.
+    if (FLAG(f) && lstat(*s, &st) && errno == ENOENT) continue;
 
     // There's a race here where a file removed between the above check and
     // dirtree's stat would report the nonexistence as an error, but that's
-- 
2.30.0.365.g02bc693789-goog



More information about the Toybox mailing list