* [git pull] FireWire updates post 2.6.32
@ 2009-12-06 17:56 Stefan Richter
2009-12-11 20:59 ` [git pull] FireWire fix Stefan Richter
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Richter @ 2009-12-06 17:56 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel
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 minor IEEE 1394/ FireWire subsystem update.
Akinobu Mita (1):
ieee1394: Use hweight32
Stefan Richter (9):
firewire: cdev: fix memory leak in an error path
firewire: normalize style of queue_work wrappers
firewire: cdev: normalize variable names
firewire: optimize config ROM creation
firewire: core: clarify generate_config_rom usage
firewire: core: optimize Topology Map creation
firewire: core: WARN on wrong usage of core transaction functions
firewire: ohci: 0 may be a valid DMA address
firewire: cdev: reduce stack usage by ioctl_dispatch
drivers/firewire/core-card.c | 75 ++++++++----------
drivers/firewire/core-cdev.c | 113 ++++++++++++++++-----------
drivers/firewire/core-topology.c | 17 +++--
drivers/firewire/core-transaction.c | 19 ++---
drivers/firewire/core.h | 9 +-
drivers/firewire/ohci.c | 39 ++++++----
drivers/firewire/sbp2.c | 9 ++-
drivers/ieee1394/ohci1394.c | 8 +--
include/linux/firewire.h | 17 +----
9 files changed, 159 insertions(+), 147 deletions(-)
--
Stefan Richter
-=====-==--= ==-- --==-
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 5+ messages in thread
* [git pull] FireWire fix
2009-12-06 17:56 [git pull] FireWire updates post 2.6.32 Stefan Richter
@ 2009-12-11 20:59 ` Stefan Richter
2009-12-31 18:55 ` [git pull] FireWire fixes; new firewire stack is recommended to distributors and users Stefan Richter
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Richter @ 2009-12-11 20:59 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel
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 a FireWire subsystem fix.
Jay Fenlason (1):
firewire: ohci: handle receive packets with a data length of zero
drivers/firewire/ohci.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
Thanks.
commit 8c0c0cc2d9f4c523fde04bdfe41e4380dec8ee54
Author: Jay Fenlason <fenlason@redhat.com>
Date: Fri Dec 11 14:23:58 2009 -0500
firewire: ohci: handle receive packets with a data length of zero
Queueing to receive an ISO packet with a payload length of zero
silently does nothing in dualbuffer mode, and crashes the kernel in
packet-per-buffer mode. Return an error in dualbuffer mode, because
the DMA controller won't let us do what we want, and work correctly in
packet-per-buffer mode.
Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: stable@kernel.org
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index a714775..553c74e 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -2189,6 +2189,13 @@ static int ohci_queue_iso_receive_dualbuffer(struct fw_iso_context *base,
page = payload >> PAGE_SHIFT;
offset = payload & ~PAGE_MASK;
rest = p->payload_length;
+ /*
+ * The controllers I've tested have not worked correctly when
+ * second_req_count is zero. Rather than do something we know won't
+ * work, return an error
+ */
+ if (rest == 0)
+ return -EINVAL;
/* FIXME: make packet-per-buffer/dual-buffer a context option */
while (rest > 0) {
@@ -2242,7 +2249,7 @@ static int ohci_queue_iso_receive_packet_per_buffer(struct fw_iso_context *base,
unsigned long payload)
{
struct iso_context *ctx = container_of(base, struct iso_context, base);
- struct descriptor *d = NULL, *pd = NULL;
+ struct descriptor *d, *pd;
struct fw_iso_packet *p = packet;
dma_addr_t d_bus, page_bus;
u32 z, header_z, rest;
@@ -2280,8 +2287,9 @@ static int ohci_queue_iso_receive_packet_per_buffer(struct fw_iso_context *base,
d->data_address = cpu_to_le32(d_bus + (z * sizeof(*d)));
rest = payload_per_buffer;
+ pd = d;
for (j = 1; j < z; j++) {
- pd = d + j;
+ pd++;
pd->control = cpu_to_le16(DESCRIPTOR_STATUS |
DESCRIPTOR_INPUT_MORE);
--
Stefan Richter
-=====-==--= ==-- -=-==
http://arcgraph.de/sr/
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [git pull] FireWire fixes; new firewire stack is recommended to distributors and users
2009-12-11 20:59 ` [git pull] FireWire fix Stefan Richter
@ 2009-12-31 18:55 ` Stefan Richter
2010-01-28 17:05 ` [git pull] FireWire fixes Stefan Richter
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Richter @ 2009-12-31 18:55 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel
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 a few fixes to the FireWire subsystem and a status update WRT
maintenance of the older vs. the newer FireWire driver stack. Shortlog,
diffstat and full diffs follow below.
Distributors who still ship the old stack (ieee1394, ohci1394, raw1394,
sbp2, eth1394 and more) should now switch to the new one (firewire-core,
firewire-ohci, firewire-sbp2, firewire-net). In the first iteration,
those distributors might want to ship the old stack also (but
blacklisted) as a fallback for their users if unforeseen problems with
the newer replacement drivers are encountered.
The older FireWire stack contains several known problems which are not
going to be fixed; instead, those issues are addressed by the new stack.
An incomplete list of these issues is kept in bugzilla:
http://bugzilla.kernel.org/show_bug.cgi?id=10046
We have a guide on migration from the older to the newer stack:
http://ieee1394.wiki.kernel.org/index.php/Juju_Migration
Thanks.
Clemens Ladisch (1):
firewire: fix use of multiple AV/C devices, allow multiple FCP listeners
Stefan Richter (4):
firewire: cdev: fix another memory leak in an error path
firewire: ohci: always use packet-per-buffer mode for isochronous reception
firewire, ieee1394: update MAINTAINERS entries
firewire, ieee1394: update Kconfig help
MAINTAINERS | 15 +---
drivers/Kconfig | 2 +-
drivers/firewire/Kconfig | 44 +++------
drivers/firewire/core-cdev.c | 27 +++--
drivers/firewire/core-transaction.c | 118 +++++++++++++++++++----
drivers/firewire/ohci.c | 4 +-
drivers/ieee1394/Kconfig | 59 ++++++++----
drivers/media/dvb/firewire/firedtv-fw.c | 12 +--
include/linux/firewire-cdev.h | 3 +
include/linux/firewire.h | 4 +-
10 files changed, 182 insertions(+), 106 deletions(-)
commit 5d7db0499e5bb13381a7fbfdd0d913b966545e75
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Sat Dec 26 01:36:53 2009 +0100
firewire, ieee1394: update Kconfig help
Update the Kconfig help texts of both stacks to encourage a general move
from the older to the newer drivers. However, do not label ieee1394 as
"Obsolete" yet, as the newer drivers have not been deployed as default
stack in the majority of Linux distributions yet, and those who start
doing so now may still want to install the old drivers as fallback for
unforeseen issues.
Since Linux 2.6.32, FireWire audio devices can be driven by the newer
firewire driver stack too, hence remove an outdated comment about audio
devices. Also remove comments about library versions since the 2nd
generation of libraw1394 and libdc1394 is now in common use; details on
library versions can be read at the wiki link from the help texts.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 8a07363..368ae6d 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -28,7 +28,7 @@ source "drivers/md/Kconfig"
source "drivers/message/fusion/Kconfig"
-source "drivers/ieee1394/Kconfig"
+source "drivers/firewire/Kconfig"
source "drivers/message/i2o/Kconfig"
diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig
index 13efcd3..a9371b3 100644
--- a/drivers/firewire/Kconfig
+++ b/drivers/firewire/Kconfig
@@ -1,5 +1,10 @@
+menu "IEEE 1394 (FireWire) support"
+ depends on PCI || BROKEN
+ # firewire-core does not depend on PCI but is
+ # not useful without PCI controller driver
+
comment "You can enable one or both FireWire driver stacks."
-comment "See the help texts for more information."
+comment "The newer stack is recommended."
config FIREWIRE
tristate "FireWire driver stack"
@@ -15,16 +20,6 @@ config FIREWIRE
To compile this driver as a module, say M here: the module will be
called firewire-core.
- This module functionally replaces ieee1394, raw1394, and video1394.
- To access it from application programs, you generally need at least
- libraw1394 v2. IIDC/DCAM applications need libdc1394 v2.
- No libraries are required to access storage devices through the
- firewire-sbp2 driver.
-
- NOTE:
- FireWire audio devices currently require the old drivers (ieee1394,
- ohci1394, raw1394).
-
config FIREWIRE_OHCI
tristate "OHCI-1394 controllers"
depends on PCI && FIREWIRE
@@ -34,22 +29,7 @@ config FIREWIRE_OHCI
is the only chipset in use, so say Y here.
To compile this driver as a module, say M here: The module will be
- called firewire-ohci. It replaces ohci1394 of the classic IEEE 1394
- stack.
-
- NOTE:
- If you want to install firewire-ohci and ohci1394 together, you
- should configure them only as modules and blacklist the driver(s)
- which you don't want to have auto-loaded. Add either
-
- blacklist firewire-ohci
- or
- blacklist ohci1394
- blacklist video1394
- blacklist dv1394
-
- to /etc/modprobe.conf or /etc/modprobe.d/* and update modprobe.conf
- depending on your distribution.
+ called firewire-ohci.
config FIREWIRE_OHCI_DEBUG
bool
@@ -66,8 +46,7 @@ config FIREWIRE_SBP2
like scanners.
To compile this driver as a module, say M here: The module will be
- called firewire-sbp2. It replaces sbp2 of the classic IEEE 1394
- stack.
+ called firewire-sbp2.
You should also enable support for disks, CD-ROMs, etc. in the SCSI
configuration section.
@@ -83,5 +62,8 @@ config FIREWIRE_NET
NOTE, this driver is not stable yet!
To compile this driver as a module, say M here: The module will be
- called firewire-net. It replaces eth1394 of the classic IEEE 1394
- stack.
+ called firewire-net.
+
+source "drivers/ieee1394/Kconfig"
+
+endmenu
diff --git a/drivers/ieee1394/Kconfig b/drivers/ieee1394/Kconfig
index f102fcc..e02096c 100644
--- a/drivers/ieee1394/Kconfig
+++ b/drivers/ieee1394/Kconfig
@@ -1,8 +1,3 @@
-menu "IEEE 1394 (FireWire) support"
- depends on PCI || BROKEN
-
-source "drivers/firewire/Kconfig"
-
config IEEE1394
tristate "Legacy alternative FireWire driver stack"
depends on PCI || BROKEN
@@ -16,8 +11,13 @@ config IEEE1394
is the core support only, you will also need to select a driver for
your IEEE 1394 adapter.
- To compile this driver as a module, say M here: the
- module will be called ieee1394.
+ To compile this driver as a module, say M here: the module will be
+ called ieee1394.
+
+ NOTE:
+ ieee1394 is superseded by the newer firewire-core driver. See
+ http://ieee1394.wiki.kernel.org/index.php/Juju_Migration for
+ further information on how to switch to the new FireWire drivers.
config IEEE1394_OHCI1394
tristate "OHCI-1394 controllers"
@@ -29,19 +29,23 @@ config IEEE1394_OHCI1394
use one of these chipsets. It should work with any OHCI-1394
compliant card, however.
- To compile this driver as a module, say M here: the
- module will be called ohci1394.
+ To compile this driver as a module, say M here: the module will be
+ called ohci1394.
NOTE:
+ ohci1394 is superseded by the newer firewire-ohci driver. See
+ http://ieee1394.wiki.kernel.org/index.php/Juju_Migration for
+ further information on how to switch to the new FireWire drivers.
+
If you want to install firewire-ohci and ohci1394 together, you
should configure them only as modules and blacklist the driver(s)
which you don't want to have auto-loaded. Add either
- blacklist firewire-ohci
- or
blacklist ohci1394
blacklist video1394
blacklist dv1394
+ or
+ blacklist firewire-ohci
to /etc/modprobe.conf or /etc/modprobe.d/* and update modprobe.conf
depending on your distribution.
@@ -58,8 +62,8 @@ config IEEE1394_PCILYNX
Instruments PCILynx chip. Note: this driver is written for revision
2 of this chip and may not work with revision 0.
- To compile this driver as a module, say M here: the
- module will be called pcilynx.
+ To compile this driver as a module, say M here: the module will be
+ called pcilynx.
Only some old and now very rare PCI and CardBus cards and
PowerMacs G3 B&W contain the PCILynx controller. Therefore
@@ -79,6 +83,14 @@ config IEEE1394_SBP2
You should also enable support for disks, CD-ROMs, etc. in the SCSI
configuration section.
+ To compile this driver as a module, say M here: the module will be
+ called sbp2.
+
+ NOTE:
+ sbp2 is superseded by the newer firewire-sbp2 driver. See
+ http://ieee1394.wiki.kernel.org/index.php/Juju_Migration for
+ further information on how to switch to the new FireWire drivers.
+
config IEEE1394_SBP2_PHYS_DMA
bool "Enable replacement for physical DMA in SBP2"
depends on IEEE1394_SBP2 && VIRT_TO_BUS && EXPERIMENTAL
@@ -111,6 +123,11 @@ config IEEE1394_ETH1394
The module is called eth1394 although it does not emulate Ethernet.
+ NOTE:
+ eth1394 is superseded by the newer firewire-net driver. See
+ http://ieee1394.wiki.kernel.org/index.php/Juju_Migration for
+ further information on how to switch to the new FireWire drivers.
+
config IEEE1394_RAWIO
tristate "raw1394 userspace interface"
depends on IEEE1394
@@ -123,6 +140,11 @@ config IEEE1394_RAWIO
To compile this driver as a module, say M here: the module will be
called raw1394.
+ NOTE:
+ raw1394 is superseded by the newer firewire-core driver. See
+ http://ieee1394.wiki.kernel.org/index.php/Juju_Migration for
+ further information on how to switch to the new FireWire drivers.
+
config IEEE1394_VIDEO1394
tristate "video1394 userspace interface"
depends on IEEE1394 && IEEE1394_OHCI1394
@@ -136,13 +158,18 @@ config IEEE1394_VIDEO1394
To compile this driver as a module, say M here: the module will be
called video1394.
+ NOTE:
+ video1394 is superseded by the newer firewire-core driver. See
+ http://ieee1394.wiki.kernel.org/index.php/Juju_Migration for
+ further information on how to switch to the new FireWire drivers.
+
config IEEE1394_DV1394
tristate "dv1394 userspace interface (deprecated)"
depends on IEEE1394 && IEEE1394_OHCI1394
help
The dv1394 driver is unsupported and may be removed from Linux in a
- future release. Its functionality is now provided by raw1394 together
- with libraries such as libiec61883.
+ future release. Its functionality is now provided by either
+ raw1394 or firewire-core together with libraries such as libiec61883.
config IEEE1394_VERBOSEDEBUG
bool "Excessive debugging output"
@@ -153,5 +180,3 @@ config IEEE1394_VERBOSEDEBUG
will quickly result in large amounts of data sent to the system log.
Say Y if you really need the debugging output. Everyone else says N.
-
-endmenu
commit 958a29cb1c9161d61dcd4ced51f260578e19823c
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Sat Dec 26 01:36:12 2009 +0100
firewire, ieee1394: update MAINTAINERS entries
Ben and Kristian have not been involved in maintenance of the IEEE 1394
drivers for quite some time; submitters are not required to Cc them on
patches.
The linux1394.org domain has been dead for a while and is no longer
under control of a Linux developer. The current web site of the
Linux 1394 project is http://ieee1394.wiki.kernel.org/.
The classic drivers/ieee1394/ stack is now obsolete from the development
point of view, though still a useful alternative in productive use. But
nobody should attempt to submit style cleanup patches for it or to
develop new drivers on top of this stack, hence mark its MAINTAINERS
entry as Obsolete.
drivers/ieee1394/raw1394*, like the rest of the old stack, does not
receive bigger code changes anymore, hence shrink the MAINTAINERS
database a bit by dropping raw1394's special entry. If something
important and urgent is going to come up for raw1394, I will make sure
that Dan will be notified of it besides via linux1394-devel.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Dan Dennedy <dan@dennedy.org>
Cc: Ben Collins <ben.collins@ubuntu.com>
Cc: Kristian Hoegsberg <krh@bitplanet.net>
diff --git a/MAINTAINERS b/MAINTAINERS
index 66f5f7d..c22d597 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2169,10 +2169,9 @@ F: drivers/hwmon/f75375s.c
F: include/linux/f75375s.h
FIREWIRE SUBSYSTEM
-M: Kristian Hoegsberg <krh@redhat.com>
M: Stefan Richter <stefanr@s5r6.in-berlin.de>
L: linux1394-devel@lists.sourceforge.net
-W: http://www.linux1394.org/
+W: http://ieee1394.wiki.kernel.org/
T: git git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git
S: Maintained
F: drivers/firewire/
@@ -2705,22 +2704,14 @@ S: Supported
F: drivers/idle/i7300_idle.c
IEEE 1394 SUBSYSTEM
-M: Ben Collins <ben.collins@ubuntu.com>
M: Stefan Richter <stefanr@s5r6.in-berlin.de>
L: linux1394-devel@lists.sourceforge.net
-W: http://www.linux1394.org/
+W: http://ieee1394.wiki.kernel.org/
T: git git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git
-S: Maintained
+S: Obsolete
F: Documentation/debugging-via-ohci1394.txt
F: drivers/ieee1394/
-IEEE 1394 RAW I/O DRIVER
-M: Dan Dennedy <dan@dennedy.org>
-M: Stefan Richter <stefanr@s5r6.in-berlin.de>
-L: linux1394-devel@lists.sourceforge.net
-S: Maintained
-F: drivers/ieee1394/raw1394*
-
IEEE 802.15.4 SUBSYSTEM
M: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
M: Sergey Lapin <slapin@ossfans.org>
commit 090699c0530ae5380a9b8511d76f656cc437bb6e
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Sat Dec 26 01:35:14 2009 +0100
firewire: ohci: always use packet-per-buffer mode for isochronous reception
This is a minimal change meant for the short term: Never set the
ohci->use_dualbuffer flag to true.
There are two reasons to do so:
- Packet-per-buffer mode and dual-buffer mode do not behave the same
under certain circumstances, notably if several packets are covered
by a single fw_cdev_iso_packet descriptor.
http://marc.info/?l=linux1394-devel&m=124965653718313
Therefore the driver stack should not silently choose one or the
other mode but should leave the choice to the high-level driver
(regardless if kernel driver or userspace driver). Or simply always
only offer packet-per-buffer mode, since a considerable number of
controllers, even current ones, does not offer dual-buffer support.
- Even under circumstances where packet-per-buffer mode and
dual-buffer mode behave exactly the same --- notably when used
through libraw1394, libdc1394, as well as the current two kernel
drivers which use isochronous reception (firewire-net and firedtv)
--- we are still faced with the problem that several OHCI 1.1
controllers have bugs in dual-buffer mode. Although it looks like
we have identified most of those buggy controllers by now, we
cannot be quite sure about that.
So, use packet-per-buffer by default from now on. This change should
be followed up by a more complete solution: Either extend the
in-kernel API and the userspace ABI by a choice between the two IR modes
or remove all dual-buffer related code from firewire-ohci.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 96768e1..a61571c 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -2226,7 +2226,6 @@ static int ohci_queue_iso_receive_dualbuffer(struct fw_iso_context *base,
if (rest == 0)
return -EINVAL;
- /* FIXME: make packet-per-buffer/dual-buffer a context option */
while (rest > 0) {
d = context_get_descriptors(&ctx->context,
z + header_z, &d_bus);
@@ -2470,7 +2469,10 @@ static int __devinit pci_probe(struct pci_dev *dev,
}
version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff;
+#if 0
+ /* FIXME: make it a context option or remove dual-buffer mode */
ohci->use_dualbuffer = version >= OHCI_VERSION_1_1;
+#endif
/* dual-buffer mode is broken if more than one IR context is active */
if (dev->vendor == PCI_VENDOR_ID_AGERE &&
commit cf0e575dcc4cab9fd955e9bec49df7e8ee30a7cf
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Sat Dec 26 01:34:29 2009 +0100
firewire: cdev: fix another memory leak in an error path
If copy_from_user in an FW_CDEV_IOC_SEND_RESPONSE ioctl failed, the
fw_request pointed to by the inbound_transaction_resource is no
longer referenced and needs to be freed.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 2cb22d1..e6d6384 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -723,6 +723,7 @@ static int ioctl_send_response(struct client *client, void *buffer)
if (copy_from_user(r->data, u64_to_uptr(request->data),
r->length)) {
ret = -EFAULT;
+ kfree(r->request);
goto out;
}
fw_send_response(client->device->card, r->request,
commit db5d247ae811f49185a71e703b65acad845e4b18
Author: Clemens Ladisch <cladisch@fastmail.net>
Date: Thu Dec 24 12:05:58 2009 +0100
firewire: fix use of multiple AV/C devices, allow multiple FCP listeners
Control of more than one AV/C device at once --- e.g. camcorders, tape
decks, audio devices, TV tuners --- failed or worked only unreliably,
depending on driver implementation. This affected kernelspace and
userspace drivers alike and was caused by firewire-core's inability to
accept multiple registrations of FCP listeners.
The fix allows multiple address handlers to be registered for the FCP
command and response registers. When a request for these registers is
received, all handlers are invoked, and the Firewire response is
generated by the core and not by any handler.
The cdev API does not change, i.e., userspace is still expected to send
a response for FCP requests; this response is silently ignored.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (changelog, rebased, whitespace)
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 231e6ee..2cb22d1 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -601,8 +601,9 @@ static void release_request(struct client *client,
struct inbound_transaction_resource *r = container_of(resource,
struct inbound_transaction_resource, resource);
- fw_send_response(client->device->card, r->request,
- RCODE_CONFLICT_ERROR);
+ if (r->request)
+ fw_send_response(client->device->card, r->request,
+ RCODE_CONFLICT_ERROR);
kfree(r);
}
@@ -645,7 +646,8 @@ static void handle_request(struct fw_card *card, struct fw_request *request,
failed:
kfree(r);
kfree(e);
- fw_send_response(card, request, RCODE_CONFLICT_ERROR);
+ if (request)
+ fw_send_response(card, request, RCODE_CONFLICT_ERROR);
}
static void release_address_handler(struct client *client,
@@ -715,15 +717,17 @@ static int ioctl_send_response(struct client *client, void *buffer)
r = container_of(resource, struct inbound_transaction_resource,
resource);
- if (request->length < r->length)
- r->length = request->length;
-
- if (copy_from_user(r->data, u64_to_uptr(request->data), r->length)) {
- ret = -EFAULT;
- goto out;
+ if (r->request) {
+ if (request->length < r->length)
+ r->length = request->length;
+ if (copy_from_user(r->data, u64_to_uptr(request->data),
+ r->length)) {
+ ret = -EFAULT;
+ goto out;
+ }
+ fw_send_response(client->device->card, r->request,
+ request->rcode);
}
-
- fw_send_response(client->device->card, r->request, request->rcode);
out:
kfree(r);
diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 842739d..495849e 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -432,14 +432,20 @@ static struct fw_address_handler *lookup_overlapping_address_handler(
return NULL;
}
+static bool is_enclosing_handler(struct fw_address_handler *handler,
+ unsigned long long offset, size_t length)
+{
+ return handler->offset <= offset &&
+ offset + length <= handler->offset + handler->length;
+}
+
static struct fw_address_handler *lookup_enclosing_address_handler(
struct list_head *list, unsigned long long offset, size_t length)
{
struct fw_address_handler *handler;
list_for_each_entry(handler, list, link) {
- if (handler->offset <= offset &&
- offset + length <= handler->offset + handler->length)
+ if (is_enclosing_handler(handler, offset, length))
return handler;
}
@@ -465,6 +471,12 @@ const struct fw_address_region fw_unit_space_region =
{ .start = 0xfffff0000900ULL, .end = 0x1000000000000ULL, };
#endif /* 0 */
+static bool is_in_fcp_region(u64 offset, size_t length)
+{
+ return offset >= (CSR_REGISTER_BASE | CSR_FCP_COMMAND) &&
+ offset + length <= (CSR_REGISTER_BASE | CSR_FCP_END);
+}
+
/**
* fw_core_add_address_handler - register for incoming requests
* @handler: callback
@@ -477,8 +489,11 @@ const struct fw_address_region fw_unit_space_region =
* give the details of the particular request.
*
* Return value: 0 on success, non-zero otherwise.
+ *
* The start offset of the handler's address region is determined by
* fw_core_add_address_handler() and is returned in handler->offset.
+ *
+ * Address allocations are exclusive, except for the FCP registers.
*/
int fw_core_add_address_handler(struct fw_address_handler *handler,
const struct fw_address_region *region)
@@ -498,10 +513,12 @@ int fw_core_add_address_handler(struct fw_address_handler *handler,
handler->offset = region->start;
while (handler->offset + handler->length <= region->end) {
- other =
- lookup_overlapping_address_handler(&address_handler_list,
- handler->offset,
- handler->length);
+ if (is_in_fcp_region(handler->offset, handler->length))
+ other = NULL;
+ else
+ other = lookup_overlapping_address_handler
+ (&address_handler_list,
+ handler->offset, handler->length);
if (other != NULL) {
handler->offset += other->length;
} else {
@@ -668,6 +685,9 @@ static struct fw_request *allocate_request(struct fw_packet *p)
void fw_send_response(struct fw_card *card,
struct fw_request *request, int rcode)
{
+ if (WARN_ONCE(!request, "invalid for FCP address handlers"))
+ return;
+
/* unified transaction or broadcast transaction: don't respond */
if (request->ack != ACK_PENDING ||
HEADER_DESTINATION_IS_BROADCAST(request->request_header[0])) {
@@ -686,26 +706,15 @@ void fw_send_response(struct fw_card *card,
}
EXPORT_SYMBOL(fw_send_response);
-void fw_core_handle_request(struct fw_card *card, struct fw_packet *p)
+static void handle_exclusive_region_request(struct fw_card *card,
+ struct fw_packet *p,
+ struct fw_request *request,
+ unsigned long long offset)
{
struct fw_address_handler *handler;
- struct fw_request *request;
- unsigned long long offset;
unsigned long flags;
int tcode, destination, source;
- if (p->ack != ACK_PENDING && p->ack != ACK_COMPLETE)
- return;
-
- request = allocate_request(p);
- if (request == NULL) {
- /* FIXME: send statically allocated busy packet. */
- return;
- }
-
- offset =
- ((unsigned long long)
- HEADER_GET_OFFSET_HIGH(p->header[1]) << 32) | p->header[2];
tcode = HEADER_GET_TCODE(p->header[0]);
destination = HEADER_GET_DESTINATION(p->header[0]);
source = HEADER_GET_SOURCE(p->header[1]);
@@ -732,6 +741,73 @@ void fw_core_handle_request(struct fw_card *card, struct fw_packet *p)
request->data, request->length,
handler->callback_data);
}
+
+static void handle_fcp_region_request(struct fw_card *card,
+ struct fw_packet *p,
+ struct fw_request *request,
+ unsigned long long offset)
+{
+ struct fw_address_handler *handler;
+ unsigned long flags;
+ int tcode, destination, source;
+
+ if ((offset != (CSR_REGISTER_BASE | CSR_FCP_COMMAND) &&
+ offset != (CSR_REGISTER_BASE | CSR_FCP_RESPONSE)) ||
+ request->length > 0x200) {
+ fw_send_response(card, request, RCODE_ADDRESS_ERROR);
+
+ return;
+ }
+
+ tcode = HEADER_GET_TCODE(p->header[0]);
+ destination = HEADER_GET_DESTINATION(p->header[0]);
+ source = HEADER_GET_SOURCE(p->header[1]);
+
+ if (tcode != TCODE_WRITE_QUADLET_REQUEST &&
+ tcode != TCODE_WRITE_BLOCK_REQUEST) {
+ fw_send_response(card, request, RCODE_TYPE_ERROR);
+
+ return;
+ }
+
+ spin_lock_irqsave(&address_handler_lock, flags);
+ list_for_each_entry(handler, &address_handler_list, link) {
+ if (is_enclosing_handler(handler, offset, request->length))
+ handler->address_callback(card, NULL, tcode,
+ destination, source,
+ p->generation, p->speed,
+ offset, request->data,
+ request->length,
+ handler->callback_data);
+ }
+ spin_unlock_irqrestore(&address_handler_lock, flags);
+
+ fw_send_response(card, request, RCODE_COMPLETE);
+}
+
+void fw_core_handle_request(struct fw_card *card, struct fw_packet *p)
+{
+ struct fw_request *request;
+ unsigned long long offset;
+
+ if (p->ack != ACK_PENDING && p->ack != ACK_COMPLETE)
+ return;
+
+ request = allocate_request(p);
+ if (request == NULL) {
+ /* FIXME: send statically allocated busy packet. */
+ return;
+ }
+
+ offset = ((u64)HEADER_GET_OFFSET_HIGH(p->header[1]) << 32) |
+ p->header[2];
+
+ if (!is_in_fcp_region(offset, request->length))
+ handle_exclusive_region_request(card, p, request, offset);
+ else
+ handle_fcp_region_request(card, p, request, offset);
+
+}
EXPORT_SYMBOL(fw_core_handle_request);
void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
diff --git a/drivers/media/dvb/firewire/firedtv-fw.c b/drivers/media/dvb/firewire/firedtv-fw.c
index fe44789..6223bf0 100644
--- a/drivers/media/dvb/firewire/firedtv-fw.c
+++ b/drivers/media/dvb/firewire/firedtv-fw.c
@@ -202,14 +202,8 @@ static void handle_fcp(struct fw_card *card, struct fw_request *request,
unsigned long flags;
int su;
- if ((tcode != TCODE_WRITE_QUADLET_REQUEST &&
- tcode != TCODE_WRITE_BLOCK_REQUEST) ||
- offset != CSR_REGISTER_BASE + CSR_FCP_RESPONSE ||
- length == 0 ||
- (((u8 *)payload)[0] & 0xf0) != 0) {
- fw_send_response(card, request, RCODE_TYPE_ERROR);
+ if (length < 2 || (((u8 *)payload)[0] & 0xf0) != 0)
return;
- }
su = ((u8 *)payload)[1] & 0x7;
@@ -230,10 +224,8 @@ static void handle_fcp(struct fw_card *card, struct fw_request *request,
}
spin_unlock_irqrestore(&node_list_lock, flags);
- if (fdtv) {
+ if (fdtv)
avc_recv(fdtv, payload, length);
- fw_send_response(card, request, RCODE_COMPLETE);
- }
}
static struct fw_address_handler fcp_handler = {
diff --git a/include/linux/firewire-cdev.h b/include/linux/firewire-cdev.h
index c6b3ca3..1f716d9 100644
--- a/include/linux/firewire-cdev.h
+++ b/include/linux/firewire-cdev.h
@@ -340,6 +340,9 @@ struct fw_cdev_send_response {
* The @closure field is passed back to userspace in the response event.
* The @handle field is an out parameter, returning a handle to the allocated
* range to be used for later deallocation of the range.
+ *
+ * The address range is allocated on all local nodes. The address allocation
+ * is exclusive except for the FCP command and response registers.
*/
struct fw_cdev_allocate {
__u64 offset;
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index 9416a46..a0e6715 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -248,8 +248,8 @@ typedef void (*fw_transaction_callback_t)(struct fw_card *card, int rcode,
void *data, size_t length,
void *callback_data);
/*
- * Important note: The callback must guarantee that either fw_send_response()
- * or kfree() is called on the @request.
+ * Important note: Except for the FCP registers, the callback must guarantee
+ * that either fw_send_response() or kfree() is called on the @request.
*/
typedef void (*fw_address_callback_t)(struct fw_card *card,
struct fw_request *request,
--
Stefan Richter
-=====-==--= ==-- =====
http://arcgraph.de/sr/
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [git pull] FireWire fixes
2009-12-31 18:55 ` [git pull] FireWire fixes; new firewire stack is recommended to distributors and users Stefan Richter
@ 2010-01-28 17:05 ` Stefan Richter
2010-02-15 22:19 ` Stefan Richter
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Richter @ 2010-01-28 17:05 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel
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 updates to the FireWire subsystem.
Thanks.
Stefan Richter (4):
firewire: core: add_descriptor size check
firewire: cdev: add_descriptor documentation fix
firewire: core: fix use-after-free regression in FCP handler
firewire: ohci: fix crashes with TSB43AB23 on 64bit systems
drivers/firewire/core-card.c | 41 ++++++++++++++++++--------
drivers/firewire/core-cdev.c | 50 +++++++++++++++++++++++---------
drivers/firewire/ohci.c | 4 ++-
include/linux/firewire-cdev.h | 4 ++-
4 files changed, 70 insertions(+), 29 deletions(-)
commit 7a481436787cbc932af6c407b317ac603969a242
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Tue Jan 26 21:39:07 2010 +0100
firewire: ohci: fix crashes with TSB43AB23 on 64bit systems
Unsurprisingly, Texas Instruments TSB43AB23 exhibits the same behaviour
as TSB43AB22/A in dual buffer IR DMA mode: If descriptors are located
at physical addresses above the 31 bit address range (2 GB), the
controller will overwrite random memory. With luck, this merely
prevents video reception. With only a little less luck, the machine
crashes.
We use the same workaround here as with TSB43AB22/A: Switch off the
dual buffer capability flag and use packet-per-buffer IR DMA instead.
Another possible workaround would be to limit the coherent DMA mask to
31 bits.
In Linux 2.6.33, this change serves effectively only as documentation
since dual buffer mode is not used for any controller anymore. But
somebody might want to re-enable it in the future to make use of
features of dual buffer DMA that are not available in packet-per-buffer
mode.
In Linux 2.6.32 and older, this update is vital for anyone with this
controller, more than 2 GB RAM, a 64 bit kernel, and FireWire video or
audio applications.
We have at least four reports:
http://bugzilla.kernel.org/show_bug.cgi?id=13808
http://marc.info/?l=linux1394-user&m=126154279004083
https://bugzilla.redhat.com/show_bug.cgi?id=552142
http://marc.info/?l=linux1394-user&m=126432246128386
Reported-by: Paul Johnson
Reported-by: Ronneil Camara
Reported-by: G Zornetzer
Reported-by: Mark Thompson
Cc: stable@kernel.org
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index a61571c..2345d41 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -2420,6 +2420,7 @@ static void ohci_pmac_off(struct pci_dev *dev)
#define PCI_VENDOR_ID_AGERE PCI_VENDOR_ID_ATT
#define PCI_DEVICE_ID_AGERE_FW643 0x5901
+#define PCI_DEVICE_ID_TI_TSB43AB23 0x8024
static int __devinit pci_probe(struct pci_dev *dev,
const struct pci_device_id *ent)
@@ -2488,7 +2489,8 @@ static int __devinit pci_probe(struct pci_dev *dev,
#if !defined(CONFIG_X86_32)
/* dual-buffer mode is broken with descriptor addresses above 2G */
if (dev->vendor == PCI_VENDOR_ID_TI &&
- dev->device == PCI_DEVICE_ID_TI_TSB43AB22)
+ (dev->device == PCI_DEVICE_ID_TI_TSB43AB22 ||
+ dev->device == PCI_DEVICE_ID_TI_TSB43AB23))
ohci->use_dualbuffer = false;
#endif
commit 281e20323ab72180137824a298ee9e21e6f9acf6
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Sun Jan 24 16:45:03 2010 +0100
firewire: core: fix use-after-free regression in FCP handler
Commit db5d247a "firewire: fix use of multiple AV/C devices, allow
multiple FCP listeners" introduced a regression into 2.6.33-rc3:
The core freed payloads of incoming requests to FCP_Request or
FCP_Response before a userspace driver accessed them.
We need to copy such payloads for each registered userspace client
and free the copies according to the lifetime rules of non-FCP client
request resources.
(This could possibly be optimized by reference counts instead of
copies.)
The presently only kernelspace driver which listens for FCP requests,
firedtv, was not affected because it already copies FCP frames into an
own buffer before returning to firewire-core's FCP handler dispatcher.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index e6d6384..4eeaed5 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -35,6 +35,7 @@
#include <linux/preempt.h>
#include <linux/sched.h>
#include <linux/spinlock.h>
+#include <linux/string.h>
#include <linux/time.h>
#include <linux/uaccess.h>
#include <linux/vmalloc.h>
@@ -595,13 +596,20 @@ static int ioctl_send_request(struct client *client, void *buffer)
client->device->max_speed);
}
+static inline bool is_fcp_request(struct fw_request *request)
+{
+ return request == NULL;
+}
+
static void release_request(struct client *client,
struct client_resource *resource)
{
struct inbound_transaction_resource *r = container_of(resource,
struct inbound_transaction_resource, resource);
- if (r->request)
+ if (is_fcp_request(r->request))
+ kfree(r->data);
+ else
fw_send_response(client->device->card, r->request,
RCODE_CONFLICT_ERROR);
kfree(r);
@@ -616,6 +624,7 @@ static void handle_request(struct fw_card *card, struct fw_request *request,
struct address_handler_resource *handler = callback_data;
struct inbound_transaction_resource *r;
struct inbound_transaction_event *e;
+ void *fcp_frame = NULL;
int ret;
r = kmalloc(sizeof(*r), GFP_ATOMIC);
@@ -627,6 +636,18 @@ static void handle_request(struct fw_card *card, struct fw_request *request,
r->data = payload;
r->length = length;
+ if (is_fcp_request(request)) {
+ /*
+ * FIXME: Let core-transaction.c manage a
+ * single reference-counted copy?
+ */
+ fcp_frame = kmemdup(payload, length, GFP_ATOMIC);
+ if (fcp_frame == NULL)
+ goto failed;
+
+ r->data = fcp_frame;
+ }
+
r->resource.release = release_request;
ret = add_client_resource(handler->client, &r->resource, GFP_ATOMIC);
if (ret < 0)
@@ -640,13 +661,15 @@ static void handle_request(struct fw_card *card, struct fw_request *request,
e->request.closure = handler->closure;
queue_event(handler->client, &e->event,
- &e->request, sizeof(e->request), payload, length);
+ &e->request, sizeof(e->request), r->data, length);
return;
failed:
kfree(r);
kfree(e);
- if (request)
+ kfree(fcp_frame);
+
+ if (!is_fcp_request(request))
fw_send_response(card, request, RCODE_CONFLICT_ERROR);
}
@@ -717,18 +740,17 @@ static int ioctl_send_response(struct client *client, void *buffer)
r = container_of(resource, struct inbound_transaction_resource,
resource);
- if (r->request) {
- if (request->length < r->length)
- r->length = request->length;
- if (copy_from_user(r->data, u64_to_uptr(request->data),
- r->length)) {
- ret = -EFAULT;
- kfree(r->request);
- goto out;
- }
- fw_send_response(client->device->card, r->request,
- request->rcode);
+ if (is_fcp_request(r->request))
+ goto out;
+
+ if (request->length < r->length)
+ r->length = request->length;
+ if (copy_from_user(r->data, u64_to_uptr(request->data), r->length)) {
+ ret = -EFAULT;
+ kfree(r->request);
+ goto out;
}
+ fw_send_response(client->device->card, r->request, request->rcode);
out:
kfree(r);
commit 6d3faf6f431bafb25f4b9926c50a7e5c267738c6
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Sun Jan 24 14:48:00 2010 +0100
firewire: cdev: add_descriptor documentation fix
struct fw_cdev_add_descriptor.length is in quadlets, not in bytes.
Also remove any doubts about the endianess of descriptor data.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
diff --git a/include/linux/firewire-cdev.h b/include/linux/firewire-cdev.h
index 1f716d9..520ecf8 100644
--- a/include/linux/firewire-cdev.h
+++ b/include/linux/firewire-cdev.h
@@ -380,7 +380,7 @@ struct fw_cdev_initiate_bus_reset {
* @immediate: If non-zero, immediate key to insert before pointer
* @key: Upper 8 bits of root directory pointer
* @data: Userspace pointer to contents of descriptor block
- * @length: Length of descriptor block data, in bytes
+ * @length: Length of descriptor block data, in quadlets
* @handle: Handle to the descriptor, written by the kernel
*
* Add a descriptor block and optionally a preceding immediate key to the local
@@ -394,6 +394,8 @@ struct fw_cdev_initiate_bus_reset {
* If not 0, the @immediate field specifies an immediate key which will be
* inserted before the root directory pointer.
*
+ * @immediate, @key, and @data array elements are CPU-endian quadlets.
+ *
* If successful, the kernel adds the descriptor and writes back a handle to the
* kernel-side object to be used for later removal of the descriptor block and
* immediate key.
commit e300839da40e99581581c5d053a95a172651fec8
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Sun Jan 24 14:47:02 2010 +0100
firewire: core: add_descriptor size check
Presently, firewire-core only checks whether descriptors that are to be
added by userspace drivers to the local node's config ROM do not exceed
a size of 256 quadlets. However, the sum of the bare minimum ROM plus
all descriptors (from firewire-core, from firewire-net, from userspace)
must not exceed 256 quadlets.
Otherwise, the bounds of a statically allocated buffer will be
overwritten. If the kernel survives that, firewire-core will
subsequently be unable to parse the local node's config ROM.
(Note, userspace drivers can add descriptors only through device files
of local nodes. These are usually only accessible by root, unlike
device files of remote nodes which may be accessible to lesser
privileged users.)
Therefore add a test which takes the actual present and required ROM
size into account for all descriptors of kernelspace and userspace
drivers.
Cc: stable@kernel.org
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c
index 7083bcc..5045156 100644
--- a/drivers/firewire/core-card.c
+++ b/drivers/firewire/core-card.c
@@ -57,6 +57,8 @@ static LIST_HEAD(descriptor_list);
static int descriptor_count;
static __be32 tmp_config_rom[256];
+/* ROM header, bus info block, root dir header, capabilities = 7 quadlets */
+static size_t config_rom_length = 1 + 4 + 1 + 1;
#define BIB_CRC(v) ((v) << 0)
#define BIB_CRC_LENGTH(v) ((v) << 16)
@@ -73,7 +75,7 @@ static __be32 tmp_config_rom[256];
#define BIB_CMC ((1) << 30)
#define BIB_IMC ((1) << 31)
-static size_t generate_config_rom(struct fw_card *card, __be32 *config_rom)
+static void generate_config_rom(struct fw_card *card, __be32 *config_rom)
{
struct fw_descriptor *desc;
int i, j, k, length;
@@ -130,23 +132,30 @@ static size_t generate_config_rom(struct fw_card *card, __be32 *config_rom)
for (i = 0; i < j; i += length + 1)
length = fw_compute_block_crc(config_rom + i);
- return j;
+ WARN_ON(j != config_rom_length);
}
static void update_config_roms(void)
{
struct fw_card *card;
- size_t length;
list_for_each_entry (card, &card_list, link) {
- length = generate_config_rom(card, tmp_config_rom);
- card->driver->set_config_rom(card, tmp_config_rom, length);
+ generate_config_rom(card, tmp_config_rom);
+ card->driver->set_config_rom(card, tmp_config_rom,
+ config_rom_length);
}
}
+static size_t required_space(struct fw_descriptor *desc)
+{
+ /* descriptor + entry into root dir + optional immediate entry */
+ return desc->length + 1 + (desc->immediate > 0 ? 1 : 0);
+}
+
int fw_core_add_descriptor(struct fw_descriptor *desc)
{
size_t i;
+ int ret;
/*
* Check descriptor is valid; the length of all blocks in the
@@ -162,15 +171,21 @@ int fw_core_add_descriptor(struct fw_descriptor *desc)
mutex_lock(&card_mutex);
- list_add_tail(&desc->link, &descriptor_list);
- descriptor_count++;
- if (desc->immediate > 0)
+ if (config_rom_length + required_space(desc) > 256) {
+ ret = -EBUSY;
+ } else {
+ list_add_tail(&desc->link, &descriptor_list);
+ config_rom_length += required_space(desc);
descriptor_count++;
- update_config_roms();
+ if (desc->immediate > 0)
+ descriptor_count++;
+ update_config_roms();
+ ret = 0;
+ }
mutex_unlock(&card_mutex);
- return 0;
+ return ret;
}
EXPORT_SYMBOL(fw_core_add_descriptor);
@@ -179,6 +194,7 @@ void fw_core_remove_descriptor(struct fw_descriptor *desc)
mutex_lock(&card_mutex);
list_del(&desc->link);
+ config_rom_length -= required_space(desc);
descriptor_count--;
if (desc->immediate > 0)
descriptor_count--;
@@ -428,7 +444,6 @@ EXPORT_SYMBOL(fw_card_initialize);
int fw_card_add(struct fw_card *card,
u32 max_receive, u32 link_speed, u64 guid)
{
- size_t length;
int ret;
card->max_receive = max_receive;
@@ -437,8 +452,8 @@ int fw_card_add(struct fw_card *card,
mutex_lock(&card_mutex);
- length = generate_config_rom(card, tmp_config_rom);
- ret = card->driver->enable(card, tmp_config_rom, length);
+ generate_config_rom(card, tmp_config_rom);
+ ret = card->driver->enable(card, tmp_config_rom, config_rom_length);
if (ret == 0)
list_add_tail(&card->link, &card_list);
--
Stefan Richter
-=====-==-=- ---= ===--
http://arcgraph.de/sr/
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [git pull] FireWire fixes
2010-01-28 17:05 ` [git pull] FireWire fixes Stefan Richter
@ 2010-02-15 22:19 ` Stefan Richter
0 siblings, 0 replies; 5+ messages in thread
From: Stefan Richter @ 2010-02-15 22:19 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel
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 updates to the FireWire subsystem.
Thanks.
Clemens Ladisch (1):
firewire: ohci: retransmit isochronous transmit packets on cycle loss
Stefan Richter (1):
firewire: net: fix panic in fwnet_write_complete
drivers/firewire/net.c | 53 ++++++++++++++++++++++++++++----------
drivers/firewire/ohci.c | 13 ++++++---
2 files changed, 47 insertions(+), 19 deletions(-)
commit 7f51a100bba517196ac4bdf29408d20ee1c771e8
Author: Clemens Ladisch <clemens@ladisch.de>
Date: Mon Feb 8 08:30:03 2010 +0100
firewire: ohci: retransmit isochronous transmit packets on cycle loss
In isochronous transmit DMA descriptors, link the skip address pointer
back to the descriptor itself. When a cycle is lost, the controller
will send the packet in the next cycle, instead of terminating the
entire DMA program.
There are two reasons for this:
* This behaviour is compatible with the old IEEE1394 stack. Old
applications would not expect the DMA program to stop in this case.
* Since the OHCI driver does not report any uncompleted packets, the
context would stop silently; clients would not have any chance to
detect and handle this error without a watchdog timer.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Pieter Palmers notes:
"The reason I added this retry behavior to the old stack is because some
cards now and then fail to send a packet (e.g. the o2micro card in my
dell laptop). I couldn't figure out why exactly this happens, my best
guess is that the card cannot fetch the payload data on time. This
happens much more frequently when sending large packets, which leads me
to suspect that there are some contention issues with the DMA that fills
the transmit FIFO.
In the old stack it was a pretty critical issue as it resulted in a
freeze of the userspace application.
The omission of a packet doesn't necessarily have to be an issue. E.g.
in IEC61883 streams the DBC field can be used to detect discontinuities
in the stream. So as long as the other side doesn't bail when no
[packet] is present in a cycle, there is not really a problem.
I'm not convinced though that retrying is the proper solution, but it is
simple and effective for what it had to do. And I think there are no
reasons not to do it this way. Userspace can still detect this by
checking the cycle the descriptor was sent in."
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (changelog, comment)
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 2345d41..43ebf33 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -2101,11 +2101,6 @@ static int ohci_queue_iso_transmit(struct fw_iso_context *base,
u32 payload_index, payload_end_index, next_page_index;
int page, end_page, i, length, offset;
- /*
- * FIXME: Cycle lost behavior should be configurable: lose
- * packet, retransmit or terminate..
- */
-
p = packet;
payload_index = payload;
@@ -2135,6 +2130,14 @@ static int ohci_queue_iso_transmit(struct fw_iso_context *base,
if (!p->skip) {
d[0].control = cpu_to_le16(DESCRIPTOR_KEY_IMMEDIATE);
d[0].req_count = cpu_to_le16(8);
+ /*
+ * Link the skip address to this descriptor itself. This causes
+ * a context to skip a cycle whenever lost cycles or FIFO
+ * overruns occur, without dropping the data. The application
+ * should then decide whether this is an error condition or not.
+ * FIXME: Make the context's cycle-lost behaviour configurable?
+ */
+ d[0].branch_address = cpu_to_le32(d_bus | z);
header = (__le32 *) &d[1];
header[0] = cpu_to_le32(IT_HEADER_SY(p->sy) |
commit 110f82d7a2e0ff5a17617a9672f1ccb7e44bc0c6
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date: Mon Jan 18 22:36:49 2010 +0100
firewire: net: fix panic in fwnet_write_complete
In the transmit path of firewire-net (IPv4 over 1394), the following
race condition may occur:
- The networking soft IRQ inserts a datagram into the 1394 async
request transmit DMA.
- The 1394 async transmit completion tasklet runs to finish cleaning
up (unlink datagram from list of pending ones, release skb and
outbound 1394 transaction object) --- before the networking soft IRQ
had a chance to proceed and add the datagram to the list of pending
datagrams.
This caused a panic in the 1394 async transmit completion tasklet when
it dereferenced unitialized list heads:
http://bugzilla.kernel.org/show_bug.cgi?id=15077
The fix is to add checks in the tx soft IRQ and in the tasklet to
determine which of these two is the last referrer to the transaction
object. Then handle the cleanup of the object by the last referrer
rather than assuming that the tasklet is always the last one.
There is another similar race: Between said tasklet and fwnet_close,
i.e. at ifdown. However, that race is much less likely to occur in
practice and shall be fixed in a separate update.
Reported-by: Илья Басин <basinilya@gmail.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index cbaf420..2d3dc7d 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -893,20 +893,31 @@ static void fwnet_receive_broadcast(struct fw_iso_context *context,
static struct kmem_cache *fwnet_packet_task_cache;
+static void fwnet_free_ptask(struct fwnet_packet_task *ptask)
+{
+ dev_kfree_skb_any(ptask->skb);
+ kmem_cache_free(fwnet_packet_task_cache, ptask);
+}
+
static int fwnet_send_packet(struct fwnet_packet_task *ptask);
static void fwnet_transmit_packet_done(struct fwnet_packet_task *ptask)
{
- struct fwnet_device *dev;
+ struct fwnet_device *dev = ptask->dev;
unsigned long flags;
-
- dev = ptask->dev;
+ bool free;
spin_lock_irqsave(&dev->lock, flags);
- list_del(&ptask->pt_link);
- spin_unlock_irqrestore(&dev->lock, flags);
- ptask->outstanding_pkts--; /* FIXME access inside lock */
+ ptask->outstanding_pkts--;
+
+ /* Check whether we or the networking TX soft-IRQ is last user. */
+ free = (ptask->outstanding_pkts == 0 && !list_empty(&ptask->pt_link));
+
+ if (ptask->outstanding_pkts == 0)
+ list_del(&ptask->pt_link);
+
+ spin_unlock_irqrestore(&dev->lock, flags);
if (ptask->outstanding_pkts > 0) {
u16 dg_size;
@@ -951,10 +962,10 @@ static void fwnet_transmit_packet_done(struct fwnet_packet_task *ptask)
ptask->max_payload = skb->len + RFC2374_FRAG_HDR_SIZE;
}
fwnet_send_packet(ptask);
- } else {
- dev_kfree_skb_any(ptask->skb);
- kmem_cache_free(fwnet_packet_task_cache, ptask);
}
+
+ if (free)
+ fwnet_free_ptask(ptask);
}
static void fwnet_write_complete(struct fw_card *card, int rcode,
@@ -977,6 +988,7 @@ static int fwnet_send_packet(struct fwnet_packet_task *ptask)
unsigned tx_len;
struct rfc2734_header *bufhdr;
unsigned long flags;
+ bool free;
dev = ptask->dev;
tx_len = ptask->max_payload;
@@ -1022,12 +1034,16 @@ static int fwnet_send_packet(struct fwnet_packet_task *ptask)
generation, SCODE_100, 0ULL, ptask->skb->data,
tx_len + 8, fwnet_write_complete, ptask);
- /* FIXME race? */
spin_lock_irqsave(&dev->lock, flags);
- list_add_tail(&ptask->pt_link, &dev->broadcasted_list);
+
+ /* If the AT tasklet already ran, we may be last user. */
+ free = (ptask->outstanding_pkts == 0 && list_empty(&ptask->pt_link));
+ if (!free)
+ list_add_tail(&ptask->pt_link, &dev->broadcasted_list);
+
spin_unlock_irqrestore(&dev->lock, flags);
- return 0;
+ goto out;
}
fw_send_request(dev->card, &ptask->transaction,
@@ -1035,12 +1051,19 @@ static int fwnet_send_packet(struct fwnet_packet_task *ptask)
ptask->generation, ptask->speed, ptask->fifo_addr,
ptask->skb->data, tx_len, fwnet_write_complete, ptask);
- /* FIXME race? */
spin_lock_irqsave(&dev->lock, flags);
- list_add_tail(&ptask->pt_link, &dev->sent_list);
+
+ /* If the AT tasklet already ran, we may be last user. */
+ free = (ptask->outstanding_pkts == 0 && list_empty(&ptask->pt_link));
+ if (!free)
+ list_add_tail(&ptask->pt_link, &dev->sent_list);
+
spin_unlock_irqrestore(&dev->lock, flags);
dev->netdev->trans_start = jiffies;
+ out:
+ if (free)
+ fwnet_free_ptask(ptask);
return 0;
}
@@ -1298,6 +1321,8 @@ static netdev_tx_t fwnet_tx(struct sk_buff *skb, struct net_device *net)
spin_unlock_irqrestore(&dev->lock, flags);
ptask->max_payload = max_payload;
+ INIT_LIST_HEAD(&ptask->pt_link);
+
fwnet_send_packet(ptask);
return NETDEV_TX_OK;
--
Stefan Richter
-=====-==-=- --=- -====
http://arcgraph.de/sr/
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-02-15 22:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-12-06 17:56 [git pull] FireWire updates post 2.6.32 Stefan Richter
2009-12-11 20:59 ` [git pull] FireWire fix Stefan Richter
2009-12-31 18:55 ` [git pull] FireWire fixes; new firewire stack is recommended to distributors and users Stefan Richter
2010-01-28 17:05 ` [git pull] FireWire fixes Stefan Richter
2010-02-15 22:19 ` Stefan Richter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).