nagai/kernel
3
0
Fork 0
forked from mirrors/linux
Code Pull requests Activity

scsi: iscsi: iscsi_tcp: Fix null-ptr-deref while calling getpeername()

Browse source 
Fix a NULL pointer crash that occurs when we are freeing the socket at the
same time we access it via sysfs.

The problem is that:

 1. iscsi_sw_tcp_conn_get_param() and iscsi_sw_tcp_host_get_param() take
    the frwd_lock and do sock_hold() then drop the frwd_lock. sock_hold()
    does a get on the "struct sock".

 2. iscsi_sw_tcp_release_conn() does sockfd_put() which does the last put
    on the "struct socket" and that does __sock_release() which sets the
    sock->ops to NULL.

 3. iscsi_sw_tcp_conn_get_param() and iscsi_sw_tcp_host_get_param() then
    call kernel_getpeername() which accesses the NULL sock->ops.

Above we do a get on the "struct sock", but we needed a get on the "struct
socket". Originally, we just held the frwd_lock the entire time but in
commit bcf3a2953d ("scsi: iscsi: iscsi_tcp: Avoid holding spinlock while
calling getpeername()") we switched to refcount based because the network
layer changed and started taking a mutex in that path, so we could no
longer hold the frwd_lock.

Instead of trying to maintain multiple refcounts, this just has us use a
mutex for accessing the socket in the interface code paths.

Link: https://lore.kernel.org/r/20220907221700.10302-1-michael.christie@oracle.com
Fixes: bcf3a2953d ("scsi: iscsi: iscsi_tcp: Avoid holding spinlock while calling getpeername()")
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
This commit is contained in:
Mike Christie 2022-09-07 17:17:00 -05:00 committed by Martin K. Petersen
parent c863a2dcb9
commit 57569c37f0
2 changed files with 55 additions and 21 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

73
drivers/scsi/iscsi_tcp.c
View file

@ -595,6 +595,8 @@ iscsi_sw_tcp_conn_create(struct iscsi_cls_session *cls_session,
INIT_WORK(&conn->recvwork, iscsi_sw_tcp_recv_data_work);
tcp_sw_conn->queue_recv = iscsi_recv_from_iscsi_q;
mutex_init(&tcp_sw_conn->sock_lock);
tfm = crypto_alloc_ahash("crc32c", 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(tfm))
goto free_conn;
@ -629,11 +631,15 @@ iscsi_sw_tcp_conn_create(struct iscsi_cls_session *cls_session,
static void iscsi_sw_tcp_release_conn(struct iscsi_conn *conn)
{
struct iscsi_session *session = conn->session;
struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
struct socket *sock = tcp_sw_conn->sock;
/*
* The iscsi transport class will make sure we are not called in
* parallel with start, stop, bind and destroys. However, this can be
* called twice if userspace does a stop then a destroy.
*/
if (!sock)
return;
@ -649,9 +655,9 @@ static void iscsi_sw_tcp_release_conn(struct iscsi_conn *conn)
iscsi_suspend_rx(conn);
spin_lock_bh(&session->frwd_lock);
mutex_lock(&tcp_sw_conn->sock_lock);
tcp_sw_conn->sock = NULL;
spin_unlock_bh(&session->frwd_lock);
mutex_unlock(&tcp_sw_conn->sock_lock);
sockfd_put(sock);
}
@ -703,7 +709,6 @@ iscsi_sw_tcp_conn_bind(struct iscsi_cls_session *cls_session,
struct iscsi_cls_conn *cls_conn, uint64_t transport_eph,
int is_leading)
{
struct iscsi_session *session = cls_session->dd_data;
struct iscsi_conn *conn = cls_conn->dd_data;
struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
@ -723,10 +728,10 @@ iscsi_sw_tcp_conn_bind(struct iscsi_cls_session *cls_session,
if (err)
goto free_socket;
spin_lock_bh(&session->frwd_lock);
mutex_lock(&tcp_sw_conn->sock_lock);
/* bind iSCSI connection and socket */
tcp_sw_conn->sock = sock;
spin_unlock_bh(&session->frwd_lock);
mutex_unlock(&tcp_sw_conn->sock_lock);
/* setup Socket parameters */
sk = sock->sk;
@ -763,8 +768,15 @@ static int iscsi_sw_tcp_conn_set_param(struct iscsi_cls_conn *cls_conn,
break;
case ISCSI_PARAM_DATADGST_EN:
iscsi_set_param(cls_conn, param, buf, buflen);
mutex_lock(&tcp_sw_conn->sock_lock);
if (!tcp_sw_conn->sock) {
mutex_unlock(&tcp_sw_conn->sock_lock);
return -ENOTCONN;
}
tcp_sw_conn->sendpage = conn->datadgst_en ?
sock_no_sendpage : tcp_sw_conn->sock->ops->sendpage;
mutex_unlock(&tcp_sw_conn->sock_lock);
break;
case ISCSI_PARAM_MAX_R2T:
return iscsi_tcp_set_max_r2t(conn, buf);
@ -779,8 +791,8 @@ static int iscsi_sw_tcp_conn_get_param(struct iscsi_cls_conn *cls_conn,
enum iscsi_param param, char *buf)
{
struct iscsi_conn *conn = cls_conn->dd_data;
struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
struct iscsi_sw_tcp_conn *tcp_sw_conn;
struct iscsi_tcp_conn *tcp_conn;
struct sockaddr_in6 addr;
struct socket *sock;
int rc;
@ -790,21 +802,36 @@ static int iscsi_sw_tcp_conn_get_param(struct iscsi_cls_conn *cls_conn,
case ISCSI_PARAM_CONN_ADDRESS:
case ISCSI_PARAM_LOCAL_PORT:
spin_lock_bh(&conn->session->frwd_lock);
if (!tcp_sw_conn || !tcp_sw_conn->sock) {
if (!conn->session->leadconn) {
spin_unlock_bh(&conn->session->frwd_lock);
return -ENOTCONN;
}
sock = tcp_sw_conn->sock;
sock_hold(sock->sk);
/*
* The conn has been setup and bound, so just grab a ref
* incase a destroy runs while we are in the net layer.
*/
iscsi_get_conn(conn->cls_conn);
spin_unlock_bh(&conn->session->frwd_lock);
tcp_conn = conn->dd_data;
tcp_sw_conn = tcp_conn->dd_data;
mutex_lock(&tcp_sw_conn->sock_lock);
sock = tcp_sw_conn->sock;
if (!sock) {
rc = -ENOTCONN;
goto sock_unlock;
}
if (param == ISCSI_PARAM_LOCAL_PORT)
rc = kernel_getsockname(sock,
(struct sockaddr *)&addr);
else
rc = kernel_getpeername(sock,
(struct sockaddr *)&addr);
sock_put(sock->sk);
sock_unlock:
mutex_unlock(&tcp_sw_conn->sock_lock);
iscsi_put_conn(conn->cls_conn);
if (rc < 0)
return rc;
@ -842,17 +869,21 @@ static int iscsi_sw_tcp_host_get_param(struct Scsi_Host *shost,
}
tcp_conn = conn->dd_data;
tcp_sw_conn = tcp_conn->dd_data;
sock = tcp_sw_conn->sock;
if (!sock) {
spin_unlock_bh(&session->frwd_lock);
return -ENOTCONN;
}
sock_hold(sock->sk);
/*
* The conn has been setup and bound, so just grab a ref
* incase a destroy runs while we are in the net layer.
*/
iscsi_get_conn(conn->cls_conn);
spin_unlock_bh(&session->frwd_lock);
rc = kernel_getsockname(sock,
(struct sockaddr *)&addr);
sock_put(sock->sk);
mutex_lock(&tcp_sw_conn->sock_lock);
sock = tcp_sw_conn->sock;
if (!sock)
rc = -ENOTCONN;
else
rc = kernel_getsockname(sock, (struct sockaddr *)&addr);
mutex_unlock(&tcp_sw_conn->sock_lock);
iscsi_put_conn(conn->cls_conn);
if (rc < 0)
return rc;

3
drivers/scsi/iscsi_tcp.h
View file

@ -28,6 +28,9 @@ struct iscsi_sw_tcp_send {
struct iscsi_sw_tcp_conn {
struct socket *sock;
/* Taken when accessing the sock from the netlink/sysfs interface */
struct mutex sock_lock;
struct work_struct recvwork;
bool queue_recv;