Subject: Re: admin/15840: "raidctl -s" useless when filesystem mounted
To: Greg Oster <oster@cs.usask.ca>
From: Simon Burge <simonb@wasabisystems.com>
List: netbsd-bugs
Date: 03/10/2002 10:52:18
Simon Burge wrote:

> Frederick Bruckman wrote:
> 
> > In article <20020309000123.B172511119@www.netbsd.org>,
> > 	anne@alcor.concordia.ca writes:
> > 
> > > Make a RAID set, put a filesystem on it, mount the filesystem, and
> > > try "raidctl -s" on the RAID device.
> > 
> > Evidently, you need to be root, or else your user needs to have write
> > privileges on /dev/raid0d. A similar situation exists with the CD ROM
> > devices -- you need to have write privs just to get status. The defaults
> > for such devices should probably be "g+w", at least.
> 
> raidctl should be changed so that operations that don't require
> write access don't open the device with the write flag set.  I
> just tried changing the "O_RDWR" to "O_RDONLY" in raidctl.c and
> now I can "raidctl -s" as a user with group operator on a mounted
> device where the device is mode 640.

Here's a patch to do this on -current and the 1.5 branch.  Does it
look ok to you, Greg?  If so I'll apply the patch to -current and
ask for a pullup to the 1.5 branch.

Simon.
--
Simon Burge                            <simonb@wasabisystems.com>
NetBSD CDs, Support and Service:    http://www.wasabisystems.com/

Here's a patch for NetBSD -current (compiles but untested):

Index: raidctl.c
===================================================================
RCS file: /cvsroot/basesrc/sbin/raidctl/raidctl.c,v
retrieving revision 1.29
diff -d -p -u -r1.29 raidctl.c
--- raidctl.c	2001/10/04 16:02:08	1.29
+++ raidctl.c	2002/03/09 23:46:55
@@ -109,6 +109,7 @@ main(argc,argv)
 	struct stat st;
 	int fd;
 	int force;
+	int openmode;
 
 	num_options = 0;
 	action = 0;
@@ -117,6 +118,7 @@ main(argc,argv)
 	do_rewrite = 0;
 	is_clean = 0;
 	force = 0;
+	openmode = O_RDWR;	/* default to read/write */
 
 	while ((ch = getopt(argc, argv, "a:A:Bc:C:f:F:g:GiI:l:r:R:sSpPuv")) 
 	       != -1)
@@ -162,10 +164,12 @@ main(argc,argv)
 		case 'g':
 			action = RAIDFRAME_GET_COMPONENT_LABEL;
 			strncpy(component, optarg, PATH_MAX);
+			openmode = O_RDONLY;
 			num_options++;
 			break;
 		case 'G':
 			action = RAIDFRAME_GET_INFO;
+			openmode = O_RDONLY;
 			do_output = 1;
 			num_options++;
 			break;
@@ -195,14 +199,17 @@ main(argc,argv)
 			break;
 		case 's':
 			action = RAIDFRAME_GET_INFO;
+			openmode = O_RDONLY;
 			num_options++;
 			break;
 		case 'S':
 			action = RAIDFRAME_CHECK_RECON_STATUS_EXT;
+			openmode = O_RDONLY;
 			num_options++;
 			break;
 		case 'p':
 			action = RAIDFRAME_CHECK_PARITY;
+			openmode = O_RDONLY;
 			num_options++;
 			break;
 		case 'P':
@@ -230,7 +237,7 @@ main(argc,argv)
 		usage();
 
 	strncpy(name,argv[0],PATH_MAX);
-	fd = opendisk(name, O_RDWR, dev_name, sizeof(dev_name), 1);
+	fd = opendisk(name, openmode, dev_name, sizeof(dev_name), 1);
 	if (fd == -1) {
 		fprintf(stderr, "%s: unable to open device file: %s\n",
 			getprogname(), name);

and here's a patch for the NetBSD 1.5 branch (tested):

Index: raidctl.c
===================================================================
RCS file: /cvsroot/basesrc/sbin/raidctl/raidctl.c,v
retrieving revision 1.23.2.1
diff -d -p -u -r1.23.2.1 raidctl.c
--- raidctl.c	2000/08/19 20:10:51	1.23.2.1
+++ raidctl.c	2002/03/09 23:48:44
@@ -108,6 +108,7 @@ main(argc,argv)
 	struct stat st;
 	int fd;
 	int force;
+	int openmode;
 
 	num_options = 0;
 	action = 0;
@@ -115,6 +116,7 @@ main(argc,argv)
 	do_rewrite = 0;
 	is_clean = 0;
 	force = 0;
+	openmode = O_RDWR;	/* default to read/write */
 
 	while ((ch = getopt(argc, argv, "a:A:Bc:C:f:F:g:iI:l:r:R:sSpPuv")) 
 	       != -1)
@@ -160,6 +162,7 @@ main(argc,argv)
 		case 'g':
 			action = RAIDFRAME_GET_COMPONENT_LABEL;
 			strncpy(component, optarg, PATH_MAX);
+			openmode = O_RDONLY;
 			num_options++;
 			break;
 		case 'i':
@@ -188,14 +191,17 @@ main(argc,argv)
 			break;
 		case 's':
 			action = RAIDFRAME_GET_INFO;
+			openmode = O_RDONLY;
 			num_options++;
 			break;
 		case 'S':
 			action = RAIDFRAME_CHECK_RECON_STATUS_EXT;
+			openmode = O_RDONLY;
 			num_options++;
 			break;
 		case 'p':
 			action = RAIDFRAME_CHECK_PARITY;
+			openmode = O_RDONLY;
 			num_options++;
 			break;
 		case 'P':
@@ -250,7 +256,7 @@ main(argc,argv)
 
 	raidID = RF_DEV2RAIDID(st.st_rdev);
 
-	if ((fd = open( dev_name, O_RDWR, 0640)) < 0) {
+	if ((fd = open(dev_name, openmode)) < 0) {
 		fprintf(stderr, "%s: unable to open device file: %s\n",
 			__progname, dev_name);
 		exit(1);