Skip site navigation (1)Skip section navigation (2)
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>