29 Sep, 2018

1 commit

  • commit 674d9de02aa7d521ebdf66c3958758bdd9c64e11 upstream.

    When handling SHDLC I-Frame commands "pipe" field used for indexing
    into an array should be checked before usage. If left unchecked it
    might access memory outside of the array of size NFC_HCI_MAX_PIPES(127).

    Malformed NFC HCI frames could be injected by a malicious NFC device
    communicating with the device being attacked (remote attack vector),
    or even by an attacker with physical access to the I2C bus such that
    they could influence the data transfers on that bus (local attack vector).
    skb->data is controlled by the attacker and has only been sanitized in
    the most trivial ways (CRC check), therefore we can consider the
    create_info struct and all of its members to tainted. 'create_info->pipe'
    with max value of 255 (uint8) is used to take an offset of the
    hdev->pipes array of 127 elements which can lead to OOB write.

    Cc: Samuel Ortiz
    Cc: Allen Pais
    Cc: "David S. Miller"
    Suggested-by: Kevin Deus
    Signed-off-by: Suren Baghdasaryan
    Acked-by: Kees Cook
    Cc: stable
    Signed-off-by: David S. Miller
    Signed-off-by: Greg Kroah-Hartman

    Suren Baghdasaryan
     

22 Jul, 2018

1 commit

  • commit 3bc53be9db21040b5d2de4d455f023c8c494aa68 upstream.

    syzbot is reporting stalls at nfc_llcp_send_ui_frame() [1]. This is
    because nfc_llcp_send_ui_frame() is retrying the loop without any delay
    when nonblocking nfc_alloc_send_skb() returned NULL.

    Since there is no need to use MSG_DONTWAIT if we retry until
    sock_alloc_send_pskb() succeeds, let's use blocking call.
    Also, in case an unexpected error occurred, let's break the loop
    if blocking nfc_alloc_send_skb() failed.

    [1] https://syzkaller.appspot.com/bug?id=4a131cc571c3733e0eff6bc673f4e36ae48f19c6

    Signed-off-by: Tetsuo Handa
    Reported-by: syzbot
    Signed-off-by: David S. Miller
    Signed-off-by: Greg Kroah-Hartman

    Tetsuo Handa
     

30 May, 2018

1 commit

  • [ Upstream commit fe9c842695e26d8116b61b80bfb905356f07834b ]

    The tlv_len is u8, so we need to limit the size of the SDP URI. Enforce
    this both in the NLA policy and in the code that performs the allocation
    and copy, to avoid writing past the end of the allocated buffer.

    Fixes: d9b8d8e19b073 ("NFC: llcp: Service Name Lookup netlink interface")
    Signed-off-by: Kees Cook
    Signed-off-by: David S. Miller
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Kees Cook
     

30 Nov, 2017

1 commit

  • commit c45e3e4c5b134b081e8af362109905427967eb19 upstream.

    A recent change fixing NFC device allocation itself introduced an
    error-handling bug by returning an error pointer in case device-id
    allocation failed. This is clearly broken as the callers still expected
    NULL to be returned on errors as detected by Dan's static checker.

    Fix this up by returning NULL in the event that we've run out of memory
    when allocating a new device id.

    Note that the offending commit is marked for stable (3.8) so this fix
    needs to be backported along with it.

    Fixes: 20777bc57c34 ("NFC: fix broken device allocation")
    Reported-by: Dan Carpenter
    Signed-off-by: Johan Hovold
    Signed-off-by: Samuel Ortiz
    Signed-off-by: Greg Kroah-Hartman

    Johan Hovold
     

02 Nov, 2017

