Commit Graph

141 Commits

Author SHA1 Message Date
Stefan Richter 7416da6112 Be more careful when copying response payloads on firewire-core
When faced with bogus config ROM read responses from an audio device
that did not support block requests as advertized, libffado's csr1212
code was able to recover when running on top of raw1394 but corrupted
its config ROM cache when running on top of firewire-core.
http://subversion.ffado.org/ticket/299

While the actual cause was a combination of firmware bug of the device
and flaw in csr1212.c of libffado, the much less graceful behavior when
running on firewire-core was obviously due to libraw1394's
firewire-core backend.  Hence,
  - do not write into the client's buffer if rcode != RCODE_COMPLETE,
  - do not copy more data than the actual response contained.

The latter safeguard is not overly effective though.  The libraw1394 API
has no means to inform a client about the error case that a responder
node sent less bytes than were requested.  (The case that the responder
sent more bytes than requested is covered up by the kernel already.)
Should we synthesize an I/O failure?  Does not sound ideal either.
However, such a size mismatch should never happen; the important part of
this change is the RCODE_COMPLETE check.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-09-07 11:48:18 +02:00
Stefan Richter 824ababa4d Implement raw1394_(start_)phy_packet_write() on firewire-core
Requires kernel 2.6.36 or newer at runtime and linux-headers 2.6.36 or newer
at build time.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-09-07 11:27:09 +02:00
Stefan Richter 926e9ea3ad Filter incoming requests per card
If multiple cards are installed, firewire-core will emit requests from
nodes on any of the cards to clients.  This is not expected by
libraw1394 clients since a raw1394handle_t is bound to a single card
alias port.

On kernel 2.6.36 and newer we can filter out requests from other cards.
Note that we still need to call the response ioctl in order to release
kernel resources associated with an inbound transaction.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-09-07 11:27:03 +02:00
Stefan Richter d3ace3dfb4 Fix FCP and ARM source node ID on firewire-core
The firewire-core (juju) backend of libraw1394 installs address range
mappings on the default ioctl fd, i.e. a file that represents a random
device on the chosen port.  It receives incoming requests from any
sender node via this address range mapping.  Due to a kernel ABI
limitation, the sender node ID is not known though.  So far libraw1394
simply assumed the node ID of the device that provided the default
ioctl fd.  This only works if there is only one accessible fd on the
entire bus.

This limitation caused for example libffado to fail to work with
another AV/C or IIDC device attached to the bus, because node IDs of
FCP requests and FCP responses did not match since the latter were
wrong.  FCP clients which did not check sender node IDs were seemingly
not affected by this bug.  The bug is fixed by a kernel ABI extension
in Linux 2.6.36.  This libraw1394 change implements libraw1394's
counterpart to this ABI extension.

Hence this libraw1394 fix requires
  - kernel-headers 2.6.36 or later at build time of libraw1394
  - kernel 2.6.36 or later at runtime.
Otherwise, libraw1394 simply degrades to the faulty previous behaviour.

Side note:  The change of IMPLEMENTED_CDEV_ABI_VERSION to 4 requires
that we fill in struct fw_cdev_allocate.region_end which was added in
the ABI v4.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-09-07 11:27:01 +02:00
Stefan Richter 3fc1f00be3 Rename a few kernel ABI testing helpers
Use more uniform names along the lines of abi_has_some_feature(...).

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-09-07 11:26:58 +02:00
Stefan Richter a29d69af5d Do not use a random FW_CDEV_VERSION as our implemented ABI version
Since linux/firewire-cdev.h header file and libraw1394 sources are
distributed separately, it is wrong to fill in a constant from that
header into the FW_CDEV_IOC_GET_INFO ioctl as the ABI version which
libraw1394 supports.  This may not be forward compatible if an old
libraw1394 is compiled with a new kernel header and ran on top of a
kernel that implements new features that require a compatible
userland.

OK, the damage is already done in released versions of libraw1394.
Hence the FW_CDEV_VERSION of the kernel header file is not going to
be updated anymore in future kernel versions.  (Only the version
internally to firewire-core will be incremented further.)

