On 3/1/22 2:23 AM, Mike Christie wrote:
> On 2/22/22 6:42 AM, Petr Pavlu wrote:
>> From: Petr Pavlu <petr.pavlu@suse.com>
>>
>> Function iscsi_target_do_login() attempts to cancel a connection when
>> a number of login exchanges reaches MAX_LOGIN_PDUS (7). This is done by
>> having a local counter and incrementing+checking it as the function
>> processes requests in a loop. A problem is that since the login rework in
>> back in 2013, the function always processes only a single request and the
>> loop is terminated at the end of the first iteration. This means the
>> counter reaches only value 1 and any excess number of login requests is
>> never rejected.
>>
>> Fix the problem by introducing iscsi_login.negotiation_exchanges counter
>> and update the logic to count exchanges per each login phase as described
>> in RFC 7143:
>>> 6.2. Text Mode Negotiation:
>>> [...]
>>> In the Login Phase (see Section 6.3), every stage is a separate
>>> negotiation. [...]
>>> [...]
>>> An iSCSI initiator or target MAY terminate a negotiation that does
>>> not terminate within an implementation-specific reasonable time or
>>> number of exchanges but SHOULD allow at least six (6) exchanges.
>>
> 
> It wasn't clear to me what this fixes. Today, are initiators sending more
> than 6 exchanges and if so what happens to the target? Is it crashing or
> annoying to user or cause some sort of endless login so we run out of
> resources? Or is this more of code cleanup?
> 
> When does this happen and with what initiators?

This issue is only something that I noticed while reading through the target
code because of some different problem. In that sense, the patch is more
a code cleanup. My tests to verify the patch were also artificial (attached).

I have now additionally tried some simple examples with sending extensive
number of Login requests in a loop to the target and did not observe any
immediate problem with running out of resources. A possible alternative might
be therefore to remove this logic, not sure.

Thanks,
Petr