Date: Thu, 28 Oct 2010 13:14:14 +0000 From: Alexander Best <arundel@freebsd.org> To: Ceri Davies <ceri@submonkey.net> Cc: Doug Barton <dougb@FreeBSD.org>, d@delphij.net, svn-src-all@FreeBSD.org, Gary Jennejohn <gljennjohn@googlemail.com>, src-committers@FreeBSD.org, Bruce Evans <brde@optusnet.com.au>, svn-src-head@FreeBSD.org, Dag-Erling Smorgrav <des@FreeBSD.org> Subject: Re: svn commit: r214431 - head/bin/rm Message-ID: <20101028131414.GA79254@freebsd.org> In-Reply-To: <20101028103725.GC5488@submonkey.net> References: <201010271848.o9RImNSR019344@svn.freebsd.org> <20101027212601.GA78062@freebsd.org> <4CC899C3.7040107@FreeBSD.org> <20101027214822.GA82697@freebsd.org> <4CC8A89D.5070909@delphij.net> <20101028152418.A916@besplex.bde.org> <20101028095538.24147119@ernst.jennejohn.org> <20101028102547.GA56817@freebsd.org> <20101028103725.GC5488@submonkey.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--3MwIy2ne0vdjdPXF
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Thu Oct 28 10, Ceri Davies wrote:
> On Thu, Oct 28, 2010 at 10:25:47AM +0000, Alexander Best wrote:
> > On Thu Oct 28 10, Gary Jennejohn wrote:
> > > On Thu, 28 Oct 2010 16:22:05 +1100 (EST)
> > > Bruce Evans <brde@optusnet.com.au> wrote:
> > >
> > > > On Wed, 27 Oct 2010, Xin LI wrote:
> > > >
> > > > > I think what really defeats -P is the fact that the file system or
> > > > > underlying data storage would not overwrite data on a file at sync().
> > > > > COW is of course one of the case, journaling MAY defeat -P but is not
> > > > > guaranteed. FS with variable block size - I believe this really depends
> > > > > on the implementation.
> > > > >
> > > > > If I understood the code correctly, UFS, UFS+SU, UFS+SUJ, msdosfs and
> > > > > ext2fs supports rm -P as long as they are not being put on gjournal'ed
> > > > > disk, ZFS zvol, etc., and no snapshot is being used.
> > > >
> > > > And that the underlying data storage us dumb. Any flash drive now
> > > > tries to minimise writes. It wouldn't take much buffering to defeat
> > > > the 0xff, 0,0xff pattern. Wear leveling should result in different
> > > > physical blocks being written each time if the writes get to the
> > > > lowest level of storage.
> > > >
> > > > And that block reallocation (done by ffs1 and ffs2) doesn't choose
> > > > different blocks.
> > > >
> > > > > It seems to be hard for me to conclude all cases in short, plain English
> > > > > but I'm all for improvements to the manual page to describe that in an
> > > > > elegant and precise manner.
> > > > >
> > > > > Maybe something like:
> > > > >
> > > > > ===============
> > > > > BUGS
> > > > >
> > > > > The -P option assumes that the underlying storage overwrites file block
> > > > > when data is written on existing offset. Several factors including the
> > > > > file system and its backing store could defeat the assumption, this
> > > > > includes, but is not limited to file systems that uses Copy-On-Write
> > > > > strategy (e.g. ZFS or UFS when snapshot is being used), or backing
> > > > > datastore that does journaling, etc. In addition, only regular files
> > > > > are overwritten, other types of files are not.
> > > > > ===============
> > > >
> > > > Summary: it is very hard to tell whether -P works, even when you think
> > > > you know what all the subsystems are doing.
> > > >
> > >
> > > All this discussion leads me to the conclusion that we should just
> > > remove the -P functionality and add a remark to the man page that that
> > > was done because it isn't guaranteed to work on all file systems.
> > >
> > > Why give users a false sense of security? If they're concerned about
> > > data security then they should use geli or something similar.
> >
> > that might be the ultimate solution. also one could use security/srm instead.
> >
> > +1 here. ;)
> >
> > question is: should -P be removed entirely or be made a no op?
>
> Probably best that it fail.
how about the following patch?
cheers.
alex
>
> Ceri
> --
> Haffely, Gaffely, Gaffely, Gonward.
--
a13x
--3MwIy2ne0vdjdPXF
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="rm.diff"
diff --git a/bin/rm/rm.1 b/bin/rm/rm.1
index 11bc77d..569e7db 100644
--- a/bin/rm/rm.1
+++ b/bin/rm/rm.1
@@ -84,22 +84,6 @@ directory is being recursively removed.
This is a far less intrusive option than
.Fl i
yet provides almost the same level of protection against mistakes.
-.It Fl P
-Overwrite regular files before deleting them.
-Files are overwritten three times, first with the byte pattern 0xff,
-then 0x00, and then 0xff again, before they are deleted.
-Files with multiple links will not be overwritten nor deleted
-and a warning will be issued.
-If the
-.Fl f
-option is specified, files with multiple links will also be overwritten
-and deleted.
-No warning will be issued.
-.Pp
-Specifying this flag for a read only file will cause
-.Nm
-to generate an error message and exit.
-The file will not be removed or overwritten.
.It Fl R
Attempt to remove the file hierarchy rooted in each
.Ar file
@@ -182,12 +166,6 @@ For example:
.Pp
.Dl "rm /home/user/-filename"
.Dl "rm ./-filename"
-.Pp
-When
-.Fl P
-is specified with
-.Fl f
-the file will be overwritten and removed even if it has hard links.
.Sh COMPATIBILITY
The
.Nm
@@ -203,6 +181,23 @@ Also, historical
.Bx
implementations prompted on the standard output,
not the standard error output.
+.Pp
+Previous
+.Bx
+versions of
+.Nm
+featured a
+.Fl P
+option which instructed
+.Nm
+to overwrite regular files three times with the byte pattern
+0xff, then 0x00, and then 0xff again, before they were deleted.
+Support for the
+.Fl P
+flag was removed in
+.Fx 9.0 ,
+because overwriting specific blocks cannot be assured
+on certain filesystems and media.
.Sh SEE ALSO
.Xr chflags 1 ,
.Xr rmdir 1 ,
@@ -226,11 +221,3 @@ A
.Nm
command appeared in
.At v1 .
-.Sh BUGS
-The
-.Fl P
-option assumes that the underlying file system updates existing blocks
-in-place and does not store new data in a new location.
-This is true for UFS but not for ZFS, which is using a Copy-On-Write strategy.
-In addition, only regular files are overwritten, other types of files
-are not.
diff --git a/bin/rm/rm.c b/bin/rm/rm.c
index 653833a..1aff60f 100644
--- a/bin/rm/rm.c
+++ b/bin/rm/rm.c
@@ -57,7 +57,7 @@ __FBSDID("$FreeBSD$");
#include <sysexits.h>
#include <unistd.h>
-int dflag, eval, fflag, iflag, Pflag, vflag, Wflag, stdin_ok;
+int dflag, eval, fflag, iflag, vflag, Wflag, stdin_ok;
int rflag, Iflag;
uid_t uid;
volatile sig_atomic_t info;
@@ -67,7 +67,6 @@ int check2(char **);
void checkdot(char **);
void checkslash(char **);
void rm_file(char **);
-int rm_overwrite(char *, struct stat *);
void rm_tree(char **);
static void siginfo(int __unused);
void usage(void);
@@ -105,8 +104,8 @@ main(int argc, char *argv[])
exit(eval);
}
- Pflag = rflag = 0;
- while ((ch = getopt(argc, argv, "dfiIPRrvW")) != -1)
+ rflag = 0;
+ while ((ch = getopt(argc, argv, "dfiIRrvW")) != -1)
switch(ch) {
case 'd':
dflag = 1;
@@ -122,9 +121,6 @@ main(int argc, char *argv[])
case 'I':
Iflag = 1;
break;
- case 'P':
- Pflag = 1;
- break;
case 'R':
case 'r': /* Compatibility. */
rflag = 1;
@@ -302,9 +298,6 @@ rm_tree(char **argv)
continue;
/* FALLTHROUGH */
default:
- if (Pflag)
- if (!rm_overwrite(p->fts_accpath, NULL))
- continue;
rval = unlink(p->fts_accpath);
if (rval == 0 || (fflag && errno == ENOENT)) {
if (rval == 0 && vflag)
@@ -374,12 +367,8 @@ rm_file(char **argv)
rval = undelete(f);
else if (S_ISDIR(sb.st_mode))
rval = rmdir(f);
- else {
- if (Pflag)
- if (!rm_overwrite(f, &sb))
- continue;
+ else
rval = unlink(f);
- }
}
if (rval && (!fflag || errno != ENOENT)) {
warn("%s", f);
@@ -394,77 +383,6 @@ rm_file(char **argv)
}
}
-/*
- * rm_overwrite --
- * Overwrite the file 3 times with varying bit patterns.
- *
- * XXX
- * This is a cheap way to *really* delete files. Note that only regular
- * files are deleted, directories (and therefore names) will remain.
- * Also, this assumes a fixed-block file system (like FFS, or a V7 or a
- * System V file system). In a logging or COW file system, you'll have to
- * have kernel support.
- */
-int
-rm_overwrite(char *file, struct stat *sbp)
-{
- struct stat sb;
- struct statfs fsb;
- off_t len;
- int bsize, fd, wlen;
- char *buf = NULL;
-
- fd = -1;
- if (sbp == NULL) {
- if (lstat(file, &sb))
- goto err;
- sbp = &sb;
- }
- if (!S_ISREG(sbp->st_mode))
- return (1);
- if (sbp->st_nlink > 1 && !fflag) {
- warnx("%s (inode %u): not overwritten due to multiple links",
- file, sbp->st_ino);
- return (0);
- }
- if ((fd = open(file, O_WRONLY, 0)) == -1)
- goto err;
- if (fstatfs(fd, &fsb) == -1)
- goto err;
- bsize = MAX(fsb.f_iosize, 1024);
- if ((buf = malloc(bsize)) == NULL)
- err(1, "%s: malloc", file);
-
-#define PASS(byte) { \
- memset(buf, byte, bsize); \
- for (len = sbp->st_size; len > 0; len -= wlen) { \
- wlen = len < bsize ? len : bsize; \
- if (write(fd, buf, wlen) != wlen) \
- goto err; \
- } \
-}
- PASS(0xff);
- if (fsync(fd) || lseek(fd, (off_t)0, SEEK_SET))
- goto err;
- PASS(0x00);
- if (fsync(fd) || lseek(fd, (off_t)0, SEEK_SET))
- goto err;
- PASS(0xff);
- if (!fsync(fd) && !close(fd)) {
- free(buf);
- return (1);
- }
-
-err: eval = 1;
- if (buf)
- free(buf);
- if (fd != -1)
- close(fd);
- warn("%s", file);
- return (0);
-}
-
-
int
check(char *path, char *name, struct stat *sp)
{
@@ -489,10 +407,6 @@ check(char *path, char *name, struct stat *sp)
strmode(sp->st_mode, modep);
if ((flagsp = fflagstostr(sp->st_flags)) == NULL)
err(1, "fflagstostr");
- if (Pflag)
- errx(1,
- "%s: -P was specified, but file is not writable",
- path);
(void)fprintf(stderr, "override %s%s%s/%s %s%sfor %s? ",
modep + 1, modep[9] == ' ' ? "" : " ",
user_from_uid(sp->st_uid, 0),
@@ -610,7 +524,7 @@ usage(void)
{
(void)fprintf(stderr, "%s\n%s\n",
- "usage: rm [-f | -i] [-dIPRrvW] file ...",
+ "usage: rm [-f | -i] [-dIRrvW] file ...",
" unlink file");
exit(EX_USAGE);
}
--3MwIy2ne0vdjdPXF--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20101028131414.GA79254>