14 Dec, 2010

1 commit

  • Change the header of PHY packets to be sent to include a pseudo
    transaction code. This makes the header consistent with that of
    received PHY packets, and allows at_context_queue_packet() and
    log_ar_at_event() to see the packet type directly instead of having
    to deduce it from the header length or even from the header contents.

    Signed-off-by: Clemens Ladisch
    Signed-off-by: Stefan Richter

    Clemens Ladisch
     

02 Aug, 2010

1 commit


30 Jul, 2010

3 commits

  • This adds the DMA context programming and userspace ABI for multichannel
    reception, i.e. for listening on multiple channel numbers by means of a
    single DMA context.

    The use case is reception of more streams than there are IR DMA units
    offered by the link layer. This is already implemented by the older
    ohci1394 + ieee1394 + raw1394 stack. And as discussed recently on
    linux1394-devel, this feature is occasionally used in practice.

    The big drawbacks of this mode are that buffer layout and interrupt
    generation necessarily differ from single-channel reception: Headers
    and trailers are not stripped from packets, packets are not aligned with
    buffer chunks, interrupts are per buffer chunk, not per packet.

    These drawbacks also cause a rather hefty code footprint to support this
    rarely used OHCI-1394 feature. (367 lines added, among them 94 lines of
    added userspace ABI documentation.)

    This implementation enforces that a multichannel reception context may
    only listen to channels to which no single-channel context on the same
    link layer is presently listening to. OHCI-1394 would allow to overlay
    single-channel contexts by the multi-channel context, but this would be
    a departure from the present first-come-first-served policy of IR
    context creation.

    The implementation is heavily based on an earlier one by Jay Fenlason.
    Thanks Jay.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • Make a note on the seemingly unused linux/sched.h.
    Rename an irritatingly named variable.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • ioctl_create_iso_context enforces ctx->header_size >= 4.

    Signed-off-by: Stefan Richter

    Stefan Richter
     

23 Jul, 2010

5 commits

  • In both the ieee1394 stack and the firewire stack, the core treats
    kernelspace drivers better than userspace drivers when it comes to
    CSR address range allocation: The former may request a register to be
    placed automatically at a free spot anywhere inside a specified address
    range. The latter may only request a register at a fixed offset.

    Hence, userspace drivers which do not require a fixed offset potentially
    need to implement a retry loop with incremented offset in each retry
    until the kernel does not fail allocation with EBUSY. This awkward
    procedure is not fundamentally necessary as the core already provides a
    superior allocation API to kernelspace drivers.

    Therefore change the ioctl() ABI by addition of a region_end member in
    the existing struct fw_cdev_allocate. Userspace and kernelspace APIs
    work the same way now.

    There is a small cost to pay by clients though: If client source code
    is required to compile with older kernel headers too, then any use of
    the new member fw_cdev_allocate.region_end needs to be enclosed by
    #ifdef/#endif directives. However, any client program that seriously
    wants to use address range allocations will require a kernel of cdev ABI
    version >= 4 at runtime and a linux/firewire-cdev.h header of >= 4
    anyway. This is because v4 brings FW_CDEV_EVENT_REQUEST2. The only
    client program in which build-time compatibility with struct
    fw_cdev_allocate as found in older kernel headers makes sense is
    libraw1394.

    (libraw1394 uses the older broken FW_CDEV_EVENT_REQUEST to implement a
    makeshift, incorrect transaction responder that does at least work
    somewhat in many simple scenarios, relying on guesswork by libraw1394
    and by libraw1394 based applications. Plus, address range allocation
    and transaction responder is only one of many features that libraw1394
    needs to provide, and these other features need to work with kernel and
    kernel-headers as old as possible. Any new linux/firewire-cdev.h based
    client that implements a transaction responder should never attempt to
    do it like libraw1394; instead it should make a header and kernel of v4
    or later a hard requirement.)

    While we are at it, update the struct fw_cdev_allocate documentation to
    better reflect the recent fw_cdev_event_request2 ABI addition.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • This extends the FW_CDEV_IOC_SEND_PHY_PACKET ioctl() for /dev/fw* to be
    useful for ping time measurements. One application for it would be gap
    count optimization in userspace that is based on ping times rather than
    hop count. (The latter is implemented in firewire-core itself but is
    not applicable to beta PHYs that act as repeater.)

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • Add an FW_CDEV_IOC_RECEIVE_PHY_PACKETS ioctl() and
    FW_CDEV_EVENT_PHY_PACKET_RECEIVED poll()/read() event for /dev/fw*.
    This can be used to get information from remote PHYs by remote access
    PHY packets.

    This is also the 2nd half of the functionality (the receive part) to
    support a userspace implementation of a VersaPHY transaction layer.

    Safety considerations:

    - PHY packets are generally broadcasts, hence some kind of elevated
    privileges should be required of a process to be able to listen in
    on PHY packets. This implementation assumes that a process that is
    allowed to open the /dev/fw* of a local node does have this
    privilege.

    There was an inconclusive discussion about introducing POSIX
    capabilities as a means to check for user privileges for these
    kinds of operations.

    Other limitations:

    - PHY packet reception may be switched on by ioctl() but cannot be
    switched off again. It would be trivial to provide an off switch,
    but this is not worth the code. The client should simply close()
    the fd then, or just ignore further events.

    - For sake of simplicity of API and kernel-side implementation, no
    filter per packet content is provided.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • Add an FW_CDEV_IOC_SEND_PHY_PACKET ioctl() for /dev/fw* which can be
    used to implement bus management related functionality in userspace.

    This is also half of the functionality (the transmit part) that is
    needed to support a userspace implementation of a VersaPHY transaction
    layer.

    Safety considerations:

    - PHY packets are generally broadcasts and may have interesting
    effects on PHYs and the bus, e.g. make asynchronous arbitration
    impossible due to too low gap count. Hence some kind of elevated
    privileges should be required of a process to be able to send
    PHY packets. This implementation assumes that a process that is
    allowed to open the /dev/fw* of a local node does have this
    privilege.

    There was an inconclusive discussion about introducing POSIX
    capabilities as a means to check for user privileges for these
    kinds of operations.

    - The kernel does not check integrity of the supplied packet data.
    That would be far too much code, considering the many kinds of
    PHY packets. A process which got the privilege to send these
    packets is trusted to do it correctly.

    Just like with the other "send packet" ioctls, a non-blocking API is
    chosen; i.e. the ioctl may return even before AT DMA started. After
    transmission, an event for poll()/read() is enqueued. Most users are
    going to need a blocking API, but a blocking userspace wrapper is easy
    to implement, and the second of the two existing libraw1394 calls
    raw1394_phy_packet_write() and raw1394_start_phy_packet_write() can be
    better supported that way.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • to make the correspondence of ioctl numbers and handlers more obvious.

    Signed-off-by: Stefan Richter

    Stefan Richter
     