1 commit

  • Many source files in the tree are missing licensing information, which
    makes it harder for compliance tools to determine the correct license.

    By default all files without license information are under the default
    license of the kernel, which is GPL version 2.

    Update the files which contain no license information with the 'GPL-2.0'
    SPDX license identifier. The SPDX identifier is a legally binding
    shorthand, which can be used instead of the full boiler plate text.

    This patch is based on work done by Thomas Gleixner and Kate Stewart and
    Philippe Ombredanne.

    How this work was done:

    Patches were generated and checked against linux-4.14-rc6 for a subset of
    the use cases:
    - file had no licensing information it it.
    - file was a */uapi/* one with no licensing information in it,
    - file was a */uapi/* one with existing licensing information,

    Further patches will be generated in subsequent months to fix up cases
    where non-standard license headers were used, and references to license
    had to be inferred by heuristics based on keywords.

    The analysis to determine which SPDX License Identifier to be applied to
    a file was done in a spreadsheet of side by side results from of the
    output of two independent scanners (ScanCode & Windriver) producing SPDX
    tag:value files created by Philippe Ombredanne. Philippe prepared the
    base worksheet, and did an initial spot review of a few 1000 files.

    The 4.13 kernel was the starting point of the analysis with 60,537 files
    assessed. Kate Stewart did a file by file comparison of the scanner
    results in the spreadsheet to determine which SPDX license identifier(s)
    to be applied to the file. She confirmed any determination that was not
    immediately clear with lawyers working with the Linux Foundation.

    Criteria used to select files for SPDX license identifier tagging was:
    - Files considered eligible had to be source code files.
    - Make and config files were included as candidates if they contained >5
    lines of source
    - File already had some variant of a license header in it (even if
    Reviewed-by: Philippe Ombredanne
    Reviewed-by: Thomas Gleixner
    Signed-off-by: Greg Kroah-Hartman

    Greg Kroah-Hartman
     

23 Jun, 2017

8 commits

  • Verify that the caller-provided sockaddr structure is large enough to
    contain the sa_family field, before accessing it in bind() handlers of the
    AF_NFC socket. Since the syscall doesn't enforce a minimum size of the
    corresponding memory region, very short sockaddrs (zero or one byte long)
    result in operating on uninitialized memory while referencing .sa_family.

    Signed-off-by: Mateusz Jurczyk
    Signed-off-by: Samuel Ortiz

    Mateusz Jurczyk
     
  • Remove unnecessary NULL check for pointer conn_info.
    conn_info is set in list_for_each_entry() using container_of(),
    which is never NULL.

    Addresses-Coverity-ID: 1362349
    Reviewed-by: Guenter Roeck
    Signed-off-by: Gustavo A. R. Silva
    Signed-off-by: Samuel Ortiz

    Gustavo A. R. Silva
     
  • Check that the NFC_ATTR_TARGET_INDEX and NFC_ATTR_PROTOCOLS attributes (in
    addition to NFC_ATTR_DEVICE_INDEX) are provided by the netlink client
    prior to accessing them. This prevents potential unhandled NULL pointer
    dereference exceptions which can be triggered by malicious user-mode
    programs, if they omit one or both of these attributes.

    Signed-off-by: Mateusz Jurczyk
    Acked-by: Kees Cook
    Signed-off-by: Samuel Ortiz

    Mateusz Jurczyk
     
  • Fix the sockaddr length verification in the connect() handler of NFC/LLCP
    sockets, to compare against the size of the actual structure expected on
    input (sockaddr_nfc_llcp) instead of its shorter version (sockaddr_nfc).

    Both structures are defined in include/uapi/linux/nfc.h. The fields
    specific to the _llcp extended struct are as follows:

    276 __u8 dsap; /* Destination SAP, if known */
    277 __u8 ssap; /* Source SAP to be bound to */
    278 char service_name[NFC_LLCP_MAX_SERVICE_NAME]; /* Service name URI */;
    279 size_t service_name_len;

    If the caller doesn't provide a sufficiently long sockaddr buffer, these
    fields remain uninitialized (and they currently originate from the stack
    frame of the top-level sys_connect handler). They are then copied by
    llcp_sock_connect() into internal storage (nfc_llcp_sock structure), and
    could be subsequently read back through the user-mode getsockname()
    function (handled by llcp_sock_getname()). This would result in the
    disclosure of up to ~70 uninitialized bytes from the kernel stack to
    user-mode clients capable of creating AFC_NFC sockets.

    Signed-off-by: Mateusz Jurczyk
    Acked-by: Kees Cook
    Signed-off-by: Samuel Ortiz

    Mateusz Jurczyk
     
  • Version 1.1 of the NFC Forum's NFC Digital Protocol Technical
    Specification dated 2014-07-14 specifies that the NFC-DEP Protocol's
    Target WT(nfcdep,max) value is 14. In version 1.0 it was 8 so change
    the value in the Linux NFC-DEP Protocol code accordingly.

    Signed-off-by: Mark Greer
    Signed-off-by: Samuel Ortiz

    Mark Greer
     
  • Section 4.8.2 (SEL_RES Response) of NFC Forum's NFC Digital Protocol
    Technical Specification dated 2010-11-17 clearly states that the size
    of a SEL_RES Response is one byte. Enforce this restriction in the
    code.

    Signed-off-by: Mark Greer
    Signed-off-by: Samuel Ortiz

    Mark Greer
     
  • Omit an extra message for a memory allocation failure in this function.

    This issue was detected by using the Coccinelle software.

    Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
    Signed-off-by: Markus Elfring
    Signed-off-by: Samuel Ortiz

    Markus Elfring
     
  • Replace the specification of four data structures by pointer dereferences
    as the parameter for the operator "sizeof" to make the corresponding size
    determination a bit safer according to the Linux coding style convention.

    Signed-off-by: Markus Elfring
    Signed-off-by: Samuel Ortiz

    Markus Elfring
     

