16 Jun, 2017

2 commits

  • 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
     

02 Mar, 2017

1 commit


20 Oct, 2016

1 commit


06 Oct, 2016

3 commits


20 Sep, 2016

4 commits

  • This patch enables prepending appearance value to scan response data.
    It also adds support for setting appearance value through mgmt command.
    If currently advertised instance has apperance flag set it is expired
    immediately.

    Signed-off-by: Michał Narajowski
    Signed-off-by: Szymon Janc
    Signed-off-by: Marcel Holtmann

    Michał Narajowski
     
  • This patch enables appending local name to scan response data. If
    currently advertised instance has name flag set it is expired
    immediately.

    Signed-off-by: Michał Narajowski
    Signed-off-by: Szymon Janc
    Signed-off-by: Marcel Holtmann

    Michał Narajowski
     
  • A comment in the code states that SCO connection should be rejected
    with the proper error value between 0xd-0xf. The code uses
    HCI_ERROR_REMOTE_LOW_RESOURCES which is 0x14.

    This led to following error:
    < HCI Command: Reject Synchronous Co.. (0x01|0x002a) plen 7
    Address: 34:51:C9:EF:02:CA (Apple, Inc.)
    Reason: Remote Device Terminated due to Low Resources (0x14)
    > HCI Event: Command Status (0x0f) plen 4
    Reject Synchronous Connection Request (0x01|0x002a) ncmd 1
    Status: Invalid HCI Command Parameters (0x12)

    Instead make use of HCI_ERROR_REJ_LIMITED_RESOURCES which is 0xd.

    Signed-off-by: Frédéric Dalleau
    Signed-off-by: Marcel Holtmann

    Frédéric Dalleau
     
  • When an Advertising Instance is removed, the Advertising Removed event
    shouldn't be sent to the same socket that issued the Remove
    Advertising command (it gets a command complete event instead). The
    mgmt_advertising_removed() function already has a parameter for
    skipping a specific socket, but there was no code to propagate the
    right value to this parameter. This patch fixes the issue by making
    sure the intermediate hci_req_clear_adv_instance() function gets the
    socket pointer.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     

24 Aug, 2016

1 commit

  • In hci_req_sync_complete the event skb is referenced in hdev->req_skb.
    It is used (via hci_req_run_skb) from either __hci_cmd_sync_ev which will
    pass the skb to the caller, or __hci_req_sync which leaks.

    unreferenced object 0xffff880005339a00 (size 256):
    comm "kworker/u3:1", pid 1011, jiffies 4294671976 (age 107.389s)
    backtrace:
    [] kmemleak_alloc+0x49/0xa0
    [] kmem_cache_alloc+0x128/0x180
    [] skb_clone+0x4f/0xa0
    [] hci_event_packet+0xc1/0x3290
    [] hci_rx_work+0x18b/0x360
    [] process_one_work+0x14a/0x440
    [] worker_thread+0x43/0x4d0
    [] kthread+0xc4/0xe0
    [] ret_from_fork+0x1f/0x40
    [] 0xffffffffffffffff

    Signed-off-by: Frédéric Dalleau
    Signed-off-by: Marcel Holtmann

    Frederic Dalleau
     

09 Apr, 2016

1 commit

  • If we're dealing with a single-mode controller or BR/EDR is disable
    for a dual-mode one, the NO_BREDR flag needs to be unconditionally
    present in the advertising data. This patch moves it out from behind
    an extra condition to be always set in the create_instance_adv_data()
    function if BR/EDR is disabled.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     

11 Mar, 2016

3 commits

  • A recent change added MGMT_ADV_FLAG_DISCOV to the flags returned by
    get_adv_instance_flags(), however failed to take into account limited
    discoverable mode. This patch fixes the issue by setting the correct
    discoverability flag in the AD data.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • Introduce a limited privacy mode indicated by value 0x02 to the mgmt
    Set Privacy command.

    With value 0x02 the kernel will use privacy mode with a resolvable
    private address. In case the controller is bondable and discoverable
    the identity address will be used.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • When lookup up the advertising instance flags for the default
    advertising instance (0) the discoverable flag should be filled in
    based on the HCI_DISCOVERABLE flag.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     

29 Jan, 2016

1 commit

  • The commit cad20c278085d893ebd616cd20c0747a8e9d53c7 was supposed to
    fix handling of devices first using public addresses and then
    switching to RPAs after pairing. Unfortunately it missed a couple of
    key places in the code.

    1. When evaluating which devices should be removed from the existing
    white list we also need to consider whether we have an IRK for them or
    not, i.e. a call to hci_find_irk_by_addr() is needed.

    2. In smp_notify_keys() we should not be requiring the knowledge of
    the RPA, but should simply keep the IRK around if the other conditions
    require it.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann
    Cc: stable@vger.kernel.org # 4.4+

    Johan Hedberg
     

06 Jan, 2016

1 commit


10 Dec, 2015