But let's remove the buggy usage of FW_CDEV_VERSION nevertheless.
Developers of other firewire-cdev client programs might look at
libraw1394 sources.  The libraw1394 sources should not teach them
how to do it wrong.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-09-07 11:26:56 +02:00
Stefan Richter 808cc7ee2c Always imply iso shutdown in fw_destroy_handle
because ieee1394_destroy_handle does it too.  Otherwise, clients which
rely on the ieee1394 backend behaviour leak memory when running on the
juju backend.

Also add the ability to call fw_iso_shutdown multiple times or before
a successful context initialization.  Faulty clients might rely on it
based on ieee1394 backend behaviour.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-09-07 11:25:22 +02:00
Stefan Richter c18094afb2 Treat the kernel's iso context handle as opaque item
Libraw1394 must not rely on the kernel always handing out the value 0
as handle of the (first) allocated isochronous I/O context.  For now
this assumption is true but it may not stay that way forever.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-09-07 11:18:10 +02:00
Stefan Richter c321058ae3 Fix raw1394_iso_stop on firewire-core
The argument to FW_CDEV_IOC_STOP_ISO was missing.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-09-07 11:18:08 +02:00
Stefan Richter 9d47becf25 Add missing malloc failure checks
Also add errno = ENOMEM because it is said that that some malloc
implementations might miss to do so.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-09-07 11:18:05 +02:00
Peter Hurley b15039ceb8 Fix for overlooked device HUP with 'firewire' stack
When EPOLLHUP event is received in fw_loop_iterate(), it is or'd
with EPOLLERR. The EPOLLHUP event was then overlooked in
handle_device_event() with unpredictable-but-generally bad results.

This problem has been rediscovered several times.
http://thread.gmane.org/gmane.linux.kernel.firewire.devel/13330
http://thread.gmane.org/gmane.linux.kernel.firewire.devel/13779

Reported-by: B.J. Buchalter <bj@mhlabs.com>
Reported-by: Michael Thireos <mthireos@vanteon.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-04-17 10:36:00 +02:00
Stefan Richter f806f3d0de Fix documentation of return values of raw1394_start_ family of functions
If running on top of the raw1394 kernel driver (any kernel version) or
on top of firewire-core (kernel version <= 2.6.29), raw1394_start_*
functions will return a value > 0 on success, not == 0.  Only with
firewire-core of kernel 2.6.30 or later, == 0 is returned on success.

The exact value depends on which driver is used, on CPU architecture,
and on request payload size in case of some types of requests.  In any
case, only that the value is > or == 0 on success (but == -1 on
failure) is significant to libraw1394 client applications.

This mismatch between documentation and implementation was already
present in older libraw1394 versions, including v1.x.  For the time
being, do not change the implementation, only adjust the documentation.

Reported-by: Dr. David Alan Gilbert
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-02-01 22:10:12 +01:00
Stefan Richter 6b7b3cbc1e Fix "make doc".
Reported by Guus Sliepen:  "make doc" failed due to missing doctype,
unknown elements, and duplicate element IDs in libraw1394.sgml.

The fix is to declare a recent DTD (matching the one which is used
in current Linux kernel documentation docbooks) and to make the
conflicting element IDs unique.

The latter part of the fix is just temporary.  In order to avoid the
conflict when the documentation is updated the next time, also fix the
kerneldoc comments of the respective API elements:  These are typedefs,
hence kernel-doc needs their comments prepended by "typedef ".

Tested with Gentoo's docbook-xml-dtd 4.5, docbook-xsl-stylesheets
1.75.2, docbook-sgml-utils 0.6.14, and openjade 1.3.2-r1.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Dan Dennedy <dan@dennedy.org>
2010-01-10 14:47:44 -08:00
Stefan Richter 4c4d62f248 Addendum to 'Calculate iso receive cycles on firewire-core'
The number of packets is a 4th of the number of header bytes (in case of
ABI version 1).  Also, wrap after an increment over 8000.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-11-23 21:41:02 +01:00
Jay Fenlason 72e5368aed Initialize unused fields in ioctl arguments
This change is essentially cosmetic:  Set fields of structs passed to
the kernel via ioctl so that valgrind will not complain about them.

Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (changelog, comments)
2009-11-22 23:34:47 +01:00
Jay Fenlason 4e2fd98144 Calculate iso receive cycles on firewire-core at ABI version 1
More accurately report the cycle on which isochronous packets were
received.  Only affects libraw1394 when used with kernel 2.6.29 or
older.

Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (changelog, whitespace)
2009-11-22 23:17:49 +01:00
Jay Fenlason ce82d255ef Fix reporting of isochronous transmit cycles on firewire-core
While firewire-core's iso reception ABI was fixed in its version 2 to
report the cycle of each received packet to userspace like rawiso does,
this same enhancement was forgotten to add to the iso transmission ABI,
causing FFADO to fail to set up and maintain streaming.