19 Jun, 2017

1 commit

  • Commit 7eda8b8e9677 ("NFC: Use IDR library to assing NFC devices IDs")
    moved device-id allocation and struct-device initialisation from
    nfc_allocate_device() to nfc_register_device().

    This broke just about every nfc-device-registration error path, which
    continue to call nfc_free_device() that tries to put the device
    reference of the now uninitialised (but zeroed) struct device:

    kobject: '(null)' (ce316420): is not initialized, yet kobject_put() is being called.

    The late struct-device initialisation also meant that various work
    queues whose names are derived from the nfc device name were also
    misnamed:

    421 root 0 SW< [(null)_nci_cmd_]
    422 root 0 SW< [(null)_nci_rx_w]
    423 root 0 SW< [(null)_nci_tx_w]

    Move the id-allocation and struct-device initialisation back to
    nfc_allocate_device() and fix up the single call site which did not use
    nfc_free_device() in its error path.

    Fixes: 7eda8b8e9677 ("NFC: Use IDR library to assing NFC devices IDs")
    Cc: stable # 3.8
    Cc: Samuel Ortiz
    Signed-off-by: Johan Hovold
    Signed-off-by: Samuel Ortiz

    Johan Hovold
     

16 Jun, 2017

4 commits

  • Joe and Bjørn suggested that it'd be nicer to not have the
    cast in the fairly common case of doing
    *(u8 *)skb_put(skb, 1) = c;

    Add skb_put_u8() for this case, and use it across the code,
    using the following spatch:

    @@
    expression SKB, C, S;
    typedef u8;
    identifier fn = {skb_put};
    fresh identifier fn2 = fn ## "_u8";
    @@
    - *(u8 *)fn(SKB, S) = C;
    + fn2(SKB, C);

    Note that due to the "S", the spatch isn't perfect, it should
    have checked that S is 1, but there's also places that use a
    sizeof expression like sizeof(var) or sizeof(u8) etc. Turns
    out that nobody ever did something like
    *(u8 *)skb_put(skb, 2) = c;

    which would be wrong anyway since the second byte wouldn't be
    initialized.

    Suggested-by: Joe Perches
    Suggested-by: Bjørn Mork
    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     
  • It seems like a historic accident that these return unsigned char *,
    and in many places that means casts are required, more often than not.

    Make these functions return void * and remove all the casts across
    the tree, adding a (u8 *) cast only where the unsigned char pointer
    was used directly, all done with the following spatch:

    @@
    expression SKB, LEN;
    typedef u8;
    identifier fn = { skb_push, __skb_push, skb_push_rcsum };
    @@
    - *(fn(SKB, LEN))
    + *(u8 *)fn(SKB, LEN)

    @@
    expression E, SKB, LEN;
    identifier fn = { skb_push, __skb_push, skb_push_rcsum };
    type T;
    @@
    - E = ((T *)(fn(SKB, LEN)))
    + E = fn(SKB, LEN)

    @@
    expression SKB, LEN;
    identifier fn = { skb_push, __skb_push, skb_push_rcsum };
    @@
    - fn(SKB, LEN)[0]
    + *(u8 *)fn(SKB, LEN)

    Note that the last part there converts from push(...)[0] to the
    more idiomatic *(u8 *)push(...).

    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     
  • It seems like a historic accident that these return unsigned char *,
    and in many places that means casts are required, more often than not.

    Make these functions (skb_put, __skb_put and pskb_put) return void *
    and remove all the casts across the tree, adding a (u8 *) cast only
    where the unsigned char pointer was used directly, all done with the
    following spatch:

    @@
    expression SKB, LEN;
    typedef u8;
    identifier fn = { skb_put, __skb_put };
    @@
    - *(fn(SKB, LEN))
    + *(u8 *)fn(SKB, LEN)

    @@
    expression E, SKB, LEN;
    identifier fn = { skb_put, __skb_put };
    type T;
    @@
    - E = ((T *)(fn(SKB, LEN)))
    + E = fn(SKB, LEN)

    which actually doesn't cover pskb_put since there are only three
    users overall.

    A handful of stragglers were converted manually, notably a macro in
    drivers/isdn/i4l/isdn_bsdcomp.c and, oddly enough, one of the many
    instances in net/bluetooth/hci_sock.c. In the former file, I also
    had to fix one whitespace problem spatch introduced.

    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     
  • A common pattern with skb_put() is to just want to memcpy()
    some data into the new space, introduce skb_put_data() for
    this.

    An spatch similar to the one for skb_put_zero() converts many
    of the places using it:

    @@
    identifier p, p2;
    expression len, skb, data;
    type t, t2;
    @@
    (
    -p = skb_put(skb, len);
    +p = skb_put_data(skb, data, len);
    |
    -p = (t)skb_put(skb, len);
    +p = skb_put_data(skb, data, len);
    )
    (
    p2 = (t2)p;
    -memcpy(p2, data, len);
    |
    -memcpy(p, data, len);
    )

    @@
    type t, t2;
    identifier p, p2;
    expression skb, data;
    @@
    t *p;
    ...
    (
    -p = skb_put(skb, sizeof(t));
    +p = skb_put_data(skb, data, sizeof(t));
    |
    -p = (t *)skb_put(skb, sizeof(t));
    +p = skb_put_data(skb, data, sizeof(t));
    )
    (
    p2 = (t2)p;
    -memcpy(p2, data, sizeof(*p));
    |
    -memcpy(p, data, sizeof(*p));
    )

    @@
    expression skb, len, data;
    @@
    -memcpy(skb_put(skb, len), data, len);
    +skb_put_data(skb, data, len);

    (again, manually post-processed to retain some comments)

    Reviewed-by: Stephen Hemminger
    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     

