78 lines
2.5 KiB
Diff
78 lines
2.5 KiB
Diff
|
From 8e92c2b0cb50a31e2956760498bc8cdb72993fb3 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Quentin Casasnovas <quentin.casasnovas@oracle.com>
|
||
|
|
Date: Fri, 16 Oct 2015 17:11:42 +0200
|
||
|
|
Subject: [PATCH] RDS: fix race condition when sending a message on unbound
|
||
|
|
socket.
|
||
|
|
|
||
|
|
Sasha's found a NULL pointer dereference in the RDS connection code when
|
||
|
|
sending a message to an apparently unbound socket. The problem is caused
|
||
|
|
by the code checking if the socket is bound in rds_sendmsg(), which checks
|
||
|
|
the rs_bound_addr field without taking a lock on the socket. This opens a
|
||
|
|
race where rs_bound_addr is temporarily set but where the transport is not
|
||
|
|
in rds_bind(), leading to a NULL pointer dereference when trying to
|
||
|
|
dereference 'trans' in __rds_conn_create().
|
||
|
|
|
||
|
|
Vegard wrote a reproducer for this issue, so kindly ask him to share if
|
||
|
|
you're interested.
|
||
|
|
|
||
|
|
I cannot reproduce the NULL pointer dereference using Vegard's reproducer
|
||
|
|
with this patch, whereas I could without.
|
||
|
|
|
||
|
|
Complete earlier incomplete fix to CVE-2015-6937:
|
||
|
|
|
||
|
|
74e98eb08588 ("RDS: verify the underlying transport exists before creating a connection")
|
||
|
|
|
||
|
|
Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
|
||
|
|
Reviewed-by: Vegard Nossum <vegard.nossum@oracle.com>
|
||
|
|
Reviewed-by: Sasha Levin <sasha.levin@oracle.com>
|
||
|
|
Cc: Vegard Nossum <vegard.nossum@oracle.com>
|
||
|
|
Cc: Sasha Levin <sasha.levin@oracle.com>
|
||
|
|
Cc: Chien Yen <chien.yen@oracle.com>
|
||
|
|
Cc: Santosh Shilimkar <santosh.shilimkar@oracle.com>
|
||
|
|
Cc: David S. Miller <davem@davemloft.net>
|
||
|
|
Cc: stable@vger.kernel.org
|
||
|
|
---
|
||
|
|
net/rds/connection.c | 6 ------
|
||
|
|
net/rds/send.c | 4 +++-
|
||
|
|
2 files changed, 3 insertions(+), 7 deletions(-)
|
||
|
|
|
||
|
|
diff --git a/net/rds/connection.c b/net/rds/connection.c
|
||
|
|
index 49adeef8090c..9b2de5e67d79 100644
|
||
|
|
--- a/net/rds/connection.c
|
||
|
|
+++ b/net/rds/connection.c
|
||
|
|
@@ -190,12 +190,6 @@ new_conn:
|
||
|
|
}
|
||
|
|
}
|
||
|
|
|
||
|
|
- if (trans == NULL) {
|
||
|
|
- kmem_cache_free(rds_conn_slab, conn);
|
||
|
|
- conn = ERR_PTR(-ENODEV);
|
||
|
|
- goto out;
|
||
|
|
- }
|
||
|
|
-
|
||
|
|
conn->c_trans = trans;
|
||
|
|
|
||
|
|
ret = trans->conn_alloc(conn, gfp);
|
||
|
|
diff --git a/net/rds/send.c b/net/rds/send.c
|
||
|
|
index 4df61a515b83..859de6f32521 100644
|
||
|
|
--- a/net/rds/send.c
|
||
|
|
+++ b/net/rds/send.c
|
||
|
|
@@ -1009,11 +1009,13 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
|
||
|
|
release_sock(sk);
|
||
|
|
}
|
||
|
|
|
||
|
|
- /* racing with another thread binding seems ok here */
|
||
|
|
+ lock_sock(sk);
|
||
|
|
if (daddr == 0 || rs->rs_bound_addr == 0) {
|
||
|
|
+ release_sock(sk);
|
||
|
|
ret = -ENOTCONN; /* XXX not a great errno */
|
||
|
|
goto out;
|
||
|
|
}
|
||
|
|
+ release_sock(sk);
|
||
|
|
|
||
|
|
if (payload_len > rds_sk_sndbuf(rs)) {
|
||
|
|
ret = -EMSGSIZE;
|
||
|
|
--
|
||
|
|
2.4.3
|
||
|
|
|