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, as long as a few packets includes such jumbo payload; i.e. a packet to several received packets.

Here, Behringer F-Control Audio 202 (based on OXFW 960) has a quirk to postpone transferring isochronous packet till finish handling any asynchronous packets. In this case, this model is lazy, transfers no packets according to 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 | 21 +++++++++++++++++++-- sound/firewire/amdtp.h | 4 ++++ sound/firewire/oxfw/oxfw-stream.c | 10 ++++++++-- 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c index d882ca5..4eb8dc9 100644 --- a/sound/firewire/amdtp.c +++ b/sound/firewire/amdtp.c @@ -255,7 +255,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_PAYLOAD) + multiplier = 5; + + return 8 + s->syt_interval * s->data_block_quadlets * 4 * multiplier; } EXPORT_SYMBOL(amdtp_stream_get_max_payload);

@@ -811,12 +816,16 @@ static void in_stream_callback(struct fw_iso_context *context, u32 cycle, void *private_data) { struct amdtp_stream *s = private_data; - unsigned int p, syt, packets, payload_quadlets; + unsigned int p, syt, packets; + unsigned int payload_quadlets, max_payload_quadlets; __be32 *buffer, *headers = header;

/* The number of packets in buffer */ packets = header_length / IN_PACKET_HEADER_SIZE;

+ /* For buffer-over-run prevention. */ + max_payload_quadlets = amdtp_stream_get_max_payload(s) / 4; + for (p = 0; p < packets; p++) { if (s->packet_index < 0) break; @@ -832,6 +841,14 @@ static void in_stream_callback(struct fw_iso_context *context, u32 cycle, /* The number of quadlets in this packet */ payload_quadlets = (be32_to_cpu(headers[p]) >> ISO_DATA_LENGTH_SHIFT) / 4; + if (payload_quadlets > max_payload_quadlets) { + dev_err(&s->unit->device, + "Detect jumbo payload: %02x %02x\n", + payload_quadlets, max_payload_quadlets); + s->packet_index = -1; + break; + } + handle_in_packet(s, payload_quadlets, buffer); }

diff --git a/sound/firewire/amdtp.h b/sound/firewire/amdtp.h index 8a03a91..26b9093 100644 --- a/sound/firewire/amdtp.h +++ b/sound/firewire/amdtp.h @@ -29,6 +29,9 @@ * packet is not continuous from an initial value. * @CIP_EMPTY_HAS_WRONG_DBC: Only for in-stream. The value of dbc in empty * packet is wrong but the others are correct. + * @CIP_JUMBO_PAYLOAD: Only for in-stream. The number of data blocks in an + * packet is larger than IEC 61883-6 defines. Current implementation + * allows 5 times as large as IEC 61883-6 defines. */ enum cip_flags { CIP_NONBLOCKING = 0x00, @@ -40,6 +43,7 @@ enum cip_flags { CIP_SKIP_DBC_ZERO_CHECK = 0x20, CIP_SKIP_INIT_DBC_CHECK = 0x40, CIP_EMPTY_HAS_WRONG_DBC = 0x80, + CIP_JUMBO_PAYLOAD = 0x100, };

/** diff --git a/sound/firewire/oxfw/oxfw-stream.c b/sound/firewire/oxfw/oxfw-stream.c index e6757cd..873d40f 100644 --- a/sound/firewire/oxfw/oxfw-stream.c +++ b/sound/firewire/oxfw/oxfw-stream.c @@ -232,9 +232,15 @@ int snd_oxfw_stream_init_simplex(struct snd_oxfw *oxfw, goto end; }

- /* OXFW starts to transmit packets with non-zero dbc. */ + /* + * OXFW starts to transmit packets with non-zero dbc. + * OXFW postpone transferring packets till handling any asynchronous + * packets. As a result, next isochronous packet includes more data + * blocks than IEC 61883-6 defines. + */ if (stream == &oxfw->tx_stream) - oxfw->tx_stream.flags |= CIP_SKIP_INIT_DBC_CHECK; + oxfw->tx_stream.flags |= CIP_SKIP_INIT_DBC_CHECK | + CIP_JUMBO_PAYLOAD; end: return err; }

Current implementation reuses the value of syt field in incoming packet to outgoing packet for full duplex timestamp synchronization, while the number of data blocks in outgoing packets refers to hard-coded table and the synchronization cannot be applied to non-blocking stream.

This commit passes the number of data blocks from incoming packet processing to outgoing packet processing for the synchronization. For normal mode, isochronous callback handler is changed to generate the values of syt and data blocks.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp.c | 54 ++++++++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c index ab75ed9..c5034e7 100644 --- a/sound/firewire/amdtp.c +++ b/sound/firewire/amdtp.c @@ -651,17 +651,16 @@ static inline int queue_in_packet(struct amdtp_stream *s) amdtp_stream_get_max_payload(s), false); }

