[PATCH 0/6] firewire: minor fixes and cleanups

[PATCH 0/6] firewire: minor fixes and cleanups

[Stefan Richter]

Following as replies to this message:

[1/6 update] firewire: core: reduce stack usage in bus reset tasklet
[2/6] firewire: ohci: fix Self ID Count register mask (safeguard against buffer overflow)
[3/6] firewire: core: header file cleanup
[4/6] firewire: core: fix race with parallel PCI device probe
[5/6] firewire: reduce some memsets
[6/6] firewire: sbp2: fix status reception

 drivers/firewire/core-card.c     |   32 +++++++++++++++----------------
 drivers/firewire/core-topology.c |    2 -
 drivers/firewire/core.h          |   16 ++++++++++++++-
 drivers/firewire/ohci.c          |   21 +++++++++++++-------
 drivers/firewire/sbp2.c          |   13 ++++++------
 include/linux/firewire.h         |   28 ---------------------------
 6 files changed, 53 insertions(+), 59 deletions(-)
-- 
Stefan Richter
-=====-==--= =--= --==-
http://arcgraph.de/sr/

[PATCH 1/6] firewire: core: reduce stack usage in bus reset tasklet

[Stefan Richter]

fw_compute_block_crc() used 1024 bytes of the kernel stack.  This
function is called
  - in process context when the local node's config ROM is updated,
  - in tasklet context of the bus reset handler.

Slab-allocate the buffer instead.

A drawback is that unlike before, the function may now fail.  This is
very unlikely though and the damage is limited (Config ROM or Topology
Map without CRC).  We don't pass an error code to callers because they
couldn't do anything about it anyway.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---

Update:  kmalloc() and kfree() the buffer on the fly instead of
retaining 1 kB of a static buffer.

 drivers/firewire/core-card.c     |   15 ++++++++++-----
 drivers/firewire/core-topology.c |    2 +-
 drivers/firewire/core.h          |    2 +-
 3 files changed, 12 insertions(+), 7 deletions(-)

Index: linux-2.6.31-rc9/drivers/firewire/core-card.c
===================================================================
--- linux-2.6.31-rc9.orig/drivers/firewire/core-card.c
+++ linux-2.6.31-rc9/drivers/firewire/core-card.c
@@ -38,16 +38,21 @@
 
 #include "core.h"
 
-int fw_compute_block_crc(u32 *block)
+int fw_compute_block_crc(u32 *block, gfp_t flags)
 {
-	__be32 be32_block[256];
-	int i, length;
+	static __be32 *be32_block;
+	int i, length = (*block >> 16) & 0xff;
+
+	be32_block = kmalloc(length * 4, flags);
+	if (WARN_ON(!be32_block))
+		goto out;
 
-	length = (*block >> 16) & 0xff;
 	for (i = 0; i < length; i++)
 		be32_block[i] = cpu_to_be32(block[i + 1]);
 	*block |= crc_itu_t(0, (u8 *) be32_block, length * 4);
 
+	kfree(be32_block);
+ out:
 	return length;
 }
 