13 Jul, 2010

3 commits

  • Bus resets which are triggered
    - by the kernel drivers after updates of the local nodes' config ROM,
    - by userspace software via ioctl
    shall be deferred until after >=2 seconds after the last bus reset.

    If multiple modifications of the local nodes' config ROM happen in a row,
    only a single bus reset should happen after them.

    When the local node's link goes from inactive to active or vice versa,
    and at the two occasions of bus resets mentioned above --- and if the
    current gap count differs from 63 --- the bus reset should be preceded
    by a PHY configuration packet that reaffirms the gap count. Otherwise a
    bus manager would have to reset the bus again right after that.

    This is necessary to promote bus stability, e.g. leave grace periods for
    allocations and reallocations of isochronous channels and bandwidth,
    SBP-2 reconnections etc.; see IEEE 1394 clause 8.2.1.

    This change implements all of the above by moving bus reset initiation
    into a delayed work (except for bus resets which are triggered by the
    bus manager workqueue job and are performed there immediately). It
    comes with a necessary addition to the card driver methods that allows
    to get the current gap count from PHY registers.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • The FW_ISO_ constants of the in-kernel API of firewire-core and
    FW_CDEV_ISO_ constants of the userspace API of firewire-core have
    nothing to do with each other --- except that the core-cdev.c
    implementation relies on them having the same values.

    Hence put some compile-time assertions into core-cdev.c. It's lame but
    I prefer it over including the userspace API header into the kernelspace
    API header and defining kernelspace API constants from userspace API
    constants. Nor do I want to expose the kernelspace constants in one of
    the two firewire headers that are exported to userland since this only
    concerns the core-cdev.c implementation.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • Check that the data length of a write quadlet request actually is large
    enough for a quadlet. Otherwise, fw_fill_request could access the four
    bytes after the end of the outbound_transaction_event structure.

    Signed-off-by: Clemens Ladisch

    Modification of Clemens' change: Consolidate the check into
    init_request() which is used by the affected ioctl_send_request() and
    ioctl_send_broadcast_request() and the unaffected
    ioctl_send_stream_packet(), to save a few lines of code.

    Note, since struct outbound_transaction_event *e is slab-allocated, such
    an out-of-bounds access won't hit unallocated memory but may result in a
    (virtually impossible to exploit) information disclosure.

    Signed-off-by: Stefan Richter

    Clemens Ladisch
     