16 May, 2017

1 commit


22 Apr, 2017

1 commit

  • Samuel Ortiz says:

    ====================
    NFC 4.12 pull request

    This is the NFC pull request for 4.12. We have:

    - Improvements for the pn533 command queue handling and device
    registration order.
    - Removal of platform data for the pn544 and st21nfca drivers.
    - Additional device tree options to support more trf7970a hardware options.
    - Support for Sony's RC-S380P through the port100 driver.
    - Removal of the obsolte nfcwilink driver.
    - Headers inclusion cleanups (miscdevice.h, unaligned.h) for many drivers.
    ====================

    Signed-off-by: David S. Miller

    David S. Miller
     

14 Apr, 2017

2 commits

  • This is an add-on to the previous patch that passes the extended ACK
    structure where it's already available by existing genl_info or extack
    function arguments.

    This was done with this spatch (with some manual adjustment of
    indentation):

    @@
    expression A, B, C, D, E;
    identifier fn, info;
    @@
    fn(..., struct genl_info *info, ...) {
    ...
    -nlmsg_parse(A, B, C, D, E, NULL)
    +nlmsg_parse(A, B, C, D, E, info->extack)
    ...
    }

    @@
    expression A, B, C, D, E;
    identifier fn, info;
    @@
    fn(..., struct genl_info *info, ...) {
    extack)
    ...>
    }

    @@
    expression A, B, C, D, E;
    identifier fn, extack;
    @@
    fn(..., struct netlink_ext_ack *extack, ...) {

    }

    @@
    expression A, B, C, D, E;
    identifier fn, extack;
    @@
    fn(..., struct netlink_ext_ack *extack, ...) {

    }

    @@
    expression A, B, C, D, E;
    identifier fn, extack;
    @@
    fn(..., struct netlink_ext_ack *extack, ...) {
    ...
    -nlmsg_parse(A, B, C, D, E, NULL)
    +nlmsg_parse(A, B, C, D, E, extack)
    ...
    }

    @@
    expression A, B, C, D;
    identifier fn, extack;
    @@
    fn(..., struct netlink_ext_ack *extack, ...) {

    }

    @@
    expression A, B, C, D;
    identifier fn, extack;
    @@
    fn(..., struct netlink_ext_ack *extack, ...) {

    }

    @@
    expression A, B, C, D;
    identifier fn, extack;
    @@
    fn(..., struct netlink_ext_ack *extack, ...) {

    }

    @@
    expression A, B, C;
    identifier fn, extack;
    @@
    fn(..., struct netlink_ext_ack *extack, ...) {

    }

    Signed-off-by: Johannes Berg
    Reviewed-by: Jiri Pirko
    Signed-off-by: David S. Miller

    Johannes Berg
     
  • Pass the new extended ACK reporting struct to all of the generic
    netlink parsing functions. For now, pass NULL in almost all callers
    (except for some in the core.)

    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     