-static void handle_out_packet(struct amdtp_stream *s, unsigned int syt) +static void handle_out_packet(struct amdtp_stream *s, unsigned int data_blocks, + unsigned int syt) { __be32 *buffer; - unsigned int data_blocks, payload_length; + unsigned int payload_length; struct snd_pcm_substream *pcm;

if (s->packet_index < 0) return;

- data_blocks = calculate_data_blocks(s, syt); - buffer = s->buffer.packets[s->packet_index].buffer; buffer[0] = cpu_to_be32(ACCESS_ONCE(s->source_node_id_field) | (s->data_block_quadlets << AMDTP_DBS_SHIFT) | @@ -691,13 +690,12 @@ static void handle_out_packet(struct amdtp_stream *s, unsigned int syt) update_pcm_pointers(s, pcm, data_blocks); }

-static void handle_in_packet(struct amdtp_stream *s, - unsigned int payload_quadlets, - __be32 *buffer) +static int handle_in_packet(struct amdtp_stream *s, + unsigned int payload_quadlets, __be32 *buffer) { u32 cip_header[2]; - unsigned int data_blocks, data_block_quadlets, data_block_counter, - dbc_interval; + unsigned int data_blocks; + unsigned int data_block_quadlets, data_block_counter, dbc_interval; struct snd_pcm_substream *pcm = NULL; bool lost;

@@ -714,6 +712,7 @@ static void handle_in_packet(struct amdtp_stream *s, dev_info_ratelimited(&s->unit->device, "Invalid CIP header for AMDTP: %08X:%08X\n", cip_header[0], cip_header[1]); + data_blocks = 0; goto end; }

@@ -730,7 +729,7 @@ static void handle_in_packet(struct amdtp_stream *s, dev_info_ratelimited(&s->unit->device, "Detect invalid value in dbs field: %08X\n", cip_header[0]); - goto err; + return -EIO; } if (s->flags & CIP_WRONG_DBS) data_block_quadlets = s->data_block_quadlets; @@ -763,7 +762,7 @@ static void handle_in_packet(struct amdtp_stream *s, dev_info(&s->unit->device, "Detect discontinuity of CIP: %02X %02X\n", s->data_block_counter, data_block_counter); - goto err; + return -EIO; }

if (data_blocks > 0) { @@ -784,15 +783,12 @@ static void handle_in_packet(struct amdtp_stream *s, (data_block_counter + data_blocks) & 0xff; end: if (queue_in_packet(s) < 0) - goto err; + return -EIO;

if (pcm) update_pcm_pointers(s, pcm, data_blocks);

- return; -err: - s->packet_index = -1; - amdtp_stream_pcm_abort(s); + return data_blocks; }

static void out_stream_callback(struct fw_iso_context *context, u32 cycle, @@ -801,6 +797,7 @@ static void out_stream_callback(struct fw_iso_context *context, u32 cycle, { struct amdtp_stream *s = private_data; unsigned int i, syt, packets = header_length / 4; + unsigned int data_blocks;

/* * Compute the cycle of the last queued packet. @@ -811,7 +808,9 @@ static void out_stream_callback(struct fw_iso_context *context, u32 cycle,

for (i = 0; i < packets; ++i) { syt = calculate_syt(s, ++cycle); - handle_out_packet(s, syt); + data_blocks = calculate_data_blocks(s, syt); + + handle_out_packet(s, data_blocks, syt); } fw_iso_context_queue_flush(s->context); } @@ -823,6 +822,7 @@ static void in_stream_callback(struct fw_iso_context *context, u32 cycle, struct amdtp_stream *s = private_data; unsigned int p, syt, packets; unsigned int payload_quadlets, max_payload_quadlets; + unsigned int data_blocks; __be32 *buffer, *headers = header;

/* The number of packets in buffer */ @@ -837,12 +837,6 @@ static void in_stream_callback(struct fw_iso_context *context, u32 cycle,

buffer = s->buffer.packets[s->packet_index].buffer;

- /* Process sync slave stream */ - if (s->sync_slave && s->sync_slave->callbacked) { - syt = be32_to_cpu(buffer[1]) & CIP_SYT_MASK; - handle_out_packet(s->sync_slave, syt); - } - /* The number of quadlets in this packet */ payload_quadlets = (be32_to_cpu(headers[p]) >> ISO_DATA_LENGTH_SHIFT) / 4; @@ -854,11 +848,23 @@ static void in_stream_callback(struct fw_iso_context *context, u32 cycle, break; }

- handle_in_packet(s, payload_quadlets, buffer); + data_blocks = handle_in_packet(s, payload_quadlets, buffer); + if (data_blocks < 0) { + s->packet_index = -1; + break; + } + + /* Process sync slave stream */ + if (s->sync_slave && s->sync_slave->callbacked) { + syt = be32_to_cpu(buffer[1]) & CIP_SYT_MASK; + handle_out_packet(s->sync_slave, data_blocks, syt); + } }

/* Queueing error or detecting discontinuity */ if (s->packet_index < 0) { + amdtp_stream_pcm_abort(s); + /* Abort sync slave. */ if (s->sync_slave) { s->sync_slave->packet_index = -1;

Former patches allow non-blocking streams to synchronize with timestamp. This patch removes the restriction.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c index 5c0e3ce..bed8e10 100644 --- a/sound/firewire/amdtp.c +++ b/sound/firewire/amdtp.c @@ -913,7 +913,7 @@ static void amdtp_stream_first_callback(struct fw_iso_context *context,

if (s->direction == AMDTP_IN_STREAM) context->callback.sc = in_stream_callback; - else if ((s->flags & CIP_BLOCKING) && (s->flags & CIP_SYNC_TO_DEVICE)) + else if (s->flags & CIP_SYNC_TO_DEVICE) context->callback.sc = slave_stream_callback; else context->callback.sc = out_stream_callback;