From: Crystal Kolipe <kolipe.c@exoticsilicon.com>
Subject: Read-only softraid volumes
To: tech@openbsd.org
Date: Thu, 12 Jun 2025 10:58:34 -0300

Ping?

The softraid subsystem really shouldn't fail just because a volume is
read-only, so this is more of a bug-fix than a new feature...

On Thu, May 15, 2025 at 12:59:38PM -0300, Crystal Kolipe wrote:
> Currently it is not possible to attach softraid volumes that reside on
> read-only media, as the existing code assumes that it can always open the
> device for read/write access.
> 
> So, for example, a usb drive with a physical write-protect switch that is used
> to hold a softraid crypto volume can't be mounted if it's set to read-only.
> 
> I recently had a look at what was needed to support doing this.
> 
> A full detailed write up for anybody who is interested is here:
> 
> https://research.exoticsilicon.com/articles/read_only_softraid
> 
> But essentially we just need to modify softraid_meta_probe() to retry the
> initial VOP_OPEN() as read-only, and then add support for softraid sd devices
> to respond to the SCSI MODE_SENSE opcode with a read-only flag in the header.
> 
> A couple of small changes to /sbin/bioctl and /sbin/scsi make things work as
> expected in userland.
> 
> Separately, the /sbin/scsi utility has some bugs in it's hex dump printing
> code of raw mode pages.  I'll post details of these in a separate thread, as
> they are not directly releated to the read-only code.
> 
> 
> --- sbin/scsi/scsi.c.dist	Sun Dec  4 23:50:47 2022
> +++ sbin/scsi/scsi.c		Thu May 15 15:17:51 2025
> @@ -113,7 +113,8 @@
>  			break;
>  		case 'f':
>  			if ((fd = scsi_open(optarg, O_RDWR)) < 0)
> -				err(1, "unable to open device %s", optarg);
> +				if ((fd = scsi_open(optarg, O_RDONLY)) < 0)
> +					err(1, "unable to open device %s", optarg);
>  			fflag = 1;
>  			break;
>  		case 'd':
> 
> --- sbin/bioctl/bioctl.c.dist	Mon Apr  7 04:25:01 2025
> +++ sbin/bioctl/bioctl.c	Thu May 15 15:20:45 2025
> @@ -230,7 +230,9 @@
>  	if (devicename == NULL)
>  		errx(1, "need device");
>  
> -	devh = opendev(devicename, O_RDWR, OPENDEV_PART, &realname);
> +	devh = opendev(devicename, (func == BIOC_DELETERAID ||
> +	    func == BIOC_INQ ? O_RDONLY : O_RDWR), OPENDEV_PART, &realname);
> +
>  	if (devh == -1) {
>  		devh = open("/dev/bio", O_RDWR);
>  		if (devh == -1)
> 
> --- sys/dev/softraid.c.dist	Mon Apr  7 04:42:29 2025
> +++ sys/dev/softraid.c		Fri May  9 13:10:01 2025
> @@ -334,6 +334,10 @@
>  			 * and figure out the open/close dance for unwind.
>  			 */
>  			error = VOP_OPEN(vn, FREAD | FWRITE, NOCRED, curproc);
> +			if (error == EACCES || error == EROFS) {
> +				sd->sd_capabilities |= SR_CAP_READONLY;
> +				error = VOP_OPEN(vn, FREAD, NOCRED, curproc);
> +				}
>  			if (error) {
>  				DNPRINTF(SR_D_META,"%s: sr_meta_probe can't "
>  				    "open %s\n", DEVNAME(sc), devname);
> @@ -2321,6 +2325,7 @@
>  	struct sr_softc		*sc = link->bus->sb_adapter_softc;
>  	struct sr_workunit	*wu = xs->io;
>  	struct sr_discipline	*sd;
> +	unsigned char		mode_sense_header[4];
>  
>  	DNPRINTF(SR_D_CMD, "%s: sr_scsi_cmd target %d xs %p flags %#x\n",
>  	    DEVNAME(sc), link->target, xs, xs->flags);
> @@ -2395,6 +2400,19 @@
>  		    DEVNAME(sc));
>  		if (sd->sd_scsi_req_sense(wu))
>  			goto stuffup;
> +		goto complete;
> +
> +	case MODE_SENSE:
> +		if ((xs->cmd.bytes[1] & 0x3f) != 0 && (xs->cmd.bytes[1] & 0x3f) != 0x3f) {
> +			goto stuffup;
> +			}
> +		bzero (&mode_sense_header, 4);
> +		mode_sense_header[0] = 0x03;
> +		if (sd->sd_capabilities & SR_CAP_READONLY) {
> +			mode_sense_header[2] = 0x80;
> +			}
> +		scsi_copy_internal_data(xs, &mode_sense_header, 4);
> +		xs->datalen = 4;
>  		goto complete;
>  
>  	default:
> 
> --- sys/dev/softraidvar.h.dist	Mon Apr  7 04:25:11 2025
> +++ sys/dev/softraidvar.h	Tue May  6 09:17:54 2025
> @@ -535,6 +535,7 @@
>  #define SR_CAP_REBUILD		0x00000004	/* Supports rebuild. */
>  #define SR_CAP_NON_COERCED	0x00000008	/* Uses non-coerced size. */
>  #define SR_CAP_REDUNDANT	0x00000010	/* Redundant copies of data. */
> +#define SR_CAP_READONLY		0x00000020	/* Read-only. */
>  
>  	union {
>  	    struct sr_raid0	mdd_raid0;
>