Since kernel commit 31769cef2e973544164aa7d0db2e2024660d5e21, we also
get iso xmit cycles in fw_cdev_event_iso_interrupt.header.  Pass these
to the iso receive handler.  In case of older kernels, calculate cycles
based on the cycle of the iso interrupt event.  These are inaccurate but
better than nothing.

Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (changelog, whitespace)
2009-11-22 22:42:16 +01:00
Jay Fenlason 1fb09ead37 Fix default isochronous IRQ interval on firewire-core
libraw1394 takes a negative IRQ interval to mean "every 256 packets"
with the juju backend, which doesn't work well if you don't queue that
many.  Use buf_packets / 4 like the ieee1394 version.

Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (order, comment)
2009-11-22 20:55:34 +01:00
Dan Dennedy cc16f4ddbe Fix build always expecting FW_DIR.
Date: Sun, 14 Jun 2009 11:51:33 +0100
From: Mike Auty <mike.auty@gmail.com>
Subject: [patch libraw1394-2] src/makefile.am expects FW_DIR to be set,
but configure only sets it if given --with-fw-dir

Here's a very small patch for the configure system of
libraw1394-2.0.{0,1,2}.  At the moment, if configure is called without
--with-fw-dir, then FW_DIR doesn't get specified.  The Makefile includes
the line INCLUDES=-I$(FW_DIR) and so in the compilation we get a -I not
followed by anything sensible.  That can cause compilation issues in
certain circumstances (see Gentoo bug 272540), so this patch ensures
that INCLUDES is only set if --with-fw-dir was specified.

Please let me know if there's any problems with the patch or if I've
submitted it to the wrong place or in the wrong way.  Thanks...

Mike  5:)

[1] http://bugs.gentoo.org/272540

Signed-off-by: Dan Dennedy <dan@dennedy.org>
2009-06-18 23:15:19 -07:00
Stefan Richter 5824e34d08 Match only /dev/fw[0-9]* as firewire-core device files
Previously, /dev/fw* and hence files like /dev/fwmonitor were probed
which may have bad effects if the client runs with access privileges.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-05-31 00:30:33 +02:00
Stefan Richter 489981d4ae Remove an unused struct member
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-05-30 14:16:12 +02:00
Stefan Richter caf94aaeab Fix memory leaks with async requests on firewire-core
Each request allocated a struct request_closure which was never freed.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-05-30 14:11:27 +02:00
Stefan Richter efb814334e Use new async stream ioctl
This implements asynchronous streams on juju, i.e. enables
raw1394_async_stream() and raw1394_start_async_stream() to work
with the new firewire kernel stack.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-05-30 13:18:59 +02:00
Stefan Richter 49dda1ef90 Iso reception: Use packet timestamps in juju ABI v2
In the firewire-cdev ABI v1, the kernel exported only the timestamp
of interrupt packets.  libraw1394 estimated the cycle of all packets
between interrupt packets by continuously incrementing the cycle.

In v2 of the ABI, we can obtain an accurate timestamp of each packet
as provided by the OHCI controller.  AFAIU, this is also what you got
from raw1394/ ohci1394.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-05-30 10:17:43 +02:00
Stefan Richter c58e16442b Use new iso resource allocation ioctls
This allows raw1394_bandwidth_modify() and raw1394_channel_modify()
to work on juju without write access to the IRM's character device file.

