| Message ID | b1d71ba992d0adab2519dff17f6d241279c0f5f1.camel@debian.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | dpt_i2o fixes for stable | expand |
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
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?
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
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.
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
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?
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.
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
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) {