08 Jul, 2010

1 commit


21 Jun, 2010

6 commits

  • The problem:

    A target-like userspace driver, e.g. AV/C target or SBP-2/3 target,
    needs to be able to act as responder and requester. In the latter role,
    it needs to send requests to nods from which it received requests. This
    is currently impossible because fw_cdev_event_request lacks information
    about sender node ID.
    Reported-by: Jay Fenlason

    Libffado + libraw1394 + firewire-core is currently unable to drive two
    or more audio devices on the same bus.
    Reported-by: Arnold Krille

    This is because libffado requires destination node ID of FCP requests
    and sender node ID of FCP responses to match. It even prohibits
    libffado from working with a bus on which libraw1394 opens a /dev/fw* as
    default ioctl device that does not correspond with the audio device.
    This is because libraw1394 does not receive the sender node ID from the
    kernel.

    Moreover, fw_cdev_event_request makes it impossible to tell unicast and
    broadcast write requests apart.

    The fix:

    Add a replacement of struct fw_cdev_event_request request, boringly
    called struct fw_cdev_event_request2. The new event will be sent to a
    userspace client instead of the old one if the client claims
    compatibility with ABI version 4 or later.

    libraw1394 needs to be extended to make use of the new event, in order
    to properly support libffado and other FCP or address range mapping
    users who require correct sender node IDs.

    Further notes:

    While we are at it, change back the range of possible values of
    fw_cdev_event_request.tcode to 0x0...0xb like in ABI version

    Stefan Richter
     
  • libraw1394 v2.0.0...v2.0.5 takes FW_CDEV_VERSION from an externally
    installed header file and uses it to declare its own implementation
    level in FW_CDEV_IOC_GET_INFO. This is wrong; it should set the real
    version for which it was actually written.

    If we add features to the kernel ABI that require the kernel to check
    a client's implementation level, we can not trust the client version if
    it was set from FW_CDEV_VERSION.

    Hence freeze FW_CDEV_VERSION at the current value (no damage has been
    done yet), clearly document FW_CDEV_VERSION as a dummy version and what
    clients are expected to do with fw_cdev_get_info.version, and use a new
    defined constant (which is not placed into the exported header file) as
    kernel implementation level.

    Note, in order to check in client program source code which features are
    present in an externally installed linux/firewire-cdev.h, use
    preprocessor directives like
    #ifdef FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE
    or
    #ifdef FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED
    instead of a check of FW_CDEV_VERSION.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • If a request comes in to an address range managed by a userspace driver
    i.e. client, the card instance of request and
    response may differ from the card instance of the client device.
    Therefore we need to take a reference of the card until the response was
    sent.

    I thought about putting the reference counting into core-transaction.c,
    but the various high-level drivers besides cdev clients (firewire-net,
    firewire-sbp2, firedtv) use the card pointer in their fw_address_handler
    address_callback method only to look up devices of which they already
    hold the necessary references. So this seems to be a specific
    firewire-cdev issue which is better addressed locally.

    We do not need the reference
    - in case of FCP_REQUEST or FCP_RESPONSE requests because then the
    firewire-core will send the split transaction response for us
    already in the context of the request handler,
    - if it is the same card as the client device's because we hold a
    card reference indirectly via teh client->device reference.
    To keep things simple, we take the reference nevertheless.

    Jay Fenlason wrote:
    > there's no way for the core to tell cdev "this card is gone,
    > kill any inbound transactions on it", while cdev holds the transaction
    > open until userspace issues a SEND_RESPONSE ioctl, which may be a very,
    > very long time. But when it does, it calls fw_send_response(), which
    > will dereference the card...
    >
    > So how unhappy are we about userspace potentially holding a fw_card
    > open forever?

    While termination of inbound transcations at card removal could be
    implemented, it is IMO not worth the effort. Currently, the effect of
    holding a reference of a card that has been removed is to block the
    process that called the pci_remove of the card. This is
    - either a user process ran by root. Root can find and kill processes
    that have /dev/fw* open, if desired.
    - a kernel thread (which one?) in case of hot removal of a PCCard or
    ExpressCard.
    The latter case could be a problem indeed. firewire-core's card
    shutdown and card release should probably be improved not to block in
    shutdown, just to defer freeing of memory until release.

    This is not a new problem though; the same already always happens with
    the client->device->card without the need of inbound transactions or
    other special conditions involved, other than the client not closing the
    file.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • My box has two firewire cards in it: card0 and card1.
    My application opens /dev/fw0 (card 0) and allocates an address space.
    The core makes the address space available on both cards.
    Along comes the remote device, which sends a READ_QUADLET_REQUEST to
    card1. The request gets passed up to my application, which calls
    ioctl_send_response().

    ioctl_send_response() then calls fw_send_response() with card0,
    because that's the card it's bound to.
    Card0's driver drops the response, because it isn't part of
    a transaction that it has outstanding.

    So in core-cdev: handle_request(), we need to stash the
    card of the inbound request in the struct inbound_transaction_resource and
    use that card to send the response to.

    The hard part will be refcounting the card correctly
    so it can't get deallocated while we hold a pointer to it.

    Here's a trivial patch, which does not do the card refcounting, but at
    least demonstrates what the problem is.

    Note that we can't depend on the fact that the core-cdev:client
    structure holds a card open, because in this case the card it holds
    open is not the card the request came in on.

    ..and there's no way for the core to tell cdev "this card is gone,
    kill any inbound transactions on it", while cdev holds the transaction
    open until userspace issues a SEND_RESPONSE ioctl, which may be a very,
    very long time. But when it does, it calls fw_send_response(), which
    will dereference the card...

    So how unhappy are we about userspace potentially holding a fw_card
    open forever?

    Signed-off-by: Jay Fenlason

    Reference counting to be addressed in a separate change.

    Signed-off-by: Stefan Richter (whitespace)

    Jay Fenlason
     
  • Protect the client's iso context pointer against a race that can happen
    when more than one creation call is executed at the same time.

    Signed-off-by: Clemens Ladisch
    Signed-off-by: Stefan Richter

    Clemens Ladisch
     
  • void (*fw_address_callback_t)(..., int speed, ...) is the speed that a
    remote node chose to transmit a request to us. In case of split
    transactions, firewire-core will transmit the response at that speed.

    Upper layer drivers on the other hand (firewire-net, -sbp2, firedtv, and
    userspace drivers) cannot do anything useful with that speed datum,
    except log it for debug purposes. But data that is merely potentially
    (not even actually) used for debug purposes does not belong into the API.

    Signed-off-by: Stefan Richter

    Stefan Richter
     

