Re: [PATCH v2] dmaengine: loongson1-apb-dma: avoid using iterator variable after list_for_each_entry()

Hi Frank:

Sorry for the late reply.

On 2026/8/15 03:33, Frank Li wrote:
> On Wed, Jul 29, 2026 at 02:32:47PM +0000, Mahad Ibrahim wrote:
>> ls1x_dma_tx_status() locates the descriptor actively being processed by
>> walking the LLI list and comparing the hardware reported next descriptor
>> pointer against each element's next-descriptor pointer.
>>
>> A list_for_each_entry macro is used in the comparison phase. Which at
>> the end of the loop leaves the lli pointer at the currently executing LLI.
>> However this also subsequently runs for a non-match lli, in which it
>> points at the head. This causes a type confusion bug which treats the
>> head, which is a ls1x_dma_desc, as a ls1x_dma_lli object. Additionally it
>> goes forwards and prints garbage via the dev_dbg.
>>
>> Fix the type confusion bug by only allowing matched LLI descriptor chains
>> to print the current LLI and residue calculation, as failing to match
>> should be treated as an unexpected condition.
>>
>> Found by the following Coccinelle check:
>>
>>    scripts/coccinelle/iterators/use_after_iter.cocci
>>
>>    drivers/dma/loongson/loongson1-apb-dma.c:461:6-9: ERROR: invalid
>>    reference to the index variable of the iterator on line 450
>>
>> I did not see a bug upstream detailing this error, nor do I have the
>> hardware to confirm this bug or error, all this is from a pure code
>> examination.
>>
>> As I do not possess the hardware, I cannot test the patch. Compile tested
>> only with mips64-linux-gnu-gcc.
> 
> Avoid post new patch on old email thread.
> 
> Just simple said
> 
> Compile test only.
> 
>>
>> Signed-off-by: Mahad Ibrahim <[email protected]>
>> ---
>>
>> v2:
>> - encapsulate residue calculation and dev_dbg inside the
>>    list_for_each_entry() macro. Treat non-matching LLI as an unexpected
>>    case.
>>
>>
>>   drivers/dma/loongson/loongson1-apb-dma.c | 27 ++++++++++++++----------
>>   1 file changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/dma/loongson/loongson1-apb-dma.c b/drivers/dma/loongson/loongson1-apb-dma.c
>> index 89786cbd20ab..8658d5377795 100644
>> --- a/drivers/dma/loongson/loongson1-apb-dma.c
>> +++ b/drivers/dma/loongson/loongson1-apb-dma.c
>> @@ -446,22 +446,27 @@ static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
>>
>>   			/* locate the current lli */
>>   			next_phys = chan->curr_lli->hw[LS1X_DMADESC_NEXT];
>> -			list_for_each_entry(lli, &desc->lli_list, node)
>> -				if (lli->hw[LS1X_DMADESC_NEXT] == next_phys)
>> -					break;
>> +			list_for_each_entry(lli, &desc->lli_list, node) {
>> +				if (lli->hw[LS1X_DMADESC_NEXT] != next_phys)
>> +					continue;
>>
>> -			dev_dbg(chan2dev(dchan), "current lli_phys=%pad",
>> -				&lli->phys);
>> +				dev_dbg(chan2dev(dchan), "current lli_phys=%pad\n",
>> +					&lli->phys);
>>
>> -			/* count the residues */
>> -			list_for_each_entry_from(lli, &desc->lli_list, node)
>> -				bytes += lli->hw[LS1X_DMADESC_LENGTH] *
>> -					 chan->bus_width;
>> +				/* count the residues */
>> +				list_for_each_entry_from(lli, &desc->lli_list, node)
>> +					bytes += lli->hw[LS1X_DMADESC_LENGTH] *
>> +						 chan->bus_width;
>> +
>> +				dma_set_residue(state, bytes);
>> +				return status;
>> +			}
>> +
>> +			dev_warn(chan2dev(dchan),
>> +				 "unable to locate current lli.\n");
> 
> most likely logic is correct, it is not obvious, hope some one who have
> hardware help test it.
> 
> Binbin Zhou just add add this driver recently, he should have hardware to
> test it.

Emm, sorry, I don't have any test equipment available right now. As far 
as I know, Keguang is the maintainer of this driver, and he has the 
necessary test equipment.
> 
> Frank
>>   		}
>>   	}
>>
>> -	dma_set_residue(state, bytes);
>> -
>>   	return status;
>>   }
>>
>> --
>> 2.54.0
>>

-- 
Thanks.
Binbin