05 Apr, 2017

1 commit

  • It looks like a typo to assign a return code to a variable which is not
    used. Found due to a compiler warning:

    net/nfc/netlink.c: In function ‘nfc_genl_activate_target’:
    net/nfc/netlink.c:903:6: warning: variable ‘rc’ set but not used [-Wunused-but-set-variable]
    int rc;
    ^~

    Signed-off-by: Andy Shevchenko
    Signed-off-by: Samuel Ortiz

    Andy Shevchenko
     

02 Apr, 2017

1 commit


10 Mar, 2017

1 commit

  • Lockdep issues a circular dependency warning when AFS issues an operation
    through AF_RXRPC from a context in which the VFS/VM holds the mmap_sem.

    The theory lockdep comes up with is as follows:

    (1) If the pagefault handler decides it needs to read pages from AFS, it
    calls AFS with mmap_sem held and AFS begins an AF_RXRPC call, but
    creating a call requires the socket lock:

    mmap_sem must be taken before sk_lock-AF_RXRPC

    (2) afs_open_socket() opens an AF_RXRPC socket and binds it. rxrpc_bind()
    binds the underlying UDP socket whilst holding its socket lock.
    inet_bind() takes its own socket lock:

    sk_lock-AF_RXRPC must be taken before sk_lock-AF_INET

    (3) Reading from a TCP socket into a userspace buffer might cause a fault
    and thus cause the kernel to take the mmap_sem, but the TCP socket is
    locked whilst doing this:

    sk_lock-AF_INET must be taken before mmap_sem

    However, lockdep's theory is wrong in this instance because it deals only
    with lock classes and not individual locks. The AF_INET lock in (2) isn't
    really equivalent to the AF_INET lock in (3) as the former deals with a
    socket entirely internal to the kernel that never sees userspace. This is
    a limitation in the design of lockdep.

    Fix the general case by:

    (1) Double up all the locking keys used in sockets so that one set are
    used if the socket is created by userspace and the other set is used
    if the socket is created by the kernel.

    (2) Store the kern parameter passed to sk_alloc() in a variable in the
    sock struct (sk_kern_sock). This informs sock_lock_init(),
    sock_init_data() and sk_clone_lock() as to the lock keys to be used.

    Note that the child created by sk_clone_lock() inherits the parent's
    kern setting.

    (3) Add a 'kern' parameter to ->accept() that is analogous to the one
    passed in to ->create() that distinguishes whether kernel_accept() or
    sys_accept4() was the caller and can be passed to sk_alloc().

    Note that a lot of accept functions merely dequeue an already
    allocated socket. I haven't touched these as the new socket already
    exists before we get the parameter.

    Note also that there are a couple of places where I've made the accepted
    socket unconditionally kernel-based:

    irda_accept()
    rds_rcp_accept_one()
    tcp_accept_from_sock()

    because they follow a sock_create_kern() and accept off of that.

    Whilst creating this, I noticed that lustre and ocfs don't create sockets
    through sock_create_kern() and thus they aren't marked as for-kernel,
    though they appear to be internal. I wonder if these should do that so
    that they use the new set of lock keys.

    Signed-off-by: David Howells
    Signed-off-by: David S. Miller

    David Howells
     

