Bug 24271

On Fri, 12 Jan 2001 sam@inf.enst.fr wrote:

> >Description:
> 	dumpon should check that the argument is not a disk slice, or should
> 	require a flag (-f?) in this case. It would prevent bad crashes :/

This is a feature.  dumpon (like swapon and newfs) should work on any disk
device, including the whole disk.  It currently only works on disk devices
whose containing slice is labeled, so it can only be misused to clobber
BSD slices and not OtherOS slices (this is a bug :-).

Bruce

On 12/01, Bruce Evans wrote:
| On Fri, 12 Jan 2001 sam@inf.enst.fr wrote:
| 
| > >Description:
| > 	dumpon should check that the argument is not a disk slice, or should
| > 	require a flag (-f?) in this case. It would prevent bad crashes :/
| 
| This is a feature.  dumpon (like swapon and newfs) should work on any disk
| device, including the whole disk.  It currently only works on disk devices
| whose containing slice is labeled, so it can only be misused to clobber
| BSD slices and not OtherOS slices (this is a bug :-).

I would recommend to add a "-f" option anyway. It is way too easy to set
(as I did yesterday) "dumpdev=/dev/ad0s2" in your rc.conf and be very
surprised after a crash. If you really want to do this, you could do it
with a "dumpon_flags=-f".

  Sam

On Fri, 12 Jan 2001, Samuel Tardieu wrote:

> On 12/01, Bruce Evans wrote:
> | On Fri, 12 Jan 2001 sam@inf.enst.fr wrote:
> | 
> | > >Description:
> | > 	dumpon should check that the argument is not a disk slice, or should
> | > 	require a flag (-f?) in this case. It would prevent bad crashes :/
> | 
> | This is a feature.  dumpon (like swapon and newfs) should work on any disk
> | device, including the whole disk.  It currently only works on disk devices
> | whose containing slice is labeled, so it can only be misused to clobber
> | BSD slices and not OtherOS slices (this is a bug :-).
> 
> I would recommend to add a "-f" option anyway. It is way too easy to set
> (as I did yesterday) "dumpdev=/dev/ad0s2" in your rc.conf and be very
> surprised after a crash. If you really want to do this, you could do it
> with a "dumpon_flags=-f".

It's no easier than setting "dumpdev=/dev/ad0s2c" or configuring swap on
/dev/ad0s2c when you really meant /dev/ad0s2b.

Bruce

On 13/01, David Malone wrote:

| In tune with the Unix way (providing you with enough rope to shoot
| yourself in the foot) it probably shouldn't check even if it was
| easy to.  After all, it's just as easy to accidently dump onto one
| of your filesystems, and there's no way to accidently detect that!

Sure, but even newfs makes some checks before formatting a filesystem.

The following patch adds two things:

  - if the special_file indicated is not tagged as swap in /etc/fstab,
    then -f must be used; this is consistent with the possibility of
    using a shared swap with another system;

  - as a bonus, you can now use raw names such as da0s2b instead of
    fully qualified names

Documentation and startup scripts changes are also integrated in this patch.

  Sam

diff -r -u /usr/src/etc/defaults/rc.conf ./etc/defaults/rc.conf
--- /usr/src/etc/defaults/rc.conf	Thu Jan 11 14:00:54 2001
+++ ./etc/defaults/rc.conf	Sat Jan 13 13:31:04 2001
@@ -303,6 +303,7 @@
 sendmail_enable="NO"	# Run the sendmail daemon (YES/NO).
 sendmail_flags="-bd -q30m" # Flags to sendmail (if enabled)
 dumpdev="NO"		# Device name to crashdump to (or NO).
+dumpon_flags=""		# Flags to pass to dumpon (if dumpdev set).
 enable_quotas="NO"      # turn on quotas on startup (or NO).
 check_quotas="YES"	# Check quotas on startup (or NO).
 accounting_enable="NO"	# Turn on process accounting (or NO).
diff -r -u /usr/src/etc/rc ./etc/rc
--- /usr/src/etc/rc	Thu Jan 11 14:00:41 2001
+++ ./etc/rc	Sat Jan 13 13:24:52 2001
@@ -440,7 +440,7 @@
 	;;
 *)
 	if [ -e "${dumpdev}" -a -d /var/crash ]; then
-		dumpon -v ${dumpdev}
+		dumpon ${dumpon_flags} -v ${dumpdev}
 		echo -n 'Checking for core dump: '
 		savecore /var/crash
 	fi
diff -r -u /usr/src/sbin/dumpon/dumpon.8 ./sbin/dumpon/dumpon.8
--- /usr/src/sbin/dumpon/dumpon.8	Thu Dec 14 12:47:12 2000
+++ ./sbin/dumpon/dumpon.8	Sat Jan 13 13:19:17 2001
@@ -40,6 +40,7 @@
 .Nd "specify a device for crash dumps"
 .Sh SYNOPSIS
 .Nm
+.Op Fl f
 .Op Fl v
 .Ar special_file
 .Nm
@@ -61,6 +62,13 @@
 .Pp
 The size of the specified dump device must be at least 64 KB greater the 
 size of physical memory.
