This performs a check of target code using it as index to
retrieve target string via the target code string array.
Signed-off-by: Daniel Leung <daniel.leung@intel.com>
(cherry picked from commit a9226324e8)
When a characteristic declaration is passed instead of a value
attribute, ensure the associated characteristic value attribute
permissions are checked before sending notifications, indications
or multiple notifications.
Signed-off-by: Yuan Ye <1275552818@qq.com>
(cherry picked from commit c3386f92fe)
Fix insufficient buffer length validation in bt_sdp_parse_attribute().
The original check only verified space for the type byte and attribute
ID, but did not account for the type variable itself that is read from
the buffer immediately after the check.
This could lead to a buffer over-read if the buffer contains exactly
sizeof(uint8_t) + sizeof(attr->id) bytes but not enough for the
additional type field.
Add sizeof(type) to the length check to ensure all required data is
present before parsing.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
(cherry picked from commit dfac5224ab)
Updates the 'src_id' check in z_vrfy_log_filter_check() so that
negative values are also excluded.
Signed-off-by: Peter Mitsis <peter.mitsis@intel.com>
(cherry picked from commit 56a15114c6)
Route handling can legitimately resend a packet on the same
interface when a route lookup or on-link lookup keeps the
original egress interface. Those packets are routed, but they
must not be treated as forwarded traffic.
Set net_pkt_forwarding() only when the matching IPv4 or IPv6
forwarding option is enabled and the selected egress interface
differs from the ingress interface. Apply the same rule in the
generic net_route_packet_if() path so the forwarding flag stays
consistent for both explicit-route and on-link routing cases.
[david.brown: Applied to the IPv6-only code on this branch. It predates
the IPv4 forwarding feature and the route.c -> route_ipv4.c/route_ipv6.c
split, so the change is re-pathed into the monolithic route.c:
net_route_packet() gains the cross-interface decision and the forwarded
hop-limit decrement, and net_route_packet_if() the cross-interface gate,
both gated on CONFIG_NET_ROUTING because
CONFIG_NET_IPV4_FORWARDING/CONFIG_NET_IPV6_FORWARDING do not exist here.
The IPv4-only hunks, route_ipv4.c, and the test-split commit are omitted
as inapplicable to this branch.]
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Assisted-By: Claude:opus-4.8
Signed-off-by: David Brown <david.brown@linaro.org>
In `l2cap_br_conf_req()` and `l2cap_br_conf_rsp()`, `buf->len` is used
to validate the minimum packet size. However, `buf->len` may exceed the
actual command data length (the `len` parameter from the L2CAP
signaling header), as the buffer can contain data beyond the current
command.
When the command data length `len` is smaller than the minimum packet
size, but `buf->len` is not less than the minimum packet size, the
validation passes incorrectly. Subsequently, when calculating `opt_len`
(`len - sizeof(*req)`), an underflow occurs duw to the value of type
`uint16_t`, resulting in an out-of-bounds buffer access issue.
Fix by validating against the `len` parameter instead of `buf->len` in
both `l2cap_br_conf_req()` and `l2cap_br_conf_rsp()`, since `len`
reflects the actual command data length.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
(cherry picked from commit 1d45168337)
Fix a race condition in RFCOMM session disconnection when both local
and peer devices initiate disconnection simultaneously.
Add state check in `rfcomm_session_disconnected()` to only transition
to `DISCONNECTED` state if the session is not already in
`DISCONNECTING` state. This prevents the race condition where both
sides set the session to disconnected, causing the disconnection
process to not complete properly and leaving the L2CAP connection
unreleased.
Without this check, subsequent RFCOMM channel connection requests
would fail due to the invalid session state (the expected state is
`IDLE`, while the actual state is `DISCONNECTED`).
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Signed-off-by: David Brown <david.brown@linaro.org>
Assisted-By: Claude:fable-5
The result of `mctp_pktbuf_alloc` wasn't being checked on both
controller and target. While a simple check is enough for the
controller, the target fix is a bit more involved.
As the target was allocating the buffer when it received the length
of the message on its pseudoregister, a buggy controller could write
this more than once, making previous allocations leak. This is solved by
only noting the size of the message written to the pseudoregister, and
doing the allocation when the first byte of the message was received.
This flow also ensures that a buggy controller can't skip sending the
length of the message, as this will result in a zero-sized MCTP packet,
which will be handled by libmctp.
Signed-off-by: Ederson de Souza <ederson.desouza@intel.com>
Signed-off-by: David Brown <david.brown@linaro.org>
Assisted-By: Claude:opus-4.8
Add http_server_normalize_url() to resolve '.' and '..' segments in
client->url_buffer once the URL is fully assembled to avoid a remote
client to read files outside the configured web root.
Signed-off-by: Flavio Ceolin <flavio@hubblenetwork.com>
(cherry picked from commit f4a423c985)
Since the Broadcast Assistant implementation may have a
pending request _per_ instance, each instance needs its own
buffer to accomodate that, otherwise we may risk overwriting
data between instances. This follows the design used in the
BAP Unicast Client
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Signed-off-by: David Brown <david.brown@linaro.org>
Assisted-By: Claude:opus-4.8
Make sure that if the connection is closed but we still received
a SYN packet, we do not try to access already closed connection.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
(cherry picked from commit 74931644b5)
In prov_msg_recv(), the protocol timer was reset unconditionally at
the top of the function, before the FCS check and before the
ADV_LINK_INVALID check. When the link has been marked invalid (e.g.
after a provisioning failure), any incoming PB-ADV packet with a
passing FCS would still reset the timer, preventing
protocol_timeout() from firing and closing the link via
prov_link_close().
Move k_work_reschedule() to after the ADV_LINK_INVALID check so the
timer is only reset for valid PDUs on an active, non-failed link.
Move the FCS check before the timer reset for the same reason.
Once ADV_LINK_INVALID is set the protocol timer is no longer
extended by incoming packets, and the link is closed by
protocol_timeout() as intended, after which the unprovisioned
device beacon and PB-ADV link acceptance are restored.
Signed-off-by: Aleksandr Khromykh <aleksandr.khromykh@nordicsemi.no>
(cherry picked from commit 3f3c37edf8)
bt_iso_recv pulls the SDU header (with or without) timestamp,
but did not check the length of `buf` before doing so.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
(cherry picked from commit 756b16b643)
The two reference counts in the net_buf library -- the per-header
`buf->ref` and the per-data-block `*ref_count` byte at the start of
each variable-data allocation -- were manipulated with plain non-atomic
C operators (`++`, `--`, `if (--rc)`, `if (!rc)`).
The documented contract says otherwise. The Network Buffers chapter of
the Zephyr docs (`doc/services/net_buf/index.rst`) states:
"The buffers have native support for being passed through k_fifo
kernel objects. Use k_fifo_put and k_fifo_get to pass buffer from
one thread to another."
"The reference count can be incremented with net_buf_ref() or
decremented with net_buf_unref(). When the count drops to zero the
buffer is automatically placed back to the free buffers pool."
There is no requirement for callers to hold a higher-level lock around
ref/unref. The API is documented as self-synchronizing, and existing
users (notably zbus's msg-subscriber path) rely on exactly that:
a producer clones a buffer N times and hands the clones off to N
subscriber threads via their FIFOs, after which the N+1 holders
independently call `net_buf_unref()` with no surrounding lock.
With non-atomic decrement-and-test, two CPUs can concurrently observe
the same prior value (e.g. 1), both decrement, and both conclude they
were the last reference. Concrete failure modes:
* `mem_pool_data_unref`: both CPUs call `k_heap_free(pool, ref_count)`
on the same block. `k_heap_free` is internally serialized, so the
duplicate free typically corrupts heap metadata silently.
* `heap_data_unref`: both CPUs call `k_free(ref_count)` on the same
block. `k_free` reads the owning `struct k_heap *` from the 8 bytes
immediately preceding `ref_count`. The first call frees the block
and the heap-hardening fill replaces those 8 bytes with the poison
pattern (0xcfdfdfdfdfdfdfcf). The second call then dereferences a
poisoned pointer and faults inside `k_spin_lock` (translation
fault on the bogus heap address).
* `net_buf_unref`: two CPUs racing the per-header decrement-and-test
can both decide "I am the last reference," both proceed to
`net_buf_destroy()`, and the buffer is returned to the pool's LIFO
twice -- silently corrupting the free list.
Fix: use atomic operations on both reference counts.
The per-data-block refcount changes from `uint8_t` to `atomic_t`. This
fits inside the existing `GET_ALIGN(pool)` reservation (>= sizeof(void
*)) at no memory cost.
The per-header `buf->ref` is overlaid in a union with three small
adjacent uint8_t fields (`flags`, `pool_id`, `user_data_size`) and an
`atomic_t ref_word` view of the same storage:
union {
atomic_t ref_word;
struct {
uint8_t ref;
uint8_t flags;
uint8_t pool_id;
uint8_t user_data_size;
};
};
(Byte order conditional on endianness so `ref` is always the LSB of
`ref_word`; on big-endian 64-bit, the byte struct is shifted by 4
bytes of padding for the same reason.)
Net_buf internals issue `atomic_inc(&buf->ref_word)` /
`atomic_dec(&buf->ref_word)` and narrow the returned word value to
`uint8_t` to extract the ref byte. Because the ref count is bounded
to 254 (already implicit in its uint8_t domain), atomic_inc/dec
adjusts only the LSB; the other three bytes are untouched. Plain
uint8_t reads of `buf->ref` from non-atomic call sites continue to
work, so the change is transparent to the dozens of consumers that
read it for diagnostics.
`flags`, `pool_id` and `user_data_size` are written exactly once at
allocation time on a single thread (or, for `flags`, from a context
that owns the buf exclusively such as bt_buf_make_view on a fresh
view), so there are no concurrent byte writes that could conflict
with the atomic word update. struct net_buf does not grow on either
32-bit or 64-bit: on 32-bit the four bytes are exactly `sizeof(long)`,
on 64-bit they fit in alignment padding the next field already
required.
A BUILD_ASSERT in lib/net_buf/buf.c documents the
`atomic_t == long` assumption that the conditional padding relies on.
In `net_buf_unref`, the per-header refcount and the fields needed for
the debug log (`buf->pool_id`) are captured into local variables
*before* the atomic decrement -- once the reference is dropped, another
CPU may immediately free the buffer, so the buffer must not be read
again. The post-decrement diagnostic log uses the value returned by
`atomic_dec` rather than re-reading `buf->ref`. The `pool->avail_count`
sanity check uses the value returned by `atomic_inc` to avoid a
follow-up `atomic_get` of memory another CPU may have changed.
`net_pkt_frag_unref()` previously had the racy
`if (frag->ref == 1U) alloc_del(); net_buf_unref();` pattern; it is
restructured to do the atomic decrement here and slot the tracker call
in atomically with the "I'm the last reference" decision, with
`net_pkt_frag_del()` routed through it.
This bug had been latent. On real SMP hardware the race window is very
small and the typical net_buf consumers (Bluetooth, networking) tend
to use fixed-data pools (`fixed_data_unref` is a no-op). The race
manifests reliably under FVP, where the FastModel's quantum-based
execution model can schedule N threads to all reach the unref point in
the same simulated moment. We discovered it through the zbus
`msg_subscriber_dynamic_isolated` sample, which exchanges shared data
buffers among 16+ subscribers running on 4 SMP cores.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
(cherry picked from commit 9bb2878319)
Fix `sntp_close_async` closing the socket while the socket service is
still polling it by deferring the close operation to the socket service.
Signed-off-by: Jordan Yates <jordan@embeint.com>
Closing a socket while it is being polled by another thread is
discouraged and should be avoided. This results in a problem when
attempting to unregister a service via `net_socket_service_unregister`,
the caller has no way of knowing when the socket service has stopped
polling on the socket and it is safe to close.
Solve this issue by introducing `net_socket_service_close`, which
signals the socket service to automatically close the sockets associated
with the service when it stops polling them.
Signed-off-by: Jordan Yates <jordan@embeint.com>
ext2_fetch_direntry() trusted the on-disk de_rec_len and de_name_len,
and the lookup and readdir paths advanced traversal by an unvalidated
de_rec_len. A crafted ext2 image could trigger an out-of-bounds read
past the directory block buffer or a zero-progress loop in any path
that walks a directory.
Validate rec_len and name_len in the parser, and reject entries whose
header does not fit in the remaining block or whose rec_len would
cross the block boundary in each caller.
Signed-off-by: Flavio Ceolin <flavio@hubblenetwork.com>
(cherry picked from commit 7cdb534a3c)
Fix issue that would be trapped by the address sanitizer, would always
read 7 bytes even though ptr might be shorter, and would therefore
read out of bounds if e.g. the string ".org" was passed.
Signed-off-by: Egill Sigurdur <egill@egill.xyz>
(cherry picked from commit 448a21da12)
In some cases the stream->qos pointer pointed to the
qos argument, and sometimes it pointed to the ep->qos.
Now all qos arguments are copied to ep->qos, and
stream->qos always points to stream->ep.qos.
Some modules had some refactoring done to properly store
the QoS. The unicast client had some additional checks
done or redone, and some now-unused code removed.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Assisted-By: Claude:opus-4.8
Signed-off-by: David Brown <david.brown@linaro.org>
Cancellation can run on timeout paths where a context-based buffer
allocation timeout can expire immediately.
- allocate the temporary packed-name net_buf with K_FOREVER
- keep existing ENOMEM handling for pool exhaustion
Assisted-by: Codex:gpt-5.3-codex
Signed-off-by: Adam Szewczyk <a.szewczyk@cthings.co>
Cancel each timed-out DNS request before retrying and reset the local
semaphore state between attempts. This prevents stale delayed callbacks
from touching stack-backed getaddrinfo state after timeout progression.
Assisted-by: Codex:gpt-5.3-Codex
Signed-off-by: Adam Szewczyk <a.szewczyk@cthings.co>
Add validation to ensure the indicator index is within the valid range
of the ind_table array before accessing it in cind_handle_values().
Without this check, an out-of-bounds index could lead to buffer overrun
when the index is used to access hf->ind_table array elements later in
the function.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
(cherry picked from commit cf7693a826)
Problem:
When fs_open() is called with FS_O_TRUNC, the FS backend
opens the underlying file via mp->fs->open() before the
truncate is attempted. If mp->fs->truncate() then fails,
the previous code cleared zfp->mp and returned right away,
which causes two issues:
- the backend's close hook is never invoked, so the
resources allocated during open (file slab entries,
internal caches, backend-specific structures such as
lfs_file, etc.) stay permanently allocated;
- a follow-up fs_close(zfp) cannot recover them either,
because zfp->mp has already been NULL'd and fs_close()
returns early.
The leak is reproducible on every backend (LittleFS, FAT,
...) and accumulates one slot per failed call until the
file slab is exhausted.
Solution:
Close the backend file explicitly in the truncate failure
path, before clearing zfp->mp, so the FS-specific close
hook can release everything it allocated during open().
If close itself fails, log the secondary error but still
return the original truncate error code, since that is
the root cause callers diagnose against.
Signed-off-by: Shuai Ma <malin719426@gmail.com>
(cherry picked from commit 66ee97f4d9)
When sc=0, a framed ISO PDU segment header includes a 3-byte time_offset
field, so seg_hdr->len must be at least PDU_ISO_SEG_TIMEOFFSET_SIZE.
isoal_check_seg_header() accepted segments with sc=0 and len<3 as valid,
allowing isoal_rx_framed_consume() to underflow, causing an
out-of-bounds read of up to 255 bytes of adjacent memory into an HCI ISO
packet delivered to the host.
Signed-off-by: Flavio Ceolin <flavio@hubblenetwork.com>
(cherry picked from commit 28080d80fc)
subnet_keys_destroy() guarded the destroy of keys->priv_beacon with
#if defined(CONFIG_BT_MESH_V1d1), while net_keys_create() guards the
matching import with #if defined(CONFIG_BT_MESH_PRIV_BEACONS).
Commit 981c79b7ce ("Bluetooth: Mesh: Drop explicit support for
Bluetooth Mesh 1.0.1") removed the CONFIG_BT_MESH_V1d1 Kconfig but
left this stray reference behind. With V1d1 gone, the destroy branch
is permanently dead code, so every successful net_keys_create() leaks
one PSA key slot.
In a long-running test that repeatedly provisions and resets a node
(no persistent settings, in-RAM CDB only), the leak accumulates one
PSA volatile-key slot per provisioning round. With the default
CONFIG_MBEDTLS_PSA_KEY_SLOT_COUNT=16, bt_mesh_private_beacon_key()
fails after ~12 rounds with:
<err> bt_mesh_net_keys: Unable to generate private beacon key
<err> bt_mesh_net: Failed creating subnet
<err> bt_mesh_main: Failed to create network
Align the destroy guard with the import guard so the leak goes away.
Signed-off-by: Lingao Meng <menglingao@xiaomi.com>
(cherry picked from commit f573da9f53)
Use NET_CMSG_SPACE() when checking ancillary buffer capacity and
account for aligned cmsg storage in msg_controllen.
This keeps recvmsg() control-data handling consistent with cmsghdr
layout and avoids under-reporting consumed control-buffer space.
Signed-off-by: Philipp Steiner <philipp.steiner1987@gmail.com>
Avoid accessing the packet after sending it, as the driver may
have already unreferenced or freed it. Use iface argument instead
of calling net_pkt_iface() on a potentially freed packet when
updating packet statistics.
Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
(cherry picked from commit aaed8332a6)
Avoid accessing the packet after sending it, as the driver may
have already unreferenced or freed it. Store the iface before
sending instead of calling net_pkt_iface() on a potentially
freed packet when updating packet statistics.
Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
(cherry picked from commit 3159c53e8e)
Avoid accessing the packet after sending it, as the driver may
have already unreferenced or freed it. Store the iface before
sending instead of calling net_pkt_iface() on a potentially
freed packet when updating packet statistics.
Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
(cherry picked from commit 0223e5e3ec)
Avoid accessing the packet after sending it, as the driver may
have already unreferenced or freed it. Store the iface before
sending instead of calling net_pkt_iface() on a potentially
freed packet when updating packet statistics.
Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
(cherry picked from commit 09c8578c66)
Avoid accessing the packet after sending it, as the driver may
have already unreferenced or freed it. Store the iface before
sending instead of calling net_pkt_iface() on a potentially
freed packet when updating packet statistics.
Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
(cherry picked from commit 86e21665d4)
The cdc_ncm_send() function ignores the return value from
usbd_ep_enqueue(). If the enqueue fails, the code proceeds to block
forever on k_sem_take() waiting for a completion callback that will
never arrive, causing a deadlock.
This was discovered by comparing the CDC-NCM implementation with
CDC-ECM, which correctly checks the return value:
ret = usbd_ep_enqueue(c_data, buf);
if (ret) {
LOG_ERR("Failed to enqueue net_buf for 0x%02x", ep);
net_buf_unref(buf);
return ret;
}
The NCM driver was missing this error handling, leading to potential
hangs if usbd_ep_enqueue() fails for any reason (e.g., endpoint not
ready, USB disconnected, buffer issues).
Fix by checking the return value and properly cleaning up (freeing
the buffer) before returning the error code to the caller.
Signed-off-by: Jay Beavers <jay@tolttechnologies.com>
(cherry picked from commit 255bccc1ba)
Include guard was missing in lwm2m_pull_context.h internal header.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
(cherry picked from commit 55be451592)
Use the same size for the URI buffer in the FW object implementation as
in the FW pull download helper module. That way, if the server writes
too long URI to handle in the FW pull mode, it'll get an error response
immediately instead of failing at firmware download start.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
(cherry picked from commit b96deb12ad)
Verify the URI length provided to lwm2m_pull_context_start_transfer()
before use, otherwise in case the URI string is longer than the buffer,
only part of it was copied w/o NULL terminator, which could lead to
out-of-bound reads and other undefined behavior.
As the string length is now validated, just use strcpy() instead of
memcpy(), no need to copy the whole buffer.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
(cherry picked from commit 99a164df5c)
In case socket is closed during an async TCP handshake, the TCP context
should be closed immediately, otherwise the connection could be
established after the socket was closed, causing TCP context leak.
To avoid race between socket close and resend timer (i.e. socket being
closed at the same time as the retransmission limit is reached), add
extra state checks before attempting to close the TCP context.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
(cherry picked from commit 8588b08808)
validate the assumptions about buffers returned by alloc_buf() in the
LE CoC receive path.
if the returned buffer does not provide enough user_data space for the
internal segment counter, disconnect and drop the buffer instead of
reading or writing past the metadata area.
also document that alloc_buf() must return a buffer with at least
sizeof(uint16_t) bytes of user_data.
Signed-off-by: Oleh Konko <security@1seal.org>
(cherry picked from commit 09ad7174e9)
ffa_create_top() was opening files with FS_O_CREATE | FS_O_WRITE
only, which caused host applications like 'cat' to fail with
Permission denied when trying to read the file through FUSE. This
is because FAT filesystem explicitly checks the READ flag before
allowing read access.
Fix by replacing FS_O_WRITE with FS_O_RDWR so the file is opened
with both read and write access.
Signed-off-by: Surya Prakash T <suryat@aerlync.com>
(cherry picked from commit 8f959ad0e6)
Make sure we will not overflow the ipaddress buffer if
port number is given.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
(cherry picked from commit 6e119a636a)
Make sure we will not overflow the ipaddress buffer if
port number is given.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
(cherry picked from commit 1c8d19a51f)
Add bounds checking to PTP management interval parameters to prevent
undefined behavior from bitwise shift operations.
The port_timer_set_timeout() and port_timer_set_timeout_random()
functions perform left and right shift operations using the
log_announce_interval and log_sync_interval values as shift counts.
Without bounds validation, these int8_t parameters could be set to
extreme values (e.g., via PTP management messages) that exceed the
valid shift range, causing undefined behavior.
(C11, 6.5.7p3)
> If the value of the right operand is negative or is greater than
or equal to the width of the promoted left operand,
the behavior is undefined
(C++11, 5.8p1)
> The behavior is undefined if the right operand is negative,
or greater than or equal to the length in bits of the promoted
left operand.
Limit both log_announce_interval and log_sync_interval to the range
[-63, 63] using MIN/MAX macros when accepting values from PTP management
frames. This range is preventing shift operations outside
the 64-bit width used in the timeout calculations.
Fixes: GHSA-3v98-458v-388r
Signed-off-by: Adam Wojasinski <awojasinski@baylibre.com>
(cherry picked from commit 5b32348c1b)
dns_unpack_answer() validated only the fixed RR header size and
accepted any rdlength, even one extending past the end of the packet.
TXT and SRV consumers in resolve.c then read up to rdlength bytes from
the message buffer, causing an out-of-bounds read on a truncated or
crafted response.
Reject any RR whose declared rdata extends past dns_msg->msg_size at
the single chokepoint in dns_unpack_answer(), so all current and
future RR consumers are covered.
Signed-off-by: Flavio Ceolin <flavio@hubblenetwork.com>
(cherry picked from commit 58b46c81c6)
The checks validating RA, NS and NA packets content on input were not
correct - packets should be dropped in case any of those checks failed,
however current logic was invalid, causing other checks to be ignored as
long as the ICMPv6 code was correct (i. e. 0).
Apart from fixing the logic, split the single convoluted if condition
into separate if checks for better readability.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
(cherry picked from commit 095f064c94)
If CONFIG_HTTP_SERVER_MAX_HEADER_LEN is increased from the default of
32, a compiler warning pops in the HTTP websocket code:
zephyr/subsys/net/lib/http/http_server_http1.c:
In function 'on_header_value':
zephyr/subsys/net/lib/http/http_server_http1.c:898:33:
warning: 'strncpy' output may be truncated copying between 0 and 32
bytes from a string of length 47 [-Wstringop-truncation]
898 | strncpy(ctx->ws_sec_key, ctx->header_buffer,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
899 | MIN(sizeof(ctx->ws_sec_key), offset));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This comes from:
if (ctx->websocket_sec_key_next) {
#if defined(CONFIG_WEBSOCKET)
strncpy(ctx->ws_sec_key, ctx->header_buffer,
MIN(sizeof(ctx->ws_sec_key), offset));
#endif
If eg. header_buffer is 48 bytes and holds a string >= 32 bytes then
ws_sec_key can end up non-nul terminated. That can then lead to buffer
overflow in handle_http1_to_websocket_upgrade().
Add a check to make sure the header value fits in ws_sec_key, if not
reject the request with a HTTP 500. The websocket key is not expected to
be > 31 bytes.
Once the check is in place, it's safe to use memcpy() for the copy, and
then add the terminating nul manually.
Signed-off-by: Michael Ellerman <mpe@oss.tenstorrent.com>
(cherry picked from commit 5d653fcfd4)
Fix a scenario where the logging thread can enter an infinite
loop and starve lower-priority threads when encountering an
uncommitted log message.
If a lower-priority thread is preempted before committing a
message, and a higher-priority thread triggers log processing,
the logging thread may repeatedly attempt to claim it. Since the
message is pending but not committed, z_log_msg_claim() returns
NULL while z_log_msg_pending() remains true, resulting in a
livelock.
Update the log processing logic to avoid looping on uncommitted
messages, allowing lower-priority threads to resume and complete
the commit.
Fixes#101401
Signed-off-by: Javier Romera <lromerajdev@gmail.com>
(cherry picked from commit 5184eac621)
When logging statically configured network SSID, use the Kconfig string
instead of a SSID buffer where it was copied to, as the latter is not
guarantee to be NULL terminated.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
(cherry picked from commit 4f09d6bcbc)
In case CONFIG_WIFI_CREDENTIALS_STATIC is used, verify the statically
configured SSID/password lengths to guarantee they don't exceed the
allowed SSID and password character limits and thus overflow credential
buffers.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
(cherry picked from commit 90d9d8e12d)
net_tcp_foreach() drops tcp_lock before the callback and re-acquires
it afterwards. A concurrent tcp_conn_release() can free the next
node cached by SYS_SLIST_FOR_EACH_CONTAINER_SAFE during this window,
causing the iterator to follow a dangling pointer on the next
iteration.
Move context teardown in tcp_conn_release() inside the tcp_lock
critical section and keep tcp_lock held across the callback in
net_tcp_foreach(). No current callback acquires tcp_lock.
Signed-off-by: Ofir Shemesh <ofirshemesh777@gmail.com>
(cherry picked from commit cd85e0e890)
Fixes an issue whereby a write might have partially been successful
but failed due to insufficient space in the storage device by
re-attempting the write again with the offset, and if it still
fails, return an error
Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
(cherry picked from commit f99dbd622e)