02 Mar, 2017

1 commit


28 Oct, 2016

4 commits

  • Now genl_register_family() is the only thing (other than the
    users themselves, perhaps, but I didn't find any doing that)
    writing to the family struct.

    In all families that I found, genl_register_family() is only
    called from __init functions (some indirectly, in which case
    I've add __init annotations to clarifly things), so all can
    actually be marked __ro_after_init.

    This protects the data structure from accidental corruption.

    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     
  • Instead of providing macros/inline functions to initialize
    the families, make all users initialize them statically and
    get rid of the macros.

    This reduces the kernel code size by about 1.6k on x86-64
    (with allyesconfig).

    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     
  • Static family IDs have never really been used, the only
    use case was the workaround I introduced for those users
    that assumed their family ID was also their multicast
    group ID.

    Additionally, because static family IDs would never be
    reserved by the generic netlink code, using a relatively
    low ID would only work for built-in families that can be
    registered immediately after generic netlink is started,
    which is basically only the control family (apart from
    the workaround code, which I also had to add code for so
    it would reserve those IDs)

    Thus, anything other than GENL_ID_GENERATE is flawed and
    luckily not used except in the cases I mentioned. Move
    those workarounds into a few lines of code, and then get
    rid of GENL_ID_GENERATE entirely, making it more robust.

    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     
  • This helper function allows family implementations to access
    their family's attrbuf. This gets rid of the attrbuf usage
    in families, and also adds locking validation, since it's not
    valid to use the attrbuf with parallel_ops or outside of the
    dumpit callback.

    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     

11 Jul, 2016

8 commits

  • When the target needs more time to process the received PDU, it sends
    Response Timeout Extension (RTOX) PDU.

    When the initiator receives a RTOX PDU, it must reply with a RTOX PDU
    and extends the current rwt value with the formula:
    rwt_int = rwt * rtox

    This patch takes care of the rtox value passed by the target in the RTOX
    PDU and extends the timeout for the next response accordingly.

    Signed-off-by: Thierry Escande
    Signed-off-by: Samuel Ortiz

    Thierry Escande
     
  • When sending an ATR_REQ, the initiator must wait for the ATR_RES at
    least 'RWT(nfcdep,activation) + dRWT(nfcdep)' and no more than
    'RWT(nfcdep,activation) + dRWT(nfcdep) + dT(nfcdep,initiator)'. This
    gives a timeout value between 1237 ms and 1337 ms. This patch defines
    DIGITAL_ATR_RES_RWT to 1337 used for the timeout value of ATR_REQ
    command.

    For other DEP PDUs, the initiator must wait between 'RWT + dRWT(nfcdep)'
    and 'RWT + dRWT(nfcdep) + dT(nfcdep,initiator)' where RWT is given by
    the following formula: '(256 * 16 / f(c)) * 2^wt' where wt is the value
    of the TO field in the ATR_RES response and is in the range between 0
    and 14. This patch declares a mapping table for wt values and gives RWT
    max values between 100 ms and 5049 ms.

    This patch also defines DIGITAL_ATR_RES_TO_WT, the maximum wt value in
    target mode, to 8.

    Signed-off-by: Thierry Escande
    Signed-off-by: Samuel Ortiz

    Thierry Escande
     
  • This patch frees the RTOX resp sk_buff in initiator mode. It also makes
    use of the free_resp exit point for ATN supervisor PDUs in both
    initiator and target mode.

    Signed-off-by: Thierry Escande
    Signed-off-by: Samuel Ortiz

    Thierry Escande
     
  • With this patch, ACK PDU sk_buffs are now freed and code has been
    refactored for better errors handling.

    Signed-off-by: Thierry Escande
    Signed-off-by: Samuel Ortiz

    Thierry Escande
     
  • When the target receives a NACK PDU, it re-sends the last sent PDU.

    ACK PDUs are received by the target as a reply from the initiator to
    chained I-PDUs. There are 3 cases to handle:
    - If the target has previously received 1 or more ATN PDUs and the PNI
    in the ACK PDU is equal to the target PNI - 1, then it means that the
    initiator did not received the last issued PDU from the target. In
    this case it re-sends this PDU.
    - If the target has received 1 or more ATN PDUs but the ACK PNI is not
    the target PNI - 1, then this means that this ACK is the reply of the
    previous chained I-PDU sent by the target. The target did not received
    it on the first attempt and it is being re-sent by the initiator. The
    process continues as usual.
    - No ATN PDU received before this ACK PDU. This is the reply of a
    chained I-PDU. The target keeps on processing its chained I-PDU.

    The code has been refactored to avoid too many indentation levels.

    Also, ACK and NACK PDUs were not freed. This is now fixed.

    Signed-off-by: Thierry Escande
    Signed-off-by: Samuel Ortiz

    Thierry Escande
     
  • When the initiator sends a DEP_REQ I-PDU, the target device may not
    reply in a timely manner. In this case the initiator device must send an
    attention PDU (ATN) and if the recipient replies with an ATN PDU in
    return, then the last I-PDU must be sent again by the initiator.

    This patch fixes how the target handles I-PDU received after an ATN PDU
    has been received.

    There are 2 possible cases:
    - The target has received the initial DEP_REQ and sends back the DEP_RES
    but the initiator did not receive it. In this case, after the
    initiator has sent an ATN PDU and the target replied it (with an ATN
    as well), the initiator sends the saved skb of the initial DEP_REQ
    again and the target replies with the saved skb of the initial
    DEP_RES.
    - Or the target did not even received the initial DEP_REQ. In this case,
    after the ATN PDUs exchange, the initiator sends the saved skb and the
    target simply passes it up, just as usual.

    This behavior is controlled using the atn_count and the PNI field of the
    digital device structure.

    Signed-off-by: Thierry Escande
    Signed-off-by: Samuel Ortiz

    Thierry Escande
     
  • When allocating chained I-PDUs, there is no need to call skb_reserve()
    since it's already done by digital_alloc_skb() and contains enough room
    for the driver head and tail data.

    Signed-off-by: Thierry Escande
    Signed-off-by: Samuel Ortiz

    Thierry Escande
     
  • This patch fixes the way an I-PDU is saved in case it needs to be sent
    again. It is now copied using pskb_copy() and not simply referenced
    using skb_get() since it could be modified by the driver.

    digital_in_send_saved_skb() and digital_tg_send_saved_skb() still get a
    reference on the saved skb which is re-sent but release it if the send
    operation fails. That way the caller doesn't have to take care about skb
    ref in case of error.

    RTOX supervisor PDU must not be saved as this can override a previously
    saved I-PDU that should be re-sent later on.

    Signed-off-by: Thierry Escande
    Signed-off-by: Samuel Ortiz

    Thierry Escande
     

06 Jul, 2016

2 commits

  • With this patch, the Digital Protocol layer abort the last issued
    command when the dep link goes down. That way it does not have to wait
    for the driver to reply with a timeout error before sending a new
    command (i.e. a start poll command if constant polling is on).

    Signed-off-by: Thierry Escande
    Signed-off-by: Samuel Ortiz

    Thierry Escande
     
  • There is a flag in the command structure indicating that this command is
    pending. It was checked before sending the command to not send the same
    command twice but it was actually never set. This is now fixed.

    Signed-off-by: Thierry Escande
    Signed-off-by: Samuel Ortiz

    Thierry Escande