16 commits

  • We can simplify a lot of code by making sure hdev->cur_adv_instance is
    always up-to-date. This allows e.g. the removal of the
    get_current_adv_instance() helper function and the special
    HCI_ADV_CURRENT value. This patch also makes selecting instance 0x00
    explicit in the various calls where advertising instances aren't
    enabled, e.g. when HCI_ADVERTISING is set or we've just finished
    enabling LE.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • The logic in powered_update_hci() to initialize the advertising data &
    state is a bit more complicated than it needs to be. It was previously
    not doing anything if HCI_LE_ENABLED wasn't set, but this was not
    obvious by quickly looking at the code. Now the conditions for the
    various actions are more explicit. Another simplification is due to
    the fact that __hci_req_schedule_adv_instance() takes care of setting
    hdev->cur_adv_instance so there's no need to set it before calling the
    function.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • The hci_req_run() function already checks for empty cmd_q and bails
    out if necessary. Also, req.cmd_q should really be treated as private
    data of the request and not accessed directly.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • The __hci_req_update_scan_rsp_data gets the instance to be updated
    which should get passed to update_inst_scan_rsp_data() instead of
    always enabling the current instance.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • This flag just tells us whether hdev->adv_instances is empty or not.
    We can equally well use the list_empty() function to get this
    information.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • The request to update HCI during power on is always coming either from
    hdev->req_workqueue or through an ioctl, so it's safe to use
    hci_req_sync for it. This way we also eliminate potential races with
    incoming mgmt commands or other actions while powering on.

    Part of this refactoring is the splitting of mgmt_powered() into
    mgmt_power_on() and __mgmt_power_off() functions. The main reason is
    the different requirements as far as hdev locking is concerned, as
    highlighted with the __ prefix of the power off API.

    Since the power on in the case of clearing the AUTO_OFF flag cannot be
    done synchronously in the set_powered mgmt handler, the hci_power_on
    work callback is extended to cover this (which also simplifies the
    set_powered helper a lot).

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • We'll soon need this both in hci_request.c and mgmt.c so move it to
    hci_request.c as a generic helper.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • We'll soon need to update the EIR both from hci_request.c and mgmt.c
    so move update_eir() as a more generic request helper to
    hci_request.c.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • We'll soon need this both from hci_request.c and mgmt.c so move it as
    a request helper function to hci_request.c.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • Since the other discoverable changes are behind req_workqueue now it
    only makes sense to move the discoverable timeout there as well.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • The discoverable mode is intrinsically linked with the connectable
    mode e.g. through sharing the same HCI command (Write Scan Enable) for
    BR/EDR. It makes therefore sense to move it to hci_request.c and run
    the changes through the same hdev->req_workqueue.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • The Class of Device needs to be changed e.g. for limited discoverable
    mode. In preparation of moving the discoverable mode to hci_request.c
    and hdev->req_workqueue, move the Class of Device helpers there first.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • This way the connectable changes are synchronized against each other,
    which helps avoid potential races. The connectable mode is also linked
    together with LE advertising which makes is more convenient to have it
    behind the same workqueue.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • This paves the way for eventually performing advertising changes
    through the hdev->req_workqueue. Some new APIs need to be exposed from
    mgmt.c to hci_request.c and vice-versa, but many of them will go away
    once hdev->req_workqueue gets used.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • This way we avoid the need to do a forward declaration in later
    patches.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • Since Add/Remove Device perform the page scan updates independently
    from the HCI command completion we've introduced a potential race when
    multiple mgmt commands are queued. Doing the page scan updates through
    the req_workqueue ensures that the state changes are performed in a
    race-free manner.

    At the same time, to make the request helper more widely usable,
    extend it to also cover Inquiry Scan changes since those are behind
    the same HCI command. This is also reflected in the new name of the
    API as well as the work struct name.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     

23 Nov, 2015

1 commit


20 Nov, 2015

5 commits

  • The hci_req_sync_cancel() is just as much related to the request
    cleanup as hci_request_cancel_all() is. Just move the former into the
    latter and do the cleanup from a single place in hci_dev_do_close().
    The important thing is to avoid deadlocks by holding the req_sync
    lock: previously hci_request_cancel_all was done right after releasing
    the lock and with this patch it's right before taking it.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • The only user of this, le_scan_restart_work(), is so short and simple
    that it makes sense to just merge the code there.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • Merge le_scan_disable_work_complete into the main le_scan_disable_work
    function and take advantage of the updated bredr_inquiry() to run the
    Inquiry through hci_req_sync().

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • Passing the needed inquiry length to bredr_inquiry() makes it possible
    to also use this helper for interleaved discovery where the controller
    doesn't support simultaneous Inquiry & LE scan.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     
  • Since discovery also deals with LE scanning it makes sense to move it
    behind the same req_workqueue as other LE scanning changes. This also
    simplifies the logic since we do many of the actions in a synchronous
    manner.

    Part of this refactoring is moving hci_req_stop_discovery() to
    hci_request.c. At the same time the function receives support for
    properly handling the STOPPING state since that's the state we'll be
    in when stopping through the req_workqueue.

    Signed-off-by: Johan Hedberg
    Signed-off-by: Marcel Holtmann

    Johan Hedberg