From a2064710ba2b38a4f07c1b273c389b70b14b2d18 Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Fri, 22 May 2015 23:00:50 +0900
Subject: ALSA: firewire-lib: add buffer-over-run protection at receiving more
 data blocks than expected

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>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/firewire/amdtp.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

(limited to 'sound/firewire/amdtp.c')

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index e061355f535f..a3970043e472 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_PAYLOAD)
+		multiplier = 5;
+
+	return 8 + s->syt_interval * s->data_block_quadlets * 4 * multiplier;
 }
 EXPORT_SYMBOL(amdtp_stream_get_max_payload);
 
@@ -807,12 +812,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;
@@ -828,6 +837,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);
 	}
 
-- 
cgit v1.2.3


From 875be09160345442196d0889ddf48f747701e12c Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Fri, 22 May 2015 23:00:51 +0900
Subject: ALSA: firewire-lib: simplify function to calculate the number of data
 blocks

This function is called according to conditions between the value of
syt and streaming mode(blocking or non-blocking).

To simplify caller's work, this commit push these conditions to the
function.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/firewire/amdtp.c | 49 +++++++++++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 22 deletions(-)

(limited to 'sound/firewire/amdtp.c')

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index a3970043e472..d9af99b9a863 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -321,17 +321,25 @@ void amdtp_stream_pcm_prepare(struct amdtp_stream *s)
 }
 EXPORT_SYMBOL(amdtp_stream_pcm_prepare);
 
-static unsigned int calculate_data_blocks(struct amdtp_stream *s)
+static unsigned int calculate_data_blocks(struct amdtp_stream *s,
+					  unsigned int syt)
 {
 	unsigned int phase, data_blocks;
 
-	if (s->flags & CIP_BLOCKING)
-		data_blocks = s->syt_interval;
-	else if (!cip_sfc_is_base_44100(s->sfc)) {
-		/* Sample_rate / 8000 is an integer, and precomputed. */
-		data_blocks = s->data_block_state;
+	/* Blocking mode. */
+	if (s->flags & CIP_BLOCKING) {
+		/* This module generate empty packet for 'no data'. */
+		if (syt == CIP_SYT_NO_INFO)
+			data_blocks = 0;
+		else
+			data_blocks = s->syt_interval;
+	/* Non-blocking mode. */
 	} else {
-		phase = s->data_block_state;
+		if (!cip_sfc_is_base_44100(s->sfc)) {
+			/* Sample_rate / 8000 is an integer, and precomputed. */
+			data_blocks = s->data_block_state;
+		} else {
+			phase = s->data_block_state;
 
 		/*
 		 * This calculates the number of data blocks per packet so that
@@ -341,16 +349,17 @@ static unsigned int calculate_data_blocks(struct amdtp_stream *s)
 		 *    as possible in the sequence (to prevent underruns of the
 		 *    device's buffer).
 		 */
-		if (s->sfc == CIP_SFC_44100)
-			/* 6 6 5 6 5 6 5 ... */
-			data_blocks = 5 + ((phase & 1) ^
-					   (phase == 0 || phase >= 40));
-		else
-			/* 12 11 11 11 11 ... or 23 22 22 22 22 ... */
-			data_blocks = 11 * (s->sfc >> 1) + (phase == 0);
-		if (++phase >= (80 >> (s->sfc >> 1)))
-			phase = 0;
-		s->data_block_state = phase;
+			if (s->sfc == CIP_SFC_44100)
+				/* 6 6 5 6 5 6 5 ... */
+				data_blocks = 5 + ((phase & 1) ^
+						   (phase == 0 || phase >= 40));
+			else
+				/* 12 11 11 11 11 ... or 23 22 22 22 22 ... */
+				data_blocks = 11 * (s->sfc >> 1) + (phase == 0);
+			if (++phase >= (80 >> (s->sfc >> 1)))
+				phase = 0;
+			s->data_block_state = phase;
+		}
 	}
 
 	return data_blocks;
@@ -647,11 +656,7 @@ static void handle_out_packet(struct amdtp_stream *s, unsigned int syt)
 	if (s->packet_index < 0)
 		return;
 
-	/* this module generate empty packet for 'no data' */
-	if (!(s->flags & CIP_BLOCKING) || (syt != CIP_SYT_NO_INFO))
-		data_blocks = calculate_data_blocks(s);
-	else
-		data_blocks = 0;
+	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) |
-- 
cgit v1.2.3


From 6fc6b9ce41c6e6ee123f0da5d3bfd7b628be2bd0 Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Fri, 22 May 2015 23:00:52 +0900
Subject: ALSA: firewire-lib: pass the number of data blocks in incoming
 packets to outgoing packets

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>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/firewire/amdtp.c | 54 ++++++++++++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

(limited to 'sound/firewire/amdtp.c')

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index d9af99b9a863..c69c3ab90acc 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -647,17 +647,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) |
@@ -687,13 +686,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;
 
