|
 |
ad57ac |
From 909e24465c59ba4df4124d7caa0730f609c33369 Mon Sep 17 00:00:00 2001
|
|
|
ad57ac |
From: Joe Lawrence <joe.lawrence@redhat.com>
|
|
|
ad57ac |
Date: Tue, 26 Oct 2021 18:00:19 -0400
|
|
|
ad57ac |
Subject: [KPATCH CVE-2020-36385] RDMA/ucma: kpatch fixes for CVE-2020-36385
|
|
|
ad57ac |
|
|
|
ad57ac |
Changes since last build:
|
|
|
ad57ac |
arches: x86_64 ppc64le
|
|
|
ad57ac |
ucma.o: changed function: ucma_migrate_id
|
|
|
ad57ac |
---------------------------
|
|
|
ad57ac |
|
|
|
ad57ac |
Kernels:
|
|
|
ad57ac |
4.18.0-305.el8
|
|
|
ad57ac |
4.18.0-305.3.1.el8_4
|
|
|
ad57ac |
4.18.0-305.7.1.el8_4
|
|
|
ad57ac |
4.18.0-305.10.2.el8_4
|
|
|
ad57ac |
4.18.0-305.12.1.el8_4
|
|
|
ad57ac |
4.18.0-305.17.1.el8_4
|
|
|
ad57ac |
4.18.0-305.19.1.el8_4
|
|
|
ad57ac |
|
|
|
ad57ac |
Modifications:
|
|
|
ad57ac |
Kpatch-MR: https://gitlab.com/redhat/prdsc/rhel/src/kpatch/rhel-8/-/merge_requests/1
|
|
|
ad57ac |
Approved-by: Artem Savkov (@artem.savkov)
|
|
|
ad57ac |
- Avoid the complications of reworking all the locks (and preceding
|
|
|
ad57ac |
commits) and apply a minimal patch to avoid the CVE condition.
|
|
|
ad57ac |
- Always inline ucma_unlock_files() to avoid new function on x64_64
|
|
|
ad57ac |
|
|
|
ad57ac |
Z-MR: https://gitlab.com/redhat/rhel/src/kernel/rhel-8/-/merge_requests/961
|
|
|
ad57ac |
|
|
|
ad57ac |
KT0 test PASS: https://beaker.engineering.redhat.com/jobs/5944203
|
|
|
ad57ac |
for kpatch-patch-4_18_0-305-1-6.el8 scratch build:
|
|
|
ad57ac |
https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=40633078
|
|
|
ad57ac |
|
|
|
ad57ac |
commit 15152e7e191054883e2859892e77b442c531d1e1
|
|
|
ad57ac |
Author: Kamal Heib <kheib@redhat.com>
|
|
|
ad57ac |
Date: Sun Mar 7 11:30:04 2021 +0200
|
|
|
ad57ac |
|
|
|
ad57ac |
RDMA/ucma: Rework ucma_migrate_id() to avoid races with destroy
|
|
|
ad57ac |
|
|
|
ad57ac |
Bugzilla: https://bugzilla.redhat.com/1982040
|
|
|
ad57ac |
CVE: CVE-2020-36385
|
|
|
ad57ac |
Y-Commit: 20c1e291ce96ca474f5e94272d2e6511c6a38d58
|
|
|
ad57ac |
|
|
|
ad57ac |
O-Bugzilla: http://bugzilla.redhat.com/1931846
|
|
|
ad57ac |
O-CVE: CVE-2020-36385
|
|
|
ad57ac |
|
|
|
ad57ac |
commit f5449e74802c1112dea984aec8af7a33c4516af1
|
|
|
ad57ac |
Author: Jason Gunthorpe <jgg@nvidia.com>
|
|
|
ad57ac |
Date: Mon Sep 14 08:59:56 2020 -0300
|
|
|
ad57ac |
|
|
|
ad57ac |
RDMA/ucma: Rework ucma_migrate_id() to avoid races with destroy
|
|
|
ad57ac |
|
|
|
ad57ac |
ucma_destroy_id() assumes that all things accessing the ctx will do so via
|
|
|
ad57ac |
the xarray. This assumption violated only in the case the FD is being
|
|
|
ad57ac |
closed, then the ctx is reached via the ctx_list. Normally this is OK
|
|
|
ad57ac |
since ucma_destroy_id() cannot run concurrenty with release(), however
|
|
|
ad57ac |
with ucma_migrate_id() is involved this can violated as the close of the
|
|
|
ad57ac |
2nd FD can run concurrently with destroy on the first:
|
|
|
ad57ac |
|
|
|
ad57ac |
CPU0 CPU1
|
|
|
ad57ac |
ucma_destroy_id(fda)
|
|
|
ad57ac |
ucma_migrate_id(fda -> fdb)
|
|
|
ad57ac |
ucma_get_ctx()
|
|
|
ad57ac |
xa_lock()
|
|
|
ad57ac |
_ucma_find_context()
|
|
|
ad57ac |
xa_erase()
|
|
|
ad57ac |
xa_unlock()
|
|
|
ad57ac |
xa_lock()
|
|
|
ad57ac |
ctx->file = new_file
|
|
|
ad57ac |
list_move()
|
|
|
ad57ac |
xa_unlock()
|
|
|
ad57ac |
ucma_put_ctx()
|
|
|
ad57ac |
|
|
|
ad57ac |
ucma_close(fdb)
|
|
|
ad57ac |
_destroy_id()
|
|
|
ad57ac |
kfree(ctx)
|
|
|
ad57ac |
|
|
|
ad57ac |
_destroy_id()
|
|
|
ad57ac |
wait_for_completion()
|
|
|
ad57ac |
// boom, ctx was freed
|
|
|
ad57ac |
|
|
|
ad57ac |
The ctx->file must be modified under the handler and xa_lock, and prior to
|
|
|
ad57ac |
modification the ID must be rechecked that it is still reachable from
|
|
|
ad57ac |
cur_file, ie there is no parallel destroy or migrate.
|
|
|
ad57ac |
|
|
|
ad57ac |
To make this work remove the double locking and streamline the control
|
|
|
ad57ac |
flow. The double locking was obsoleted by the handler lock now directly
|
|
|
ad57ac |
preventing new uevents from being created, and the ctx_list cannot be read
|
|
|
ad57ac |
while holding fgets on both files. Removing the double locking also
|
|
|
ad57ac |
removes the need to check for the same file.
|
|
|
ad57ac |
|
|
|
ad57ac |
Fixes: 88314e4dda1e ("RDMA/cma: add support for rdma_migrate_id()")
|
|
|
ad57ac |
Link: https://lore.kernel.org/r/0-v1-05c5a4090305+3a872-ucma_syz_migrate_jgg@nvidia.com
|
|
|
ad57ac |
Reported-and-tested-by: syzbot+cc6fc752b3819e082d0c@syzkaller.appspotmail.com
|
|
|
ad57ac |
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
|
|
|
ad57ac |
|
|
|
ad57ac |
Signed-off-by: Kamal Heib <kheib@redhat.com>
|
|
|
ad57ac |
|
|
|
ad57ac |
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
|
|
|
ad57ac |
---
|
|
|
ad57ac |
drivers/infiniband/core/ucma.c | 11 ++++++++++-
|
|
|
ad57ac |
1 file changed, 10 insertions(+), 1 deletion(-)
|
|
|
ad57ac |
|
|
|
ad57ac |
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
|
|
|
ad57ac |
index 138e0096ca8e..2cd4277bf4a8 100644
|
|
|
ad57ac |
--- a/drivers/infiniband/core/ucma.c
|
|
|
ad57ac |
+++ b/drivers/infiniband/core/ucma.c
|
|
|
ad57ac |
@@ -1626,7 +1626,7 @@ static void ucma_lock_files(struct ucma_file *file1, struct ucma_file *file2)
|
|
|
ad57ac |
}
|
|
|
ad57ac |
}
|
|
|
ad57ac |
|
|
|
ad57ac |
-static void ucma_unlock_files(struct ucma_file *file1, struct ucma_file *file2)
|
|
|
ad57ac |
+static __always_inline void ucma_unlock_files(struct ucma_file *file1, struct ucma_file *file2)
|
|
|
ad57ac |
{
|
|
|
ad57ac |
if (file1 < file2) {
|
|
|
ad57ac |
mutex_unlock(&file2->mut);
|
|
|
ad57ac |
@@ -1689,6 +1689,14 @@ static ssize_t ucma_migrate_id(struct ucma_file *new_file,
|
|
|
ad57ac |
ucma_lock_files(cur_file, new_file);
|
|
|
ad57ac |
xa_lock(&ctx_table);
|
|
|
ad57ac |
|
|
|
ad57ac |
+ /* CVE-2020-36385 kpatch: double check the context one last time */
|
|
|
ad57ac |
+ if (_ucma_find_context(cmd.id, cur_file) != ctx) {
|
|
|
ad57ac |
+ xa_unlock(&ctx_table);
|
|
|
ad57ac |
+ ucma_unlock_files(cur_file, new_file);
|
|
|
ad57ac |
+ ret = -ENOENT;
|
|
|
ad57ac |
+ goto err_unlock;
|
|
|
ad57ac |
+ }
|
|
|
ad57ac |
+
|
|
|
ad57ac |
list_move_tail(&ctx->list, &new_file->ctx_list);
|
|
|
ad57ac |
ucma_move_events(ctx, new_file);
|
|
|
ad57ac |
ctx->file = new_file;
|
|
|
ad57ac |
@@ -1702,6 +1710,7 @@ static ssize_t ucma_migrate_id(struct ucma_file *new_file,
|
|
|
ad57ac |
&resp, sizeof(resp)))
|
|
|
ad57ac |
ret = -EFAULT;
|
|
|
ad57ac |
|
|
|
ad57ac |
+err_unlock:
|
|
|
ad57ac |
ucma_put_ctx(ctx);
|
|
|
ad57ac |
file_put:
|
|
|
ad57ac |
fdput(f);
|
|
|
ad57ac |
--
|
|
|
ad57ac |
2.31.1
|
|
|
ad57ac |
|
|
|
ad57ac |
|