Hi,

Gregoire Jadi wrote on Fri, Mar 30, 2018 at 06:07:42PM +0200:

> While working on a port of keyringer, I observed the following behavior
> of rm(1) with the -P option: if the file does not have write permission,
> the file is removed without being overwritten.
> 
> This is not the same behavior as shred(1) (from sysutils/coreutils) which
> do not remove the file if it cannot be overwritten. If the -f option is
> used with shred(1), the permission of the file are changed to allow
> writing if necessary.
> 
> Is the current behavior desired? (need to update the manpage?)
> 
> Or should `rm -P` and `rm -Pf` have the same behavior as shred. That is:
> `rm -P` should fail without removing the file if the file cannot be
> overwritten and `rm -Pf` should change the permission and overwrite
> file.

That makes more sense to me than the current behaviour.

In addition, when running with -P and without -f and the user
interactive confirms that the missing write protection shall be
overriden, changing the permissions seems right to me, too.

Here is a patch implementing that.

Lightly tested by hand by running various combinations of rm(1)
 * with and without -P
 * with and without -f
 * on files with different permissions

Feedback is welcome both on the general idea and on the specific
implementation.

Note that the return value of rm_overwrite() is currently unused.
So we are free to change it to reflect success or failure, such
that deleting the file can be skipped in case of failure even
after trying to relax the permissions.  Also note that checking
fflag in rw_overwrite is not needed (and would even be wrong)
because the program doesn't get there in the first place unless
fflag was given or deletion was interactively confirmed.

Yours,
  Ingo


Index: rm.c
===================================================================
RCS file: /cvs/src/bin/rm/rm.c,v
retrieving revision 1.42
diff -u -p -r1.42 rm.c
--- rm.c        27 Jun 2017 21:49:47 -0000      1.42
+++ rm.c        31 Mar 2018 17:02:37 -0000
@@ -103,7 +103,7 @@ main(int argc, char *argv[])
        argv += optind;
 
        if (Pflag) {
-               if (pledge("stdio rpath wpath cpath getpw", NULL) == -1)
+               if (pledge("stdio rpath wpath cpath fattr getpw", NULL) == -1)
                        err(1, "pledge");
        } else {
                if (pledge("stdio rpath cpath getpw", NULL) == -1)
@@ -213,9 +213,9 @@ rm_tree(char **argv)
 
                case FTS_F:
                case FTS_NSOK:
-                       if (Pflag)
-                               rm_overwrite(p->fts_accpath, p->fts_info ==
-                                   FTS_NSOK ? NULL : p->fts_statp);
+                       if (Pflag && !rm_overwrite(p->fts_accpath,
+                           p->fts_info == FTS_NSOK ? NULL : p->fts_statp))
+                               continue;
                        /* FALLTHROUGH */
                default:
                        if (!unlink(p->fts_accpath) ||
@@ -264,8 +264,8 @@ rm_file(char **argv)
                else if (S_ISDIR(sb.st_mode))
                        rval = rmdir(f);
                else {
-                       if (Pflag)
-                               rm_overwrite(f, &sb);
+                       if (Pflag && !rm_overwrite(f, &sb))
+                               continue;
                        rval = unlink(f);
                }
                if (rval && (!fflag || errno != ENOENT)) {
@@ -308,9 +308,12 @@ rm_overwrite(char *file, struct stat *sb
        if (sbp->st_nlink > 1) {
                warnx("%s (inode %llu): not overwritten due to multiple links",
                    file, (unsigned long long)sbp->st_ino);
-               return (0);
+               return (1);
        }
-       if ((fd = open(file, O_WRONLY|O_NONBLOCK|O_NOFOLLOW, 0)) == -1)
+       if ((fd = open(file, O_WRONLY|O_NONBLOCK|O_NOFOLLOW, 0)) == -1 &&
+           (errno != EACCES ||
+            chmod(file, sb.st_mode | S_IWUSR) == -1 ||
+            (fd = open(file, O_WRONLY|O_NONBLOCK|O_NOFOLLOW, 0)) == -1))
                goto err;
        if (fstat(fd, &sb2))
                goto err;

Reply via email to