RFC 2460 Sec. 5 requires that a ICMPv6 Time Exceeded message is sent
upon reassembly timeout, if we received the first fragment (i.e. the one
with a Fragment Offset of zero).
Implement this requirement.
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
The purpose of shift_packets() is to make room to insert one fragment in
the list. This is not what it does currently, potentially leading to
-ENOMEM even if there is enough free room.
To see the current behaviour, let's assume that we receive 3 fragments
in reverse order:
- Frag3(offset = 0x40, M=0)
- Frag2(offset = 0x20, M=1)
- Frag1(offset = 0x00, M=1)
After receiving Frag3 and Frag2, pkt[] will look like:
.-------.-------.-------.
| Frag2 | Frag3 | NULL |
| 0x20 | 0x40 | |
'-------'-------'-------'
pkt[0] pkt[1] pkt[2]
When receiving Frag1, shift_packets(pos = 0) is called to make some room
at position 0. It will iterate up to i = 2 where there is a free
element. The current algorithm will try to shift pkt[0] to pkt[2], which
is indeed impossible but also unnecessary. It is only required to shift
pkt[0] and pkt[1] by one element in order to free pkt[0] to insert
Frag1.
Update the algorithm in order to shift the memory only by one element.
As a result, the ENOMEM test is only simpler: as long as we encounter
one free element, we are guaranteed that we can shift by one element.
Also assign a NULL value to the newly freed element since memmove() only
copy bytes.
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
Currently net_ipv6_handle_fragment_hdr() performs 2 distinct tests: it
checks the M-bit of the most recent fragment to decide if we can proceed
with the reassembly. Then it performs some sanity checks which can lead
to dropping the whole packet if not successful.
The test on the M-bit assumes that fragments arrive in order. But this
will fail if packets arrive out-of-order, since the last fragment can
arrive before some other fragments. In that case, we proceed with the
reassembly but it will fail because not all the fragments have been
received.
We need a more complete check before proceeding with the reassembly:
- We received the first fragment (offset = 0)
- All intermediate fragments are contiguous
- The More bit of the last fragment is 0
Since these conditions can also detect a malformed fragmented packet, we
can replace the existing sanity check that is performed before
reassembly. As a bonus, we can now detect and rejected overlapping
fragments, since this can have some security issues (see RFC 5722).
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
Currently we only store the fragment offset. But in some cases it might
be necessary to also inspect the M-bit (More Fragment) of all received
fragments.
Modify the semantics of the field to store all the flags, rename the
setter to account for this change, and add a getter for the M-bit.
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
The special handling of the 1st fragment in unnecessary, since it will
be correctly handled even without it. Moreover it causes some corner
cases, like a single packet with a fragment header (M=0), to be
incorrectly handled since the reassembly code is skipped.
Remove the special handling of the 1st fragment to fix these problems.
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
Currently the requirement of the length being a multiple of 8 is not
tested for the first fragment, since the first fragment takes a
different path due to the goto.
Move the test earlier in the process, so that it is performed on all
fragments, including the first one.
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
If we have less fragments than what can be stored in the reassembly
array, some loops will blindly dereference NULL pointers.
Add checks for NULL pointers when necessary and exit the loop.
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
Currently the stack is limited to a maximum of 2 incoming fragments per
packet. While this can be enough in most cases, it might not be enough
in other cases.
Make this value configurable at build time.
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
Currently prev_hdr_offset always equals 6, which is the offset of
the nexthdr field in the IPv6 header. This value is used to overwrite it
when removing an IPv6 Fragment header, so it will work as long as there
is no other Extension header between the IPv6 header and the Fragment
header.
However this does not work in the other cases: the nexthdr field of the
IPv6 header will be overwritten instead of the nexthdr field of the last
Extension header before the Fragment, leading to unwanted results.
Update prev_hdr_offset so that it always point to the nexthdr field of
the previous header, either the IPv6 header or an Extension header.
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
The current validation code waits to process the header before rejecting
it, while some checks can be already enforced when reading the nexthdr
field of the previous header.
The main problem is a wrong pointer field in the resulting ICMPv6 error
message: the pointer should have the offset of the invalid nexthdr
field, while currently it will the offset the invalid header.
To solve that problem, reorganize the loop in two parts: the first
switch validates nexthdr, while the second switch processes the current
header. This allows to reject invalid nexthdr earlier.
The check for duplicated headers is also generalized, so that we can
catch other kind of headers (like the Fragment header).
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
By definition, NET_IPV6_NEXTHDR_NONE is void. So we must stop processing
before trying to read any data, since we will start reading values that
are outside the Extension Header (likely the payload, if any).
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
When an unknown option is encountered, an ICMPv6 error message must be
sent in some cases. The message contains a pointer field, which must be
the offset to the unknown option. Currently the offset is computed from
the beginning of the option list, while it should be computed with
respect to the beginning of the IPv6 header.
Record the offset when reading the option type and pass it later to
ipv6_drop_on_unknown_option() to correctly set the pointer field. Also
rename the argument in ipv6_drop_on_unknown_option() to make the
purpose more clear.
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
Currently PADN data are not skipped, which results in the stack to think
that the next header starts in the middle of the padding. We have to
skip the bytes before going on.
Also clarify the PAD1 does not have any length field.
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
The current names are confusing. Indeed "nexthdr" if the type of the
header currently processed, while "next_nexthdr" is the nexthdr field of
the current header.
Rename them to improve readability and make it less error-prone.
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
ICMPv6 error messages are not sent (on native_posix) because the first
net_pkt_write() returns an error.
pkt has just been allocated using net_pkt_alloc_with_buffer(). Trying to
write an empty packet in overwrite mode will result in an error. There
is no need to be in overwrite mode, since we want to write the LL
src/dst addresses at the beginning.
Signed-off-by: Florian Vaussard <florian.vaussard@gmail.com>
Refactor IPv4 multicast address to MAC multicast address conversion
into its own function, so it can be reused by drivers as it is already
possible for IPv6 multicast addresses.
Signed-off-by: Markus Fuchs <markus.fuchs@ch.sauter-bc.com>
This patch fixes the issue that can cause a deadlock in shell.
When two threads simultaneously poll the TXDONE signal, only one
of them will receive it, leaving second one stalled.
The problem was that shell's context contains k_poll_event objects that
were polled by multiple threads. Polling it overwrites the poller field
that was set by previous thread.
Instead, the k_poll_event object must be created on the stack by every
thread that wants to poll the TXDONE signal.
This makes sure that no thread will be left waiting for this signal
forever.
Signed-off-by: Michał Barnaś <mb@semihalf.com>
When falling back to L2CAP for connection parameter updates, the
interval min and maxes should also be saved.
Fixes#38613.
Signed-off-by: Eric Johnson <eric@liveathos.com>
When calling ull_conn_iso_cis_stop with no pending LLL events,
cis_disabled_cb may be called recursively if more than one CIS is
associated with a disconnecting ACL connection.
To prevent ticker_stop being called more than once, it shall be
registered at entry of cis_disabled_cb whether this is the last CIS.
Only then shall ticker_stop be called.
Signed-off-by: Morten Priess <mtpr@oticon.com>
CIS LLL events pending was checked incorrectly using a
mayfly incorrect set to be called from ULL LOW context,
but the actual call was from ULL HIGH context.
Beside this, the code was refactored to have file static
functions after global scope functions.
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
In case LwM2M server or bootstrap server rejected
Registration/Registration Update/Deregsitration attempt, there were no
reasonable notification to the application. Fix this by reporting
LWM2M_RD_CLIENT_EVENT_*_FAILURE in such case.
Addtitionaly, remove pointless ENGINE_DEREGISTER_FAILED event, which
have no use in the state machine.
Finally, simplify the response code logging to prevent code duplication.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Replace the old master and slave prefixes with the new central and
peripehral ones from the Bluetooth spec v5.3.
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
Replace the old whitelist-related terms with the new filter accept list
one from the Bluetooth spec v5.3.
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
In k_mem_paging_eviction_select(), the returned dirty bit value
may not be actually associated with the page selected, but
rather the last page examined. So fix this.
Signed-off-by: Daniel Leung <daniel.leung@intel.com>
Align with the new inclusive naming terms in the v5.3 spec in the
Bluetooth Host implementation.
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
Align with the new inclusive naming terms in the v5.3 spec in the
Bluetooth shell implementation.
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
To avoid hitting the following violation:
Violation to rule 5.7 (Tag name should be unique)
Replace the use of "shell" in the shell as an instance of a pointer to
const struct shell.
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
Align with the new inclusive naming terms in the v5.3 spec in the
controller's HCI implementation.
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
The Counter resource in the IPSO Push Button object was incremented
silently by the post write handler of the State resource. This prevented
the resource from being marked as updated, effectively preventing
the engine from sending notifications to observers.
Fix this, by setting the new counter value with `lwm2m_engine_set_u64()`
instead. This will update the resource value, and trigger the engine to
send notifications if needed.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Add an array similar to the bt_conn (ACL/L2CAP)
tx sent callback, and initialize it.
This increases the number of bt_conn_tx available
such that ISO does not take any of "L2CAP's" buffers,
but also ensures that the sent callback is called
for a broadcast iso only build.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
The new inclusive naming terminology changes in v5.3 of the Bluetooth
specification affect the HCI layer, so apply all relevant changes to
align with it.
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
Fix the definiion of Common Extended Payload Format data
field in the PDU definitions to be zero-length array,
because PDU size are configurable and to avoid allocations
being made using these PDU structs.
Corrected the extended scan response length check code to
use the correct define instead.
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Fix Extended Advertising stop on duration by using the
ticks_drift which now includes the random delay and any
ticker rescheduling of advertising radio events due to
collision with other radio events.
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
The ticker `ticks_drift` is propagated via the ticker
elapsed callback, in order to provide necessary information
to correctly calculate total elapsed durations by states and
roles that use ticker extensions to mitigate scheduling
collisions by drifting within a permitted window.
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
HTTP_DATA_FINAL was incorrectly notified in case Content Length field
was present in the HTTP respone - in such case it was set for every
response fragment, not only the last one.
Fix this by relying on `message_complete` flag instead of
`http_should_keep_alive()` function to determine whether to notify
HTTP_DATA_FINAL or not. As the HTTP parser calls the
`on_message_complete()` callback in either case (response is chunked or
not), this seems to be a more reasonable apporach to determine whether
the fragment is final or not.
Additinally, instead of calling response callback for
`on_body`/`on_message_complete` separately, call it directly from
`http_wait_data()` function, after the parsing round. This fixes the
case when headers were not reported correctly when the provided buffer
was smaller than the total headers length, resulting in corrupted data
being reported to the user.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
The helper function to conver 32-bit binary float value to
float32_value_t incorrectly identified the "hidden" bit, resulting in
invalid conversion when TLV encoding was used to write a resource
(the output value was divided by 2).
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
There were several issues preventing JSON format writes to work
correctly:
1. The formatter wrongly assumed that Base Name and Relative Name values
read from the message are NULL terminated, which in result could give
invalid results when combining them.
2. The formatter wrongly assumed that Relative Name is always present,
which is not always the case. In result, it failed to parse messages,
which contained full path in their Base Name Field
3. There were no boundaries check when reading JSON variable name/value,
which could lead to buffer overflow in case malformed data was
received.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Just like plain text, JSON parser ignored leading zeros after decimal
point, giving invalid results. Fix this in a similar way as for the
plain text.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Floating point parser for plain text format was parsing floats wrongly,
ignoring leading zeros after decimal points.
Fix this, by reusing atof32() function, already avaialbe in a different
part of the engine, which did the parsing correctly.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
According to the specificaion, resources are not predefined to use 32 or
64 bit floating point numbers, but should rather accept any of them (as
indicated by the size of the TLV in the message). This lead to issues
for instance with Eclipse Leshan LWM2M server, where Leshan sent 64-bit
value, while Zephyr expected 32-bit, making it impossible to write
the resource.
Therefore, unify the float usage to 32-bit float representation and fix
the TLV parsing functions, to accept values sent as either 32 or 64 bit
float.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
The `poll()` function did not report POLLHUP if the peer ended the DTLS
session, making it impossible to detect such event on the application
side.
On the other hand, TLS erroneusely reported POLLHUP along with each
POLLIN event, as the 0 returned by the `recv()` socket call was
wrongly interpreted (it was expected to get 0 in return as 0 bytes were
requested).
Fix this by introducing a helper function to process the mbedtls context
and verify if new application data is pendingi or session has ended.
Use this new function in the poll handler, instead of a socket `recv()`
call, to remove any ambiguity in the usage, for both TLS and DTLS.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Notify the application when DTLS client session ends by returning
ENOTCONN on such event. Additionally, reset the mbed TLS session
structures, allowing to reinstante the session on the next send() call.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
ECONNABORTED was returned in case tls_mbedtls_reset() function for
resetting session failed, which can be caused by memory shortage. Return
ENOMEM instead.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