20 Jun, 2010

1 commit


19 Jun, 2010

1 commit


10 Jun, 2010

2 commits


28 May, 2010

1 commit

  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6:
    ieee1394: schedule for removal
    firewire: core: use separate timeout for each transaction
    firewire: core: Fix tlabel exhaustion problem
    firewire: core: make transaction label allocation more robust
    firewire: core: clean up config ROM related defined constants
    ieee1394: mark char device files as not seekable
    firewire: cdev: mark char device files as not seekable
    firewire: ohci: cleanups and fix for nonstandard build without debug facility
    firewire: ohci: wait for PHY register accesses to complete
    firewire: ohci: fix up configuration of TI chips
    firewire: ohci: enable 1394a enhancements
    firewire: ohci: do not clear PHY interrupt status inadvertently
    firewire: ohci: add a function for reading PHY registers

    Trivial conflicts in Documentation/feature-removal-schedule.txt

    Linus Torvalds
     

16 Apr, 2010

1 commit


10 Apr, 2010

4 commits

  • The character device file ABI (i.e. /dev/fw*
    character device file interface) does not make any use of lseek(),
    pread(), pwrite() (or any kind of write() at all).

    Use nonseekable_open() and, redundantly, set file_operations.llseek to
    no_llseek to remove any doubt whether the BKL-grabbing default_llseek
    handler is used. (Also shuffle file_operations initialization according
    to the order of handler definitions.)

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • 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
    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • 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
    Signed-off-by: Stefan Richter

    Clemens Ladisch
     
  • 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
    Signed-off-by: Stefan Richter

    Clemens Ladisch
     

30 Mar, 2010

