On 9/14/21 5:03 AM, Dmitry Bogdanov wrote:
> +bool iscsi_conn_auth_required(struct iscsi_conn *conn)
> +{
> + struct se_node_acl *se_nacl;
> +
> + if (conn->sess->sess_ops->SessionType) {
> + /*
> + * For SessionType=Discovery
> + */
> + return conn->tpg->tpg_attrib.authentication;
> + }
> + /*
> + * For SessionType=Normal
> + */
> + se_nacl = conn->sess->se_sess->se_node_acl;
> + if (!se_nacl) {
> + pr_debug("Unknown ACL %s is trying to connect\n",
> + se_nacl->initiatorname);
> + return true;
Before the patch, if we didn't have an ACL we would go by
conn->tpg->tpg_attrib.authentication. But with the patch, if
we don't have an ACL, then it looks like we always require authentication
which I don't think is right.
Is the code above supposed to return the value of
conn->tpg->tpg_attrib.authentication?
> + }
> +
> + if (se_nacl->dynamic_node_acl) {
> + pr_debug("Dynamic ACL %s is trying to connect\n",
> + se_nacl->initiatorname);
> + return conn->tpg->tpg_attrib.authentication;
> + }
> +
> + pr_debug("Known ACL %s is trying to connect\n",
> + se_nacl->initiatorname);
> + return conn->tpg->tpg_attrib.authentication;
> +}
> +
> static int iscsi_target_handle_csg_zero(
> struct iscsi_conn *conn,
> struct iscsi_login *login)
> @@ -874,22 +903,26 @@ static int iscsi_target_handle_csg_zero(
> return -1;
>
> if (!iscsi_check_negotiated_keys(conn->param_list)) {
> - if (conn->tpg->tpg_attrib.authentication &&
> - !strncmp(param->value, NONE, 4)) {
> - pr_err("Initiator sent AuthMethod=None but"
> - " Target is enforcing iSCSI Authentication,"
> - " login failed.\n");
> - iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_INITIATOR_ERR,
> - ISCSI_LOGIN_STATUS_AUTH_FAILED);
> - return -1;
> - }
> + bool auth_required = iscsi_conn_auth_required(conn);
> +
In __iscsi_target_login_thread we have:
if (conn->sess)
conn->sess->se_sess->sup_prot_ops =
conn->conn_transport->iscsit_get_sup_prot_ops(conn);
before we call:
iscsi_target_start_negotiation -> iscsi_target_do_login- > iscsi_target_handle_csg_zero
and hit the code above.
If conn->sess can be NULL then iscsi_conn_auth_required will crash.
However, I can't tell how conn->sess can be NULL in that code path. Is the conn->sess
check in __iscsi_target_login_thread not needed?
