From: Eric Biggers <ebiggers(a)google.com>
Once all I/O using a blk_crypto_key has completed, filesystems can call
blk_crypto_evict_key(). However, the block layer currently doesn't call
blk_crypto_put_keyslot() until the request is being freed, which happens
after upper layers have been told (via bio_endio()) the I/O has
completed. This causes a race condition where blk_crypto_evict_key()
can see 'slot_refs != 0' without there being an actual bug.
This makes __blk_crypto_evict_key() hit the
'WARN_ON_ONCE(atomic_read(&slot->slot_refs) != 0)' and return without
doing anything, eventually causing a use-after-free in
blk_crypto_reprogram_all_keys(). (This is a very rare bug and has only
been seen when per-file keys are being used with fscrypt.)
There are two options to fix this: either release the keyslot before
bio_endio() is called on the request's last bio, or make
__blk_crypto_evict_key() ignore slot_refs. Let's go with the first
solution, since it preserves the ability to report bugs (via
WARN_ON_ONCE) where a key is evicted while still in-use.
Fixes: a892c8d52c02 ("block: Inline encryption support for blk-mq")
Cc: stable(a)vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers(a)google.com>
---
block/blk-crypto-internal.h | 15 +++++++++++----
block/blk-crypto.c | 24 ++++++++++++------------
block/blk-mq.c | 15 ++++++++++++++-
3 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/block/blk-crypto-internal.h b/block/blk-crypto-internal.h
index a8cdaf26851e1..73609902349b6 100644
--- a/block/blk-crypto-internal.h
+++ b/block/blk-crypto-internal.h
@@ -153,14 +153,21 @@ static inline bool blk_crypto_bio_prep(struct bio **bio_ptr)
return true;
}
-blk_status_t __blk_crypto_init_request(struct request *rq);
-static inline blk_status_t blk_crypto_init_request(struct request *rq)
+blk_status_t __blk_crypto_rq_get_keyslot(struct request *rq);
+static inline blk_status_t blk_crypto_rq_get_keyslot(struct request *rq)
{
if (blk_crypto_rq_is_encrypted(rq))
- return __blk_crypto_init_request(rq);
+ return __blk_crypto_rq_get_keyslot(rq);
return BLK_STS_OK;
}
+void __blk_crypto_rq_put_keyslot(struct request *rq);
+static inline void blk_crypto_rq_put_keyslot(struct request *rq)
+{
+ if (blk_crypto_rq_is_encrypted(rq))
+ __blk_crypto_rq_put_keyslot(rq);
+}
+
void __blk_crypto_free_request(struct request *rq);
static inline void blk_crypto_free_request(struct request *rq)
{
@@ -199,7 +206,7 @@ static inline blk_status_t blk_crypto_insert_cloned_request(struct request *rq)
{
if (blk_crypto_rq_is_encrypted(rq))
- return blk_crypto_init_request(rq);
+ return blk_crypto_rq_get_keyslot(rq);
return BLK_STS_OK;
}
diff --git a/block/blk-crypto.c b/block/blk-crypto.c
index 45378586151f7..8e5612364c48c 100644
--- a/block/blk-crypto.c
+++ b/block/blk-crypto.c
@@ -224,27 +224,27 @@ static bool bio_crypt_check_alignment(struct bio *bio)
return true;
}
-blk_status_t __blk_crypto_init_request(struct request *rq)
+blk_status_t __blk_crypto_rq_get_keyslot(struct request *rq)
{
return blk_crypto_get_keyslot(rq->q->crypto_profile,
rq->crypt_ctx->bc_key,
&rq->crypt_keyslot);
}
-/**
- * __blk_crypto_free_request - Uninitialize the crypto fields of a request.
- *
- * @rq: The request whose crypto fields to uninitialize.
- *
- * Completely uninitializes the crypto fields of a request. If a keyslot has
- * been programmed into some inline encryption hardware, that keyslot is
- * released. The rq->crypt_ctx is also freed.
- */
-void __blk_crypto_free_request(struct request *rq)
+void __blk_crypto_rq_put_keyslot(struct request *rq)
{
blk_crypto_put_keyslot(rq->crypt_keyslot);
+ rq->crypt_keyslot = NULL;
+}
+
+void __blk_crypto_free_request(struct request *rq)
+{
mempool_free(rq->crypt_ctx, bio_crypt_ctx_pool);
- blk_crypto_rq_set_defaults(rq);
+ rq->crypt_ctx = NULL;
+
+ /* The keyslot, if one was needed, should have been released earlier. */
+ if (WARN_ON_ONCE(rq->crypt_keyslot))
+ __blk_crypto_rq_put_keyslot(rq);
}
/**
diff --git a/block/blk-mq.c b/block/blk-mq.c
index d3494a796ba80..738e81f518227 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -840,6 +840,12 @@ static void blk_complete_request(struct request *req)
req->q->integrity.profile->complete_fn(req, total_bytes);
#endif
+ /*
+ * Upper layers may call blk_crypto_evict_key() anytime after the last
+ * bio_endio(). Therefore, the keyslot must be released before that.
+ */
+ blk_crypto_rq_put_keyslot(req);
+
blk_account_io_completion(req, total_bytes);
do {
@@ -905,6 +911,13 @@ bool blk_update_request(struct request *req, blk_status_t error,
req->q->integrity.profile->complete_fn(req, nr_bytes);
#endif
+ /*
+ * Upper layers may call blk_crypto_evict_key() anytime after the last
+ * bio_endio(). Therefore, the keyslot must be released before that.
+ */
+ if (blk_crypto_rq_is_encrypted(req) && nr_bytes >= blk_rq_bytes(req))
+ __blk_crypto_rq_put_keyslot(req);
+
if (unlikely(error && !blk_rq_is_passthrough(req) &&
!(req->rq_flags & RQF_QUIET)) &&
!test_bit(GD_DEAD, &req->q->disk->state)) {
@@ -2967,7 +2980,7 @@ void blk_mq_submit_bio(struct bio *bio)
blk_mq_bio_to_request(rq, bio, nr_segs);
- ret = blk_crypto_init_request(rq);
+ ret = blk_crypto_rq_get_keyslot(rq);
if (ret != BLK_STS_OK) {
bio->bi_status = ret;
bio_endio(bio);
--
2.39.2
Hi Sasha,
sashal(a)kernel.org wrote on Sat, 4 Mar 2023 22:50:38 -0500:
> This is a note to let you know that I've just added the patch titled
>
> mtd: rawnand: fsl_elbc: Propagate HW ECC settings to HW
>
> to the 5.4-stable tree which can be found at:
> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum…
>
> The filename of the patch is:
> mtd-rawnand-fsl_elbc-propagate-hw-ecc-settings-to-hw.patch
> and it can be found in the queue-5.4 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable(a)vger.kernel.org> know about it.
+Marek
As reported by kernel test robot, this commit does not apply on 5.4
because many changes have happened in the core since the introduction
of the fixed patch. In practice the driver works in most cases,
but does not in few rare cases (IIUC) so I would be in favor of just
dropping this commit from the queue/5.4 branch. If someone feels like
this commit should be backported there, please send an updated fix.
Link: https://lore.kernel.org/oe-kbuild-all/202303060514.1ziBICF7-lkp@intel.com/
Thanks,
Miquèl
>
>
>
> commit 839c09472742b383f65eb7c1f7e576ebfb6a1f62
> Author: Pali Rohár <pali(a)kernel.org>
> Date: Sat Jan 28 14:41:11 2023 +0100
>
> mtd: rawnand: fsl_elbc: Propagate HW ECC settings to HW
>
> [ Upstream commit b56265257d38af5abf43bd5461ca166b401c35a5 ]
>
> It is possible that current chip->ecc.engine_type value does not match to
> configured HW value (if HW ECC checking and generating is enabled or not).
>
> This can happen with old U-Boot bootloader version which either does not
> initialize NAND (and let it in some default unusable state) or initialize
> NAND with different parameters than what is specified in kernel DTS file.
>
> So if kernel chose to use some chip->ecc.engine_type settings (e.g. from
> DTS file) then do not depend on bootloader HW configuration and configures
> HW ECC settings according to chip->ecc.engine_type value.
>
> BR_DECC must be set to BR_DECC_CHK_GEN when HW is doing ECC (both
> generating and checking), or to BR_DECC_OFF when HW is not doing ECC.
>
> This change fixes usage of SW ECC support in case bootloader explicitly
> enabled HW ECC support and kernel DTS file has specified to use SW ECC.
> (Of course this works only in case when NAND is not a boot device and both
> bootloader and kernel are loaded from different location, e.g. FLASH NOR.)
>
> Fixes: f6424c22aa36 ("mtd: rawnand: fsl_elbc: Make SW ECC work")
> Signed-off-by: Pali Rohár <pali(a)kernel.org>
> Signed-off-by: Miquel Raynal <miquel.raynal(a)bootlin.com>
> Link: https://lore.kernel.org/linux-mtd/20230128134111.32559-1-pali@kernel.org
> Signed-off-by: Sasha Levin <sashal(a)kernel.org>
>
> diff --git a/drivers/mtd/nand/raw/fsl_elbc_nand.c b/drivers/mtd/nand/raw/fsl_elbc_nand.c
> index 634c550db13a7..e900c0eddc21d 100644
> --- a/drivers/mtd/nand/raw/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/raw/fsl_elbc_nand.c
> @@ -727,6 +727,7 @@ static int fsl_elbc_attach_chip(struct nand_chip *chip)
> struct fsl_lbc_ctrl *ctrl = priv->ctrl;
> struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
> unsigned int al;
> + u32 br;
>
> switch (chip->ecc.mode) {
> /*
> @@ -762,6 +763,13 @@ static int fsl_elbc_attach_chip(struct nand_chip *chip)
> return -EINVAL;
> }
>
> + /* enable/disable HW ECC checking and generating based on if HW ECC was chosen */
> + br = in_be32(&lbc->bank[priv->bank].br) & ~BR_DECC;
> + if (chip->ecc.engine_type == NAND_ECC_ENGINE_TYPE_ON_HOST)
> + out_be32(&lbc->bank[priv->bank].br, br | BR_DECC_CHK_GEN);
> + else
> + out_be32(&lbc->bank[priv->bank].br, br | BR_DECC_OFF);
> +
> /* calculate FMR Address Length field */
> al = 0;
> if (chip->pagemask & 0xffff0000)
After upgrading to make v4.4.1 (released last week), there's no longer any
progress output from builds of the Linux kernel v4.19 and earlier. It seems the
actual build still works, but it's now silent except for warnings and errors.
It bisects to the following 'make' commit:
commit dc2d963989b96161472b2cd38cef5d1f4851ea34
Author: Dmitry Goncharov <dgoncharov(a)users.sf.net>
Date: Sun Nov 27 14:09:17 2022 -0500
[SV 63347] Always add command line variable assignments to MAKEFLAGS
Is this an intentional breakage from the 'make' side?
- Eric
ksm_exit() may remove the mm from the ksm_scan between the unlocking of
the ksm_mmlist and the start of the VMA iteration. This results in the
mmap_read_lock() not being taken and a report from lockdep that the mm
isn't locked in the maple tree code.
Fix the race by checking if this mm has been removed before iterating
the VMAs. __ksm_exit() uses the mmap lock to synchronize the freeing of
an mm, so it is safe to keep iterating over the VMAs when it is going to
be freed.
This change will slow down the mm exit during the race condition, but
will speed up the non-race scenarios iteration over the VMA list, which
should be much more common.
Reported-by: Pengfei Xu <pengfei.xu(a)intel.com>
Link: https://lore.kernel.org/lkml/ZAdUUhSbaa6fHS36@xpf.sh.intel.com/
Reported-by: syzbot+2ee18845e89ae76342c5(a)syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?id=64a3e95957cd3deab99df7cd7b5a9475af92c9…
Cc: linux-mm(a)kvack.org
Cc: linux-kernel(a)vger.kernel.org
Cc: Andrew Morton <akpm(a)linux-foundation.org>
Cc: Matthew Wilcox (Oracle) <willy(a)infradead.org>
Cc: heng.su(a)intel.com
Cc: lkp(a)intel.com
Cc: <Stable(a)vger.kernel.org>
Fixes: a5f18ba07276 ("mm/ksm: use vma iterators instead of vma linked list")
Signed-off-by: Liam R. Howlett <Liam.Howlett(a)oracle.com>
---
mm/ksm.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/mm/ksm.c b/mm/ksm.c
index 525c3306e78b..723ddbe6ea97 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -1044,9 +1044,10 @@ static int unmerge_and_remove_all_rmap_items(void)
mm = mm_slot->slot.mm;
mmap_read_lock(mm);
+ if (ksm_test_exit(mm))
+ goto mm_exiting;
+
for_each_vma(vmi, vma) {
- if (ksm_test_exit(mm))
- break;
if (!(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma)
continue;
err = unmerge_ksm_pages(vma,
@@ -1055,6 +1056,7 @@ static int unmerge_and_remove_all_rmap_items(void)
goto error;
}
+mm_exiting:
remove_trailing_rmap_items(&mm_slot->rmap_list);
mmap_read_unlock(mm);
--
2.39.2