If either the build-time requirement of firewire-cdev header ABI >= v.2
or the runtime requirement of firewire-core ABI >= v.2 is not satisfied,
the code falls back to transactions to the IRM as before.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-05-30 10:17:43 +02:00
Stefan Richter da5156af5a Use new broadcast request ioctl
This implements broadcast transactions on juju.
(Broadcast transactions are write transactions to PHY ID 63,
not to be confused with isochronous or asynchronous streams.)

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-05-30 10:17:43 +02:00
Dan Dennedy 775c2d6275 Change the license of the "juju" fw*.[hc] files to LGPL v2.1 as approved
by Kristian Hogsberg in an e-mail to the linux1394-devel mailing list
on Feb 3, 2009.

Signed-off-by: Dan Dennedy <dan@dennedy.org>
2009-02-03 09:56:26 -08:00
Stefan Richter 8b17c2a49f Set errno = ENOSYS in unimplemented functions
Most of them do this already, only a few missed it.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Dan Dennedy <dan@dennedy.org>
2009-01-12 20:15:21 -08:00
Stefan Richter a59532c835 Work without permission to access local node's /dev/fw*
On 10 Jan, David Moore wrote:
> On Sat, 2009-01-10 at 19:28 +0100, Stefan Richter wrote:
>> @@ -161,14 +160,16 @@ scan_devices(fw_handle_t handle)
...
>> +		for (j = 0; j < i; j++)
>> +			if (ports[j].card == get_info.card)
>> +				continue;
>> +
>
> That continue statement doesn't do what you intended I think.

From: Stefan Richter <stefanr@s5r6.in-berlin.de>
Subject: [PATCH] Work without permission to access local node's /dev/fw*

Fix for juju backend:

libraw1394 required write permission to the character device file of
the local node(s) in order to enumerate cards and for a number of
other operations.  This forced users to either run applications like
dvgrab and kino with elevated privileges, or to configure write
permission for all /dev/fw* or at least for local nodes' /dev/fw*.

We now use the first accessible file which was found for each card
for as many tasks as possible, instead of the local node's file.

This allows distributors or admins to implement stricter access
rights (default off, e.g. only on for AV/C and IIDC devices)
without sacrificing functionality of said class of applications.
Access to the local node is now only required by low-level tools
like gscanbus.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Dan Dennedy <dan@dennedy.org>
2009-01-10 16:50:28 -08:00
David Moore cb8b7bf86a Fix stack corruption during juju lock transactions
When performing a lock transaction (such as with fw_lock) under Juju, 4
bytes of the stack gets corrupted.  This is because the lock transaction
has 8 bytes of data sent and 4 bytes received.  Since the transaction
"length" is specified as 8, handle_device_event() copies 8 bytes into
the destination variable instead of the desired 4, and overflows into
the stack by 4 bytes.

This patch fixes the corruption by adding an extra "out_length" argument
to the send_request() function so that both in_length and out_length can
be specified separately.

Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-12-29 11:12:32 -08:00
Jarod Wilson d69397ae8f Fix iso_shutdown with juju firewire stack
Make iso start/stop/start sequences on the same handle, such as those used
by apps such as MythTV behave as expected. I can finally watch video off my
cable box over FireWire using MythTV w/the juju stack now. :)

Initially, seemed a one-liner might be the ticket (setting handle->iso.fd = -1
at the end of fw_iso_shutdown()), but that led to memory corruption and a
locked up system. What ultimately worked was essentially mimicking what the
old stack did to track iso state, and call fw_iso_stop() from
fw_iso_shutdown() as needed.

Nb: Only lightly tested with iso receive via MythTV, but its all fairly
straight-forward, I think.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-12-08 21:13:41 -08:00
Dan Dennedy 4904ff698d Let fw_read_cycle_timer() use the local fd instead of requiring iso to be initialized
Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-12-07 23:02:27 -08:00
Stefan Richter fbe1fa46c6 [libraw1394 patch] Unify {ieee1394,fw}_bandwidth_modify()
because they do the same.

We only may want a separate fw_bandwidth_modify() in the future when
firewire-core gains a special ioctl() for that.

(Not runtime-tested, but it looks good to me.)

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-12-03 19:56:36 -08:00
Stefan Richter 788442d403 [libraw1394 patch] Fix raw1394_channel_modify() on firewire-core
(juju)

Reported by Adrian Knoth:  fw_channel_modify() was unable to allocate
some channels which were actually free.
http://marc.info/?l=linux1394-devel&t=122818128900002

