On Wed, 2012-08-22 at 18:27 +0800, Benjamin Wang wrote:
> Hi Nicholas:
> I am tracing the source code of iscsi target in kernel 3.4.6, and
> it is found that there is lack of the error handling of
> idr_get_new().
>
>
So looking at current v3.6-rc2 code, there is one remaining case of
idr_get_new() usage in iscsi_target_login.c:iscsi_login_zero_tsih_s1()
that needs to have it's return value handled properly..
> For example, in iscsi_target_login.c,
>
>
> if (!idr_pre_get(&sess_idr, GFP_KERNEL)) {
> pr_err("idr_pre_get() for sess_idr failed\n");
> iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
> ISCSI_LOGIN_STATUS_NO_RESOURCES);
> kfree(sess);
> return -ENOMEM;
> }
> spin_lock(&sess_idr_lock);
> idr_get_new(&sess_idr, NULL, &sess->session_index);
> spin_unlock(&sess_idr_lock);
>
>
> Suppose it could be:
> TRY_AGAIN:
> if (!idr_pre_get(&sess_idr, GFP_KERNEL)) {
> pr_err("idr_pre_get() for sess_idr failed\n");
> iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
> ISCSI_LOGIN_STATUS_NO_RESOURCES);
> kfree(sess);
> return -ENOMEM;
> }
> spin_lock(&sess_idr_lock);
> ret = idr_get_new(&sess_idr, NULL, &sess->session_index);
> spin_unlock(&sess_idr_lock);
> if (ret == -EAGAIN)
> goto TRY_AGAIN ;
>
>
This would cause an infinite loop if sess_idr space is completely
exhausted..
How above just failing w/ ISCSI_LOGIN_STATUS_NO_RESOURCES status +
cleanup along the lines of what the !sess->sess_ops failure path already
does..?
Can you generate a bugfix patch against v3.4.2 for this one case that I
can apply to target-pending for mainline..?
Thanks Benjamin!
--nab
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
