I got a kernel NULL pointer derference report as below:
BUG: kernel NULL pointer dereference, address: 0000000000000038
CPU: 4 PID: 1050 Comm: cat Not tainted 5.19.0 #38
RIP: 0010:kernel_getpeername+0xf/0x30
Call Trace:
<TASK>
? iscsi_sw_tcp_conn_get_param+0x11f/0x17f
show_conn_ep_param_ISCSI_PARAM_CONN_ADDRESS+0x90/0xb0
dev_attr_show+0x1d/0x50
sysfs_kf_seq_show+0xad/0x120
kernfs_seq_show+0x2c/0x40
seq_read_iter+0x12e/0x4d0
? aa_file_perm+0x177/0x590
kernfs_fop_read_iter+0x183/0x210
new_sync_read+0xfe/0x180
? 0xffffffff81000000
vfs_read+0x14d/0x1a0
ksys_read+0x6d/0xf0
__x64_sys_read+0x1a/0x20
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x63/0xcd
The problem scenario is:
CPU1 CPU2
------------------------- ------------------------
iscsi_sw_tcp_conn_get_param
spin_lock_bh(&frwd_lock)
if (!tcp_sw_conn || !tcp_sw_conn->sock)
spin_unlock_bh(&frwd_lock)
return -ENOTCONN
sock = tcp_sw_conn->sock;
sock_hold(sock->sk)
spin_unlock_bh(&frwd_lock)
iscsi_sw_tcp_release_conn
spin_lock_bh(&frwd_lock)
tcp_sw_conn->sock = NULL
spin_unlock_bh(&frwd_lock)
sockfd_put(sock)
task_work
__fput
sock_close
__sock_release
sock->sk = NULL
sock->ops = NULL
sock->file = NULL
kernel_getpeername
sock->ops->getname
^^^^^^^^^
NULL pointer dereference of sock->ops
sock_hold() and sock_put() can't ensure that sock_close() won't be
called before calling getpeername() or getsockname(). Using fget()
and sockfd_put() replace sock_hold() and sock_put(), and put them
under frwd_lock protection, to ensure that the socket struct is
preserved until after the getpeernaem() or getsockname() complete.
Fixes: bcf3a2953d36 ("scsi: iscsi: iscsi_tcp: Avoid holding spinlock while
calling getpeername()")
Signed-off-by: Li Jinlin <[email protected]>
---
drivers/scsi/iscsi_tcp.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 9fee70d6434a..63532dc3970d 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -612,8 +612,8 @@ static void iscsi_sw_tcp_release_conn(struct iscsi_conn
*conn)
spin_lock_bh(&session->frwd_lock);
tcp_sw_conn->sock = NULL;
- spin_unlock_bh(&session->frwd_lock);
sockfd_put(sock);
+ spin_unlock_bh(&session->frwd_lock);
}
static void iscsi_sw_tcp_conn_destroy(struct iscsi_cls_conn *cls_conn)
@@ -756,7 +756,7 @@ static int iscsi_sw_tcp_conn_get_param(struct
iscsi_cls_conn *cls_conn,
return -ENOTCONN;
}
sock = tcp_sw_conn->sock;
- sock_hold(sock->sk);
+ fget(sock->file);
spin_unlock_bh(&conn->session->frwd_lock);
if (param == ISCSI_PARAM_LOCAL_PORT)
@@ -765,7 +765,9 @@ static int iscsi_sw_tcp_conn_get_param(struct
iscsi_cls_conn *cls_conn,
else
rc = kernel_getpeername(sock,
(struct sockaddr *)&addr);
- sock_put(sock->sk);
+ spin_lock_bh(&conn->session->frwd_lock);
+ sockfd_put(sock);
+ spin_unlock_bh(&conn->session->frwd_lock);
if (rc < 0)
return rc;
@@ -808,12 +810,14 @@ static int iscsi_sw_tcp_host_get_param(struct Scsi_Host
*shost,
spin_unlock_bh(&session->frwd_lock);
return -ENOTCONN;
}
- sock_hold(sock->sk);
+ fget(sock->file);
spin_unlock_bh(&session->frwd_lock);
rc = kernel_getsockname(sock,
(struct sockaddr *)&addr);
- sock_put(sock->sk);
+ spin_lock_bh(&conn->session->frwd_lock);
+ sockfd_put(sock);
+ spin_unlock_bh(&conn->session->frwd_lock);
if (rc < 0)
return rc;
--
2.27.0
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/20220802101939.3972556-1-lijinlin3%40huawei.com.