This can be easily fixed by replacing fw_channel_modify() by
ieee1394_channel_modify() because this is highlevel enough to work with
Juju as well.  We only may want a separate fw_channel_modify() in the
future when firewire-core gains a special ioctl() for that.

Also fix a documentation typo:  raw1394_channel_modify() did not show up
in extracted API documentation due to a cut'n'paste typo in raw1394.h.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-12-03 19:56:32 -08:00
Stefan Richter 7b8d270352 Fix segfault in juju's handle_arm_request
The buffer pointers were uninitialized, leading to segfault in memcpy.
Bug report and initial version of the fix by Adrian Knoth.

Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-10-28 22:56:48 -07:00
Stefan Richter 0bf8132319 Reduce nesting depth in new_handle dispatchers
Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-10-28 22:56:32 -07:00
Stefan Richter d080bb3d83 Address some compiler warnings
7x unused variable, 1x assignment used as truth value, 1x pointer signedness

Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-10-28 22:56:09 -07:00
Erik Hovland 531b0787f1 Makes extra sure strings are not overrun.
When using strncpy with the exact size of the destination string the
string may end up lacking null termination because the source string is
bigger then the destination.

Signed-off-by: Erik Hovland <erik@hovland.org>
Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-10-28 22:52:41 -07:00
Erik Hovland 63b9d38820 Makes sure a value is returned by the function.
A function can compile without returning something always.

Signed-off-by: Erik Hovland <erik@hovland.org>
Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-10-28 22:52:34 -07:00
Erik Hovland 038f5f5c98 Make sure that we have the right types.
When an unsigned type is assigned a signed value, the
negatived value is never seen.

Signed-off-by: Erik Hovland <erik@hovland.org>
Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-10-28 22:52:32 -07:00
Erik Hovland 8ad324c971 Compare unsigned values instead of subtracting them.
Unsigned values do not return signed values when subtracted
and the right operand is larger then the left operand.

Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-10-28 22:52:29 -07:00
Erik Hovland 709d8791ce Protect against resource leaks.
Signed-off-by: Erik Hovland <erik@hovland.org>
Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-10-28 22:52:06 -07:00
Erik Hovland 378b935fa3 Make sure variables are initialized before used.
Signed-off-by: Erik Hovland <erik@hovland.org>
Signed-off-by: Dan Dennedy <dan@dennedy.org>
2008-10-28 22:52:04 -07:00
Dan Dennedy 6747abb057 dispatch.c: fix a memory leak on not deleting the raw1394 wrapper handle. 2008-07-21 22:01:03 -07:00
Dan Dennedy 8f942654d9 Change handle validation to prevent segfault and be more informative. 2008-07-06 12:06:40 -07:00
Stefan Richter 4ce429a797 Fix raw1394_read_cycle_timer after juju integration
The ieee1394 version of raw1394_read_cycle_timer() fell over the cliff
in "First cut at integrating juju".  This brings it back and adds a juju
version of it.

Also correct a typo in the inline documentation: s/get/read/
2008-07-05 13:16:30 -07:00
Philippe Troin 477b6eee6d Plug dir leak and initialize data structs
While trying to track down some crashes in kino, I found the following problems
with libraw1394:

 * There is a DIR* leak in raw1394_set_port().
 * Lots of data structures are not fully initialized when calling IEEE1394
   ioctl()s.  These cause valgrind errors (benign, as valgrind does not know
   how to interpret all ioctls.  However these also cause kino to crash in
   libraw1394.  I've added a bunch of memset()s to prevent this problem from
   happening.

Forward-ported to libraw1394 git tree by Jarod Wilson.
2008-07-05 13:09:29 -07:00
Dan Dennedy f9681ff59d Move the source code files in the juju directory into the src directory and
give them 'fw' names instead of 'juju.'
2008-04-30 14:37:28 -07:00
Dan Dennedy 3b4d2eed60 libraw1394.sgml, raw1394.h: remove information about deprecated isochronous
API that has been removed.
2008-04-29 23:09:22 -07:00
Dan Dennedy 0ea462ae98 First cut at integrating juju
This is currently working with legacy ieee1394 and tools/testlibraw.
2008-04-23 17:44:36 -07:00