When get_vaddr_frames() does its hacky follow_pfn() loop it should never
be allowed to extract a struct page from a normal VMA. This could allow a
serious use-after-free problem on any kernel memory.
Restrict this to only work on VMA's with one of VM_IO | VM_PFNMAP
set. This limits the use-after-free problem to only IO memory, which while
still serious, is an improvement.
Cc: stable(a)vger.kernel.org
Fixes: 8025e5ddf9c1 ("[media] mm: Provide new get_vaddr_frames() helper")
Signed-off-by: Jason Gunthorpe <jgg(a)nvidia.com>
---
mm/frame_vector.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index 10f82d5643b6de..26cb20544b6c37 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -99,6 +99,10 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
if (ret >= nr_frames || start < vma->vm_end)
break;
vma = find_vma_intersection(mm, start, start + 1);
+ if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) {
+ ret = -EINVAL;
+ goto out;
+ }
} while (vma && vma->vm_flags & (VM_IO | VM_PFNMAP));
out:
if (locked)
--
2.28.0
Arbitration Lost (IAL) can happen after every single byte transfer. If
arbitration is lost, the I2C hardware will autonomously switch from
master mode to slave. If a transfer is not aborted in this state,
consecutive transfers will not be executed by the hardware and will
timeout.
Signed-off-by: Christian Eggers <ceggers(a)arri.de>
Cc: stable(a)vger.kernel.org
---
drivers/i2c/busses/i2c-imx.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 745f4071155a..bf0e94814222 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -490,6 +490,16 @@ static int i2c_imx_trx_complete(struct imx_i2c_struct *i2c_imx, bool atomic)
dev_dbg(&i2c_imx->adapter.dev, "<%s> Timeout\n", __func__);
return -ETIMEDOUT;
}
+
+ /* check for arbitration lost */
+ if (i2c_imx->i2csr & I2SR_IAL) {
+ dev_dbg(&i2c_imx->adapter.dev, "<%s> Arbitration lost\n", __func__);
+ i2c_imx_clear_irq(i2c_imx, I2SR_IAL);
+
+ i2c_imx->i2csr = 0;
+ return -EAGAIN;
+ }
+
dev_dbg(&i2c_imx->adapter.dev, "<%s> TRX complete\n", __func__);
i2c_imx->i2csr = 0;
return 0;
--
Christian Eggers
Embedded software developer
Arnold & Richter Cine Technik GmbH & Co. Betriebs KG
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRA 57918
Persoenlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRB 54477
Geschaeftsfuehrer: Dr. Michael Neuhaeuser; Stephan Schenk; Walter Trauninger; Markus Zeiler
Changes since v9 [1]:
- (Boris) Compile out the copy_mc_fragile() infrastructure in the
CONFIG_X86_MCE=n case.
This had several knock-on effects. The proposed x86: copy_mc_generic()
was internally checking for X86_FEATURE_ERMS and falling back to
copy_mc_fragile(), however that fallback is not possible in the
CONFIG_X86_MCE=n case when copy_mc_fragile() is compiled out. Instead,
copy_mc_to_user() is rewritten similar to copy_user_generic() that walks
through several fallback implementations copy_mc_fragile ->
copy_mc_enhanced_fast_string (new) -> copy_user_generic (no #MC
recovery).
[1]: http://lore.kernel.org/r/160087928642.3520.17063139768910633998.stgit@dwill…
---
Hi Boris,
I gave this some soak time over the weekend for the robots to chew on
for regressions. No reports, and the updates pass my testing. Please
consider including this in your updates for v5.10, and thanks for
offering to pick this up.
---
The motivations to go rework memcpy_mcsafe() are that the benefit of
doing slow and careful copies is obviated on newer CPUs, and that the
current opt-in list of cpus to instrument recovery is broken relative to
those cpus. There is no need to keep an opt-in list up to date on an
ongoing basis if pmem/dax operations are instrumented for recovery by
default. With recovery enabled by default the old "mcsafe_key" opt-in to
careful copying can be made a "fragile" opt-out. Where the "fragile"
list takes steps to not consume poison across cachelines.
The discussion with Linus made clear that the current "_mcsafe" suffix
was imprecise to a fault. The operations that are needed by pmem/dax are
to copy from a source address that might throw #MC to a destination that
may write-fault, if it is a user page. So copy_to_user_mcsafe() becomes
copy_mc_to_user() to indicate the separate precautions taken on source
and destination. copy_mc_to_kernel() is introduced as a non-SMAP version
that does not expect write-faults on the destination, but is still
prepared to abort with an error code upon taking #MC.
---
Dan Williams (2):
x86, powerpc: Rename memcpy_mcsafe() to copy_mc_to_{user,kernel}()
x86/copy_mc: Introduce copy_mc_enhanced_fast_string()
arch/powerpc/Kconfig | 2
arch/powerpc/include/asm/string.h | 2
arch/powerpc/include/asm/uaccess.h | 40 +++--
arch/powerpc/lib/Makefile | 2
arch/powerpc/lib/copy_mc_64.S | 4
arch/x86/Kconfig | 2
arch/x86/Kconfig.debug | 2
arch/x86/include/asm/copy_mc_test.h | 75 +++++++++
arch/x86/include/asm/mce.h | 9 +
arch/x86/include/asm/mcsafe_test.h | 75 ---------
arch/x86/include/asm/string_64.h | 32 ----
arch/x86/include/asm/uaccess.h | 9 +
arch/x86/include/asm/uaccess_64.h | 20 --
arch/x86/kernel/cpu/mce/core.c | 8 -
arch/x86/kernel/quirks.c | 10 -
arch/x86/lib/Makefile | 1
arch/x86/lib/copy_mc.c | 96 ++++++++++++
arch/x86/lib/copy_mc_64.S | 163 ++++++++++++++++++++
arch/x86/lib/memcpy_64.S | 115 --------------
arch/x86/lib/usercopy_64.c | 21 ---
drivers/md/dm-writecache.c | 15 +-
drivers/nvdimm/claim.c | 2
drivers/nvdimm/pmem.c | 6 -
include/linux/string.h | 9 -
include/linux/uaccess.h | 13 ++
include/linux/uio.h | 10 +
lib/Kconfig | 7 +
lib/iov_iter.c | 48 +++---
tools/arch/x86/include/asm/mcsafe_test.h | 13 --
tools/arch/x86/lib/memcpy_64.S | 115 --------------
tools/objtool/check.c | 5 -
tools/perf/bench/Build | 1
tools/perf/bench/mem-memcpy-x86-64-lib.c | 24 ---
tools/testing/nvdimm/test/nfit.c | 49 +++---
.../testing/selftests/powerpc/copyloops/.gitignore | 2
tools/testing/selftests/powerpc/copyloops/Makefile | 6 -
.../selftests/powerpc/copyloops/copy_mc_64.S | 1
.../selftests/powerpc/copyloops/memcpy_mcsafe_64.S | 1
38 files changed, 484 insertions(+), 531 deletions(-)
rename arch/powerpc/lib/{memcpy_mcsafe_64.S => copy_mc_64.S} (98%)
create mode 100644 arch/x86/include/asm/copy_mc_test.h
delete mode 100644 arch/x86/include/asm/mcsafe_test.h
create mode 100644 arch/x86/lib/copy_mc.c
create mode 100644 arch/x86/lib/copy_mc_64.S
delete mode 100644 tools/arch/x86/include/asm/mcsafe_test.h
delete mode 100644 tools/perf/bench/mem-memcpy-x86-64-lib.c
create mode 120000 tools/testing/selftests/powerpc/copyloops/copy_mc_64.S
delete mode 120000 tools/testing/selftests/powerpc/copyloops/memcpy_mcsafe_64.S
base-commit: a1b8638ba1320e6684aa98233c15255eb803fac7
Every error log reported by OPAL is exported to userspace through a sysfs
interface and notified using kobject_uevent(). The userspace daemon
(opal_errd) then reads the error log and acknowledges it error log is saved
safely to disk. Once acknowledged the kernel removes the respective sysfs
file entry causing respective resources getting released including kobject.
However there are chances where user daemon may already be scanning elog
entries while new sysfs elog entry is being created by kernel. User daemon
may read this new entry and ack it even before kernel can notify userspace
about it through kobject_uevent() call. If that happens then we have a
potential race between elog_ack_store->kobject_put() and kobject_uevent
which can lead to use-after-free issue of a kernfs object resulting into a
kernel crash. This patch fixes this race by protecting a sysfs file
creation/notification by holding a reference count on kobject until we
safely send kobject_uevent().
The function create_elog_obj() returns the elog object which if used by
caller function will end up in use-after-free problem again. However, the
return value of create_elog_obj() function isn't being used today and there
is need as well. Hence change it to return void to make this fix complete.
Fixes: 774fea1a38c6 ("powerpc/powernv: Read OPAL error log and export it through sysfs")
Cc: <stable(a)vger.kernel.org> # v3.15+
Reported-by: Oliver O'Halloran <oohall(a)gmail.com>
Signed-off-by: Mahesh Salgaonkar <mahesh(a)linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar(a)linux.ibm.com>
Reviewed-by: Oliver O'Halloran <oohall(a)gmail.com>
Reviewed-by: Vasant Hegde <hegdevasant(a)linux.vnet.ibm.com>
---
Chnage in v4:
- Re-worded comments. No code change.
Change in v3:
- Change create_elog_obj function signature to return void.
Change in v2:
- Instead of mutex and use extra reference count on kobject to avoid the
race.
---
arch/powerpc/platforms/powernv/opal-elog.c | 34 ++++++++++++++++++++++++----
1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/opal-elog.c b/arch/powerpc/platforms/powernv/opal-elog.c
index 62ef7ad995da..adf4ff8d0bea 100644
--- a/arch/powerpc/platforms/powernv/opal-elog.c
+++ b/arch/powerpc/platforms/powernv/opal-elog.c
@@ -179,14 +179,14 @@ static ssize_t raw_attr_read(struct file *filep, struct kobject *kobj,
return count;
}
-static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
+static void create_elog_obj(uint64_t id, size_t size, uint64_t type)
{
struct elog_obj *elog;
int rc;
elog = kzalloc(sizeof(*elog), GFP_KERNEL);
if (!elog)
- return NULL;
+ return;
elog->kobj.kset = elog_kset;
@@ -219,18 +219,42 @@ static struct elog_obj *create_elog_obj(uint64_t id, size_t size, uint64_t type)
rc = kobject_add(&elog->kobj, NULL, "0x%llx", id);
if (rc) {
kobject_put(&elog->kobj);
- return NULL;
+ return;
}
+ /*
+ * As soon as sysfs file for this elog is created/activated there is
+ * chance opal_errd daemon might read and acknowledge this elog before
+ * kobject_uevent() is called. If that happens then we have a potential
+ * race between elog_ack_store->kobject_put() and kobject_uevent which
+ * leads to use-after-free issue of a kernfs object resulting into
+ * kernel crash.
+ *
+ * We already have one reference count on kobject and is been used for
+ * sysfs_create_bin_file() function. This initial one reference count
+ * is valid until it is dropped by elog_ack_store() function.
+ *
+ * However if userspace acknowledges the elog before this code reaches
+ * to kobject_uevent(), the reference count on kobject drops to zero
+ * and no longer stay valid for kobject_uevent() invocation. To avoid
+ * this race take reference count on kobject for bin file creation and
+ * drop it after kobject_uevent() is sent.
+ */
+
+ kobject_get(&elog->kobj); /* take a reference for the bin file. */
rc = sysfs_create_bin_file(&elog->kobj, &elog->raw_attr);
if (rc) {
kobject_put(&elog->kobj);
- return NULL;
+ /* Drop reference count taken for bin file. */
+ kobject_put(&elog->kobj);
+ return;
}
kobject_uevent(&elog->kobj, KOBJ_ADD);
+ /* Drop reference count taken for bin file. */
+ kobject_put(&elog->kobj);
- return elog;
+ return;
}
static irqreturn_t elog_event(int irq, void *data)
According to the "VFxxx Controller Reference Manual" (and the comment
block starting at line 97), Vybrid requires writing a one for clearing
an interrupt flag. Syncing the method for clearing I2SR_IIF in
i2c_imx_isr().
Signed-off-by: Christian Eggers <ceggers(a)arri.de>
Cc: stable(a)vger.kernel.org
---
drivers/i2c/busses/i2c-imx.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 0ab5381aa012..34648df7f1a6 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -424,7 +424,12 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy, bool a
/* check for arbitration lost */
if (temp & I2SR_IAL) {
- temp &= ~I2SR_IAL;
+ /*
+ * i2sr_clr_opcode is the value to clear all interrupts.
+ * Here we want to clear only I2SR_IAL, so we write
+ * ~i2sr_clr_opcode with just the I2SR_IAL bit toggled.
+ */
+ temp = ~i2c_imx->hwdata->i2sr_clr_opcode ^ I2SR_IAL;
imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR);
return -EAGAIN;
}
@@ -623,8 +628,12 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
if (temp & I2SR_IIF) {
/* save status register */
i2c_imx->i2csr = temp;
- temp &= ~I2SR_IIF;
- temp |= (i2c_imx->hwdata->i2sr_clr_opcode & I2SR_IIF);
+ /*
+ * i2sr_clr_opcode is the value to clear all interrupts.
+ * Here we want to clear only I2SR_IIF, so we write
+ * ~i2sr_clr_opcode with just the I2SR_IIF bit toggled.
+ */
+ temp = ~i2c_imx->hwdata->i2sr_clr_opcode ^ I2SR_IIF;
imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR);
wake_up(&i2c_imx->queue);
return IRQ_HANDLED;
--
Christian Eggers
Embedded software developer
Arnold & Richter Cine Technik GmbH & Co. Betriebs KG
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRA 57918
Persoenlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRB 54477
Geschaeftsfuehrer: Dr. Michael Neuhaeuser; Stephan Schenk; Walter Trauninger; Markus Zeiler
This is a note to let you know that I've just added the patch titled
extcon: ptn5150: Fix usage of atomic GPIO with sleeping GPIO chips
to my char-misc git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
in the char-misc-next branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will also be merged in the next major kernel release
during the merge window.
If you have any questions about this process, please let me know.
>From 6aaad58c872db062f7ea2761421ca748bd0931cc Mon Sep 17 00:00:00 2001
From: Krzysztof Kozlowski <krzk(a)kernel.org>
Date: Mon, 17 Aug 2020 09:00:00 +0200
Subject: extcon: ptn5150: Fix usage of atomic GPIO with sleeping GPIO chips
The driver uses atomic version of gpiod_set_value() without any real
reason. It is called in a workqueue under mutex so it could sleep
there. Changing it to "can_sleep" flavor allows to use the driver with
all GPIO chips.
Fixes: 4ed754de2d66 ("extcon: Add support for ptn5150 extcon driver")
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzk(a)kernel.org>
Reviewed-by: Vijai Kumar K <vijaikumar.kanagarajan(a)gmail.com>
Signed-off-by: Chanwoo Choi <cw00.choi(a)samsung.com>
---
drivers/extcon/extcon-ptn5150.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/extcon/extcon-ptn5150.c b/drivers/extcon/extcon-ptn5150.c
index d1c997599390..5f5252752644 100644
--- a/drivers/extcon/extcon-ptn5150.c
+++ b/drivers/extcon/extcon-ptn5150.c
@@ -127,7 +127,7 @@ static void ptn5150_irq_work(struct work_struct *work)
case PTN5150_DFP_ATTACHED:
extcon_set_state_sync(info->edev,
EXTCON_USB_HOST, false);
- gpiod_set_value(info->vbus_gpiod, 0);
+ gpiod_set_value_cansleep(info->vbus_gpiod, 0);
extcon_set_state_sync(info->edev, EXTCON_USB,
true);
break;
@@ -138,9 +138,9 @@ static void ptn5150_irq_work(struct work_struct *work)
PTN5150_REG_CC_VBUS_DETECTION_MASK) >>
PTN5150_REG_CC_VBUS_DETECTION_SHIFT);
if (vbus)
- gpiod_set_value(info->vbus_gpiod, 0);
+ gpiod_set_value_cansleep(info->vbus_gpiod, 0);
else
- gpiod_set_value(info->vbus_gpiod, 1);
+ gpiod_set_value_cansleep(info->vbus_gpiod, 1);
extcon_set_state_sync(info->edev,
EXTCON_USB_HOST, true);
@@ -156,7 +156,7 @@ static void ptn5150_irq_work(struct work_struct *work)
EXTCON_USB_HOST, false);
extcon_set_state_sync(info->edev,
EXTCON_USB, false);
- gpiod_set_value(info->vbus_gpiod, 0);
+ gpiod_set_value_cansleep(info->vbus_gpiod, 0);
}
}
--
2.28.0