@@ -129,7 +134,7 @@ static u32 *generate_config_rom(struct f
 	 * the bus info block, which is always the case for this
 	 * implementation. */
 	for (i = 0; i < j; i += length + 1)
-		length = fw_compute_block_crc(config_rom + i);
+		length = fw_compute_block_crc(config_rom + i, GFP_KERNEL);
 
 	*config_rom_length = j;
 
Index: linux-2.6.31-rc9/drivers/firewire/core-topology.c
===================================================================
--- linux-2.6.31-rc9.orig/drivers/firewire/core-topology.c
+++ linux-2.6.31-rc9/drivers/firewire/core-topology.c
@@ -517,7 +517,7 @@ static void update_topology_map(struct f
 	card->topology_map[2] = (node_count << 16) | self_id_count;
 	card->topology_map[0] = (self_id_count + 2) << 16;
 	memcpy(&card->topology_map[3], self_ids, self_id_count * 4);
-	fw_compute_block_crc(card->topology_map);
+	fw_compute_block_crc(card->topology_map, GFP_ATOMIC);
 }
 
 void fw_core_handle_bus_reset(struct fw_card *card, int node_id, int generation,
Index: linux-2.6.31-rc9/drivers/firewire/core.h
===================================================================
--- linux-2.6.31-rc9.orig/drivers/firewire/core.h
+++ linux-2.6.31-rc9/drivers/firewire/core.h
@@ -93,7 +93,7 @@ int fw_card_add(struct fw_card *card,
 		u32 max_receive, u32 link_speed, u64 guid);
 void fw_core_remove_card(struct fw_card *card);
 int fw_core_initiate_bus_reset(struct fw_card *card, int short_reset);
-int fw_compute_block_crc(u32 *block);
+int fw_compute_block_crc(u32 *block, gfp_t flags);
 void fw_schedule_bm_work(struct fw_card *card, unsigned long delay);
 
 

-- 
Stefan Richter
-=====-==--= =--= --==-
http://arcgraph.de/sr/

Re: [PATCH 1/6] firewire: core: reduce stack usage in bus reset tasklet

[pageexec]

On 6 Sep 2009 at 18:48, Stefan Richter wrote:

> Index: linux-2.6.31-rc9/drivers/firewire/core-card.c
> ===================================================================
> --- linux-2.6.31-rc9.orig/drivers/firewire/core-card.c
> +++ linux-2.6.31-rc9/drivers/firewire/core-card.c
> @@ -38,16 +38,21 @@
>  
>  #include "core.h"
>  
> -int fw_compute_block_crc(u32 *block)
> +int fw_compute_block_crc(u32 *block, gfp_t flags)
>  {
> -	__be32 be32_block[256];
> -	int i, length;
> +	static __be32 *be32_block;
       ^^^^^^

did you actually mean that to be static? if so, then you might as well allocate the
buffer statically and not worry about a runtime allocation failure.

> +	int i, length = (*block >> 16) & 0xff;
> +
> +	be32_block = kmalloc(length * 4, flags);
> +	if (WARN_ON(!be32_block))
> +		goto out;
>  
> -	length = (*block >> 16) & 0xff;
>  	for (i = 0; i < length; i++)
>  		be32_block[i] = cpu_to_be32(block[i + 1]);
>  	*block |= crc_itu_t(0, (u8 *) be32_block, length * 4);
>  
> +	kfree(be32_block);
> + out:
>  	return length;
>  }

Re: [PATCH 1/6] firewire: core: reduce stack usage in bus reset tasklet

[Stefan Richter]

pageexec@freemail.hu wrote:
> On 6 Sep 2009 at 18:48, Stefan Richter wrote:
> 
>> Index: linux-2.6.31-rc9/drivers/firewire/core-card.c
>> ===================================================================
>> --- linux-2.6.31-rc9.orig/drivers/firewire/core-card.c
>> +++ linux-2.6.31-rc9/drivers/firewire/core-card.c
>> @@ -38,16 +38,21 @@
>>  
>>  #include "core.h"
>>  
>> -int fw_compute_block_crc(u32 *block)
>> +int fw_compute_block_crc(u32 *block, gfp_t flags)
>>  {
>> -	__be32 be32_block[256];
>> -	int i, length;
>> +	static __be32 *be32_block;
>        ^^^^^^
> 
> did you actually mean that to be static? if so, then you might as well allocate the
> buffer statically and not worry about a runtime allocation failure.

Uh, that's a cut'n'paste mistake.  It shouldn't be static.  Thanks, I'll 
correct that before I put it into linux1394-2.6.git.

>> +	int i, length = (*block >> 16) & 0xff;
>> +
>> +	be32_block = kmalloc(length * 4, flags);
>> +	if (WARN_ON(!be32_block))
>> +		goto out;
>>  
>> -	length = (*block >> 16) & 0xff;
>>  	for (i = 0; i < length; i++)
>>  		be32_block[i] = cpu_to_be32(block[i + 1]);
>>  	*block |= crc_itu_t(0, (u8 *) be32_block, length * 4);
>>  
>> +	kfree(be32_block);
>> + out:
>>  	return length;
>>  }
> 
> 
> 


-- 
Stefan Richter
-=====-==--= =--= --===
http://arcgraph.de/sr/

Re: [PATCH 1/6] firewire: core: reduce stack usage in bus reset tasklet

[Stefan Richter]

Stefan Richter wrote:
> pageexec@freemail.hu wrote:
>> On 6 Sep 2009 at 18:48, Stefan Richter wrote:
>>
>>> Index: linux-2.6.31-rc9/drivers/firewire/core-card.c
>>> ===================================================================
>>> --- linux-2.6.31-rc9.orig/drivers/firewire/core-card.c
>>> +++ linux-2.6.31-rc9/drivers/firewire/core-card.c
>>> @@ -38,16 +38,21 @@
>>>  
>>>  #include "core.h"
>>>  
>>> -int fw_compute_block_crc(u32 *block)
>>> +int fw_compute_block_crc(u32 *block, gfp_t flags)
>>>  {
>>> -    __be32 be32_block[256];
>>> -    int i, length;
>>> +    static __be32 *be32_block;
>>        ^^^^^^
>>
>> did you actually mean that to be static? if so, then you might as well 
>> allocate the
>> buffer statically and not worry about a runtime allocation failure.
> 
> Uh, that's a cut'n'paste mistake.  It shouldn't be static.  Thanks, I'll 
> correct that before I put it into linux1394-2.6.git.
> 
>>> +    int i, length = (*block >> 16) & 0xff;
>>> +
>>> +    be32_block = kmalloc(length * 4, flags);
>>> +    if (WARN_ON(!be32_block))
>>> +        goto out;
>>>  
>>> -    length = (*block >> 16) & 0xff;
>>>      for (i = 0; i < length; i++)
>>>          be32_block[i] = cpu_to_be32(block[i + 1]);
>>>      *block |= crc_itu_t(0, (u8 *) be32_block, length * 4);
>>>  
>>> +    kfree(be32_block);
>>> + out:
>>>      return length;
>>>  }

This can be optimized further to get rid of the temporary be32_block 
entirely.

There are two users of fw_compute_block_crc:  The functions which 
generate the local Config ROM CSR and then pass it down to the low-level 
driver, and the function which which generates the local Topology Map 
CSR and stores it for core-transaction.c::handle_topology_map() to read 
on demand.

The low-level driver then needs to convert the Config ROM to __be32[]. 
Likewise, handle_topology_map needs to convert the Topology Map to 
__be32[].  They would be both better off if we stored their input as 
__be32[] in the first place, and of course do so before we compute the 
CRC.  Duh.

I'll rewrite patch 1/6 and 5/6 accordingly.
-- 
Stefan Richter
-=====-==--= =--= --===
http://arcgraph.de/sr/

[PATCH 2/6] firewire: ohci: fix Self ID Count register mask (safeguard against buffer overflow)

[Stefan Richter]

The selfIDSize field of Self ID Count is 9 bits wide, and we are only
interested in the high 8 bits.  Fix the mask accordingly.  The
previously too large mask didn't do damage though because the next few
bits in the register are reserved and therefore zero with presently
existing hardware.

Also, check for the maximum possible self ID count of 252 (according to
OHCI 1.1 clause 11.2 and IEEE 1394a-2000 clause 4.3.4.1, i.e. up to four
self IDs of up to 63 nodes, even though IEEE 1394 up to edition 2008
defines only up to three self IDs per node).  More than 252 self IDs
would only happen if the self ID receive DMA unit malfunctioned, which
would likely be caught by other self ID buffer checks.  However, check
it early to be sure.  More than 253 quadlets would overflow the Topology
Map CSR.

Reported-By: PaX Team
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/ohci.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6.31-rc9/drivers/firewire/ohci.c
===================================================================
--- linux-2.6.31-rc9.orig/drivers/firewire/ohci.c
+++ linux-2.6.31-rc9/drivers/firewire/ohci.c
@@ -1279,8 +1279,8 @@ static void bus_reset_tasklet(unsigned l
 	 * the inverted quadlets and a header quadlet, we shift one
 	 * bit extra to get the actual number of self IDs.
 	 */
-	self_id_count = (reg >> 3) & 0x3ff;
-	if (self_id_count == 0) {
+	self_id_count = (reg >> 3) & 0xff;
+	if (self_id_count == 0 || self_id_count > 252) {
 		fw_notify("inconsistent self IDs\n");
 		return;
 	}

-- 
Stefan Richter
-=====-==--= =--= --==-
http://arcgraph.de/sr/

Re: [PATCH 2/6] firewire: ohci: fix Self ID Count register mask (safeguard against buffer overflow)

[pageexec]

On 6 Sep 2009 at 18:49, Stefan Richter wrote:

added stable as .30 is affected, possibly older kernels as well, i didn't check.

> The selfIDSize field of Self ID Count is 9 bits wide, and we are only
> interested in the high 8 bits.  Fix the mask accordingly.  The
> previously too large mask didn't do damage though because the next few
> bits in the register are reserved and therefore zero with presently
> existing hardware.

unless something prevents one from creating a malicious device,
i wouldn't be so sure about all existing hw ;).

> Also, check for the maximum possible self ID count of 252 (according to
> OHCI 1.1 clause 11.2 and IEEE 1394a-2000 clause 4.3.4.1, i.e. up to four
> self IDs of up to 63 nodes, even though IEEE 1394 up to edition 2008
> defines only up to three self IDs per node).  More than 252 self IDs
> would only happen if the self ID receive DMA unit malfunctioned, which
> would likely be caught by other self ID buffer checks.  However, check
> it early to be sure.  More than 253 quadlets would overflow the Topology
> Map CSR.
> 
> Reported-By: PaX Team
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> ---
>  drivers/firewire/ohci.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6.31-rc9/drivers/firewire/ohci.c
> ===================================================================
> --- linux-2.6.31-rc9.orig/drivers/firewire/ohci.c
> +++ linux-2.6.31-rc9/drivers/firewire/ohci.c
> @@ -1279,8 +1279,8 @@ static void bus_reset_tasklet(unsigned l
>  	 * the inverted quadlets and a header quadlet, we shift one
>  	 * bit extra to get the actual number of self IDs.
>  	 */
> -	self_id_count = (reg >> 3) & 0x3ff;
> -	if (self_id_count == 0) {
> +	self_id_count = (reg >> 3) & 0xff;
> +	if (self_id_count == 0 || self_id_count > 252) {
>  		fw_notify("inconsistent self IDs\n");
>  		return;
>  	}
> 
> -- 
> Stefan Richter
> -=====-==--= =--= --==-
> http://arcgraph.de/sr/
> 
> 

Re: [PATCH 2/6] firewire: ohci: fix Self ID Count register mask (safeguard against buffer overflow)

[Stefan Richter]

pageexec@freemail.hu wrote:
> On 6 Sep 2009 at 18:49, Stefan Richter wrote:
> 
> added stable as .30 is affected, possibly older kernels as well, i didn't check.

The mistake which is fixed here is present since firewire-ohci was added 
in 2.6.22.  However, for a smooth submission to stable, the patched file...

>> --- linux-2.6.31-rc9.orig/drivers/firewire/ohci.c
>> +++ linux-2.6.31-rc9/drivers/firewire/ohci.c

...has to be drivers/firewire/fw-ohci.c in kernels before 2.6.31-rc1.

As hinted at in the changelog, I personally don't consider the fix 
urgent because there are a few more self ID consistency checks in place 
which catch a variety of real and theoretical corruptions, but I don't 
claim that I'm appropriately aware of all possibilities.
-- 
Stefan Richter
-=====-==--= =--= --===
http://arcgraph.de/sr/

[PATCH 3/6] firewire: core: header file cleanup

[Stefan Richter]

fw_card_get, fw_card_put, fw_card_release are currently not exported for
use outside the firewire-core.  Move their definitions/ declarations
from the subsystem header file to the core header file.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/core.h  |   14 ++++++++++++++
 include/linux/firewire.h |   14 --------------
 2 files changed, 14 insertions(+), 14 deletions(-)

Index: linux-2.6.31-rc9/drivers/firewire/core.h
===================================================================
--- linux-2.6.31-rc9.orig/drivers/firewire/core.h
+++ linux-2.6.31-rc9/drivers/firewire/core.h
@@ -96,6 +96,20 @@ int fw_core_initiate_bus_reset(struct fw
 int fw_compute_block_crc(u32 *block, gfp_t flags);
 void fw_schedule_bm_work(struct fw_card *card, unsigned long delay);
 
+static inline struct fw_card *fw_card_get(struct fw_card *card)
+{
+	kref_get(&card->kref);
+
+	return card;
+}
+
+void fw_card_release(struct kref *kref);
+
+static inline void fw_card_put(struct fw_card *card)
+{
+	kref_put(&card->kref, fw_card_release);
+}
+
 
 /* -cdev */
 
Index: linux-2.6.31-rc9/include/linux/firewire.h
===================================================================
--- linux-2.6.31-rc9.orig/include/linux/firewire.h
+++ linux-2.6.31-rc9/include/linux/firewire.h
@@ -134,20 +134,6 @@ struct fw_card {
 	u32 topology_map[(CSR_TOPOLOGY_MAP_END - CSR_TOPOLOGY_MAP) / 4];
 };
 
-static inline struct fw_card *fw_card_get(struct fw_card *card)
-{
-	kref_get(&card->kref);
-
-	return card;
-}
-
-void fw_card_release(struct kref *kref);
-
-static inline void fw_card_put(struct fw_card *card)
-{
-	kref_put(&card->kref, fw_card_release);
-}
-
 struct fw_attribute_group {
 	struct attribute_group *groups[2];
 	struct attribute_group group;

-- 
Stefan Richter
-=====-==--= =--= --==-
http://arcgraph.de/sr/

[PATCH 4/6] firewire: core: fix race with parallel PCI device probe

[Stefan Richter]

The config ROM buffer received from generate_config_rom is a globally
shared static buffer.  Extend the card_mutex protection in fw_add_card
until after the config ROM was copied into the card driver's buffer.
Otherwise, parallelized card driver probes may end up with ROM contents
that were meant for a different card.

firewire-ohci's card->driver->enable hook is safe to be called within
the card_mutex.  Furthermore, it is safe to reorder card_list update
versus card enable, which simplifies the code a little.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/core-card.c |   13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

Index: linux-2.6.31-rc9/drivers/firewire/core-card.c
===================================================================
--- linux-2.6.31-rc9.orig/drivers/firewire/core-card.c
+++ linux-2.6.31-rc9/drivers/firewire/core-card.c
@@ -449,16 +449,13 @@ int fw_card_add(struct fw_card *card,
 	card->guid = guid;
 
 	mutex_lock(&card_mutex);
-	config_rom = generate_config_rom(card, &length);
-	list_add_tail(&card->link, &card_list);
-	mutex_unlock(&card_mutex);
 
+	config_rom = generate_config_rom(card, &length);
 	ret = card->driver->enable(card, config_rom, length);
-	if (ret < 0) {
-		mutex_lock(&card_mutex);
-		list_del(&card->link);
-		mutex_unlock(&card_mutex);
-	}
+	if (ret == 0)
+		list_add_tail(&card->link, &card_list);
+
+	mutex_unlock(&card_mutex);
 
 	return ret;
 }

-- 
Stefan Richter
-=====-==--= =--= --==-
http://arcgraph.de/sr/

[PATCH 5/6] firewire: reduce some memsets

[Stefan Richter]

The 1 kBytes big config ROM was cleared to zero twice at each update.
It is only necessary to clear the trailing unused space in the ROM, and
only once.

While we are at it, replace the trivial fw_memcpy_to_be32() helper by
its open-coded equivalent.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/core-card.c |    4 +---
 drivers/firewire/ohci.c      |   17 ++++++++++++-----
 include/linux/firewire.h     |    4 ----
 3 files changed, 13 insertions(+), 12 deletions(-)

Index: linux-2.6.31-rc9/drivers/firewire/core-card.c
===================================================================
--- linux-2.6.31-rc9.orig/drivers/firewire/core-card.c
+++ linux-2.6.31-rc9/drivers/firewire/core-card.c
@@ -80,46 +80,44 @@ static int descriptor_count;
 static u32 *generate_config_rom(struct fw_card *card, size_t *config_rom_length)
 {
 	struct fw_descriptor *desc;
 	static u32 config_rom[256];
 	int i, j, length;
 
 	/*
 	 * Initialize contents of config rom buffer.  On the OHCI
 	 * controller, block reads to the config rom accesses the host
 	 * memory, but quadlet read access the hardware bus info block
 	 * registers.  That's just crack, but it means we should make
 	 * sure the contents of bus info block in host memory matches
 	 * the version stored in the OHCI registers.
 	 */
 
-	memset(config_rom, 0, sizeof(config_rom));
 	config_rom[0] = BIB_CRC_LENGTH(4) | BIB_INFO_LENGTH(4) | BIB_CRC(0);
 	config_rom[1] = 0x31333934;
 
 	config_rom[2] =
 		BIB_LINK_SPEED(card->link_speed) |
 		BIB_GENERATION(card->config_rom_generation++ % 14 + 2) |
 		BIB_MAX_ROM(2) |
 		BIB_MAX_RECEIVE(card->max_receive) |
 		BIB_BMC | BIB_ISC | BIB_CMC | BIB_IMC;
 	config_rom[3] = card->guid >> 32;
 	config_rom[4] = card->guid;
 
 	/* Generate root directory. */
-	i = 5;
-	config_rom[i++] = 0;
+	i = 6;
 	config_rom[i++] = 0x0c0083c0; /* node capabilities */
 	j = i + descriptor_count;
 
 	/* Generate root directory entries for descriptors. */
 	list_for_each_entry (desc, &descriptor_list, link) {
 		if (desc->immediate > 0)
 			config_rom[i++] = desc->immediate;
 		config_rom[i] = desc->key | (j - i);
 		i++;
 		j += desc->length;
 	}
 
 	/* Update root directory length. */
 	config_rom[5] = (i - 5 - 1) << 16;
 
Index: linux-2.6.31-rc9/drivers/firewire/ohci.c
===================================================================
--- linux-2.6.31-rc9.orig/drivers/firewire/ohci.c
+++ linux-2.6.31-rc9/drivers/firewire/ohci.c
@@ -1464,6 +1464,16 @@ static int software_reset(struct fw_ohci
 	return -EBUSY;
 }
 
+static void copy_config_rom(__be32 *dest, const u32 *src, size_t length)
+{
+	int i;
+
+	for (i = 0; i < length; i++)
+		dest[i] = cpu_to_be32(src[i]);
+
+	memset(&dest[length], 0, CONFIG_ROM_SIZE - length * 4);
+}
+
 static int ohci_enable(struct fw_card *card, u32 *config_rom, size_t length)
 {
 	struct fw_ohci *ohci = fw_ohci(card);
@@ -1565,8 +1575,7 @@ static int ohci_enable(struct fw_card *c
 		if (ohci->next_config_rom == NULL)
 			return -ENOMEM;
 
-		memset(ohci->next_config_rom, 0, CONFIG_ROM_SIZE);
-		fw_memcpy_to_be32(ohci->next_config_rom, config_rom, length * 4);
+		copy_config_rom(ohci->next_config_rom, config_rom, length);
 	} else {
 		/*
 		 * In the suspend case, config_rom is NULL, which
@@ -1659,9 +1668,7 @@ static int ohci_set_config_rom(struct fw
 		ohci->next_config_rom = next_config_rom;
 		ohci->next_config_rom_bus = next_config_rom_bus;
 
-		memset(ohci->next_config_rom, 0, CONFIG_ROM_SIZE);
-		fw_memcpy_to_be32(ohci->next_config_rom, config_rom,
-				  length * 4);
+		copy_config_rom(ohci->next_config_rom, config_rom, length);
 
 		ohci->next_header = config_rom[0];
 		ohci->next_config_rom[0] = 0;
Index: linux-2.6.31-rc9/include/linux/firewire.h
===================================================================
--- linux-2.6.31-rc9.orig/include/linux/firewire.h
+++ linux-2.6.31-rc9/include/linux/firewire.h
@@ -30,10 +30,6 @@ static inline void fw_memcpy_from_be32(v
 		dst[i] = be32_to_cpu(src[i]);
 }
 
-static inline void fw_memcpy_to_be32(void *_dst, void *_src, size_t size)
-{
-	fw_memcpy_from_be32(_dst, _src, size);
-}
 #define CSR_REGISTER_BASE		0xfffff0000000ULL
 
 /* register offsets are relative to CSR_REGISTER_BASE */

-- 
Stefan Richter
-=====-==--= =--= --==-
http://arcgraph.de/sr/

[PATCH 6/6] firewire: sbp2: fix status reception

[Stefan Richter]

Per SBP-2 clause 5.3, a target shall store 8...32 bytes of status
information.  Trailing zeros after the first 8 bytes don't need to be
stored, they are implicit.  Fix the status write handler to clear all
unwritten status data.

While we are at it, remove the trivial fw_memcpy_from_be32() helper.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/sbp2.c  |   13 +++++++------
 include/linux/firewire.h |   10 ----------
 2 files changed, 7 insertions(+), 16 deletions(-)

Index: linux-2.6.31-rc9/drivers/firewire/sbp2.c
===================================================================
--- linux-2.6.31-rc9.orig/drivers/firewire/sbp2.c
+++ linux-2.6.31-rc9/drivers/firewire/sbp2.c
@@ -425,19 +425,20 @@ static void sbp2_status_write(struct fw_
 	struct sbp2_logical_unit *lu = callback_data;
 	struct sbp2_orb *orb;
 	struct sbp2_status status;
-	size_t header_size;
 	unsigned long flags;
 
 	if (tcode != TCODE_WRITE_BLOCK_REQUEST ||
-	    length == 0 || length > sizeof(status)) {
+	    length < 8 || length > sizeof(status)) {
 		fw_send_response(card, request, RCODE_TYPE_ERROR);
 		return;
 	}
 
-	header_size = min(length, 2 * sizeof(u32));
-	fw_memcpy_from_be32(&status, payload, header_size);
-	if (length > header_size)
-		memcpy(status.data, payload + 8, length - header_size);
+	status.status  = be32_to_cpup(payload);
+	status.orb_low = be32_to_cpup(payload + 4);
+	memset(status.data, 0, sizeof(status.data));
+	if (length > 8)
+		memcpy(status.data, payload + 8, length - 8);
+
 	if (STATUS_GET_SOURCE(status) == 2 || STATUS_GET_SOURCE(status) == 3) {
 		fw_notify("non-orb related status write, not handled\n");
 		fw_send_response(card, request, RCODE_COMPLETE);
Index: linux-2.6.31-rc9/include/linux/firewire.h
===================================================================
--- linux-2.6.31-rc9.orig/include/linux/firewire.h
+++ linux-2.6.31-rc9/include/linux/firewire.h
@@ -20,16 +20,6 @@
 #define fw_notify(s, args...) printk(KERN_NOTICE KBUILD_MODNAME ": " s, ## args)
 #define fw_error(s, args...) printk(KERN_ERR KBUILD_MODNAME ": " s, ## args)
 
-static inline void fw_memcpy_from_be32(void *_dst, void *_src, size_t size)
-{
-	u32    *dst = _dst;
-	__be32 *src = _src;
-	int i;
-
-	for (i = 0; i < size / 4; i++)
-		dst[i] = be32_to_cpu(src[i]);
-}
-
 #define CSR_REGISTER_BASE		0xfffff0000000ULL
 
 /* register offsets are relative to CSR_REGISTER_BASE */

-- 
Stefan Richter
-=====-==--= =--= --==-
http://arcgraph.de/sr/