| Message ID | 20230417171801.208122-4-mlombard@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix possible hangs and race conditions during login | expand |
On 4/17/23 12:18 PM, Maurizio Lombardi wrote: > The tpg->np_login_sem is a semaphore that is used to serialize the login > process when multiple login threads run concurrently against the same > target portal group. > > The iscsi_target_locate_portal() function finds the tpg, > calls iscsit_access_np() against the np_login_sem semaphore > and saves the tpg pointer in conn->tpg; > > If iscsi_target_locate_portal() fails, the caller will check for the > conn->tpg pointer and, if it's not NULL, then it will assume > that iscsi_target_locate_portal() called iscsit_access_np() on the > semaphore. > > Make sure that conn->tpg gets initialized only if iscsit_access_np() > was successful, otherwise iscsit_deaccess_np() may end up > being called against a semaphore we never took, allowing more than one > thread to access the same tpg. > > Signed-off-by: Maurizio Lombardi <mlombard@redhat.com> Reviewed-by: Mike Christie <michael.christie@oracle.com>
diff --git a/drivers/target/iscsi/iscsi_target_nego.c b/drivers/target/iscsi/iscsi_target_nego.c index 4164977c170e..9b0c1196fe8e 100644 --- a/drivers/target/iscsi/iscsi_target_nego.c +++ b/drivers/target/iscsi/iscsi_target_nego.c @@ -1121,6 +1121,7 @@ int iscsi_target_locate_portal( iscsi_target_set_sock_callbacks(conn); login->np = np; + conn->tpg = NULL; login_req = (struct iscsi_login_req *) login->req; payload_length = ntoh24(login_req->dlength); @@ -1188,7 +1189,6 @@ int iscsi_target_locate_portal( */ sessiontype = strncmp(s_buf, DISCOVERY, 9); if (!sessiontype) { - conn->tpg = iscsit_global->discovery_tpg; if (!login->leading_connection) goto get_target; @@ -1205,9 +1205,11 @@ int iscsi_target_locate_portal( * Serialize access across the discovery struct iscsi_portal_group to * process login attempt. */ + conn->tpg = iscsit_global->discovery_tpg; if (iscsit_access_np(np, conn->tpg) < 0) { iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR, ISCSI_LOGIN_STATUS_SVC_UNAVAILABLE); + conn->tpg = NULL; ret = -1; goto out; }
The tpg->np_login_sem is a semaphore that is used to serialize the login process when multiple login threads run concurrently against the same target portal group. The iscsi_target_locate_portal() function finds the tpg, calls iscsit_access_np() against the np_login_sem semaphore and saves the tpg pointer in conn->tpg; If iscsi_target_locate_portal() fails, the caller will check for the conn->tpg pointer and, if it's not NULL, then it will assume that iscsi_target_locate_portal() called iscsit_access_np() on the semaphore. Make sure that conn->tpg gets initialized only if iscsit_access_np() was successful, otherwise iscsit_deaccess_np() may end up being called against a semaphore we never took, allowing more than one thread to access the same tpg. Signed-off-by: Maurizio Lombardi <mlombard@redhat.com> --- drivers/target/iscsi/iscsi_target_nego.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)