dpt_i2o fixes for stable

dpt_i2o fixes for stable

[Ben Hutchings]

[-- Attachment #1.1: Type: text/plain, Size: 764 bytes --]

I'm proposing to address the most obvious issues with dpt_i2o on stable
branches.  At this stage it may be better to remove it as has been done
upstream, but I'd rather limit the regression for anyone still using
the hardware.

The changes are:

- "scsi: dpt_i2o: Remove broken pass-through ioctl (I2OUSERCMD)",
  which closes security flaws including CVE-2023-2007.
- "scsi: dpt_i2o: Do not process completions with invalid addresses",
  which removes the remaining bus_to_virt() call and may slightly
  improve handling of misbehaving hardware.

These changes have been compiled on all the relevant stable branches,
but I don't have hardware to test on.

Ben.

-- 
Ben Hutchings - Debian developer, member of kernel, installer and LTS
teams

[-- Attachment #1.2: 0001-scsi-dpt_i2o-Fix-various-bugs-in-adpt_i2o_passthru-4.14.patch --]
[-- Type: text/x-patch, Size: 10567 bytes --]

From fa54f65de2a1035b47362dd86dab567703056f46 Mon Sep 17 00:00:00 2001
From: Ben Hutchings <benh@debian.org>
Date: Sat, 27 May 2023 15:34:30 +0200
Subject: [PATCH 1/2] scsi: dpt_i2o: Remove broken pass-through ioctl (I2OUSERCMD)

adpt_i2o_passthru() takes a user-provided message and passes it
through to the hardware with appropriate translation of addresses
and message IDs.  It has a number of bugs:

- When a message requires scatter/gather, it doesn't verify that the
  offset to the scatter/gather list is less than the message size.
- When a message requires scatter/gather, it overwrites the DMA
  addresses with the user-space virtual addresses before unmapping the
  DMA buffers.
- It reads the message from user memory multiple times.  This allows
  user-space to change the message and bypass validation.
- It assumes that the message is at least 4 words long, but doesn't
  check that.

I tried fixing these, but even the maintainer of the corresponding
user-space in Debian doesn't have the hardware any more.

Instead, remove the pass-through ioctl (I2OUSRCMD) and supporting
code.

There is no corresponding upstream commit, because this driver was
removed upstream.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: 67af2b060e02 ("[SCSI] dpt_i2o: move from virt_to_bus/bus_to_virt ...")
Signed-off-by: Ben Hutchings <benh@debian.org>
---
 drivers/scsi/dpt_i2o.c | 258 +----------------------------------------
 drivers/scsi/dpti.h    |   1 -
 2 files changed, 3 insertions(+), 256 deletions(-)

diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index fd172b0890d3..0773fde778b1 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -630,51 +630,6 @@ static struct scsi_cmnd *
 	return NULL;
 }
 
-/*
- *	Turn a pointer to ioctl reply data into an u32 'context'
- */
-static u32 adpt_ioctl_to_context(adpt_hba * pHba, void *reply)
-{
-#if BITS_PER_LONG == 32
-	return (u32)(unsigned long)reply;
-#else
-	ulong flags = 0;
-	u32 nr, i;
-
-	spin_lock_irqsave(pHba->host->host_lock, flags);
-	nr = ARRAY_SIZE(pHba->ioctl_reply_context);
-	for (i = 0; i < nr; i++) {
-		if (pHba->ioctl_reply_context[i] == NULL) {
-			pHba->ioctl_reply_context[i] = reply;
-			break;
-		}
-	}
-	spin_unlock_irqrestore(pHba->host->host_lock, flags);
-	if (i >= nr) {
-		printk(KERN_WARNING"%s: Too many outstanding "
-				"ioctl commands\n", pHba->name);
-		return (u32)-1;
-	}
-
-	return i;
-#endif
-}
-
-/*
- *	Go from an u32 'context' to a pointer to ioctl reply data.
- */
-static void *adpt_ioctl_from_context(adpt_hba *pHba, u32 context)
-{
-#if BITS_PER_LONG == 32
-	return (void *)(unsigned long)context;
-#else
-	void *p = pHba->ioctl_reply_context[context];
-	pHba->ioctl_reply_context[context] = NULL;
-
-	return p;
-#endif
-}
-
 /*===========================================================================
  * Error Handling routines
  *===========================================================================
@@ -1698,201 +1653,6 @@ static int adpt_close(struct inode *inode, struct file *file)
 	return 0;
 }
 
-
-static int adpt_i2o_passthru(adpt_hba* pHba, u32 __user *arg)
-{
-	u32 msg[MAX_MESSAGE_SIZE];
-	u32* reply = NULL;
-	u32 size = 0;
-	u32 reply_size = 0;
-	u32 __user *user_msg = arg;
-	u32 __user * user_reply = NULL;
-	void *sg_list[pHba->sg_tablesize];
-	u32 sg_offset = 0;
-	u32 sg_count = 0;
-	int sg_index = 0;
-	u32 i = 0;
-	u32 rcode = 0;
-	void *p = NULL;
-	dma_addr_t addr;
-	ulong flags = 0;
-
-	memset(&msg, 0, MAX_MESSAGE_SIZE*4);
-	// get user msg size in u32s 
-	if(get_user(size, &user_msg[0])){
-		return -EFAULT;
-	}
-	size = size>>16;
-
-	user_reply = &user_msg[size];
-	if(size > MAX_MESSAGE_SIZE){
-		return -EFAULT;
-	}
-	size *= 4; // Convert to bytes
-
-	/* Copy in the user's I2O command */
-	if(copy_from_user(msg, user_msg, size)) {
-		return -EFAULT;
-	}
-	get_user(reply_size, &user_reply[0]);
-	reply_size = reply_size>>16;
-	if(reply_size > REPLY_FRAME_SIZE){
-		reply_size = REPLY_FRAME_SIZE;
-	}
-	reply_size *= 4;
-	reply = kzalloc(REPLY_FRAME_SIZE*4, GFP_KERNEL);
-	if(reply == NULL) {
-		printk(KERN_WARNING"%s: Could not allocate reply buffer\n",pHba->name);
-		return -ENOMEM;
-	}
-	sg_offset = (msg[0]>>4)&0xf;
-	msg[2] = 0x40000000; // IOCTL context
-	msg[3] = adpt_ioctl_to_context(pHba, reply);
-	if (msg[3] == (u32)-1) {
-		kfree(reply);
-		return -EBUSY;
-	}
-
-	memset(sg_list,0, sizeof(sg_list[0])*pHba->sg_tablesize);
-	if(sg_offset) {
-		// TODO add 64 bit API
-		struct sg_simple_element *sg =  (struct sg_simple_element*) (msg+sg_offset);
-		sg_count = (size - sg_offset*4) / sizeof(struct sg_simple_element);
-		if (sg_count > pHba->sg_tablesize){
-			printk(KERN_DEBUG"%s:IOCTL SG List too large (%u)\n", pHba->name,sg_count);
-			kfree (reply);
-			return -EINVAL;
-		}
-
-		for(i = 0; i < sg_count; i++) {
-			int sg_size;
-
-			if (!(sg[i].flag_count & 0x10000000 /*I2O_SGL_FLAGS_SIMPLE_ADDRESS_ELEMENT*/)) {
-				printk(KERN_DEBUG"%s:Bad SG element %d - not simple (%x)\n",pHba->name,i,  sg[i].flag_count);
-				rcode = -EINVAL;
-				goto cleanup;
-			}
-			sg_size = sg[i].flag_count & 0xffffff;      
-			/* Allocate memory for the transfer */
-			p = dma_alloc_coherent(&pHba->pDev->dev, sg_size, &addr, GFP_KERNEL);
-			if(!p) {
-				printk(KERN_DEBUG"%s: Could not allocate SG buffer - size = %d buffer number %d of %d\n",
-						pHba->name,sg_size,i,sg_count);
-				rcode = -ENOMEM;
-				goto cleanup;
-			}
-			sg_list[sg_index++] = p; // sglist indexed with input frame, not our internal frame.
-			/* Copy in the user's SG buffer if necessary */
-			if(sg[i].flag_count & 0x04000000 /*I2O_SGL_FLAGS_DIR*/) {
-				// sg_simple_element API is 32 bit
-				if (copy_from_user(p,(void __user *)(ulong)sg[i].addr_bus, sg_size)) {
-					printk(KERN_DEBUG"%s: Could not copy SG buf %d FROM user\n",pHba->name,i);
-					rcode = -EFAULT;
-					goto cleanup;
-				}
-			}
-			/* sg_simple_element API is 32 bit, but addr < 4GB */
-			sg[i].addr_bus = addr;
-		}
-	}
-
-	do {
-		/*
-		 * Stop any new commands from enterring the
-		 * controller while processing the ioctl
-		 */
-		if (pHba->host) {
-			scsi_block_requests(pHba->host);
-			spin_lock_irqsave(pHba->host->host_lock, flags);
-		}
-		rcode = adpt_i2o_post_wait(pHba, msg, size, FOREVER);
-		if (rcode != 0)
-			printk("adpt_i2o_passthru: post wait failed %d %p\n",
-					rcode, reply);
-		if (pHba->host) {
-			spin_unlock_irqrestore(pHba->host->host_lock, flags);
-			scsi_unblock_requests(pHba->host);
-		}
-	} while (rcode == -ETIMEDOUT);
-
-	if(rcode){
-		goto cleanup;
-	}
-
-	if(sg_offset) {
-	/* Copy back the Scatter Gather buffers back to user space */
-		u32 j;
-		// TODO add 64 bit API
-		struct sg_simple_element* sg;
-		int sg_size;
-
-		// re-acquire the original message to handle correctly the sg copy operation
-		memset(&msg, 0, MAX_MESSAGE_SIZE*4); 
-		// get user msg size in u32s 
-		if(get_user(size, &user_msg[0])){
-			rcode = -EFAULT; 
-			goto cleanup; 
-		}
-		size = size>>16;
-		size *= 4;
-		if (size > MAX_MESSAGE_SIZE) {
-			rcode = -EINVAL;
-			goto cleanup;
-		}
-		/* Copy in the user's I2O command */
-		if (copy_from_user (msg, user_msg, size)) {
-			rcode = -EFAULT;
-			goto cleanup;
-		}
-		sg_count = (size - sg_offset*4) / sizeof(struct sg_simple_element);
-
-		// TODO add 64 bit API
-		sg 	 = (struct sg_simple_element*)(msg + sg_offset);
-		for (j = 0; j < sg_count; j++) {
-			/* Copy out the SG list to user's buffer if necessary */
-			if(! (sg[j].flag_count & 0x4000000 /*I2O_SGL_FLAGS_DIR*/)) {
-				sg_size = sg[j].flag_count & 0xffffff; 
-				// sg_simple_element API is 32 bit
-				if (copy_to_user((void __user *)(ulong)sg[j].addr_bus,sg_list[j], sg_size)) {
-					printk(KERN_WARNING"%s: Could not copy %p TO user %x\n",pHba->name, sg_list[j], sg[j].addr_bus);
-					rcode = -EFAULT;
-					goto cleanup;
-				}
-			}
-		}
-	} 
-
-	/* Copy back the reply to user space */
-	if (reply_size) {
-		// we wrote our own values for context - now restore the user supplied ones
-		if(copy_from_user(reply+2, user_msg+2, sizeof(u32)*2)) {
-			printk(KERN_WARNING"%s: Could not copy message context FROM user\n",pHba->name);
-			rcode = -EFAULT;
-		}
-		if(copy_to_user(user_reply, reply, reply_size)) {
-			printk(KERN_WARNING"%s: Could not copy reply TO user\n",pHba->name);
-			rcode = -EFAULT;
-		}
-	}
-
-
-cleanup:
-	if (rcode != -ETIME && rcode != -EINTR) {
-		struct sg_simple_element *sg =
-				(struct sg_simple_element*) (msg +sg_offset);
-		kfree (reply);
-		while(sg_index) {
-			if(sg_list[--sg_index]) {
-				dma_free_coherent(&pHba->pDev->dev,
-					sg[sg_index].flag_count & 0xffffff,
-					sg_list[sg_index],
-					sg[sg_index].addr_bus);
-			}
-		}
-	}
-	return rcode;
-}
-
 #if defined __ia64__ 
 static void adpt_ia64_info(sysInfo_S* si)
 {
@@ -2019,8 +1779,6 @@ static int adpt_ioctl(struct inode *inode, struct file *file, uint cmd, ulong ar
 			return -EFAULT;
 		}
 		break;
-	case I2OUSRCMD:
-		return adpt_i2o_passthru(pHba, argp);
 
 	case DPT_CTRLINFO:{
 		drvrHBAinfo_S HbaInfo;
@@ -2174,13 +1932,6 @@ static irqreturn_t adpt_isr(int irq, void *dev_id)
 			adpt_send_nop(pHba, old_m);
 		} 
 		context = readl(reply+8);
-		if(context & 0x40000000){ // IOCTL
-			void *p = adpt_ioctl_from_context(pHba, readl(reply+12));
-			if( p != NULL) {
-				memcpy_fromio(p, reply, REPLY_FRAME_SIZE * 4);
-			}
-			// All IOCTLs will also be post wait
-		}
 		if(context & 0x80000000){ // Post wait message
 			status = readl(reply+16);
 			if(status  >> 24){
@@ -2188,12 +1939,9 @@ static irqreturn_t adpt_isr(int irq, void *dev_id)
 			} else {
 				status = I2O_POST_WAIT_OK;
 			}
-			if(!(context & 0x40000000)) {
-				cmd = adpt_cmd_from_context(pHba,
-							readl(reply+12));
-				if(cmd != NULL) {
-					printk(KERN_WARNING"%s: Apparent SCSI cmd in Post Wait Context - cmd=%p context=%x\n", pHba->name, cmd, context);
-				}
+			cmd = adpt_cmd_from_context(pHba, readl(reply+12));
+			if(cmd != NULL) {
+				printk(KERN_WARNING"%s: Apparent SCSI cmd in Post Wait Context - cmd=%p context=%x\n", pHba->name, cmd, context);
 			}
 			adpt_i2o_post_wait_complete(context, status);
 		} else { // SCSI message
diff --git a/drivers/scsi/dpti.h b/drivers/scsi/dpti.h
index 1fa345ab8ecb..62a57324bb00 100644
--- a/drivers/scsi/dpti.h
+++ b/drivers/scsi/dpti.h
@@ -252,7 +252,6 @@ typedef struct _adpt_hba {
 	void __iomem *FwDebugBLEDflag_P;// Virtual Addr Of FW Debug BLED
 	void __iomem *FwDebugBLEDvalue_P;// Virtual Addr Of FW Debug BLED
 	u32 FwDebugFlags;
-	u32 *ioctl_reply_context[4];
 } adpt_hba;
 
 struct sg_simple_element {

[-- Attachment #1.3: 0001-scsi-dpt_i2o-Fix-various-bugs-in-adpt_i2o_passthru-4.19.patch --]
[-- Type: text/x-patch, Size: 10642 bytes --]

From ed291614d1fef50d7060599cb8630f55be9c41f6 Mon Sep 17 00:00:00 2001
From: Ben Hutchings <benh@debian.org>
Date: Sat, 27 May 2023 15:34:30 +0200
Subject: [PATCH 1/2] scsi: dpt_i2o: Remove broken pass-through ioctl (I2OUSERCMD)

adpt_i2o_passthru() takes a user-provided message and passes it
through to the hardware with appropriate translation of addresses
and message IDs.  It has a number of bugs:

- When a message requires scatter/gather, it doesn't verify that the
  offset to the scatter/gather list is less than the message size.
- When a message requires scatter/gather, it overwrites the DMA
  addresses with the user-space virtual addresses before unmapping the
  DMA buffers.
- It reads the message from user memory multiple times.  This allows
  user-space to change the message and bypass validation.
- It assumes that the message is at least 4 words long, but doesn't
  check that.

I tried fixing these, but even the maintainer of the corresponding
user-space in Debian doesn't have the hardware any more.

Instead, remove the pass-through ioctl (I2OUSRCMD) and supporting
code.

There is no corresponding upstream commit, because this driver was
removed upstream.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: 67af2b060e02 ("[SCSI] dpt_i2o: move from virt_to_bus/bus_to_virt ...")
Signed-off-by: Ben Hutchings <benh@debian.org>
---
 drivers/scsi/dpt_i2o.c | 265 +----------------------------------------
 drivers/scsi/dpti.h    |   1 -
 2 files changed, 3 insertions(+), 263 deletions(-)

diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 37de8fb186d7..b63908c359e2 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -628,51 +628,6 @@ static struct scsi_cmnd *
 	return NULL;
 }
 
-/*
- *	Turn a pointer to ioctl reply data into an u32 'context'
- */
-static u32 adpt_ioctl_to_context(adpt_hba * pHba, void *reply)
-{
-#if BITS_PER_LONG == 32
-	return (u32)(unsigned long)reply;
-#else
-	ulong flags = 0;
-	u32 nr, i;
-
-	spin_lock_irqsave(pHba->host->host_lock, flags);
-	nr = ARRAY_SIZE(pHba->ioctl_reply_context);
-	for (i = 0; i < nr; i++) {
-		if (pHba->ioctl_reply_context[i] == NULL) {
-			pHba->ioctl_reply_context[i] = reply;
-			break;
-		}
-	}
-	spin_unlock_irqrestore(pHba->host->host_lock, flags);
-	if (i >= nr) {
-		printk(KERN_WARNING"%s: Too many outstanding "
-				"ioctl commands\n", pHba->name);
-		return (u32)-1;
-	}
-
-	return i;
-#endif
-}
-
-/*
- *	Go from an u32 'context' to a pointer to ioctl reply data.
- */
-static void *adpt_ioctl_from_context(adpt_hba *pHba, u32 context)
-{
-#if BITS_PER_LONG == 32
-	return (void *)(unsigned long)context;
-#else
-	void *p = pHba->ioctl_reply_context[context];
-	pHba->ioctl_reply_context[context] = NULL;
-
-	return p;
-#endif
-}
-
 /*===========================================================================
  * Error Handling routines
  *===========================================================================
@@ -1697,208 +1652,6 @@ static int adpt_close(struct inode *inode, struct file *file)
 	return 0;
 }
 
-
-static int adpt_i2o_passthru(adpt_hba* pHba, u32 __user *arg)
-{
-	u32 msg[MAX_MESSAGE_SIZE];
-	u32* reply = NULL;
-	u32 size = 0;
-	u32 reply_size = 0;
-	u32 __user *user_msg = arg;
-	u32 __user * user_reply = NULL;
-	void **sg_list = NULL;
-	u32 sg_offset = 0;
-	u32 sg_count = 0;
-	int sg_index = 0;
-	u32 i = 0;
-	u32 rcode = 0;
-	void *p = NULL;
-	dma_addr_t addr;
-	ulong flags = 0;
-
-	memset(&msg, 0, MAX_MESSAGE_SIZE*4);
-	// get user msg size in u32s 
-	if(get_user(size, &user_msg[0])){
-		return -EFAULT;
-	}
-	size = size>>16;
-
-	user_reply = &user_msg[size];
-	if(size > MAX_MESSAGE_SIZE){
-		return -EFAULT;
-	}
-	size *= 4; // Convert to bytes
-
-	/* Copy in the user's I2O command */
-	if(copy_from_user(msg, user_msg, size)) {
-		return -EFAULT;
-	}
-	get_user(reply_size, &user_reply[0]);
-	reply_size = reply_size>>16;
-	if(reply_size > REPLY_FRAME_SIZE){
-		reply_size = REPLY_FRAME_SIZE;
-	}
-	reply_size *= 4;
-	reply = kzalloc(REPLY_FRAME_SIZE*4, GFP_KERNEL);
-	if(reply == NULL) {
-		printk(KERN_WARNING"%s: Could not allocate reply buffer\n",pHba->name);
-		return -ENOMEM;
-	}
-	sg_offset = (msg[0]>>4)&0xf;
-	msg[2] = 0x40000000; // IOCTL context
-	msg[3] = adpt_ioctl_to_context(pHba, reply);
-	if (msg[3] == (u32)-1) {
-		rcode = -EBUSY;
-		goto free;
-	}
-
-	sg_list = kcalloc(pHba->sg_tablesize, sizeof(*sg_list), GFP_KERNEL);
-	if (!sg_list) {
-		rcode = -ENOMEM;
-		goto free;
-	}
-	if(sg_offset) {
-		// TODO add 64 bit API
-		struct sg_simple_element *sg =  (struct sg_simple_element*) (msg+sg_offset);
-		sg_count = (size - sg_offset*4) / sizeof(struct sg_simple_element);
-		if (sg_count > pHba->sg_tablesize){
-			printk(KERN_DEBUG"%s:IOCTL SG List too large (%u)\n", pHba->name,sg_count);
-			rcode = -EINVAL;
-			goto free;
-		}
-
-		for(i = 0; i < sg_count; i++) {
-			int sg_size;
-
-			if (!(sg[i].flag_count & 0x10000000 /*I2O_SGL_FLAGS_SIMPLE_ADDRESS_ELEMENT*/)) {
-				printk(KERN_DEBUG"%s:Bad SG element %d - not simple (%x)\n",pHba->name,i,  sg[i].flag_count);
-				rcode = -EINVAL;
-				goto cleanup;
-			}
-			sg_size = sg[i].flag_count & 0xffffff;      
-			/* Allocate memory for the transfer */
-			p = dma_alloc_coherent(&pHba->pDev->dev, sg_size, &addr, GFP_KERNEL);
-			if(!p) {
-				printk(KERN_DEBUG"%s: Could not allocate SG buffer - size = %d buffer number %d of %d\n",
-						pHba->name,sg_size,i,sg_count);
-				rcode = -ENOMEM;
-				goto cleanup;
-			}
-			sg_list[sg_index++] = p; // sglist indexed with input frame, not our internal frame.
-			/* Copy in the user's SG buffer if necessary */
-			if(sg[i].flag_count & 0x04000000 /*I2O_SGL_FLAGS_DIR*/) {
-				// sg_simple_element API is 32 bit
-				if (copy_from_user(p,(void __user *)(ulong)sg[i].addr_bus, sg_size)) {
-					printk(KERN_DEBUG"%s: Could not copy SG buf %d FROM user\n",pHba->name,i);
-					rcode = -EFAULT;
-					goto cleanup;
-				}
-			}
-			/* sg_simple_element API is 32 bit, but addr < 4GB */
-			sg[i].addr_bus = addr;
-		}
-	}
-
-	do {
-		/*
-		 * Stop any new commands from enterring the
-		 * controller while processing the ioctl
-		 */
-		if (pHba->host) {
-			scsi_block_requests(pHba->host);
-			spin_lock_irqsave(pHba->host->host_lock, flags);
-		}
-		rcode = adpt_i2o_post_wait(pHba, msg, size, FOREVER);
-		if (rcode != 0)
-			printk("adpt_i2o_passthru: post wait failed %d %p\n",
-					rcode, reply);
-		if (pHba->host) {
-			spin_unlock_irqrestore(pHba->host->host_lock, flags);
-			scsi_unblock_requests(pHba->host);
-		}
-	} while (rcode == -ETIMEDOUT);
-
-	if(rcode){
-		goto cleanup;
-	}
-
-	if(sg_offset) {
-	/* Copy back the Scatter Gather buffers back to user space */
-		u32 j;
-		// TODO add 64 bit API
-		struct sg_simple_element* sg;
-		int sg_size;
-
-		// re-acquire the original message to handle correctly the sg copy operation
-		memset(&msg, 0, MAX_MESSAGE_SIZE*4); 
-		// get user msg size in u32s 
-		if(get_user(size, &user_msg[0])){
-			rcode = -EFAULT; 
-			goto cleanup; 
-		}
-		size = size>>16;
-		size *= 4;
-		if (size > MAX_MESSAGE_SIZE) {
-			rcode = -EINVAL;
-			goto cleanup;
-		}
-		/* Copy in the user's I2O command */
-		if (copy_from_user (msg, user_msg, size)) {
-			rcode = -EFAULT;
-			goto cleanup;
-		}
-		sg_count = (size - sg_offset*4) / sizeof(struct sg_simple_element);
-
-		// TODO add 64 bit API
-		sg 	 = (struct sg_simple_element*)(msg + sg_offset);
-		for (j = 0; j < sg_count; j++) {
-			/* Copy out the SG list to user's buffer if necessary */
-			if(! (sg[j].flag_count & 0x4000000 /*I2O_SGL_FLAGS_DIR*/)) {
-				sg_size = sg[j].flag_count & 0xffffff; 
-				// sg_simple_element API is 32 bit
-				if (copy_to_user((void __user *)(ulong)sg[j].addr_bus,sg_list[j], sg_size)) {
-					printk(KERN_WARNING"%s: Could not copy %p TO user %x\n",pHba->name, sg_list[j], sg[j].addr_bus);
-					rcode = -EFAULT;
-					goto cleanup;
-				}
-			}
-		}
-	} 
-
-	/* Copy back the reply to user space */
-	if (reply_size) {
-		// we wrote our own values for context - now restore the user supplied ones
-		if(copy_from_user(reply+2, user_msg+2, sizeof(u32)*2)) {
-			printk(KERN_WARNING"%s: Could not copy message context FROM user\n",pHba->name);
-			rcode = -EFAULT;
-		}
-		if(copy_to_user(user_reply, reply, reply_size)) {
-			printk(KERN_WARNING"%s: Could not copy reply TO user\n",pHba->name);
-			rcode = -EFAULT;
-		}
-	}
-
-
-cleanup:
-	if (rcode != -ETIME && rcode != -EINTR) {
-		struct sg_simple_element *sg =
-				(struct sg_simple_element*) (msg +sg_offset);
-		while(sg_index) {
-			if(sg_list[--sg_index]) {
-				dma_free_coherent(&pHba->pDev->dev,
-					sg[sg_index].flag_count & 0xffffff,
-					sg_list[sg_index],
-					sg[sg_index].addr_bus);
-			}
-		}
-	}
-
-free:
-	kfree(sg_list);
-	kfree(reply);
-	return rcode;
-}
-
 #if defined __ia64__ 
 static void adpt_ia64_info(sysInfo_S* si)
 {
@@ -2025,8 +1778,6 @@ static int adpt_ioctl(struct inode *inode, struct file *file, uint cmd, ulong ar
 			return -EFAULT;
 		}
 		break;
-	case I2OUSRCMD:
-		return adpt_i2o_passthru(pHba, argp);
 
 	case DPT_CTRLINFO:{
 		drvrHBAinfo_S HbaInfo;
@@ -2183,13 +1934,6 @@ static irqreturn_t adpt_isr(int irq, void *dev_id)
 			adpt_send_nop(pHba, old_m);
 		} 
 		context = readl(reply+8);
-		if(context & 0x40000000){ // IOCTL
-			void *p = adpt_ioctl_from_context(pHba, readl(reply+12));
-			if( p != NULL) {
-				memcpy_fromio(p, reply, REPLY_FRAME_SIZE * 4);
-			}
-			// All IOCTLs will also be post wait
-		}
 		if(context & 0x80000000){ // Post wait message
 			status = readl(reply+16);
 			if(status  >> 24){
@@ -2197,12 +1941,9 @@ static irqreturn_t adpt_isr(int irq, void *dev_id)
 			} else {
 				status = I2O_POST_WAIT_OK;
 			}
-			if(!(context & 0x40000000)) {
-				cmd = adpt_cmd_from_context(pHba,
-							readl(reply+12));
-				if(cmd != NULL) {
-					printk(KERN_WARNING"%s: Apparent SCSI cmd in Post Wait Context - cmd=%p context=%x\n", pHba->name, cmd, context);
-				}
+			cmd = adpt_cmd_from_context(pHba, readl(reply+12));
+			if(cmd != NULL) {
+				printk(KERN_WARNING"%s: Apparent SCSI cmd in Post Wait Context - cmd=%p context=%x\n", pHba->name, cmd, context);
 			}
 			adpt_i2o_post_wait_complete(context, status);
 		} else { // SCSI message
diff --git a/drivers/scsi/dpti.h b/drivers/scsi/dpti.h
index dfc8d2eaa09e..9a313883788a 100644
--- a/drivers/scsi/dpti.h
+++ b/drivers/scsi/dpti.h
@@ -251,7 +251,6 @@ typedef struct _adpt_hba {
 	void __iomem *FwDebugBLEDflag_P;// Virtual Addr Of FW Debug BLED
 	void __iomem *FwDebugBLEDvalue_P;// Virtual Addr Of FW Debug BLED
 	u32 FwDebugFlags;
-	u32 *ioctl_reply_context[4];
 } adpt_hba;
 
 struct sg_simple_element {

[-- Attachment #1.4: 0001-scsi-dpt_i2o-Fix-various-bugs-in-adpt_i2o_passthru-5.4-5.10-5.15.patch --]
[-- Type: text/x-patch, Size: 10965 bytes --]

From f4b327796d84579bea47fdaaaba2c9900b881237 Mon Sep 17 00:00:00 2001
From: Ben Hutchings <benh@debian.org>
Date: Sat, 27 May 2023 15:34:30 +0200
Subject: [PATCH 1/2] scsi: dpt_i2o: Remove broken pass-through ioctl (I2OUSERCMD)

adpt_i2o_passthru() takes a user-provided message and passes it
through to the hardware with appropriate translation of addresses
and message IDs.  It has a number of bugs:

- When a message requires scatter/gather, it doesn't verify that the
  offset to the scatter/gather list is less than the message size.
- When a message requires scatter/gather, it overwrites the DMA
  addresses with the user-space virtual addresses before unmapping the
  DMA buffers.
- It reads the message from user memory multiple times.  This allows
  user-space to change the message and bypass validation.
- It assumes that the message is at least 4 words long, but doesn't
  check that.

I tried fixing these, but even the maintainer of the corresponding
user-space in Debian doesn't have the hardware any more.

Instead, remove the pass-through ioctl (I2OUSRCMD) and supporting
code.

There is no corresponding upstream commit, because this driver was
removed upstream.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: 67af2b060e02 ("[SCSI] dpt_i2o: move from virt_to_bus/bus_to_virt ...")
Signed-off-by: Ben Hutchings <benh@debian.org>
---
 drivers/scsi/dpt_i2o.c | 274 ++---------------------------------------
 drivers/scsi/dpti.h    |   1 -
 2 files changed, 8 insertions(+), 267 deletions(-)

diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 4251212acbbe..85f4d6535154 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -582,51 +582,6 @@ static int adpt_show_info(struct seq_file *m, struct Scsi_Host *host)
 	return 0;
 }
 
-/*
- *	Turn a pointer to ioctl reply data into an u32 'context'
- */
-static u32 adpt_ioctl_to_context(adpt_hba * pHba, void *reply)
-{
-#if BITS_PER_LONG == 32
-	return (u32)(unsigned long)reply;
-#else
-	ulong flags = 0;
-	u32 nr, i;
-
-	spin_lock_irqsave(pHba->host->host_lock, flags);
-	nr = ARRAY_SIZE(pHba->ioctl_reply_context);
-	for (i = 0; i < nr; i++) {
-		if (pHba->ioctl_reply_context[i] == NULL) {
-			pHba->ioctl_reply_context[i] = reply;
-			break;
-		}
-	}
-	spin_unlock_irqrestore(pHba->host->host_lock, flags);
-	if (i >= nr) {
-		printk(KERN_WARNING"%s: Too many outstanding "
-				"ioctl commands\n", pHba->name);
-		return (u32)-1;
-	}
-
-	return i;
-#endif
-}
-
-/*
- *	Go from an u32 'context' to a pointer to ioctl reply data.
- */
-static void *adpt_ioctl_from_context(adpt_hba *pHba, u32 context)
-{
-#if BITS_PER_LONG == 32
-	return (void *)(unsigned long)context;
-#else
-	void *p = pHba->ioctl_reply_context[context];
-	pHba->ioctl_reply_context[context] = NULL;
-
-	return p;
-#endif
-}
-
 /*===========================================================================
  * Error Handling routines
  *===========================================================================
@@ -1648,208 +1603,6 @@ static int adpt_close(struct inode *inode, struct file *file)
 	return 0;
 }
 
-
-static int adpt_i2o_passthru(adpt_hba* pHba, u32 __user *arg)
-{
-	u32 msg[MAX_MESSAGE_SIZE];
-	u32* reply = NULL;
-	u32 size = 0;
-	u32 reply_size = 0;
-	u32 __user *user_msg = arg;
-	u32 __user * user_reply = NULL;
-	void **sg_list = NULL;
-	u32 sg_offset = 0;
-	u32 sg_count = 0;
-	int sg_index = 0;
-	u32 i = 0;
-	u32 rcode = 0;
-	void *p = NULL;
-	dma_addr_t addr;
-	ulong flags = 0;
-
-	memset(&msg, 0, MAX_MESSAGE_SIZE*4);
-	// get user msg size in u32s 
-	if(get_user(size, &user_msg[0])){
-		return -EFAULT;
-	}
-	size = size>>16;
-
-	user_reply = &user_msg[size];
-	if(size > MAX_MESSAGE_SIZE){
-		return -EFAULT;
-	}
-	size *= 4; // Convert to bytes
-
-	/* Copy in the user's I2O command */
-	if(copy_from_user(msg, user_msg, size)) {
-		return -EFAULT;
-	}
-	get_user(reply_size, &user_reply[0]);
-	reply_size = reply_size>>16;
-	if(reply_size > REPLY_FRAME_SIZE){
-		reply_size = REPLY_FRAME_SIZE;
-	}
-	reply_size *= 4;
-	reply = kzalloc(REPLY_FRAME_SIZE*4, GFP_KERNEL);
-	if(reply == NULL) {
-		printk(KERN_WARNING"%s: Could not allocate reply buffer\n",pHba->name);
-		return -ENOMEM;
-	}
-	sg_offset = (msg[0]>>4)&0xf;
-	msg[2] = 0x40000000; // IOCTL context
-	msg[3] = adpt_ioctl_to_context(pHba, reply);
-	if (msg[3] == (u32)-1) {
-		rcode = -EBUSY;
-		goto free;
-	}
-
-	sg_list = kcalloc(pHba->sg_tablesize, sizeof(*sg_list), GFP_KERNEL);
-	if (!sg_list) {
-		rcode = -ENOMEM;
-		goto free;
-	}
-	if(sg_offset) {
-		// TODO add 64 bit API
-		struct sg_simple_element *sg =  (struct sg_simple_element*) (msg+sg_offset);
-		sg_count = (size - sg_offset*4) / sizeof(struct sg_simple_element);
-		if (sg_count > pHba->sg_tablesize){
-			printk(KERN_DEBUG"%s:IOCTL SG List too large (%u)\n", pHba->name,sg_count);
-			rcode = -EINVAL;
-			goto free;
-		}
-
-		for(i = 0; i < sg_count; i++) {
-			int sg_size;
-
-			if (!(sg[i].flag_count & 0x10000000 /*I2O_SGL_FLAGS_SIMPLE_ADDRESS_ELEMENT*/)) {
-				printk(KERN_DEBUG"%s:Bad SG element %d - not simple (%x)\n",pHba->name,i,  sg[i].flag_count);
-				rcode = -EINVAL;
-				goto cleanup;
-			}
-			sg_size = sg[i].flag_count & 0xffffff;      
-			/* Allocate memory for the transfer */
-			p = dma_alloc_coherent(&pHba->pDev->dev, sg_size, &addr, GFP_KERNEL);
-			if(!p) {
-				printk(KERN_DEBUG"%s: Could not allocate SG buffer - size = %d buffer number %d of %d\n",
-						pHba->name,sg_size,i,sg_count);
-				rcode = -ENOMEM;
-				goto cleanup;
-			}
-			sg_list[sg_index++] = p; // sglist indexed with input frame, not our internal frame.
-			/* Copy in the user's SG buffer if necessary */
-			if(sg[i].flag_count & 0x04000000 /*I2O_SGL_FLAGS_DIR*/) {
-				// sg_simple_element API is 32 bit
-				if (copy_from_user(p,(void __user *)(ulong)sg[i].addr_bus, sg_size)) {
-					printk(KERN_DEBUG"%s: Could not copy SG buf %d FROM user\n",pHba->name,i);
-					rcode = -EFAULT;
-					goto cleanup;
-				}
-			}
-			/* sg_simple_element API is 32 bit, but addr < 4GB */
-			sg[i].addr_bus = addr;
-		}
-	}
-
-	do {
-		/*
-		 * Stop any new commands from enterring the
-		 * controller while processing the ioctl
-		 */
-		if (pHba->host) {
-			scsi_block_requests(pHba->host);
-			spin_lock_irqsave(pHba->host->host_lock, flags);
-		}
-		rcode = adpt_i2o_post_wait(pHba, msg, size, FOREVER);
-		if (rcode != 0)
-			printk("adpt_i2o_passthru: post wait failed %d %p\n",
-					rcode, reply);
-		if (pHba->host) {
-			spin_unlock_irqrestore(pHba->host->host_lock, flags);
-			scsi_unblock_requests(pHba->host);
-		}
-	} while (rcode == -ETIMEDOUT);
-
-	if(rcode){
-		goto cleanup;
-	}
-
-	if(sg_offset) {
-	/* Copy back the Scatter Gather buffers back to user space */
-		u32 j;
-		// TODO add 64 bit API
-		struct sg_simple_element* sg;
-		int sg_size;
-
-		// re-acquire the original message to handle correctly the sg copy operation
-		memset(&msg, 0, MAX_MESSAGE_SIZE*4); 
-		// get user msg size in u32s 
-		if(get_user(size, &user_msg[0])){
-			rcode = -EFAULT; 
-			goto cleanup; 
-		}
-		size = size>>16;
-		size *= 4;
-		if (size > MAX_MESSAGE_SIZE) {
-			rcode = -EINVAL;
-			goto cleanup;
-		}
-		/* Copy in the user's I2O command */
-		if (copy_from_user (msg, user_msg, size)) {
-			rcode = -EFAULT;
-			goto cleanup;
-		}
-		sg_count = (size - sg_offset*4) / sizeof(struct sg_simple_element);
-
-		// TODO add 64 bit API
-		sg 	 = (struct sg_simple_element*)(msg + sg_offset);
-		for (j = 0; j < sg_count; j++) {
-			/* Copy out the SG list to user's buffer if necessary */
-			if(! (sg[j].flag_count & 0x4000000 /*I2O_SGL_FLAGS_DIR*/)) {
-				sg_size = sg[j].flag_count & 0xffffff; 
-				// sg_simple_element API is 32 bit
-				if (copy_to_user((void __user *)(ulong)sg[j].addr_bus,sg_list[j], sg_size)) {
-					printk(KERN_WARNING"%s: Could not copy %p TO user %x\n",pHba->name, sg_list[j], sg[j].addr_bus);
-					rcode = -EFAULT;
-					goto cleanup;
-				}
-			}
-		}
-	} 
-
-	/* Copy back the reply to user space */
-	if (reply_size) {
-		// we wrote our own values for context - now restore the user supplied ones
-		if(copy_from_user(reply+2, user_msg+2, sizeof(u32)*2)) {
-			printk(KERN_WARNING"%s: Could not copy message context FROM user\n",pHba->name);
-			rcode = -EFAULT;
-		}
-		if(copy_to_user(user_reply, reply, reply_size)) {
-			printk(KERN_WARNING"%s: Could not copy reply TO user\n",pHba->name);
-			rcode = -EFAULT;
-		}
-	}
-
-
-cleanup:
-	if (rcode != -ETIME && rcode != -EINTR) {
-		struct sg_simple_element *sg =
-				(struct sg_simple_element*) (msg +sg_offset);
-		while(sg_index) {
-			if(sg_list[--sg_index]) {
-				dma_free_coherent(&pHba->pDev->dev,
-					sg[sg_index].flag_count & 0xffffff,
-					sg_list[sg_index],
-					sg[sg_index].addr_bus);
-			}
-		}
-	}
-
-free:
-	kfree(sg_list);
-	kfree(reply);
-	return rcode;
-}
-
 #if defined __ia64__ 
 static void adpt_ia64_info(sysInfo_S* si)
 {
@@ -1976,8 +1729,6 @@ static int adpt_ioctl(struct inode *inode, struct file *file, uint cmd, ulong ar
 			return -EFAULT;
 		}
 		break;
-	case I2OUSRCMD:
-		return adpt_i2o_passthru(pHba, argp);
 
 	case DPT_CTRLINFO:{
 		drvrHBAinfo_S HbaInfo;
@@ -2134,13 +1885,6 @@ static irqreturn_t adpt_isr(int irq, void *dev_id)
 			adpt_send_nop(pHba, old_m);
 		} 
 		context = readl(reply+8);
-		if(context & 0x40000000){ // IOCTL
-			void *p = adpt_ioctl_from_context(pHba, readl(reply+12));
-			if( p != NULL) {
-				memcpy_fromio(p, reply, REPLY_FRAME_SIZE * 4);
-			}
-			// All IOCTLs will also be post wait
-		}
 		if(context & 0x80000000){ // Post wait message
 			status = readl(reply+16);
 			if(status  >> 24){
@@ -2148,16 +1892,14 @@ static irqreturn_t adpt_isr(int irq, void *dev_id)
 			} else {
 				status = I2O_POST_WAIT_OK;
 			}
-			if(!(context & 0x40000000)) {
-				/*
-				 * The request tag is one less than the command tag
-				 * as the firmware might treat a 0 tag as invalid
-				 */
-				cmd = scsi_host_find_tag(pHba->host,
-							 readl(reply + 12) - 1);
-				if(cmd != NULL) {
-					printk(KERN_WARNING"%s: Apparent SCSI cmd in Post Wait Context - cmd=%p context=%x\n", pHba->name, cmd, context);
-				}
+			/*
+			 * The request tag is one less than the command tag
+			 * as the firmware might treat a 0 tag as invalid
+			 */
+			cmd = scsi_host_find_tag(pHba->host,
+						 readl(reply + 12) - 1);
+			if(cmd != NULL) {
+				printk(KERN_WARNING"%s: Apparent SCSI cmd in Post Wait Context - cmd=%p context=%x\n", pHba->name, cmd, context);
 			}
 			adpt_i2o_post_wait_complete(context, status);
 		} else { // SCSI message
diff --git a/drivers/scsi/dpti.h b/drivers/scsi/dpti.h
index 8a079e8d7f65..0565533e8095 100644
--- a/drivers/scsi/dpti.h
+++ b/drivers/scsi/dpti.h
@@ -248,7 +248,6 @@ typedef struct _adpt_hba {
 	void __iomem *FwDebugBLEDflag_P;// Virtual Addr Of FW Debug BLED
 	void __iomem *FwDebugBLEDvalue_P;// Virtual Addr Of FW Debug BLED
 	u32 FwDebugFlags;
-	u32 *ioctl_reply_context[4];
 } adpt_hba;
 
 struct sg_simple_element {

[-- Attachment #1.5: 0002-scsi-dpt_i2o-Do-not-process-completions-with-invalid-4.14-4.19-5.4.patch --]
[-- Type: text/x-patch, Size: 2279 bytes --]

From c5b94ca174d28cf144665527236638e96b7fc758 Mon Sep 17 00:00:00 2001
From: Ben Hutchings <benh@debian.org>
Date: Sat, 27 May 2023 15:52:48 +0200
Subject: [PATCH 2/2] scsi: dpt_i2o: Do not process completions with invalid
 addresses

adpt_isr() reads reply addresses from a hardware register, which
should always be within the DMA address range of the device's pool of
reply address buffers.  In case the address is out of range, it tries
to muddle on, converting to a virtual address using bus_to_virt().

bus_to_virt() does not take DMA addresses, and it doesn't make sense
to try to handle the completion in this case.  Ignore it and continue
looping to service the interrupt.  If a completion has been lost then
the SCSI core should eventually time-out and trigger a reset.

There is no corresponding upstream commit, because this driver was
removed upstream.

Fixes: 67af2b060e02 ("[SCSI] dpt_i2o: move from virt_to_bus/bus_to_virt ...")
Signed-off-by: Ben Hutchings <benh@debian.org>
---
 drivers/scsi/Kconfig   | 2 +-
 drivers/scsi/dpt_i2o.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 7cb6e2b9e180..6047f0284f73 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -473,7 +473,7 @@ config SCSI_MVUMI
 
 config SCSI_DPT_I2O
 	tristate "Adaptec I2O RAID support "
-	depends on SCSI && PCI && VIRT_TO_BUS
+	depends on SCSI && PCI
 	help
 	  This driver supports all of Adaptec's I2O based RAID controllers as 
 	  well as the DPT SmartRaid V cards.  This is an Adaptec maintained
diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index b63908c359e2..3f8d1c17e938 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -59,7 +59,7 @@ MODULE_DESCRIPTION("Adaptec I2O RAID Driver");
 
 #include <asm/processor.h>	/* for boot_cpu_data */
 #include <asm/pgtable.h>
-#include <asm/io.h>		/* for virt_to_bus, etc. */
+#include <asm/io.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -1914,7 +1914,7 @@ static irqreturn_t adpt_isr(int irq, void *dev_id)
 		} else {
 			/* Ick, we should *never* be here */
 			printk(KERN_ERR "dpti: reply frame not from pool\n");
-			reply = (u8 *)bus_to_virt(m);
+			continue;
 		}
 
 		if (readl(reply) & MSG_FAIL) {

[-- Attachment #1.6: 0002-scsi-dpt_i2o-Do-not-process-completions-with-invalid-5.10-5.15.patch --]
[-- Type: text/x-patch, Size: 2279 bytes --]

From 157298ea77a54f7793b370cb8cdfa967811adb66 Mon Sep 17 00:00:00 2001
From: Ben Hutchings <benh@debian.org>
Date: Sat, 27 May 2023 15:52:48 +0200
Subject: [PATCH 2/2] scsi: dpt_i2o: Do not process completions with invalid
 addresses

adpt_isr() reads reply addresses from a hardware register, which
should always be within the DMA address range of the device's pool of
reply address buffers.  In case the address is out of range, it tries
to muddle on, converting to a virtual address using bus_to_virt().

bus_to_virt() does not take DMA addresses, and it doesn't make sense
to try to handle the completion in this case.  Ignore it and continue
looping to service the interrupt.  If a completion has been lost then
the SCSI core should eventually time-out and trigger a reset.

There is no corresponding upstream commit, because this driver was
removed upstream.

Fixes: 67af2b060e02 ("[SCSI] dpt_i2o: move from virt_to_bus/bus_to_virt ...")
Signed-off-by: Ben Hutchings <benh@debian.org>
---
 drivers/scsi/Kconfig   | 2 +-
 drivers/scsi/dpt_i2o.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 701b61ec76ee..6524e1fe54d2 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -444,7 +444,7 @@ config SCSI_MVUMI
 
 config SCSI_DPT_I2O
 	tristate "Adaptec I2O RAID support "
-	depends on SCSI && PCI && VIRT_TO_BUS
+	depends on SCSI && PCI
 	help
 	  This driver supports all of Adaptec's I2O based RAID controllers as 
 	  well as the DPT SmartRaid V cards.  This is an Adaptec maintained
diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 85f4d6535154..43ec5657a935 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -56,7 +56,7 @@ MODULE_DESCRIPTION("Adaptec I2O RAID Driver");
 #include <linux/mutex.h>
 
 #include <asm/processor.h>	/* for boot_cpu_data */
-#include <asm/io.h>		/* for virt_to_bus, etc. */
+#include <asm/io.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -1865,7 +1865,7 @@ static irqreturn_t adpt_isr(int irq, void *dev_id)
 		} else {
 			/* Ick, we should *never* be here */
 			printk(KERN_ERR "dpti: reply frame not from pool\n");
-			reply = (u8 *)bus_to_virt(m);
+			continue;
 		}
 
 		if (readl(reply) & MSG_FAIL) {

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 858 bytes --]

Re: dpt_i2o fixes for stable

[Greg Kroah-Hartman]

On Sat, May 27, 2023 at 10:42:00PM +0200, Ben Hutchings wrote:
> I'm proposing to address the most obvious issues with dpt_i2o on stable
> branches.  At this stage it may be better to remove it as has been done
> upstream, but I'd rather limit the regression for anyone still using
> the hardware.
> 
> The changes are:
> 
> - "scsi: dpt_i2o: Remove broken pass-through ioctl (I2OUSERCMD)",
>   which closes security flaws including CVE-2023-2007.
> - "scsi: dpt_i2o: Do not process completions with invalid addresses",
>   which removes the remaining bus_to_virt() call and may slightly
>   improve handling of misbehaving hardware.
> 
> These changes have been compiled on all the relevant stable branches,
> but I don't have hardware to test on.

Why don't we just delete it in the stable trees as well?  If no one has
the hardware (otherwise the driver would not have been removed), who is
going to hit these issues anyway?

thanks,

greg k-h

Re: dpt_i2o fixes for stable

[Finn Thain]

On Sun, 28 May 2023, Greg Kroah-Hartman wrote:

> On Sat, May 27, 2023 at 10:42:00PM +0200, Ben Hutchings wrote:
> > I'm proposing to address the most obvious issues with dpt_i2o on stable
> > branches.  At this stage it may be better to remove it as has been done
> > upstream, but I'd rather limit the regression for anyone still using
> > the hardware.
> > 
> > The changes are:
> > 
> > - "scsi: dpt_i2o: Remove broken pass-through ioctl (I2OUSERCMD)",
> >   which closes security flaws including CVE-2023-2007.
> > - "scsi: dpt_i2o: Do not process completions with invalid addresses",
> >   which removes the remaining bus_to_virt() call and may slightly
> >   improve handling of misbehaving hardware.
> > 
> > These changes have been compiled on all the relevant stable branches,
> > but I don't have hardware to test on.
> 
> Why don't we just delete it in the stable trees as well?  If no one has
> the hardware (otherwise the driver would not have been removed), who is
> going to hit these issues anyway?
> 

It's already gone from two stable trees. Would you also have it deleted 
from users' machines, or would you have each distro separately maintain 
out-of-tree that code which it is presently shipping, or something else?

Re: dpt_i2o fixes for stable

[Greg Kroah-Hartman]

On Sun, May 28, 2023 at 07:58:11PM +1000, Finn Thain wrote:
> On Sun, 28 May 2023, Greg Kroah-Hartman wrote:
> 
> > On Sat, May 27, 2023 at 10:42:00PM +0200, Ben Hutchings wrote:
> > > I'm proposing to address the most obvious issues with dpt_i2o on stable
> > > branches.  At this stage it may be better to remove it as has been done
> > > upstream, but I'd rather limit the regression for anyone still using
> > > the hardware.
> > > 
> > > The changes are:
> > > 
> > > - "scsi: dpt_i2o: Remove broken pass-through ioctl (I2OUSERCMD)",
> > >   which closes security flaws including CVE-2023-2007.
> > > - "scsi: dpt_i2o: Do not process completions with invalid addresses",
> > >   which removes the remaining bus_to_virt() call and may slightly
> > >   improve handling of misbehaving hardware.
> > > 
> > > These changes have been compiled on all the relevant stable branches,
> > > but I don't have hardware to test on.
> > 
> > Why don't we just delete it in the stable trees as well?  If no one has
> > the hardware (otherwise the driver would not have been removed), who is
> > going to hit these issues anyway?
> > 
> 
> It's already gone from two stable trees. Would you also have it deleted 
> from users' machines, or would you have each distro separately maintain 
> out-of-tree that code which it is presently shipping, or something else?

Delete it as obviously no one actually has this hardware.  Or just leave
it alone, as obviously no one has this hardware so any changes made to
the code would not actually affect anyone.

Or am I missing something here?

thanks,

greg k-h

Re: dpt_i2o fixes for stable

[Ben Hutchings]

On Sun, 2023-05-28 at 08:02 +0100, Greg Kroah-Hartman wrote:
> On Sat, May 27, 2023 at 10:42:00PM +0200, Ben Hutchings wrote:
> > I'm proposing to address the most obvious issues with dpt_i2o on stable
> > branches.  At this stage it may be better to remove it as has been done
> > upstream, but I'd rather limit the regression for anyone still using
> > the hardware.
> > 
> > The changes are:
> > 
> > - "scsi: dpt_i2o: Remove broken pass-through ioctl (I2OUSERCMD)",
> >   which closes security flaws including CVE-2023-2007.
> > - "scsi: dpt_i2o: Do not process completions with invalid addresses",
> >   which removes the remaining bus_to_virt() call and may slightly
> >   improve handling of misbehaving hardware.
> > 
> > These changes have been compiled on all the relevant stable branches,
> > but I don't have hardware to test on.
> 
> Why don't we just delete it in the stable trees as well?  If no one has
> the hardware (otherwise the driver would not have been removed), who is
> going to hit these issues anyway?

We don't know that no-one is using the hardware, just because no-one
among a small group of kernel developers and early adopters has spoken
up yet.

Ben.

-- 
Ben Hutchings - Debian developer, member of kernel, installer and LTS
teams

Re: dpt_i2o fixes for stable

[Greg Kroah-Hartman]

On Sun, May 28, 2023 at 02:40:52PM +0200, Ben Hutchings wrote:
> On Sun, 2023-05-28 at 08:02 +0100, Greg Kroah-Hartman wrote:
> > On Sat, May 27, 2023 at 10:42:00PM +0200, Ben Hutchings wrote:
> > > I'm proposing to address the most obvious issues with dpt_i2o on stable
> > > branches.  At this stage it may be better to remove it as has been done
> > > upstream, but I'd rather limit the regression for anyone still using
> > > the hardware.
> > > 
> > > The changes are:
> > > 
> > > - "scsi: dpt_i2o: Remove broken pass-through ioctl (I2OUSERCMD)",
> > >   which closes security flaws including CVE-2023-2007.
> > > - "scsi: dpt_i2o: Do not process completions with invalid addresses",
> > >   which removes the remaining bus_to_virt() call and may slightly
> > >   improve handling of misbehaving hardware.
> > > 
> > > These changes have been compiled on all the relevant stable branches,
> > > but I don't have hardware to test on.
> > 
> > Why don't we just delete it in the stable trees as well?  If no one has
> > the hardware (otherwise the driver would not have been removed), who is
> > going to hit these issues anyway?
> 
> We don't know that no-one is using the hardware, just because no-one
> among a small group of kernel developers and early adopters has spoken
> up yet.

So what are we supposed to do here.  Take patches that even if the
driver is added back upstream will not get merged there (as it will not
be obvious they are needed)?  Or just ignore this?

Why did you work on these changes, were there reports of problems?  Or
complaints from users?  Something else?

thanks,

greg k-h

Re: dpt_i2o fixes for stable

[Finn Thain]

On Sun, 28 May 2023, Greg Kroah-Hartman wrote:

> On Sun, May 28, 2023 at 07:58:11PM +1000, Finn Thain wrote:
> > On Sun, 28 May 2023, Greg Kroah-Hartman wrote:
> > 
> > > On Sat, May 27, 2023 at 10:42:00PM +0200, Ben Hutchings wrote:
> > > > I'm proposing to address the most obvious issues with dpt_i2o on stable
> > > > branches.  At this stage it may be better to remove it as has been done
> > > > upstream, but I'd rather limit the regression for anyone still using
> > > > the hardware.
> > > > 
> > > > The changes are:
> > > > 
> > > > - "scsi: dpt_i2o: Remove broken pass-through ioctl (I2OUSERCMD)",
> > > >   which closes security flaws including CVE-2023-2007.
> > > > - "scsi: dpt_i2o: Do not process completions with invalid addresses",
> > > >   which removes the remaining bus_to_virt() call and may slightly
> > > >   improve handling of misbehaving hardware.
> > > > 
> > > > These changes have been compiled on all the relevant stable branches,
> > > > but I don't have hardware to test on.
> > > 
> > > Why don't we just delete it in the stable trees as well?  If no one has
> > > the hardware (otherwise the driver would not have been removed), who is
> > > going to hit these issues anyway?
> > > 
> > 
> > It's already gone from two stable trees. Would you also have it deleted 
> > from users' machines, or would you have each distro separately maintain 
> > out-of-tree that code which it is presently shipping, or something else?
> 
> Delete it as obviously no one actually has this hardware.  Or just leave
> it alone, as obviously no one has this hardware so any changes made to
> the code would not actually affect anyone.
> 
> Or am I missing something here?
> 

Under the assumption that the hardware does not exist, surely there's no 
value in a distro shipping the driver. No argument from me on that point. 
But the assumption is questionable and impossible to validate.

As b04e75a4a8a8 was never reverted, I infer that users of v6.0 (and later) 
do not need the driver. How do you infer that users of distro kernels are 
not using a given driver?

Re: dpt_i2o fixes for stable

[Finn Thain]

On Sun, 28 May 2023, Greg Kroah-Hartman wrote:

> 
> So what are we supposed to do here.  Take patches that even if the
> driver is added back upstream will not get merged there 
> (as it will not be obvious they are needed)?
> 

As long as there's no maintainer, I don't think you can accept the patch. 
If a maintainer volunteered themselves, I think b04e75a4a8a8 could be 
reverted. Perhaps Ben will apply for that position -- I'm unable to do so.

Re: dpt_i2o fixes for stable

[Greg Kroah-Hartman]

On Sat, May 27, 2023 at 10:42:00PM +0200, Ben Hutchings wrote:
> I'm proposing to address the most obvious issues with dpt_i2o on stable
> branches.  At this stage it may be better to remove it as has been done
> upstream, but I'd rather limit the regression for anyone still using
> the hardware.
> 
> The changes are:
> 
> - "scsi: dpt_i2o: Remove broken pass-through ioctl (I2OUSERCMD)",
>   which closes security flaws including CVE-2023-2007.
> - "scsi: dpt_i2o: Do not process completions with invalid addresses",
>   which removes the remaining bus_to_virt() call and may slightly
>   improve handling of misbehaving hardware.
> 
> These changes have been compiled on all the relevant stable branches,
> but I don't have hardware to test on.

All now queued up, thanks.

greg k-h