12 Aug, 2011

1 commit

  • Clemens points out that we need to use compat_ptr() in order to safely
    cast from u64 to addresses of a 32-bit usermode client.

    Before, our conversion went wrong
    - in practice if the client cast from pointer to integer such that
    sign-extension happened, (libraw1394 and libdc1394 at least were not
    doing that, IOW were not affected)
    or
    - in theory on s390 (which doesn't have FireWire though) and on the
    tile architecture, regardless of what the client does.
    The bug would usually be observed as the initial get_info ioctl failing
    with "Bad address" (EFAULT).

    Reported-by: Carl Karsten
    Reported-by: Clemens Ladisch
    Signed-off-by: Stefan Richter

    Stefan Richter
     

16 Jul, 2011

2 commits

  • Between open(2) of a /dev/fw* and the first FW_CDEV_IOC_GET_INFO
    ioctl(2) on it, the kernel already queues FW_CDEV_EVENT_BUS_RESET events
    to be read(2) by the client. The get_info ioctl is practically always
    issued right away after open, hence this condition only occurs if the
    client opens during a bus reset, especially during a rapid series of bus
    resets.

    The problem with this condition is twofold:

    - These bus reset events carry the (as yet undocumented) @closure
    value of 0. But it is not the kernel's place to choose closures;
    they are privat to the client. E.g., this 0 value forced from the
    kernel makes it unsafe for clients to dereference it as a pointer to
    a closure object without NULL pointer check.

    - It is impossible for clients to determine the relative order of bus
    reset events from get_info ioctl(2) versus those from read(2),
    except in one way: By comparison of closure values. Again, such a
    procedure imposes complexity on clients and reduces freedom in use
    of the bus reset closure.

    So, change the ABI to suppress queuing of bus reset events before the
    first FW_CDEV_IOC_GET_INFO ioctl was issued by the client.

    Note, this ABI change cannot be version-controlled. The kernel cannot
    distinguish old from new clients before the first FW_CDEV_IOC_GET_INFO
    ioctl.

    We will try to back-merge this change into currently maintained stable/
    longterm series, and we only document the new behaviour. The old
    behavior is now considered a kernel bug, which it basically is.

    Signed-off-by: Stefan Richter
    Cc:

    Stefan Richter
     
  • On Jun 27 Linus Torvalds wrote:
    > The correct error code for "I don't understand this ioctl" is ENOTTY.
    > The naming may be odd, but you should think of that error value as a
    > "unrecognized ioctl number, you're feeding me random numbers that I
    > don't understand and I assume for historical reasons that you tried to
    > do some tty operation on me".
    [...]
    > The EINVAL thing goes way back, and is a disaster. It predates Linux
    > itself, as far as I can tell. You'll find lots of man-pages that have
    > this line in it:
    >
    > EINVAL Request or argp is not valid.
    >
    > and it shows up in POSIX etc. And sadly, it generally shows up
    > _before_ the line that says
    >
    > ENOTTY The specified request does not apply to the kind of object
    > that the descriptor d references.
    >
    > so a lot of people get to the EINVAL, and never even notice the ENOTTY.
    [...]
    > At least glibc (and hopefully other C libraries) use a _string_ that
    > makes much more sense: strerror(ENOTTY) is "Inappropriate ioctl for
    > device"

    So let's correct this in the ABI while it is
    still young, relative to distributor adoption.

    Side note: We return -ENOTTY not only on _IOC_TYPE or _IOC_NR mismatch,
    but also on _IOC_SIZE mismatch. An ioctl with an unsupported size of
    argument structure can be seen as an unsupported version of that ioctl.

    Signed-off-by: Stefan Richter
    Cc:

    Stefan Richter
     

11 May, 2011

4 commits

  • The struct sbp2_logical_unit.work items can all be executed in parallel
    but are not reentrant. Furthermore, reconnect or re-login work must be
    executed in a WQ_MEM_RECLAIM workqueue.

    Hence replace the old single-threaded firewire-sbp2 workqueue by a
    concurrency-managed but non-reentrant workqueue with rescuer.
    firewire-core already maintains one, hence use this one.

    In earlier versions of this change, I observed occasional failures of
    parallel INQUIRY to an Initio INIC-2430 FireWire 800 to dual IDE bridge.
    More testing indicates that parallel INQUIRY is not actually a problem,
    but too quick successions of logout and login + INQUIRY, e.g. a quick
    sequence of cable plugout and plugin, can result in failed INQUIRY.
    This does not seem to be something that should or could be addressed by
    serialization.

    Another dual-LU device to which I currently have access to, an
    OXUF924DSB FireWire 800 to dual SATA bridge with firmware from MacPower,
    has been successfully tested with this too.

    This change is beneficial to environments with two or more FireWire
    storage devices, especially if they are located on the same bus.
    Management tasks that should be performed as soon and as quickly as
    possible, especially reconnect, are no longer held up by tasks on other
    devices that may take a long time, especially login with INQUIRY and sd
    or sr driver probe.

    Signed-off-by: Stefan Richter

    Stefan Richter
     
  • firewire-core manages the following types of work items:

    fw_card.br_work:
    - resets the bus on a card and possibly sends a PHY packet before that
    - does not sleep for long or not at all
    - is scheduled via fw_schedule_bus_reset() by
    - firewire-ohci's pci_probe method
    - firewire-ohci's set_config_rom method, called by kernelspace
    protocol drivers and userspace drivers which add/remove
    Configuration ROM descriptors
    - userspace drivers which use the bus reset ioctl
    - itself if the last reset happened less than 2 seconds ago

    fw_card.bm_work:
    - performs bus management duties
    - usually does not (but may in corner cases) sleep for long
    - is scheduled via fw_schedule_bm_work() by
    - firewire-ohci's self-ID-complete IRQ handler tasklet
    - firewire-core's fw_device.work instances whenever the root node
    device was (successfully or unsuccessfully) discovered,
    refreshed, or rediscovered
    - itself in case of resource allocation failures or in order to
    obey the 125ms bus manager arbitration interval

    fw_device.work:
    - performs node probe, update, shutdown, revival, removal; including
    kernel driver probe, update, shutdown and bus reset notification to
    userspace drivers
    - usually sleeps moderately long, in corner cases very long
    - is scheduled by
    - firewire-ohci's self-ID-complete IRQ handler tasklet via the
    core's fw_node_event
    - firewire-ohci's pci_remove method via core's fw_destroy_nodes/
    fw_node_event
    - itself during retries, e.g. while a node is powering up

    iso_resource.work:
    - accesses registers at the Isochronous Resource Manager node
    - usually does not (but may in corner cases) sleep for long
    - is scheduled via schedule_iso_resource() by
    - the owning userspace driver at addition and removal of the
    resource
    - firewire-core's fw_device.work instances after bus reset
    - itself in case of resource allocation if necessary to obey the
    1000ms reallocation period after bus reset

    fw_card.br_work instances should not, and instances of the others must
    not, be executed in parallel by multiple CPUs -- but were not protected
    against that. Hence allocate a non-reentrant workqueue for them.

    fw_device.work may be used in the memory reclaim path in case of SBP-2
    device updates. Hence we need a workqueue with rescuer and cannot use
    system_nrt_wq.

    Signed-off-by: Stefan Richter
    Reviewed-by: Tejun Heo

    Stefan Richter
     
  • When queueing iso packets, the run time is dominated by the two
    MMIO accesses that set the DMA context's wake bit. Because most
    drivers submit packets in batches, we can save much time by
    removing all but the last wakeup.

    The internal kernel API is changed to require a call to
    fw_iso_context_queue_flush() after a batch of queued packets.
    The user space API does not change, so one call to
    FW_CDEV_IOC_QUEUE_ISO must specify multiple packets to take
    advantage of this optimization.

    In my measurements, this patch reduces the time needed to queue
    fifty skip packets from userspace to one sixth on a 2.5 GHz CPU,
    or to one third at 800 MHz.

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

    Clemens Ladisch
     
  • We do not need slab allocations anymore in order to satisfy
    streaming DMA mapping constraints, thanks to commit da28947e7e36
    "firewire: ohci: avoid separate DMA mapping for small AT payloads".

    (Besides, the slab-allocated buffers that firewire-core, firewire-sbp2,
    and firedtv used to provide for 8-byte write and lock requests were
    still not fully portable since they crossed cacheline boundaries or
    shared a cacheline with unrelated CPU-accessed data. snd-firewire-lib
    got this aspect right by using an extra kmalloc/ kfree just for the
    8-byte transaction buffer.)

    This change replaces kmalloc'ed lock transaction scratch buffers in
    firewire-core, firedtv, and snd-firewire-lib by local stack allocations.
    Perhaps the most notable result of the change is simpler locking because
    there is no need to serialize usages of preallocated per-device buffers
    anymore. Also, allocations and deallocations are simpler.

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

    Stefan Richter
     

23 Jan, 2011

4 commits


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

3 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