1 commit

  • …it slab.h inclusion from percpu.h

    percpu.h is included by sched.h and module.h and thus ends up being
    included when building most .c files. percpu.h includes slab.h which
    in turn includes gfp.h making everything defined by the two files
    universally available and complicating inclusion dependencies.

    percpu.h -> slab.h dependency is about to be removed. Prepare for
    this change by updating users of gfp and slab facilities include those
    headers directly instead of assuming availability. As this conversion
    needs to touch large number of source files, the following script is
    used as the basis of conversion.

    http://userweb.kernel.org/~tj/misc/slabh-sweep.py

    The script does the followings.

    * Scan files for gfp and slab usages and update includes such that
    only the necessary includes are there. ie. if only gfp is used,
    gfp.h, if slab is used, slab.h.

    * When the script inserts a new include, it looks at the include
    blocks and try to put the new include such that its order conforms
    to its surrounding. It's put in the include block which contains
    core kernel includes, in the same order that the rest are ordered -
    alphabetical, Christmas tree, rev-Xmas-tree or at the end if there
    doesn't seem to be any matching order.

    * If the script can't find a place to put a new include (mostly
    because the file doesn't have fitting include block), it prints out
    an error message indicating which .h file needs to be added to the
    file.

    The conversion was done in the following steps.

    1. The initial automatic conversion of all .c files updated slightly
    over 4000 files, deleting around 700 includes and adding ~480 gfp.h
    and ~3000 slab.h inclusions. The script emitted errors for ~400
    files.

    2. Each error was manually checked. Some didn't need the inclusion,
    some needed manual addition while adding it to implementation .h or
    embedding .c file was more appropriate for others. This step added
    inclusions to around 150 files.

    3. The script was run again and the output was compared to the edits
    from #2 to make sure no file was left behind.

    4. Several build tests were done and a couple of problems were fixed.
    e.g. lib/decompress_*.c used malloc/free() wrappers around slab
    APIs requiring slab.h to be added manually.

    5. The script was run on all .h files but without automatically
    editing them as sprinkling gfp.h and slab.h inclusions around .h
    files could easily lead to inclusion dependency hell. Most gfp.h
    inclusion directives were ignored as stuff from gfp.h was usually
    wildly available and often used in preprocessor macros. Each
    slab.h inclusion directive was examined and added manually as
    necessary.

    6. percpu.h was updated not to include slab.h.

    7. Build test were done on the following configurations and failures
    were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my
    distributed build env didn't work with gcov compiles) and a few
    more options had to be turned off depending on archs to make things
    build (like ipr on powerpc/64 which failed due to missing writeq).

    * x86 and x86_64 UP and SMP allmodconfig and a custom test config.
    * powerpc and powerpc64 SMP allmodconfig
    * sparc and sparc64 SMP allmodconfig
    * ia64 SMP allmodconfig
    * s390 SMP allmodconfig
    * alpha SMP allmodconfig
    * um on x86_64 SMP allmodconfig

    8. percpu.h modifications were reverted so that it could be applied as
    a separate patch and serve as bisection point.

    Given the fact that I had only a couple of failures from tests on step
    6, I'm fairly confident about the coverage of this conversion patch.
    If there is a breakage, it's likely to be something in one of the arch
    headers which should be easily discoverable easily on most builds of
    the specific arch.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>

    Tejun Heo
     

25 Feb, 2010

4 commits


21 Feb, 2010

2 commits

  • The current implementation of Bus_Time read access was buggy since it
    did not ensure that Bus_Time.second_count_hi and second_count_lo came
    from the same 128 seconds period.

    Reported-by: Håkan Johansson

    Instead of a fix, remove Bus_Time register support altogether. The spec
    requires all cycle master capable nodes to implement this (all Linux
    nodes are cycle master capable) while it also says that it "may" be
    initialized by the bus manager or by the IRM standing in for a bus
    manager. (Neither Linux' firewire-core nor ieee1394 nodemgr implement
    this.)

    Since we cannot rely on Bus_Time having been initialized by a bus
    manager, it is better to return an error instead of a nonsensical value
    on a read request to Bus_Time.

    Alternatively, we could fix the Bus_Time read integrity bug _and_
    implement (a) cycle master's write support of the register as well as
    (b) bus manager's Bus_Time initialization service, i.e. preservation of
    the Bus_Time when the cycle master node of a bus changes. However, that
    would be quite some code for a feature that is unreliable to begin with
    and very likely unused in practice.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • ohci: Break out of the retry loop if too many attempts were necessary.
    This may theoretically happen if the chip is fatally defective or if the
    get_cycle_timer ioctl was performed after a CardBus controller was
    ejected.

    Also micro-optimize the loop by re-using the last two register reads in
    the next iteration, remove a questionable inline keyword, and shuffle a
    comment around.

    core: ioctl_get_cycle_timer() is always called with interrupts on,
    therefore local_irq_save() can be replaced by local_irq_disable().
    Disabled local IRQs imply disabled preemption, hence preempt_disable()
    can be removed.

    Signed-off-by: Stefan Richter

    Stefan Richter
     

27 Jan, 2010

1 commit

  • 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

    Stefan Richter
     

30 Dec, 2009

2 commits

  • 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

    Stefan Richter
     
  • 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
    Signed-off-by: Stefan Richter (changelog, rebased, whitespace)

    Clemens Ladisch