Re: [PATCH 2/3] scsi: libiscsi: fix invalid pointer dereference in iscsi_eh_session_reset

From: Ding Hui
Date: Sat Sep 11 2021 - 05:52:11 EST

On 2021/9/11 12:38 上午, Mike Christie wrote:
On 9/9/21 8:02 PM, Ding Hui wrote:
like commit 5db6dd14b313 ("scsi: libiscsi: Fix NULL pointer dereference in
iscsi_eh_session_reset"), access conn->persistent_address here is not safe

The persistent_address is independent of conn refcount, so maybe
already freed by iscsi_conn_teardown(), also we put the refcount of conn
above, the conn pointer may be invalid.

This shouldn't happen like you describe above, because when we wake up
we will see the session->state is ISCSI_STATE_TERMINATE. We will then
not reference the conn in that code below.

However, it looks like we could hit an issue where if a user was resetting
the persistent_address or targetname via iscsi_set_param -> iscsi_switch_str_param
then we could be accessing freed memory. I think we need the frwd_lock when swapping
the strings in iscsi_switch_str_param.

Thanks for your detailed explanation, I'll drop 2/3 and 3/3 in v2 patch.
I expect that the persistent_address issue be fixed in your future patchset.

Sorry for my ignorance.

Signed-off-by: Ding Hui <dinghui@xxxxxxxxxxxxxx>
drivers/scsi/libiscsi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 712a45368385..69b3b2148328 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -2531,8 +2531,8 @@ int iscsi_eh_session_reset(struct scsi_cmnd *sc)
if (session->state == ISCSI_STATE_LOGGED_IN) {
- "session reset succeeded for %s,%s\n",
- session->targetname, conn->persistent_address);
+ "session reset succeeded for %s\n",
+ session->targetname);
} else
goto failed;