2010-04-15 16:00:50

by Stefan Richter

[permalink] [raw]
Subject: [git pull] FireWire fixes and documentation update

Linus, please pull from the for-linus branch at

git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus

to receive the following IEEE 1394/ FireWire subsystem update.
Thanks.

Clemens Ladisch (3):
firewire: cdev: disallow receive packets without header
firewire: cdev: require quadlet-aligned headers for transmit packets
firewire: cdev: iso packet documentation

Stefan Richter (3):
firewire: cdev: fix information leak
firewire: cdev: comment fixlet
firewire: cdev: change license of exported header files to MIT license

drivers/firewire/core-cdev.c | 23 ++++++-----
include/linux/firewire-cdev.h | 78 +++++++++++++++++++++++++-----------
include/linux/firewire-constants.h | 29 ++++++++++++-
3 files changed, 95 insertions(+), 35 deletions(-)

Since several of these patches were not copied to linux-kernel yet, I
will send the full log and diff in a reply to this message.
--
Stefan Richter
-=====-==-=- -=-- -====
http://arcgraph.de/sr/


2010-04-15 16:03:25

by Stefan Richter

[permalink] [raw]
Subject: Re: [git pull] FireWire fixes and documentation update

On 15 Apr, Stefan Richter wrote:
> Linus, please pull from the for-linus branch at
>
> git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus
>
> to receive the following IEEE 1394/ FireWire subsystem update.
> Thanks.
>
> Clemens Ladisch (3):
> firewire: cdev: disallow receive packets without header
> firewire: cdev: require quadlet-aligned headers for transmit packets
> firewire: cdev: iso packet documentation
>
> Stefan Richter (3):
> firewire: cdev: fix information leak
> firewire: cdev: comment fixlet
> firewire: cdev: change license of exported header files to MIT license
>
> drivers/firewire/core-cdev.c | 23 ++++++-----
> include/linux/firewire-cdev.h | 78 +++++++++++++++++++++++++-----------
> include/linux/firewire-constants.h | 29 ++++++++++++-
> 3 files changed, 95 insertions(+), 35 deletions(-)
>
> Since several of these patches were not copied to linux-kernel yet, I
> will send the full log and diff in a reply to this message.

commit 19b3eecc21b65a24b0aae2684ca0c8e1b99ef802
Author: Stefan Richter <[email protected]>
Date: Sun Apr 11 11:52:12 2010 +0200

firewire: cdev: change license of exported header files to MIT license

Among else, this allows projects like libdc1394 to carry copies of the
ABI related header files without them or distributors having to worry
about effects on the project's overall license terms. Switch to MIT
license as suggested by Kristian. Also update the year in the
copyright statement according to source history.

Cc: Jay Fenlason <[email protected]>
Acked-by: Clemens Ladisch <[email protected]>
Signed-off-by: Stefan Richter <[email protected]>
Signed-off-by: Kristian H?gsberg <[email protected]>
---
include/linux/firewire-cdev.h | 29 +++++++++++++++++------------
include/linux/firewire-constants.h | 29 +++++++++++++++++++++++++++--
2 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/include/linux/firewire-cdev.h b/include/linux/firewire-cdev.h
index 6ffb24a..81f3b14 100644
--- a/include/linux/firewire-cdev.h
+++ b/include/linux/firewire-cdev.h
@@ -1,21 +1,26 @@
/*
* Char device interface.
*
- * Copyright (C) 2005-2006 Kristian Hoegsberg <[email protected]>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software Foundation,
- * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ * Copyright (C) 2005-2007 Kristian Hoegsberg <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
*/

#ifndef _LINUX_FIREWIRE_CDEV_H
diff --git a/include/linux/firewire-constants.h b/include/linux/firewire-constants.h
index b316770..9c63f06 100644
--- a/include/linux/firewire-constants.h
+++ b/include/linux/firewire-constants.h
@@ -1,3 +1,28 @@
+/*
+ * IEEE 1394 constants.
+ *
+ * Copyright (C) 2005-2007 Kristian Hoegsberg <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
#ifndef _LINUX_FIREWIRE_CONSTANTS_H
#define _LINUX_FIREWIRE_CONSTANTS_H

@@ -21,7 +46,7 @@
#define EXTCODE_WRAP_ADD 0x6
#define EXTCODE_VENDOR_DEPENDENT 0x7

-/* Juju specific tcodes */
+/* Linux firewire-core (Juju) specific tcodes */
#define TCODE_LOCK_MASK_SWAP (0x10 | EXTCODE_MASK_SWAP)
#define TCODE_LOCK_COMPARE_SWAP (0x10 | EXTCODE_COMPARE_SWAP)
#define TCODE_LOCK_FETCH_ADD (0x10 | EXTCODE_FETCH_ADD)
@@ -36,7 +61,7 @@
#define RCODE_TYPE_ERROR 0x6
#define RCODE_ADDRESS_ERROR 0x7