@@ -710,6 +708,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;
 	}
 
@@ -726,7 +725,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;
@@ -759,7 +758,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) {
@@ -780,15 +779,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,
@@ -797,6 +793,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.
@@ -807,7 +804,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);
 }
@@ -819,6 +818,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 */
@@ -833,12 +833,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;
@@ -850,11 +844,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;
-- 
cgit v1.2.3


From a4103bd7fdd59548580bdbc8e995ef9621ef670e Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Fri, 22 May 2015 23:00:53 +0900
Subject: ALSA: firewire-lib: set streaming error outside of packetization

In previous commit, error handling for incoming packet processing is
outside of packetization. This is nice for reading the codes.

This commit applies this idea for outgoing packet processing, too.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/firewire/amdtp.c | 39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

(limited to 'sound/firewire/amdtp.c')

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index c69c3ab90acc..c26dda7f41e6 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -647,16 +647,13 @@ 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 data_blocks,
-			      unsigned int syt)
+static int handle_out_packet(struct amdtp_stream *s, unsigned int data_blocks,
+			     unsigned int syt)
 {
 	__be32 *buffer;
 	unsigned int payload_length;
 	struct snd_pcm_substream *pcm;
 
-	if (s->packet_index < 0)
-		return;
-
 	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) |
@@ -676,14 +673,14 @@ static void handle_out_packet(struct amdtp_stream *s, unsigned int data_blocks,
 	s->data_block_counter = (s->data_block_counter + data_blocks) & 0xff;
 
 	payload_length = 8 + data_blocks * 4 * s->data_block_quadlets;
-	if (queue_out_packet(s, payload_length, false) < 0) {
-		s->packet_index = -1;
-		amdtp_stream_pcm_abort(s);
-		return;
-	}
+	if (queue_out_packet(s, payload_length, false) < 0)
+		return -EIO;
 
 	if (pcm)
 		update_pcm_pointers(s, pcm, data_blocks);
+
+	/* No need to return the number of handled data blocks. */
+	return 0;
 }
 
 static int handle_in_packet(struct amdtp_stream *s,
@@ -795,6 +792,9 @@ static void out_stream_callback(struct fw_iso_context *context, u32 cycle,
 	unsigned int i, syt, packets = header_length / 4;
 	unsigned int data_blocks;
 
+	if (s->packet_index < 0)
+		return;
+
 	/*
 	 * Compute the cycle of the last queued packet.
 	 * (We need only the four lowest bits for the SYT, so we can ignore
@@ -806,8 +806,13 @@ static void out_stream_callback(struct fw_iso_context *context, u32 cycle,
 		syt = calculate_syt(s, ++cycle);
 		data_blocks = calculate_data_blocks(s, syt);
 
-		handle_out_packet(s, data_blocks, syt);
+		if (handle_out_packet(s, data_blocks, syt) < 0) {
+			s->packet_index = -1;
+			amdtp_stream_pcm_abort(s);
+			return;
+		}
 	}
+
 	fw_iso_context_queue_flush(s->context);
 }
 
@@ -821,6 +826,9 @@ static void in_stream_callback(struct fw_iso_context *context, u32 cycle,
 	unsigned int data_blocks;
 	__be32 *buffer, *headers = header;
 
+	if (s->packet_index < 0)
+		return;
+
 	/* The number of packets in buffer */
 	packets = header_length / IN_PACKET_HEADER_SIZE;
 
@@ -828,9 +836,6 @@ static void in_stream_callback(struct fw_iso_context *context, u32 cycle,
 	max_payload_quadlets = amdtp_stream_get_max_payload(s) / 4;
 
 	for (p = 0; p < packets; p++) {
-		if (s->packet_index < 0)
-			break;
-
 		buffer = s->buffer.packets[s->packet_index].buffer;
 
 		/* The number of quadlets in this packet */
@@ -853,7 +858,11 @@ static void in_stream_callback(struct fw_iso_context *context, u32 cycle,
 		/* 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);
+			if (handle_out_packet(s->sync_slave,
+					      data_blocks, syt) < 0) {
+				s->packet_index = -1;
+				break;
+			}
 		}
 	}
 
-- 
cgit v1.2.3


From 727d3a0b1f97404eb1c3c136ba7fd41ab7ea6a22 Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Fri, 22 May 2015 23:00:54 +0900
Subject: ALSA: firewire-lib: remove restriction for non-blocking mode

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

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

(limited to 'sound/firewire/amdtp.c')

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index c26dda7f41e6..97afc887a53d 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -909,7 +909,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;
-- 
cgit v1.2.3


From 29bcae208179416048c49c7039c9d7a362b6c5bf Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Fri, 22 May 2015 23:21:11 +0900
Subject: ALSA: firewire-lib: rename local functions for code cleanup

The naming rule for local functions was inconsistent. This commit
rename them with a consistent manner.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Acked-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/firewire/amdtp.c | 60 +++++++++++++++++++++++++-------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

(limited to 'sound/firewire/amdtp.c')

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index 97afc887a53d..9d723458cdcc 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -260,15 +260,15 @@ unsigned int amdtp_stream_get_max_payload(struct amdtp_stream *s)
 }
 EXPORT_SYMBOL(amdtp_stream_get_max_payload);
 
-static void amdtp_write_s16(struct amdtp_stream *s,
-			    struct snd_pcm_substream *pcm,
-			    __be32 *buffer, unsigned int frames);
-static void amdtp_write_s32(struct amdtp_stream *s,
-			    struct snd_pcm_substream *pcm,
-			    __be32 *buffer, unsigned int frames);
-static void amdtp_read_s32(struct amdtp_stream *s,
-			   struct snd_pcm_substream *pcm,
-			   __be32 *buffer, unsigned int frames);
+static void write_pcm_s16(struct amdtp_stream *s,
+			  struct snd_pcm_substream *pcm,
+			  __be32 *buffer, unsigned int frames);
+static void write_pcm_s32(struct amdtp_stream *s,
+			  struct snd_pcm_substream *pcm,
+			  __be32 *buffer, unsigned int frames);
+static void read_pcm_s32(struct amdtp_stream *s,
+			 struct snd_pcm_substream *pcm,
+			 __be32 *buffer, unsigned int frames);
 
 /**
  * amdtp_stream_set_pcm_format - set the PCM format
@@ -291,16 +291,16 @@ void amdtp_stream_set_pcm_format(struct amdtp_stream *s,
 		/* fall through */
 	case SNDRV_PCM_FORMAT_S16:
 		if (s->direction == AMDTP_OUT_STREAM) {
-			s->transfer_samples = amdtp_write_s16;
+			s->transfer_samples = write_pcm_s16;
 			break;
 		}
 		WARN_ON(1);
 		/* fall through */
 	case SNDRV_PCM_FORMAT_S32:
 		if (s->direction == AMDTP_OUT_STREAM)
-			s->transfer_samples = amdtp_write_s32;
+			s->transfer_samples = write_pcm_s32;
 		else
-			s->transfer_samples = amdtp_read_s32;
+			s->transfer_samples = read_pcm_s32;
 		break;
 	}
 }
@@ -408,9 +408,9 @@ static unsigned int calculate_syt(struct amdtp_stream *s,
 	}
 }
 
-static void amdtp_write_s32(struct amdtp_stream *s,
-			    struct snd_pcm_substream *pcm,
-			    __be32 *buffer, unsigned int frames)
+static void write_pcm_s32(struct amdtp_stream *s,
+			  struct snd_pcm_substream *pcm,
+			  __be32 *buffer, unsigned int frames)
 {
 	struct snd_pcm_runtime *runtime = pcm->runtime;
 	unsigned int channels, remaining_frames, i, c;
@@ -433,9 +433,9 @@ static void amdtp_write_s32(struct amdtp_stream *s,
 	}
 }
 
-static void amdtp_write_s16(struct amdtp_stream *s,
-			    struct snd_pcm_substream *pcm,
-			    __be32 *buffer, unsigned int frames)
+static void write_pcm_s16(struct amdtp_stream *s,
+			  struct snd_pcm_substream *pcm,
+			  __be32 *buffer, unsigned int frames)
 {
 	struct snd_pcm_runtime *runtime = pcm->runtime;
 	unsigned int channels, remaining_frames, i, c;
@@ -458,9 +458,9 @@ static void amdtp_write_s16(struct amdtp_stream *s,
 	}
 }
 
-static void amdtp_read_s32(struct amdtp_stream *s,
-			   struct snd_pcm_substream *pcm,
-			   __be32 *buffer, unsigned int frames)
+static void read_pcm_s32(struct amdtp_stream *s,
+			 struct snd_pcm_substream *pcm,
+			 __be32 *buffer, unsigned int frames)
 {
 	struct snd_pcm_runtime *runtime = pcm->runtime;
 	unsigned int channels, remaining_frames, i, c;
@@ -482,8 +482,8 @@ static void amdtp_read_s32(struct amdtp_stream *s,
 	}
 }
 
-static void amdtp_fill_pcm_silence(struct amdtp_stream *s,
-				   __be32 *buffer, unsigned int frames)
+static void write_pcm_silence(struct amdtp_stream *s,
+			      __be32 *buffer, unsigned int frames)
 {
 	unsigned int i, c;
 
@@ -524,8 +524,8 @@ static void midi_rate_use_one_byte(struct amdtp_stream *s, unsigned int port)
 	s->midi_fifo_used[port] += amdtp_rate_table[s->sfc];
 }
 
-static void amdtp_fill_midi(struct amdtp_stream *s,
-			    __be32 *buffer, unsigned int frames)
+static void write_midi_messages(struct amdtp_stream *s,
+				__be32 *buffer, unsigned int frames)
 {
 	unsigned int f, port;
 	u8 *b;
@@ -551,8 +551,8 @@ static void amdtp_fill_midi(struct amdtp_stream *s,
 	}
 }
 
-static void amdtp_pull_midi(struct amdtp_stream *s,
-			    __be32 *buffer, unsigned int frames)
+static void read_midi_messages(struct amdtp_stream *s,
+			       __be32 *buffer, unsigned int frames)
 {
 	unsigned int f, port;
 	int len;
@@ -666,9 +666,9 @@ static int handle_out_packet(struct amdtp_stream *s, unsigned int data_blocks,
 	if (pcm)
 		s->transfer_samples(s, pcm, buffer, data_blocks);
 	else
-		amdtp_fill_pcm_silence(s, buffer, data_blocks);
+		write_pcm_silence(s, buffer, data_blocks);
 	if (s->midi_ports)
-		amdtp_fill_midi(s, buffer, data_blocks);
+		write_midi_messages(s, buffer, data_blocks);
 
 	s->data_block_counter = (s->data_block_counter + data_blocks) & 0xff;
 
@@ -766,7 +766,7 @@ static int handle_in_packet(struct amdtp_stream *s,
 			s->transfer_samples(s, pcm, buffer, data_blocks);
 
 		if (s->midi_ports)
-			amdtp_pull_midi(s, buffer, data_blocks);
+			read_midi_messages(s, buffer, data_blocks);
 	}
 
 	if (s->flags & CIP_DBC_IS_END_EVENT)
-- 
cgit v1.2.3


From 9a2820c1189bd3165accd7663783e32935bb9055 Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Fri, 22 May 2015 23:21:12 +0900
Subject: ALSA: firewire-lib: macro arrangement for code cleanup

Some macros include my misunderstanding for IEC 61883-1 or -6.
Additionally, some fixed values appear on codes.

This commit replaces these with macros with proper names.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Acked-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/firewire/amdtp.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

(limited to 'sound/firewire/amdtp.c')

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index 9d723458cdcc..29efbdad572f 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -40,24 +40,28 @@
 #define TAG_CIP			1
 
 /* common isochronous packet header parameters */
-#define CIP_EOH			(1u << 31)
+#define CIP_EOH_SHIFT		31
+#define CIP_EOH			(1u << CIP_EOH_SHIFT)
 #define CIP_EOH_MASK		0x80000000
-#define CIP_FMT_AM		(0x10 << 24)
+#define CIP_SID_SHIFT		24
+#define CIP_SID_MASK		0x3f000000
+#define CIP_DBS_MASK		0x00ff0000
+#define CIP_DBS_SHIFT		16
+#define CIP_DBC_MASK		0x000000ff
+#define CIP_FMT_SHIFT		24
 #define CIP_FMT_MASK		0x3f000000
+#define CIP_FDF_MASK		0x00ff0000
+#define CIP_FDF_SHIFT		16
 #define CIP_SYT_MASK		0x0000ffff
 #define CIP_SYT_NO_INFO		0xffff
-#define CIP_FDF_MASK		0x00ff0000
-#define CIP_FDF_SFC_SHIFT	16
 
 /*
  * Audio and Music transfer protocol specific parameters
  * only "Clock-based rate control mode" is supported
  */
-#define AMDTP_FDF_AM824		(0 << (CIP_FDF_SFC_SHIFT + 3))
+#define CIP_FMT_AM		(0x10 << CIP_FMT_SHIFT)
+#define AMDTP_FDF_AM824		(0 << (CIP_FDF_SHIFT + 3))
 #define AMDTP_FDF_NO_DATA	0xff
-#define AMDTP_DBS_MASK		0x00ff0000
-#define AMDTP_DBS_SHIFT		16
-#define AMDTP_DBC_MASK		0x000000ff
 
 /* TODO: make these configurable */
 #define INTERRUPT_INTERVAL	16
@@ -656,10 +660,10 @@ static int handle_out_packet(struct amdtp_stream *s, unsigned int data_blocks,
 
 	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) |
+				(s->data_block_quadlets << CIP_DBS_SHIFT) |
 				s->data_block_counter);
 	buffer[1] = cpu_to_be32(CIP_EOH | CIP_FMT_AM | AMDTP_FDF_AM824 |
-				(s->sfc << CIP_FDF_SFC_SHIFT) | syt);
+				(s->sfc << CIP_FDF_SHIFT) | syt);
 	buffer += 2;
 
 	pcm = ACCESS_ONCE(s->pcm);
@@ -712,11 +716,11 @@ static int handle_in_packet(struct amdtp_stream *s,
 	/* Calculate data blocks */
 	if (payload_quadlets < 3 ||
 	    ((cip_header[1] & CIP_FDF_MASK) ==
-				(AMDTP_FDF_NO_DATA << CIP_FDF_SFC_SHIFT))) {
+				(AMDTP_FDF_NO_DATA << CIP_FDF_SHIFT))) {
 		data_blocks = 0;
 	} else {
 		data_block_quadlets =
-			(cip_header[0] & AMDTP_DBS_MASK) >> AMDTP_DBS_SHIFT;
+			(cip_header[0] & CIP_DBS_MASK) >> CIP_DBS_SHIFT;
 		/* avoid division by zero */
 		if (data_block_quadlets == 0) {
 			dev_info_ratelimited(&s->unit->device,
@@ -731,7 +735,7 @@ static int handle_in_packet(struct amdtp_stream *s,
 	}
 
 	/* Check data block counter continuity */
-	data_block_counter = cip_header[0] & AMDTP_DBC_MASK;
+	data_block_counter = cip_header[0] & CIP_DBC_MASK;
 	if (data_blocks == 0 && (s->flags & CIP_EMPTY_HAS_WRONG_DBC) &&
 	    s->data_block_counter != UINT_MAX)
 		data_block_counter = s->data_block_counter;
@@ -1050,8 +1054,10 @@ EXPORT_SYMBOL(amdtp_stream_pcm_pointer);
  */
 void amdtp_stream_update(struct amdtp_stream *s)
 {
+	/* Precomputing. */
 	ACCESS_ONCE(s->source_node_id_field) =
-		(fw_parent_device(s->unit)->card->node_id & 0x3f) << 24;
+		(fw_parent_device(s->unit)->card->node_id << CIP_SID_SHIFT) &
+								CIP_SID_MASK;
 }
 EXPORT_SYMBOL(amdtp_stream_update);
 
-- 
cgit v1.2.3


From 12e0f438d312fd40815249198ec1e4f3abddacc5 Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Fri, 22 May 2015 23:21:13 +0900
Subject: ALSA: firewire-lib: use dev_err() when detecting incoming streaming
 error

When detecting invalid value in 'dbs' field of CIP header or packet
discontinuity, current implementation reports the status by err_info().
In most cases this state is caused by model-specific issue due to
vendor's customization and should be reported to developers.

This commit use dev_err() instead of dev_info() for this purpose.
In the cases, packet streaming is aborted, thus no message floading
occurs.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Acked-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/firewire/amdtp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

(limited to 'sound/firewire/amdtp.c')

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index 29efbdad572f..93cf93a66aed 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -723,7 +723,7 @@ static int handle_in_packet(struct amdtp_stream *s,
 			(cip_header[0] & CIP_DBS_MASK) >> CIP_DBS_SHIFT;
 		/* avoid division by zero */
 		if (data_block_quadlets == 0) {
-			dev_info_ratelimited(&s->unit->device,
+			dev_err(&s->unit->device,
 				"Detect invalid value in dbs field: %08X\n",
 				cip_header[0]);
 			return -EIO;
@@ -756,9 +756,9 @@ static int handle_in_packet(struct amdtp_stream *s,
 	}
 
 	if (lost) {
-		dev_info(&s->unit->device,
-			 "Detect discontinuity of CIP: %02X %02X\n",
-			 s->data_block_counter, data_block_counter);
+		dev_err(&s->unit->device,
+			"Detect discontinuity of CIP: %02X %02X\n",
+			s->data_block_counter, data_block_counter);
 		return -EIO;
 	}
 
-- 
cgit v1.2.3


From a900705491e6f377966711aa95df753b5ae16dd3 Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Fri, 22 May 2015 23:21:14 +0900
Subject: ALSA: firewire-lib: use protocol error when detecting wrong value in
 CIP header

When detecting zero in 'dbs' field of CIP header, this packet streaming
should be aborted because of avoiding division-by-zero. This is an error
in an aspect of IEC 61883-1, thus protocol error.

This commit use EPROTO instead of EIO. Actually, the returned value is
not used for userspace and this commit has no effect.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Acked-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/firewire/amdtp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'sound/firewire/amdtp.c')

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index 93cf93a66aed..2b3e8b1319f7 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -726,7 +726,7 @@ static int handle_in_packet(struct amdtp_stream *s,
 			dev_err(&s->unit->device,
 				"Detect invalid value in dbs field: %08X\n",
 				cip_header[0]);
-			return -EIO;
+			return -EPROTO;
 		}
 		if (s->flags & CIP_WRONG_DBS)
 			data_block_quadlets = s->data_block_quadlets;
-- 
cgit v1.2.3


From 31ea49baa1aa97f882ee3da8142ec5a9dac509c2 Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Thu, 28 May 2015 00:02:59 +0900
Subject: ALSA: firewire-lib: fix buffer-over-run when detecting packet
 discontinuity

When detecting packet discontinuity, handle_in_packet() returns minus value
and this value is assigned to unsigned int variable, then the variable has
huge value. As a result, the variable causes buffer-over-run in
handle_out_packet(). This brings invalid page request and system hangup.

This commit fixes the bug to add a new argument into handle_in_packet()
and the number of handled data blocks is assignd to it. The function
return value is just used to check error.

I also considered to change the type of local variable to 'int' in
in_stream_callback(). This idea is based on type-conversion in C standard,
while it may cause future problems when adding more works. Thus, I dropped
this idea.

Fixes: 6fc6b9ce41c6('ALSA: firewire-lib: pass the number of data blocks in incoming packets to outgoing packets')
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/firewire/amdtp.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

(limited to 'sound/firewire/amdtp.c')

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index 2b3e8b1319f7..7bb988fa6b6d 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -688,10 +688,10 @@ static int handle_out_packet(struct amdtp_stream *s, unsigned int data_blocks,
 }
 
 static int handle_in_packet(struct amdtp_stream *s,
-			    unsigned int payload_quadlets, __be32 *buffer)
+			    unsigned int payload_quadlets, __be32 *buffer,
+			    unsigned int *data_blocks)
 {
 	u32 cip_header[2];
-	unsigned int data_blocks;
 	unsigned int data_block_quadlets, data_block_counter, dbc_interval;
 	struct snd_pcm_substream *pcm = NULL;
 	bool lost;
@@ -709,7 +709,7 @@ static int 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;
+		*data_blocks = 0;
 		goto end;
 	}
 
@@ -717,7 +717,7 @@ static int handle_in_packet(struct amdtp_stream *s,
 	if (payload_quadlets < 3 ||
 	    ((cip_header[1] & CIP_FDF_MASK) ==
 				(AMDTP_FDF_NO_DATA << CIP_FDF_SHIFT))) {
-		data_blocks = 0;
+		*data_blocks = 0;
 	} else {
 		data_block_quadlets =
 			(cip_header[0] & CIP_DBS_MASK) >> CIP_DBS_SHIFT;
@@ -731,12 +731,12 @@ static int handle_in_packet(struct amdtp_stream *s,
 		if (s->flags & CIP_WRONG_DBS)
 			data_block_quadlets = s->data_block_quadlets;
 
-		data_blocks = (payload_quadlets - 2) / data_block_quadlets;
+		*data_blocks = (payload_quadlets - 2) / data_block_quadlets;
 	}
 
 	/* Check data block counter continuity */
 	data_block_counter = cip_header[0] & CIP_DBC_MASK;
-	if (data_blocks == 0 && (s->flags & CIP_EMPTY_HAS_WRONG_DBC) &&
+	if (*data_blocks == 0 && (s->flags & CIP_EMPTY_HAS_WRONG_DBC) &&
 	    s->data_block_counter != UINT_MAX)
 		data_block_counter = s->data_block_counter;
 
@@ -746,10 +746,10 @@ static int handle_in_packet(struct amdtp_stream *s,
 	} else if (!(s->flags & CIP_DBC_IS_END_EVENT)) {
 		lost = data_block_counter != s->data_block_counter;
 	} else {
-		if ((data_blocks > 0) && (s->tx_dbc_interval > 0))
+		if ((*data_blocks > 0) && (s->tx_dbc_interval > 0))
 			dbc_interval = s->tx_dbc_interval;
 		else
-			dbc_interval = data_blocks;
+			dbc_interval = *data_blocks;
 
 		lost = data_block_counter !=
 		       ((s->data_block_counter + dbc_interval) & 0xff);
@@ -762,30 +762,30 @@ static int handle_in_packet(struct amdtp_stream *s,
 		return -EIO;
 	}
 
-	if (data_blocks > 0) {
+	if (*data_blocks > 0) {
 		buffer += 2;
 
 		pcm = ACCESS_ONCE(s->pcm);
 		if (pcm)
-			s->transfer_samples(s, pcm, buffer, data_blocks);
+			s->transfer_samples(s, pcm, buffer, *data_blocks);
 
 		if (s->midi_ports)
-			read_midi_messages(s, buffer, data_blocks);
+			read_midi_messages(s, buffer, *data_blocks);
 	}
 
 	if (s->flags & CIP_DBC_IS_END_EVENT)
 		s->data_block_counter = data_block_counter;
 	else
 		s->data_block_counter =
-				(data_block_counter + data_blocks) & 0xff;
+				(data_block_counter + *data_blocks) & 0xff;
 end:
 	if (queue_in_packet(s) < 0)
 		return -EIO;
 
 	if (pcm)
-		update_pcm_pointers(s, pcm, data_blocks);
+		update_pcm_pointers(s, pcm, *data_blocks);
 
-	return data_blocks;
+	return 0;
 }
 
 static void out_stream_callback(struct fw_iso_context *context, u32 cycle,
@@ -853,8 +853,8 @@ static void in_stream_callback(struct fw_iso_context *context, u32 cycle,
 			break;
 		}
 
-		data_blocks = handle_in_packet(s, payload_quadlets, buffer);
-		if (data_blocks < 0) {
+		if (handle_in_packet(s, payload_quadlets, buffer,
+							&data_blocks) < 0) {
 			s->packet_index = -1;
 			break;
 		}
-- 
cgit v1.2.3