From owner-svn-src-all@FreeBSD.ORG Thu Oct 28 13:14:14 2010 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: by hub.freebsd.org (Postfix, from userid 1233) id DD8A21065679; Thu, 28 Oct 2010 13:14:14 +0000 (UTC) Date: Thu, 28 Oct 2010 13:14:14 +0000 From: Alexander Best To: Ceri Davies Message-ID: <20101028131414.GA79254@freebsd.org> 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> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="3MwIy2ne0vdjdPXF" Content-Disposition: inline In-Reply-To: <20101028103725.GC5488@submonkey.net> Cc: Doug Barton , d@delphij.net, svn-src-all@FreeBSD.org, Gary Jennejohn , src-committers@FreeBSD.org, Bruce Evans , svn-src-head@FreeBSD.org, Dag-Erling Smorgrav Subject: Re: svn commit: r214431 - head/bin/rm X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Oct 2010 13:14:15 -0000 --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 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 #include -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--