+.Pp
+The
+.Fl f
+flag is needed when
+.Ar special_file
+is not declared as a swap partition in
+.Pa /etc/fstab .
 .Pp
 The
 .Fl v
diff -r -u /usr/src/sbin/dumpon/dumpon.c ./sbin/dumpon/dumpon.c
--- /usr/src/sbin/dumpon/dumpon.c	Mon May  1 21:39:36 2000
+++ ./sbin/dumpon/dumpon.c	Sat Jan 13 13:15:59 2001
@@ -52,19 +52,26 @@
 #include <sys/sysctl.h>
 #include <sys/stat.h>
 #include <sysexits.h>
+#include <paths.h>
+#include <string.h>
+#include <fstab.h>
 
 void	usage __P((void)) __dead2;
 
 int
 main(int argc, char **argv)
 {
-	int ch, verbose, rv;
+	int ch, verbose, force, rv;
 	struct stat stab;
 	int mib[2];
+	char special[MAXPATHLEN];
 
-	verbose = rv = 0;
-	while ((ch = getopt(argc, argv, "v")) != -1)
+	verbose = force = rv = 0;
+	while ((ch = getopt(argc, argv, "fv")) != -1)
 		switch((char)ch) {
+		case 'f':
+		  force = 1;
+			break;
 		case 'v':
 			verbose = 1;
 			break;
@@ -78,15 +85,43 @@
 		usage();
 
 	if (strcmp(argv[0], "off")) {
-		rv = stat(argv[0], &stab);
+		if (strrchr(argv[0], '/') == 0) {
+			(void)snprintf(special, MAXPATHLEN, "%s%s", _PATH_DEV, argv[0]);
+		} else {
+			strncpy(special, argv[0], MAXPATHLEN);
+		}
+		rv = stat(special, &stab);
 		if (rv) {
-			err(EX_OSFILE, "%s", argv[0]);
+			err(EX_OSFILE, "%s", special);
 		}
 
 		if (!S_ISCHR(stab.st_mode)) {
 			errx(EX_USAGE,
 			     "%s: must specify a character disk device",
-			     argv[0]);
+			     special);
+		}
+		if (!force) {
+			struct fstab *fsp;
+
+			if (!setfsent ())
+				errx(EX_USAGE,
+						 "%s: use -f if %s is not available",
+						 special,
+						 _PATH_FSTAB);
+			while ((fsp = getfsent()) != NULL) {
+				if (!strcmp(fsp->fs_spec, special)) {
+					if (!strcmp(fsp->fs_type, FSTAB_SW))
+						goto fs_ok;
+					errx(EX_USAGE,
+							 "%s: -f required to use a %s partition",
+							 special,
+							 fsp->fs_vfstype);
+				}
+			}
+			errx(EX_USAGE,
+					 "%s: -f required unless using a declared swap partition",
+					 special);
+		fs_ok:
 		}
 	} else {
 		stab.st_rdev = NODEV;
@@ -106,7 +141,7 @@
 			printf("dumpon: crash dumps disabled\n");
 		} else {
 			printf("dumpon: crash dumps to %s (%lu, %lu)\n",
-			       argv[0],
+			       special,
 			       (unsigned long)major(stab.st_rdev),
 			       (unsigned long)minor(stab.st_rdev));
 		}
@@ -119,7 +154,7 @@
 usage()
 {
 	fprintf(stderr,
-		"usage: dumpon [-v] special_file\n"
+		"usage: dumpon [-f] [-v] special_file\n"
 		"       dumpon [-v] off\n");
 	exit(EX_USAGE);
 }
diff -r -u /usr/src/share/man/man5/rc.conf.5 ./share/man/man5/rc.conf.5
--- /usr/src/share/man/man5/rc.conf.5	Fri Dec 29 10:18:41 2000
+++ ./share/man/man5/rc.conf.5	Sat Jan 13 13:28:19 2001
@@ -1348,6 +1348,12 @@
 directory by the
 .Xr savecore 8
 program.
+.It Ar dumpon_flags
+(str) If
+.Ar dumpdev
+is set, these are the flags to pass to the
+.Xr dumpon 8
+program.
 .It Ar check_quotas
 (bool) Set to
 .Ar YES

Comment 5 Robert Watson 2008-02-16 13:27:29 UTC

State Changed
From-To: open->closed

After some discussion on #freebsd-bugbusters, it sounds like we should close 
this PR on the basis that UNIX likes to let you shoot your feet.  If we were 
designing swap/dump behavior from scratch, my preference would be to have a 
partition header in much the same way we have a superblock for file systems 
so that partition selection could be validated directly.  However, in the  
ame way that we don't validate partions for swap, we don't validate for 
dump, as dump and swap data may contain arbitrary contents. 

While it's not something we appear to heavily support in fstab anymore, I 
actually like the ability to dump to a separate partition or location from 
swap -- swap has a tendency to get overwritten for large file system checks 
(etc), whereas a dedicated dump partition isn't.  Likewise, on very small 
systems where textdumps are used and swapping isn't enabled, that 
distinction is also useful.