Re: [PATCH] firewire: Fix out-of-bounds access in CSR iterator

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

Hi,

Thank you for proposing this fix, however I have mixed feelings about
apply it, since the iterator is designed to traverse the cache data of the
actual configuration ROM, which has been generated by the parser.

If the configuration ROM contains any malformed entries, they should
already be detected by the parser. Therefore I don't think robustness of
the iterator is a critical concern in this case.

For the v7.4 kernel, I've merged KUnit tests for the parser
implementation. If you are still interested in discussion about this fix,
please check them at first then try to add tests specific to the iterator
itself.

On Mon, Nov 02, 2026 at 06:53:34AM +0000, Sreeraj S Kurup wrote:
> The fw_csr_iterator_next() function reads ci->p before
> checking if it has reached the end.
> 
> This can cause an out-of-bounds read when the iterator
> has no more entries.
> 
> Check the iterator position before reading the entry.
> Move the pointer forward only after reading a valid entry.
> 
> Signed-off-by: Sreeraj S Kurup <sreekuttan2156239@gmail.com>
> ---
>  drivers/firewire/core-device.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c
> index a68100d3da0b..36667841ca3c 100644
> --- a/drivers/firewire/core-device.c
> +++ b/drivers/firewire/core-device.c
> @@ -40,10 +40,14 @@ EXPORT_SYMBOL(fw_csr_iterator_init);
>  
>  int fw_csr_iterator_next(struct fw_csr_iterator *ci, int *key, int *value)
>  {
> +	if (ci->p >= ci->end)
> +		return 0;
> +
>  	*key = *ci->p >> 24;
>  	*value = *ci->p & 0xffffff;
>  
> -	return ci->p++ < ci->end;
> +	ci->p++;
> +	return 1;
>  }
>  EXPORT_SYMBOL(fw_csr_iterator_next);
>  
> -- 
> 2.55.0


Thanks

Takashi Sakamoto