The ATT request buffers are held until the ATT response has been
received. This means that the ATT request buffers are released by the
RX thread, instead of the from the RX priority context of
num_complete.
This can cause a deadlock in the RX thread when we allocate buffers
and all the available buffers are ATT requests, since the RX thread is
the only thread that can release buffers.
Release the ATT request buffers once they have been sent and instead
handle ATT request resending by reconstructing the buffer from the
GATT parameters.
Also re-order the order of resource allocation by allocating the
request context before the buffer. This ensures that we cannot
allocate more buffers for ATT requests than there are ATT requests.
Fixed a buf reference leak that could occur when the ATT request buffer
has been allocated, but GATT returns an error before handing the
responsebility of the buffer to ATT, for example when bt_att_req_alloc
fails.
This is fixed by moving the functionality of att_req_destroy to
bt_att_req_free.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Remove the ATT request destroy callback which is never assigned
by any of the ATT requests.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
ATT channels do support queueing buffer so it no longer need to block
waiting the tx_sem besides the buffer allocation already serves the
same purpose as the application will not be able to have more requests
than there are buffers available.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
When ATT resends an ATT request it is sent as a "response" instead of
as a request. This causes the ATT request buffer to be released and
the ATT request cannot be resent one more time.
This causes a problem when the ATT request requires authentication
but the elevation of security is not enforcing MITM protection.
In this case the ATT will first require security level 2 and then resend
the request once this has been reached.
This will lead to a new ATT error response and ATT will require security
level L3.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Remove ATT modifying the L2CAP channel state by unassigning the
the connection pointer on timeout.
Unassigning this pointer does not prevent the ATT channel from receiving
since bt_l2cap_recv does not inspect this pointer before calling the
channel receive function.
This prevented the disconnected callback from being called on the
channel after the channel had timed out, but since the disconnected
callback now handles this case this workaroun is no longer needed.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Handle ATT timeout on disconnected ATT channel.
When the ATT channel is disconnected with a pending request the ATT
timeout is canceled and the response processed with an error code.
However canceling of delayed work is not guaranted to succeed, e.g:
"Work queue thread has removed the work item from the queue but has not
called its handler"
This could lead to timeout handler being invoked after the disconnected
handler.
Fixes: #29098
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Fix crash in ATT when the response for a request is received after
the ATT timeout has fired and the ATT channel has been detached.
Add similar handling for all ATT channel operations.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Fix issue where the encrypt change has an error code, but the
encrypt change request was unrelated to the current ATT request.
This lead to the current ATT transaction being interpreted as finished
an the ATT client would proceed with the next ATT request, which would
fail since the ATT client is now violating the ATT single transaction
rule.
Updated similar if statement checking for the opposite to improve
readability.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Don't use the ATT structs that has contains only a flexible array
member. This is not supported by C99 standard, only through GNU C
extension with zero length array.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Make it actually give the original pointer to the attribute and its
resolved handle so static attributes don't need an extra lookup.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Since bt_l2cap_send_cb can fail returning its error is not enough as
the buffer has been modified to add the headers, so this save the state
before calling bt_conn_send_cb and takes a reference so it can be
restored its original state in case of error.
Fixes#27434
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Avoid warning in att_handle_rsp, only call this when there is a
pending ATT request. This avoids a warning happens when elevating
security fails but higher security had not been requested by an
ATT request.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
squash! Bluetooth: ATT: Remove warning when no ATT process pending
EATT channels use bt_l2cap_chan_send which does return the number of
bytes sent on success not 0 as bt_l2cap_send.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
These functions don't work with buffers that do have fragments, instead
this replaces their usage with net_buf_{put,get}.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
ATT_PENDING_SENT does severely impact the throughput since multiple
packets no longer can be scheduled at same time, so instead of always
setting it regardless of the bearer/channel it is now only used for
EATT since that cannot set its own callbacks.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
bt_l2cap_send_cb may fail if there are no context available which means
that the request would not be sent, also due to the use of custom
callback it cannot be queued either so the only option is to return the
error and let the application handle it.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Since the TX semaphore is used for all types of PDUs a request may have
to be put on the request list while there is no pending request pending
which means no response will be generated to trigger att_process,
previously this condition was handled by setting the request as
currently pending and append its buffer to tx_queue but this is no
longer efficient since there could be more than one channel active the
code should try all of them before queueing back to request list.
To fix this the request list will now be processed each time a PDU has
been sent.
Fixes#26070
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
ATT channel sent callback shall not be overwritting until the
operation completes as it can result in breaking flow control when
CONFIG_BT_ATT_ENFORCE_FLOW is enabled.
Fixes#25964Fixes#26071
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This adds support for ATT_MULTIPLE_HANDLE_VALUE_NTF,
ATT_READ_MULTIPLE_VARIABLE_REQ and ATT_READ_MULTIPLE_VARIABLE_RSP.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This adds support for EATT bearer which was introduced in 5.2, they work
as extra channels to have GATT traffic, at the moment it is completely
transparent to application when they are in use since the allocation
happens automatically.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Convert bluetooth host to using k_timeout_struct for the timeout values.
This is mostly replacing s32_t with k_timeout_t.
In l2cap the handling of no timeout in send channel request was removed
since the timeout is both documented as minimum of 1 second and never
given any no timeout value.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
When disconnecting att_reset is called and all requests are notified
but instead of passing req->user_data like it should it pass the req
itself which nowdays comes from a k_mem_slab, rather than being a
contiguous memory that would contain the request and its user data,
which would likely cause invalid access.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Fix ATT releasing the att structure back to the memory slab allocator
before the structure is actually ready to be released. The memory slab
allocator will write context data inside the freed slab which is
currently being overwritten by l2cap during channel teardown.
This manifests as an "Unable to allocate ATT context for conn" when
reconnecting with multiple connections.
Since the l2cap channel is embedded inside of the ATT context and l2cap
still has a valid referenc to the l2cap channel we need to release the
ATT context at a later time.
This should be fixed by implementing the channel destroy function and
releasing the channel there.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Add the option to provide the destroy callback to the fixed channels.
This can be used to free the memory for the L2CAP channel context which
is provided by the fixed channel in the accept callback.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
The callback function may modify the att->req and it has to be
called after the att_process. The att_process does not re-check
if att->req is still NULL.
Signed-off-by: Marek Pieta <Marek.Pieta@nordicsemi.no>
Fix assert in net_buf triggered on att encrypt change event.
ASSERTION FAIL [net_buf_simple_headroom(buf) >= len] @
ZEPHYR_BASE/subsys/net/buf.c:881
This happens because when the att request was allocated, it was not
properly initialized and req->retrying was left as true.
This caused the att encrypt change handling to assume an att request
needed to be resent, starting resending with an invalid request and
request buffer.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
The path to include/drivers should not be in the compiler include path
list, only include/. In order to make this possible, always explictly
refer to hci_driver.h via the drivers/bluetooth/ path and not only
bluetooth/.
Fixes#21974.
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
This should reduce the footprint on applications that do a lot of
requests i.e have a lot of subscriptions.
Fixes#21103
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Make the channel ops struct const since there really isn't anything
there that needs to change at runtime. The only exception is the L2CAP
shell which was playing with the recv callback, however that can be
fixed by introducing a simple bool variable.
With tests/bluetooth/shell this reduces RAM consumption by 112 bytes
while adding only 16 bytes to flash consumption.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Fix ATT MTU size of length variables resulting in wrong length values
reported to the user. Communicating with an Android device using an MTU
of 517 resulted in write commands reported as length 2 instead.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Since bt_conn_send_cb can fail to send buffer causing it to unref this
may cause buffer leaks as the caller is not aware of the error assuming
the buffer could be sent.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Fix bug in ATT reset handling, not releasing queued notification
buffers when the connection is terminated.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
This moves check_perm code under GAT and make use of it to check if CCC
write permission can be attended by the connection security level or if
it needs to be increased.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This adds support to set different permissions to CCCD so security can
be checked when enabling notification which conforms to:
BLUETOOTH CORE SPECIFICATION Version 5.1 | Vol 3, Part G page 2360:
'3.3.3.3 Client Characteristic Configuration
Authentication and authorization may be required by the server to
write the configuration descriptor.'
In addition to that also ensure that notification are not re-enabled
until the proper security level is reached to conform to the following
statement:
'10.3.1.1 Handling of GATT indications and notifications
A client “requests” a server to send indications and notifications
by appropriately configuring the server via a Client Characteristic
Configuration Descriptor. Since the configuration is persistent
across a disconnection and reconnection, security requirements must
be checked against the configuration upon a reconnection before
sending indications or notifications. When a server reconnects to a
client to send an indication or notification for which security is
required, the server shall initiate or request encryption with the
client prior to sending an indication or notification. If the client
does not have an LTK indicating that the client has lost the bond,
enabling encryption will fail.'
Fixes#17983
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Host shall ignore the unknown ATT PDU that has Command Flag set.
Fixes regression introduced in 3b271b8455.
Fixes: GATT/SR/UNS/BI-02-C
Signed-off-by: Mariusz Skamra <mariusz.skamra@codecoup.pl>
Endianness bug fix in bt_uuid_create function.
Replaced bt_uuid_create_le with bt_uuid_create which
handles both UUID from air and internal varaiable.
Fixed bug with endianess in case of big endian targets.
Signed-off-by: Akshatha Harishchandra <akhr@oticon.com>
When an opcode doesn't have a handle that doesn't mean it is unknown
just that it will not be handle as it could have been disabled.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
When CONFIG_BT_GATT_CLIENT is not set att_op_get_type would return
unkown operation instead of properly return the opcode.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Rename bt_conn_security to bt_conn_set_security, this makes the API
naming more consistent.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Rename security level enum, using level and number instead of low,
medium, high and fips.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
This is safer now that bt_conn_create_pdu can return NULL when using
syswq which can prevent things like signalling of L2CAP and ATT layers.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Find by type does only accept a UUID with the same length as the UUID
which is stored in the internal list. If a UUID is stored in the short
16 bit format then a request with 128 bit UUID will fail.
Add support for the missing formats.
Signed-off-by: Kim Sekkelund <ksek@oticon.com>
Error codes are listed in header files and in the core spec as hex
values. Always print them in hex in debug for easier error code
checking.
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
This adds a dummy cancel request which is used instead of NULL when
clearing and outstanding request thus prevent new request to take
place before a response is received while allowing the original request
to be reused.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
move misc/util.h to sys/util.h and
create a shim for backward-compatibility.
No functional changes to the headers.
A warning in the shim can be controlled with CONFIG_COMPAT_INCLUDES.
Related to #16539
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
