Re: [PATCH 1/4] ALSA: firewire-lib: add buffer-over-run protection at receiving more data blocks than expected

[Takashi Iwai <tiwai@suse.de>]

At Sat, 16 May 2015 20:22:42 +0900,
Takashi Sakamoto wrote:
> 
> In IEC 61883-6, the number of data blocks in a packet is limited up to
> the value of SYT_INTERVAL. Current implementation is compliant to the
> limitation, while it can cause buffer-over-run when the value of dbs
> field in received packet is illegally large.
> 
> This commit adds a validator to detect such illegal packets to prevent
> the buffer-over-run. Actually, the buffer is aligned to the size of memory
>  page, thus this issue hardly causes system errors due to the room to page
> alignment.
> 
> But, Behringer F-Control Audio 202 (based on OXFW 970) has a quirk to
> postpone transferring isochronous packet till finish handling any
> asynchronous packets. In this case, this model is lazy, transfers no
> packets during several cycle-start packets. After finishing, this model
> pushes required data in next isochronous packet. As a result, the
> packet include more data blocks than IEC 61883-6 defines.
> 
> To continue to support this model, this commit adds a new flag to extend
> the length of calculated payload. This flag allows the size of payload
> 5 times as large as IEC 61883-6 defines. As a result, packets from this
> model passed the validator successfully.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  sound/firewire/amdtp.c            | 15 ++++++++++++++-
>  sound/firewire/amdtp.h            |  4 ++++
>  sound/firewire/oxfw/oxfw-stream.c | 10 ++++++++--
>  3 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
> index e061355..6424382 100644
> --- a/sound/firewire/amdtp.c
> +++ b/sound/firewire/amdtp.c
> @@ -251,7 +251,12 @@ EXPORT_SYMBOL(amdtp_stream_set_parameters);
>   */
>  unsigned int amdtp_stream_get_max_payload(struct amdtp_stream *s)
>  {
> -	return 8 + s->syt_interval * s->data_block_quadlets * 4;
> +	unsigned int multiplier = 1;
> +
> +	if (s->flags & CIP_JUMBO_DATA_BLOCKS)
> +		multiplier = 5;
> +
> +	return 8 + s->syt_interval * s->data_block_quadlets * 4 * multiplier;
>  }
>  EXPORT_SYMBOL(amdtp_stream_get_max_payload);
>  
> @@ -687,6 +692,14 @@ static void handle_in_packet(struct amdtp_stream *s,
>  	struct snd_pcm_substream *pcm = NULL;
>  	bool lost;
>  
> +	/* Protect from buffer-over-run. */
> +	if (payload_quadlets > amdtp_stream_get_max_payload(s)) {
> +		dev_info(&s->unit->device,
> +			 "Too many data blocks in a packet: %02X %02X\n",
> +			 amdtp_stream_get_max_payload(s), payload_quadlets);
> +		goto err;

How often may this message appear?  My concern is the flood of such
error message.  Some messages are already with _ratelimited() for
avoiding it.


Takashi