Re: [PATCH] 2.6.8.1 Mis-detect CRDW as CDROM

[Martin Schlemmer]

On Tue, 2004-08-17 at 17:59, Andreas Messer wrote:
> Hello again,
> 
> as i get informed, that the kmail emailclient has not made
> what i want, i decided to use mutt for next time. I will
> include the patch again to make it readable. I have also
> changed the thing with MODE_SELECT_10 to write mode 
> because Christer Weinig figured out, that this CMD may
> be insecure in connection with harddisks.
> The changes to cdrom.h made by Marc Ballarin have not yet 
> been included.
> But i think, that the security model should made more 
> precise - deciding only upon the commands does not give
> the effekt of much improved security.
> 
> Here ist the patch.
> 

I am probably missing something, but cant something like attached
work ?  I am definately not sure about the mode passed to the second
verify_command in sg_scsi_ioctl ... (Note I have never really hacked
anywhere near the scsi layer, or done userspace scsi coding, so be
nice =)

Patch is based on vanilla 2.6.8.1, with the bits from Andreas' last
patch, with MODE_SELECT_10 changed to read again.


Thanks,

-- 
Martin Schlemmer
--- 1/drivers/block/scsi_ioctl.c	2004-08-17 21:36:57.680789648 +0200
+++ 2/drivers/block/scsi_ioctl.c	2004-08-17 21:35:54.000000000 +0200
@@ -110,7 +110,7 @@
 #define safe_for_read(cmd)	[cmd] = CMD_READ_SAFE
 #define safe_for_write(cmd)	[cmd] = CMD_WRITE_SAFE
 
-static int verify_command(struct file *file, unsigned char *cmd)
+static int verify_command(struct file *file, unsigned char *cmd, int mode)
 {
 	static const unsigned char cmd_type[256] = {
 
@@ -156,23 +156,90 @@
 		safe_for_write(WRITE_16),
 		safe_for_write(WRITE_BUFFER),
 		safe_for_write(WRITE_LONG),
+
+
+		/* Some additional defs for recording/reading CDs */
+
+		/* 0x01 REZERO_UNIT used by k3b, but also work without */
+               
+		/* read-mode */
+		safe_for_read(GPCMD_GET_CONFIGURATION),
+		safe_for_read(GPCMD_GET_EVENT_STATUS_NOTIFICATION),
+		safe_for_read(GPCMD_GET_PERFORMANCE),
+		safe_for_read(GPCMD_MECHANISM_STATUS),
+
+		/* should this allowed for read ? */
+		safe_for_read(GPCMD_LOAD_UNLOAD),
+		safe_for_read(GPCMD_SET_SPEED),
+		safe_for_read(GPCMD_PAUSE_RESUME),   /* playing audio cd */
+		safe_for_read(SEEK_10),              /* playing audio cd */
+		safe_for_read(GPCMD_SET_READ_AHEAD),
+		safe_for_read(GPCMD_SET_STREAMING),
+		safe_for_read(GPCMD_STOP_PLAY_SCAN), /* playing audio cd */
+
+		/* k3b wont work without read - maybe bug in k3b */ 
+		safe_for_read(GPCMD_MODE_SELECT_10), 
+
+		/* write-mode */
+		safe_for_write(GPCMD_BLANK), 
+		safe_for_write(GPCMD_CLOSE_TRACK),
+		safe_for_write(0x2c),        /* ERASE_10 */ 
+		safe_for_write(GPCMD_FORMAT_UNIT),
+		safe_for_write(GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL),
+		safe_for_write(0x5c),        /* READ_BUFFER_CAPACITY */
+		safe_for_write(GPCMD_READ_FORMAT_CAPACITIES),
+		safe_for_write(GPCMD_REPAIR_RZONE_TRACK),
+		safe_for_write(GPCMD_RESERVE_RZONE_TRACK),
+		safe_for_write(0x5d),        /* SEND_CUE_SHEET */
+		safe_for_write(0xbf),        /* SEND_DVD_STRUCTURE */
+		safe_for_write(GPCMD_SEND_KEY),
+		safe_for_write(GPCMD_SEND_OPC),
+		safe_for_write(SYNCHRONIZE_CACHE),
+		safe_for_write(VERIFY),
+
+		/* Disabled, may change firmware 
+		   safe_for_write(0x3b),  WRITE_BUFFER */
+		/* Disabled due useless without WRITE_BUFFER 
+		   safe_for_write(0x3c),  READ_BUFFER */
+
 	};
 	unsigned char type = cmd_type[cmd[0]];
 
-	/* Anybody who can open the device can do a read-safe command */
-	if (type & CMD_READ_SAFE)
-		return 0;
-
-	/* Write-safe commands just require a writable open.. */
-	if (type & CMD_WRITE_SAFE) {
-		if (file->f_mode & FMODE_WRITE)
+	switch (mode) {
+	case SG_DXFER_FROM_DEV:
+		/* Anybody who can open the device can do a read-safe
+		 * command */
+		if (type & CMD_READ_SAFE)
 			return 0;
+		break;
+	case SG_DXFER_TO_FROM_DEV:
+		/* We need to be able to read and write to the device.. */
+		if (type & CMD_WRITE_SAFE && type & CMD_READ_SAFE) {
+			if (file->f_mode & FMODE_WRITE)
+				return 0;
+		}
+		break;
+	case SG_DXFER_TO_DEV:
+		/* Write-safe commands just require a writable open.. */
+		if (type & CMD_WRITE_SAFE) {
+			if (file->f_mode & FMODE_WRITE)
+				return 0;
+		}
+		break;
 	}
 
 	/* And root can do any command.. */
 	if (capable(CAP_SYS_RAWIO))
 		return 0;
 
+        /* Added for debugging*/
+       
+	if(file->f_mode & FMODE_WRITE)
+	  printk(KERN_WARNING "SCSI-CMD Filter: 0x%x not allowed with write-mode\n",cmd[0]);
+	else
+	  printk(KERN_WARNING "SCSI-CMD Filter: 0x%x not allowed with read-mode\n",cmd[0]);
+
+
 	/* Otherwise fail it with an "Operation not permitted" */
 	return -EPERM;
 }
@@ -193,7 +260,7 @@
 		return -EINVAL;
 	if (copy_from_user(cmd, hdr->cmdp, hdr->cmd_len))
 		return -EFAULT;
-	if (verify_command(file, cmd))
+	if (verify_command(file, cmd, hdr->dxfer_direction))
 		return -EPERM;
 
 	/*
@@ -343,7 +410,7 @@
 	if (copy_from_user(buffer, sic->data + cmdlen, in_len))
 		goto error;
 
-	err = verify_command(file, rq->cmd);
+	err = verify_command(file, rq->cmd, in_len ? SG_DXFER_TO_DEV : SG_DXFER_FROM_DEV);
 	if (err)
 		goto error;

Attachment: signature.asc
Description: This is a digitally signed message part