-/* Juju specific rcodes */
+/* Linux firewire-core (Juju) specific rcodes */
#define RCODE_SEND_ERROR 0x10
#define RCODE_CANCELLED 0x11
#define RCODE_BUSY 0x12

commit ca658b1e29d6be939207532e337fb640eb697f71
Author: Stefan Richter <[email protected]>
Date: Sat Apr 10 12:23:09 2010 +0200

firewire: cdev: comment fixlet

Signed-off-by: Stefan Richter <[email protected]>
---
include/linux/firewire-cdev.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/firewire-cdev.h b/include/linux/firewire-cdev.h
index 011fdf1..6ffb24a 100644
--- a/include/linux/firewire-cdev.h
+++ b/include/linux/firewire-cdev.h
@@ -647,8 +647,8 @@ struct fw_cdev_get_cycle_timer2 {
* instead of allocated.
* An %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED event concludes this operation.
*
- * To summarize, %FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE allocates iso resources
- * for the lifetime of the fd or handle.
+ * To summarize, %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE allocates iso resources
+ * for the lifetime of the fd or @handle.
* In contrast, %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE allocates iso resources
* for the duration of a bus generation.
*

commit aa6fec3cdeb14ecc916eb78c4cd9ed79e4f7fe8d
Author: Clemens Ladisch <[email protected]>
Date: Wed Mar 31 16:26:52 2010 +0200

firewire: cdev: iso packet documentation

Add the missing documentation for iso packets.

Signed-off-by: Clemens Ladisch <[email protected]>
Signed-off-by: Stefan Richter <[email protected]>
---
include/linux/firewire-cdev.h | 39 +++++++++++++++++++++++++++++++++------
1 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/include/linux/firewire-cdev.h b/include/linux/firewire-cdev.h
index 40b1101..011fdf1 100644
--- a/include/linux/firewire-cdev.h
+++ b/include/linux/firewire-cdev.h
@@ -438,7 +438,7 @@ struct fw_cdev_remove_descriptor {
* @type: %FW_CDEV_ISO_CONTEXT_TRANSMIT or %FW_CDEV_ISO_CONTEXT_RECEIVE
* @header_size: Header size to strip for receive contexts
* @channel: Channel to bind to
- * @speed: Speed to transmit at
+ * @speed: Speed for transmit contexts
* @closure: To be returned in &fw_cdev_event_iso_interrupt
* @handle: Handle to context, written back by kernel
*
@@ -451,6 +451,9 @@ struct fw_cdev_remove_descriptor {
* If a context was successfully created, the kernel writes back a handle to the
* context, which must be passed in for subsequent operations on that context.
*
+ * For receive contexts, @header_size must be at least 4 and must be a multiple
+ * of 4.
+ *
* Note that the effect of a @header_size > 4 depends on
* &fw_cdev_get_info.version, as documented at &fw_cdev_event_iso_interrupt.
*/
@@ -481,10 +484,34 @@ struct fw_cdev_create_iso_context {
*
* &struct fw_cdev_iso_packet is used to describe isochronous packet queues.
*
- * Use the FW_CDEV_ISO_ macros to fill in @control. The sy and tag fields are
- * specified by IEEE 1394a and IEC 61883.
- *
- * FIXME - finish this documentation
+ * Use the FW_CDEV_ISO_ macros to fill in @control.
+ *
+ * For transmit packets, the header length must be a multiple of 4 and specifies
+ * the numbers of bytes in @header that will be prepended to the packet's
+ * payload; these bytes are copied into the kernel and will not be accessed
+ * after the ioctl has returned. The sy and tag fields are copied to the iso
+ * packet header (these fields are specified by IEEE 1394a and IEC 61883-1).
+ * The skip flag specifies that no packet is to be sent in a frame; when using
+ * this, all other fields except the interrupt flag must be zero.
+ *
+ * For receive packets, the header length must be a multiple of the context's
+ * header size; if the header length is larger than the context's header size,
+ * multiple packets are queued for this entry. The sy and tag fields are
+ * ignored. If the sync flag is set, the context drops all packets until
+ * a packet with a matching sy field is received (the sync value to wait for is
+ * specified in the &fw_cdev_start_iso structure). The payload length defines
+ * how many payload bytes can be received for one packet (in addition to payload
+ * quadlets that have been defined as headers and are stripped and returned in
+ * the &fw_cdev_event_iso_interrupt structure). If more bytes are received, the
+ * additional bytes are dropped. If less bytes are received, the remaining
+ * bytes in this part of the payload buffer will not be written to, not even by
+ * the next packet, i.e., packets received in consecutive frames will not
+ * necessarily be consecutive in memory. If an entry has queued multiple
+ * packets, the payload length is divided equally among them.
+ *
+ * When a packet with the interrupt flag set has been completed, the
+ * &fw_cdev_event_iso_interrupt event will be sent. An entry that has queued
+ * multiple receive packets is completed when its last packet is completed.
*/
struct fw_cdev_iso_packet {
__u32 control;
@@ -501,7 +528,7 @@ struct fw_cdev_iso_packet {
* Queue a number of isochronous packets for reception or transmission.
* This ioctl takes a pointer to an array of &fw_cdev_iso_packet structs,
* which describe how to transmit from or receive into a contiguous region
- * of a mmap()'ed payload buffer. As part of the packet descriptors,
+ * of a mmap()'ed payload buffer. As part of transmit packet descriptors,
* a series of headers can be supplied, which will be prepended to the
* payload during DMA.
*

commit 9cac00b8f0079d5d3d54ec4dae453d58dec30e7c
Author: Stefan Richter <[email protected]>
Date: Wed Apr 7 08:30:50 2010 +0200

firewire: cdev: fix information leak

A userspace client got to see uninitialized stack-allocated memory if it
specified an _IOC_READ type of ioctl and an argument size larger than
expected by firewire-core's ioctl handlers (but not larger than the
core's union ioctl_arg).

Fix this by clearing the requested buffer size to zero, but only at _IOR
ioctls. This way, there is almost no runtime penalty to legitimate
ioctls. The only legitimate _IOR is FW_CDEV_IOC_GET_CYCLE_TIMER with 12
or 16 bytes to memset.

[Another way to fix this would be strict checking of argument size (and
possibly direction) vs. command number. However, we then need a lookup
table, and we need to allow for slight size deviations in case of 32bit
userland on 64bit kernel.]

Reported-by: Clemens Ladisch <[email protected]>
Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/core-cdev.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 5eba9e0..0d3df09 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1356,24 +1356,24 @@ static int dispatch_ioctl(struct client *client,
return -ENODEV;

if (_IOC_TYPE(cmd) != '#' ||
- _IOC_NR(cmd) >= ARRAY_SIZE(ioctl_handlers))
+ _IOC_NR(cmd) >= ARRAY_SIZE(ioctl_handlers) ||
+ _IOC_SIZE(cmd) > sizeof(buffer))
return -EINVAL;

- if (_IOC_DIR(cmd) & _IOC_WRITE) {
- if (_IOC_SIZE(cmd) > sizeof(buffer) ||
- copy_from_user(&buffer, arg, _IOC_SIZE(cmd)))
+ if (_IOC_DIR(cmd) == _IOC_READ)
+ memset(&buffer, 0, _IOC_SIZE(cmd));
+
+ if (_IOC_DIR(cmd) & _IOC_WRITE)
+ if (copy_from_user(&buffer, arg, _IOC_SIZE(cmd)))
return -EFAULT;
- }

ret = ioctl_handlers[_IOC_NR(cmd)](client, &buffer);
if (ret < 0)
return ret;

- if (_IOC_DIR(cmd) & _IOC_READ) {
- if (_IOC_SIZE(cmd) > sizeof(buffer) ||
- copy_to_user(arg, &buffer, _IOC_SIZE(cmd)))
+ if (_IOC_DIR(cmd) & _IOC_READ)
+ if (copy_to_user(arg, &buffer, _IOC_SIZE(cmd)))
return -EFAULT;
- }

return ret;
}

commit 385ab5bcd4be586dffdba550b310308d89eade71
Author: Clemens Ladisch <[email protected]>
Date: Wed Mar 31 16:26:46 2010 +0200

firewire: cdev: require quadlet-aligned headers for transmit packets

The definition of struct fw_cdev_iso_packet seems to imply that the
header_length must be quadlet-aligned, and in fact, specifying an
unaligned header has never really worked when using multiple packet
structures, because the position of the next control word is computed by
rounding the header_length _down_, so the last one to three bytes of the
header would overlap the next control word.

To avoid this problem, check that the header length is properly aligned.

Signed-off-by: Clemens Ladisch <[email protected]>
Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/core-cdev.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index bbb8160..5eba9e0 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -959,6 +959,8 @@ static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg)
u.packet.header_length = GET_HEADER_LENGTH(control);

if (ctx->type == FW_ISO_CONTEXT_TRANSMIT) {
+ if (u.packet.header_length % 4 != 0)
+ return -EINVAL;
header_length = u.packet.header_length;
} else {
/*

commit 4ba1d9c0c22947a9207029e7184733252e6135f1
Author: Clemens Ladisch <[email protected]>
Date: Wed Mar 31 16:26:39 2010 +0200

firewire: cdev: disallow receive packets without header

In receive contexts, reject packets with header_length==0. This would
be an instruction to queue zero packets which would not make sense.

This prevents a division by zero in the OHCI driver.

Signed-off-by: Clemens Ladisch <[email protected]>
Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/core-cdev.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 8be720b..bbb8160 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -968,7 +968,8 @@ static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg)
if (ctx->header_size == 0) {
if (u.packet.header_length > 0)
return -EINVAL;
- } else if (u.packet.header_length % ctx->header_size != 0) {
+ } else if (u.packet.header_length == 0 ||
+ u.packet.header_length % ctx->header_size != 0) {
return -EINVAL;
}
header_length = 0;

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

2010-04-15 19:43:41

by Daniel K.

[permalink] [raw]
Subject: Re: [git pull] FireWire fixes and documentation update

Stefan Richter wrote:
> commit 19b3eecc21b65a24b0aae2684ca0c8e1b99ef802
> Author: Stefan Richter <[email protected]>
> Date: Sun Apr 11 11:52:12 2010 +0200
>
> firewire: cdev: change license of exported header files to MIT license
>
> [...]
>
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR

Where exactly does PRECISION INSIGHT AND/OR ITS SUPPLIERS enter the picture?
Are any of you employed there?

The standard license text is usually [1]:

IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM

> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.

[1] http://www.opensource.org/licenses/mit-license.php


Daniel K.

2010-04-15 20:30:32

by Stefan Richter

[permalink] [raw]
Subject: Re: [git pull] FireWire fixes and documentation update

On 15 Apr, Daniel K. wrote:
> Stefan Richter wrote:
>> firewire: cdev: change license of exported header files to MIT license
>> [...]
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>
> Where exactly does PRECISION INSIGHT AND/OR ITS SUPPLIERS enter the picture?
> Are any of you employed there?

No, this was a copy & waste error.
Thanks, fixed.

Linus, I updated linux1394-2.6.git's for-linus branch for you to pull:

git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus

Clemens Ladisch (3):
firewire: cdev: disallow receive packets without header
firewire: cdev: require quadlet-aligned headers for transmit packets
firewire: cdev: iso packet documentation

Stefan Richter (4):
firewire: cdev: fix information leak
firewire: cdev: comment fixlet
firewire: cdev: change license of exported header files to MIT license
firewire: cdev: fix cut+paste mistake in disclaimer

drivers/firewire/core-cdev.c | 23 ++++++-----
include/linux/firewire-cdev.h | 78 +++++++++++++++++++++++++-----------
include/linux/firewire-constants.h | 29 ++++++++++++-
3 files changed, 95 insertions(+), 35 deletions(-)


Added:

>From a2612cb16d4d8447793609cbdd2a2f4f156c0020 Mon Sep 17 00:00:00 2001
From: Stefan Richter <[email protected]>
Date: Thu, 15 Apr 2010 22:16:04 +0200
Subject: [PATCH] firewire: cdev: fix cut+paste mistake in disclaimer

This was supposed to be generic "authors or copyright holders";
I mistakenly picked up text from a wrong file.

Reported-by: Daniel K. <[email protected]>
Signed-off-by: Stefan Richter <[email protected]>
---
include/linux/firewire-cdev.h | 2 +-
include/linux/firewire-constants.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/firewire-cdev.h b/include/linux/firewire-cdev.h
index 81f3b14..68f883b 100644
--- a/include/linux/firewire-cdev.h
+++ b/include/linux/firewire-cdev.h
@@ -17,7 +17,7 @@
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
* OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
* ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
diff --git a/include/linux/firewire-constants.h b/include/linux/firewire-constants.h
index 9c63f06..9b4bb5f 100644
--- a/include/linux/firewire-constants.h
+++ b/include/linux/firewire-constants.h
@@ -17,7 +17,7 @@
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
* OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
* ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
--
Stefan Richter
-=====-==-=- -=-- -====
http://arcgraph.de/sr/

2010-04-22 19:46:15

by Stefan Richter

[permalink] [raw]
Subject: [git pull] FireWire fixes

Linus, please pull from the for-linus branch at

git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus

to receive some more IEEE 1394/ FireWire subsystem fixes.
Thanks.

Clemens Ladisch (4):
firewire: core: fix retries calculation in iso manage_channel()
firewire: core: fw_iso_resource_manage: return -EBUSY when out of resources
firewire: ohci: prevent aliasing of locally handled register addresses
firewire: ohci: wait for local CSR lock access to finish

Stefan Richter (1):
firewire: cdev: fix cut+paste mistake in disclaimer

drivers/firewire/core-iso.c | 14 ++++++++++----
drivers/firewire/ohci.c | 23 ++++++++++++++---------
include/linux/firewire-cdev.h | 2 +-
include/linux/firewire-constants.h | 2 +-
4 files changed, 26 insertions(+), 15 deletions(-)


Full log and diff:

commit e1393667be574807a13bfaf1bb471f5fd1a5287b
Author: Clemens Ladisch <[email protected]>
Date: Mon Apr 12 10:35:44 2010 +0200

firewire: ohci: wait for local CSR lock access to finish

Add a loop to wait for the controller to finish a locally-initiated CSR
lock operation. Google shows some occurrences of the "swap not done
yet" message which might indicate that some OHCI controllers are not
fast enough to do the lock/swap in the time needed for one PCI access.

This also correctly handles the case where the lock operation did not
finish, instead of silently returning an uninitialized value.

Signed-off-by: Clemens Ladisch <[email protected]>
Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/ohci.c | 20 +++++++++++++-------
1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 82fb2e7..6e95f8f 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -1158,7 +1158,7 @@ static void handle_local_lock(struct fw_ohci *ohci,
struct fw_packet *packet, u32 csr)
{
struct fw_packet response;
- int tcode, length, ext_tcode, sel;
+ int tcode, length, ext_tcode, sel, try;
__be32 *payload, lock_old;
u32 lock_arg, lock_data;

@@ -1185,13 +1185,19 @@ static void handle_local_lock(struct fw_ohci *ohci,
reg_write(ohci, OHCI1394_CSRCompareData, lock_arg);
reg_write(ohci, OHCI1394_CSRControl, sel);

- if (reg_read(ohci, OHCI1394_CSRControl) & 0x80000000)
- lock_old = cpu_to_be32(reg_read(ohci, OHCI1394_CSRData));
- else
- fw_notify("swap not done yet\n");
+ for (try = 0; try < 20; try++)
+ if (reg_read(ohci, OHCI1394_CSRControl) & 0x80000000) {
+ lock_old = cpu_to_be32(reg_read(ohci,
+ OHCI1394_CSRData));
+ fw_fill_response(&response, packet->header,
+ RCODE_COMPLETE,
+ &lock_old, sizeof(lock_old));
+ goto out;
+ }
+
+ fw_error("swap not done (CSR lock timeout)\n");
+ fw_fill_response(&response, packet->header, RCODE_BUSY, NULL, 0);

- fw_fill_response(&response, packet->header,
- RCODE_COMPLETE, &lock_old, sizeof(lock_old));
out:
fw_core_handle_response(&ohci->card, &response);
}

commit 2608203daf5f87311c6e5d36e5de5efcb14aab24
Author: Clemens Ladisch <[email protected]>
Date: Mon Apr 12 10:35:30 2010 +0200

firewire: ohci: prevent aliasing of locally handled register addresses

We must compute the offset from the CSR register base with the
full 48 address bits to prevent matching with addresses whose
lower 32 bits happen to be equal with one of the specially
handled registers.

Signed-off-by: Clemens Ladisch <[email protected]>
Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/ohci.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index e33917b..82fb2e7 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -1198,8 +1198,7 @@ static void handle_local_lock(struct fw_ohci *ohci,

static void handle_local_request(struct context *ctx, struct fw_packet *packet)
{
- u64 offset;
- u32 csr;
+ u64 offset, csr;

if (ctx == &ctx->ohci->at_request_ctx) {
packet->ack = ACK_PENDING;

commit d6372b6e7c6142e6cc2108b3b850584cd7ade106
Author: Clemens Ladisch <[email protected]>
Date: Mon Apr 12 10:35:18 2010 +0200

firewire: core: fw_iso_resource_manage: return -EBUSY when out of resources

Returning -EIO for all errors would not allow clients to determine if
the resource allocation process itself failed, or if the resources are
not available. (The latter information is needed by CMP to synchronize
restoring of overlayed connections after a bus reset.)

Signed-off-by: Clemens Ladisch <[email protected]>
Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/core-iso.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
index 34a5137..9198e03 100644
--- a/drivers/firewire/core-iso.c
+++ b/drivers/firewire/core-iso.c
@@ -189,7 +189,7 @@ static int manage_bandwidth(struct fw_card *card, int irm_id, int generation,
for (try = 0; try < 5; try++) {
new = allocate ? old - bandwidth : old + bandwidth;
if (new < 0 || new > BANDWIDTH_AVAILABLE_INITIAL)
- break;
+ return -EBUSY;

data[0] = cpu_to_be32(old);
data[1] = cpu_to_be32(new);
@@ -217,7 +217,7 @@ static int manage_channel(struct fw_card *card, int irm_id, int generation,
u32 channels_mask, u64 offset, bool allocate, __be32 data[2])
{
__be32 c, all, old;
- int i, retry = 5;
+ int i, ret = -EIO, retry = 5;

old = all = allocate ? cpu_to_be32(~0) : 0;

@@ -225,6 +225,8 @@ static int manage_channel(struct fw_card *card, int irm_id, int generation,
if (!(channels_mask & 1 << i))
continue;

+ ret = -EBUSY;
+
c = cpu_to_be32(1 << (31 - i));
if ((old & c) != (all & c))
continue;
@@ -253,11 +255,13 @@ static int manage_channel(struct fw_card *card, int irm_id, int generation,
if (retry) {
retry--;
i--;
+ } else {
+ ret = -EIO;
}
}
}

- return -EIO;
+ return ret;
}

static void deallocate_channel(struct fw_card *card, int irm_id,

commit 3a1f0a0e3d871e3d3e08a1429009992151becda8
Author: Clemens Ladisch <[email protected]>
Date: Mon Apr 12 10:35:05 2010 +0200

firewire: core: fix retries calculation in iso manage_channel()

If there is a permanent error condition when communicating with the IRM,
after the sixth error, the retry variable will be decremented to -1.
If, in this case, the bits in channels_mask are not yet exhausted, the
next channel is retried 2^32 times.

To fix this, check that retry is never decremented beyond zero.

Signed-off-by: Clemens Ladisch <[email protected]>
Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/core-iso.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
index 99c20f1..34a5137 100644
--- a/drivers/firewire/core-iso.c
+++ b/drivers/firewire/core-iso.c
@@ -250,8 +250,10 @@ static int manage_channel(struct fw_card *card, int irm_id, int generation,

/* 1394-1995 IRM, fall through to retry. */
default:
- if (retry--)
+ if (retry) {
+ retry--;
i--;
+ }
}
}


commit a2612cb16d4d8447793609cbdd2a2f4f156c0020
Author: Stefan Richter <[email protected]>
Date: Thu Apr 15 22:16:04 2010 +0200

firewire: cdev: fix cut+paste mistake in disclaimer

This was supposed to be generic "authors or copyright holders";
I mistakenly picked up text from a wrong file.

Reported-by: Daniel K. <[email protected]>
Signed-off-by: Stefan Richter <[email protected]>
---
include/linux/firewire-cdev.h | 2 +-
include/linux/firewire-constants.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/firewire-cdev.h b/include/linux/firewire-cdev.h
index 81f3b14..68f883b 100644
--- a/include/linux/firewire-cdev.h
+++ b/include/linux/firewire-cdev.h
@@ -17,7 +17,7 @@
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
* OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
* ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
diff --git a/include/linux/firewire-constants.h b/include/linux/firewire-constants.h
index 9c63f06..9b4bb5f 100644
--- a/include/linux/firewire-constants.h
+++ b/include/linux/firewire-constants.h
@@ -17,7 +17,7 @@
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
* OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
* ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.

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