magic unmbers in rmt protocol

[David Holland <dholland-tech%netbsd.org@localhost>]

It came up in chat today that dump/rmt uses hardcoded integer values
that are then passed as the flags argument of open. This is wrong for
several reasons.

The following patch corrects it. The rmt part is untested (don't have
a suitable setup to test it with) -- if someone who uses rmt could
check it out that would be helpful.

Based on discussion with mrg, the rmt patch accepts a bunch of open
flags that dump doesn't send, because it's apparently useful to be
able to use rmt to dump to remote regular files with a suitably
patched dump. Getting the latter written and committed is for some
future time :-)

Question: should the defines go in a new <protocols/rmt.h> or is that
overkill? Right now they're pasted in both programs, which is not
ideal but not a huge issue either.

Index: usr.sbin/rmt/rmt.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/rmt/rmt.c,v
retrieving revision 1.17
diff -u -p -r1.17 rmt.c
--- usr.sbin/rmt/rmt.c	23 Jan 2018 21:06:25 -0000	1.17
+++ usr.sbin/rmt/rmt.c	17 Mar 2021 19:18:13 -0000
@@ -59,6 +59,25 @@ __RCSID("$NetBSD: rmt.c,v 1.17 2018/01/2
 #include <string.h>
 #include <unistd.h>
 
+/*
+ * The wire protocol uses integer literals for the open mode,
+ * corresponding to the traditional O_RDONLY and O_RDWR values.
+ * Traditional dump sends only either O_RDONLY or O_RDWR, but
+ * since it's useful to use rmt to dump to remote regular files
+ * we also allow some other flags, using the NetBSD values
+ * (which are likely the historical ones, but I'm not sure).
+ */
+#define RMT_O_RDONLY	0
+#define RMT_O_RDWR	2
+#define RMT_O_ACCMODE	3
+#define RMT_O_APPEND	8
+#define RMT_O_NOFOLLOW	0x100
+#define RMT_O_CREAT	0x200
+#define RMT_O_TRUNC	0x400
+#define RMT_O_EXCL	0x800
+#define RMT_O_REGULAR	0x02000000
+#define RMT_O_FLAGS	0x02000f08
+
 int	tape = -1;
 
 int	maxrecsize = -1;
@@ -77,6 +96,7 @@ FILE	*debug;
 char	*checkbuf(char *, int);
 void	 error(int);
 void	 getstring(char *, size_t);
+static int decode_mode(const char *);
 
 int
 main(int argc, char *argv[])
@@ -85,6 +105,7 @@ main(int argc, char *argv[])
 	int rval;
 	char c;
 	int n, i, cc;
+	int openflags;
 
 	argc--, argv++;
 	if (argc > 0) {
@@ -106,7 +127,12 @@ top:
 		getstring(device, sizeof(device));
 		getstring(mode, sizeof(mode));
 		DEBUG2("rmtd: O %s %s\n", device, mode);
-		tape = open(device, atoi(mode),
+		openflags = decode_mode(mode);
+		if (openflags == -1) {
+			error(EINVAL);
+			goto top;
+		}
+		tape = open(device, openflags,
 		    S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
 		if (tape < 0)
 			goto ioerror;
@@ -246,3 +272,49 @@ error(int num)
 	(void)snprintf(resp, sizeof(resp), "E%d\n%s\n", num, strerror(num));
 	(void)write(STDOUT_FILENO, resp, strlen(resp));
 }
+
+static int
+decode_mode(const char *buf)
+{
+	unsigned val;
+	int ret;
+	char *more;
+
+	errno = 0;
+	val = strtol(buf, &more, 0);
+	if (errno != 0 || *more != '\0') {
+		return -1;
+	}
+	switch (val & RMT_O_ACCMODE) {
+	    case RMT_O_RDONLY:
+		ret = O_RDONLY;
+		break;
+	    case RMT_O_RDWR:
+		ret = O_RDWR;
+		break;
+	    default:
+		return -1;
+	}
+	if ((val & RMT_O_FLAGS) != val) {
+		return -1;
+	}
+	if (val & RMT_O_APPEND) {
+		ret |= O_APPEND;
+	}
+	if (val & RMT_O_NOFOLLOW) {
+		ret |= O_NOFOLLOW;
+	}
+	if (val & RMT_O_CREAT) {
+		ret |= O_CREAT;
+	}
+	if (val & RMT_O_TRUNC) {
+		ret |= O_TRUNC;
+	}
+	if (val & RMT_O_EXCL) {
+		ret |= O_EXCL;
+	}
+	if (val & RMT_O_REGULAR) {
+		ret |= O_REGULAR;
+	}
+	return ret;
+}
Index: sbin/dump/dump.h
===================================================================
RCS file: /cvsroot/src/sbin/dump/dump.h,v
retrieving revision 1.59
diff -u -p -r1.59 dump.h
--- sbin/dump/dump.h	3 Dec 2020 08:25:57 -0000	1.59
+++ sbin/dump/dump.h	17 Mar 2021 19:18:14 -0000
@@ -244,6 +244,8 @@ void	lfs_wrap_go(void);
 #endif
 
 /* rdump routines */
+#define RMT_O_RDONLY 	0
+#define RMT_O_RDWR 	2
 #if defined(RDUMP) || defined(RRESTORE)
 void	rmtclose(void);
 int	rmthost(const char *);
Index: sbin/dump/tape.c
===================================================================
RCS file: /cvsroot/src/sbin/dump/tape.c,v
retrieving revision 1.55
diff -u -p -r1.55 tape.c
--- sbin/dump/tape.c	1 Mar 2019 16:42:11 -0000	1.55
+++ sbin/dump/tape.c	17 Mar 2021 19:18:14 -0000
@@ -391,7 +391,7 @@ trewind(int eject)
 #ifdef RDUMP
 	if (host) {
 		rmtclose();
-		while (rmtopen(tape, 0, 0) < 0)
+		while (rmtopen(tape, RMT_O_RDONLY, 0) < 0)
 			sleep(10);
 		if (eflag && eject) {
 			msg("Ejecting %s\n", tape);
@@ -431,7 +431,7 @@ close_rewind(void)
 	if (lflag) {
 		for (i = 0; i < lflag / 10; i++) { /* wait lflag seconds */
 			if (host) {
-				if (rmtopen(tape, 0, 0) >= 0) {
+				if (rmtopen(tape, RMT_O_RDONLY, 0) >= 0) {
 					rmtclose();
 					return;
 				}
@@ -667,7 +667,7 @@ restore_check_point:
 			msg("Dumping volume %d on %s\n", tapeno, tape);
 		}
 #ifdef RDUMP
-		while ((tapefd = (host ? rmtopen(tape, 2, 1) :
+		while ((tapefd = (host ? rmtopen(tape, RMT_O_RDWR, 1) :
 			pipeout ? 1 : open(tape, O_WRONLY|O_CREAT, 0666))) < 0)
 #else
 		while ((tapefd = (pipeout ? 1 :


-- 
David A. Holland
dholland%netbsd.org@localhost