18620: core: add core_mutex_debug to aid debugging deadlocks r=maribu a=maribu
### Contribution description
Adding `USEMODULE += core_mutex_debug` to your `Makefile` results in
on log messages such as
[mutex] waiting for thread 1 (pc = 0x800024d)
being added whenever `mutex_lock()` blocks. This makes tracing down
deadlocks easier.
### Testing procedure
Run e.g.
```sh
USEMODULE=core_mutex_debug BOARD=nucleo-f767zi make -C tests/mutex_cancel flash test
```
which should provide output such as
```
Welcome to pyterm!
Type '/exit' to exit.
READY
s
[mutex] waiting for thread 1 (pc = 0x8000f35)
START
main(): This is RIOT! (Version: 2022.10-devel-841-g5cc02-core/mutex/debug)
Test Application for mutex_cancel / mutex_lock_cancelable
=========================================================
Test without cancellation: OK
Test early cancellation: OK
Verify no side effects on subsequent calls: [mutex] waiting for thread 1 (pc = 0x800024d)
OK
Test late cancellation: [mutex] waiting for thread 1 (pc = 0x0)
OK
TEST PASSED
```
```sh
$ arm-none-eabi-addr2line -a 0x800024d -e tests/mutex_cancel/bin/nucleo-f767zi/tests_mutex_cancel.elf
0x0800024d
/home/maribu/Repos/software/RIOT/tests/mutex_cancel/main.c:51
```
### Issues/PRs references
Depends on and includes https://github.com/RIOT-OS/RIOT/pull/18619
19296: nanocoap: allow to define CoAP resources as XFA r=maribu a=benpicco
19504: cpu/cc26xx_cc13xx: Fix bogus array-bound warning r=maribu a=maribu
### Contribution description
GCC 12 create a bogus array out of bounds warning as it assumes that because there is special handling for `uart == 0` and `uart == 1`, `uart` can indeed be `1`. There is an `assert(uart < UART_NUMOF)` above that would blow up prior to any out of bounds access.
In any case, optimizing out the special handling of `uart == 1` for when `UART_NUMOF == 1` likely improves the generated code and fixes the warning.
/home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:88:8: error: array subscript 1 is above array bounds of 'uart_isr_ctx_t[1]' [-Werror=array-bounds]
88 | ctx[uart].rx_cb = rx_cb;
| ~~~^~~~~~
/home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:52:23: note: while referencing 'ctx'
52 | static uart_isr_ctx_t ctx[UART_NUMOF];
| ^~~
/home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:89:8: error: array subscript 1 is above array bounds of 'uart_isr_ctx_t[1]' [-Werror=array-bounds]
89 | ctx[uart].arg = arg;
| ~~~^~~~~~
/home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:52:23: note: while referencing 'ctx'
52 | static uart_isr_ctx_t ctx[UART_NUMOF];
| ^~~
### Testing procedure
The actual change is a pretty obvious one-liner, so that code review and a green CI should be sufficient. If not, running any UART example app without regression should do.
### Issues/PRs references
None
19506: tools/openocd: Fix handling of OPENOCD_CMD_RESET_HALT r=maribu a=maribu
### Contribution description
The OPENOCD_CMD_RESET_HALT was not longer correctly passed to the script. This fixes the issue.
### Testing procedure
Flashing of e.g. the `cc2650-launchpad` with upstream OpenOCD should work again.
### Issues/PRs references
The change was added to https://github.com/RIOT-OS/RIOT/pull/19050 after testing the PR and before merging. I'm not sure if the fix never worked because of this, or if behavior of `target-export-variables` or GNU Make changed.
Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
19383: cord: include gcoap_req_send returning 0 in error r=benpicco a=bergzand
### Contribution description
gcoap_req_send returns 0 if it was unable to send the CoAP request. CoRD did not include that case in the return code checks. This changes CoRD to include it and drop the registration if CoAP could not send the request. The old behaviour made the CoRD thread lock up.
### Testing procedure
- Check with the gcoap API docs.
- I can reliable trigger the issue with a RIOT application including both the `cord_ep_standalone` module and some measurement reported both sending requests to the same application. If at some point the application is shut down, gcoap has all its memo's occupied with the measurement reporting and can't add the CoRD update request. Thus the CoRD update request fails with a zero code and the thread (previously) would lock up.
### Issues/PRs references
None
19385: cpu/stm32/periph/timer: fix clobered IRQ flag r=benpicco a=Enoch247
### Contribution description
From the git commit:
> The STM32 periph_timer driver reads the timer's status flags, then clears them all. It is possible that a timer interrupt could occur between reading the flag and clearing it. This would lead to a lost interrupt.
>
> The timer's status flags can be cleared by software, but can only be set by the hardware. This patch takes advantage of this by only clearing the flags it knows are set. The rest of the flags are set, which doesn't actually change their state.
I had trouble finding anything in ST's datasheet saying that software could not set the timer's status flags, but testing showed that this is how it works in practice. Further, [ST's own HAL ](https://github.com/STMicroelectronics/STM32CubeF4/blob/master/Drivers/STM32F4xx_HAL_Driver/Inc/stm32f4xx_hal_tim.h#L1258)confirms this. If the hardware didn't work this way, it would be impossible to atomically read-modify-write the flags.
### Testing procedure
I tested by doing the following:
1. `make -C tests/periph_timer BOARD=nucleo-f767zi all flash term`
2. press s
3. press [ENTER]
4. observe test passes
5. `make -C tests/periph_timer_periodic BOARD=nucleo-f767zi all flash term`
6. press s
7. press [ENTER]
8. observe test passes
9. `make -C tests/periph_timer_short_relative_set BOARD=nucleo-f767zi all flash term`
10. press s
11. press [ENTER]
12. observe test passes
### Issues/PRs references
- none known
Co-authored-by: Koen Zandberg <koen@bergzand.net>
Co-authored-by: Joshua DeWeese <jdeweese@primecontrols.com>
gcoap_req_send returns 0 if it was unable to send the CoAP request. CoRD
did not include that case in the return code checks. This changes CoRD
to include it and drop the registration if CoAP could not send the
request. The old behaviour made the CoRD thread lock up.
19365: bootloaders: fix bootloader button logic r=benpicco a=dylad
### Contribution description
In lastest master, the `BTN_BOOTLOADER_INVERTED` logic doesn't work as expected.
This PR fixes the underlying logic by replacing the `BTN_BOOTLOADER_INVERTED` macro definition by a runtime function.
In fact the current code:
```
#ifndef BTN_BOOTLOADER_INVERTED
#if (BTN0_MODE == GPIO_IN_PD)
#define BTN_BOOTLOADER_INVERTED false
#else
#define BTN_BOOTLOADER_INVERTED true
#endif
#endif
```
cannot work because both `BTN0_MODE` and `GPIO_IN_PD` are not known by the precompiler as they are enum values defined at cpu level.
Thus, replaces it by a runtime function in our bootloader applications.
I've also add `GPIO_OD_PU` along side `GPIO_IN_PU` and add a new define (which can be override at board level or app level) in case an external pullup is used.
### Testing procedure
Flash the riotboot_dfu bootloader:
`make BOARD=saml21-xpro -C bootloaders/riotboot_dfu flash`
Then, flash any test app:
`PROGRAMMER=dfu-util USEMODULE=usbus_dfu make BOARD=saml21-xpro -C tests/shell riotboot/flash-slot0`
With master, the application will not start.
With this PR, the application will start after flashing.
### Issues/PRs references
Fixes#19364
19366: nanocoap_sock: don't include token in empty ACK response r=benpicco a=benpicco
19367: cord: bump reference from draft to rfc r=benpicco a=bergzand
### Contribution description
The draft is an RFC, this bumps the "see also" in the docs to the rfc.
### Testing procedure
Check that the correct RFC is linked in the docs.
### Issues/PRs references
None
Co-authored-by: Dylan Laduranty <dylan.laduranty@mesotic.com>
Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
Co-authored-by: Koen Zandberg <koen@bergzand.net>
This is an API change in the latter, which would typically now take an
extra argument GCOAP_SOCKET_TYPE_UNDEF.
Follow-Up-For: https://github.com/RIOT-OS/RIOT/pull/16688
19142: sock_dtls: move common code into sock_dtls_establish_session() r=benpicco a=benpicco
Co-authored-by: Benjamin Valentin <benpicco@beuth-hochschule.de>
18773: nanocoap_sock: fix handling empty ACKs with separate response r=maribu a=benpicco
Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
If a NON confirmable message is sent with a callback function,
not receiving a response in time would lead to a retransmission.
This is of course an error, as only CON messages are to be retransmitted.
As far as I can tell, no DHCPv6 RFC specifies this option. The handling
for the zero option was added in #17736 by @benpicco to fix issues
encountered while trying to retrieve a DHCHPv6 lease. However, I
strongly suspect that the zero option was encountered in this case due
to an out-of-bounds read performed in RIOT's DHCPv6 client
implementation (i.e. the option parsing loop read beyond the packet
bounds). This issue was fixed in #18307 and I strongly suspect that it
should also fix the issue @benpicco originally encountered in #17736. As
such, I propose that we remove the if statement which treats the zero
option as an end-of-payload marker.
Fixes#18309
- most were trivial
- missing group close or open
- extra space
- no doxygen comment
- name commad might open an implicit group
this hould also be implicit cosed but does not happen somtimes
- crazy: internal declared groups have to be closed internal
As per Section 5.2.1 of the MQTT-SN specification, the MQTT-SN length
header is either 1- or 3-octet long. If it is 3-octet long then the
first octet is 0x01. The asymcute implementation currently only checks
that the incoming packet is at least 2-octet long before attempting to
parse it (MIN_PKT_LEN). However, if the first octet is 0x01 the packet
must be more than 3 octet long in order to be valid. Since asymcute
does not check this it reads one octet beyond the packet data for a
2-octet packet where the first octet has the value 0x01. This commit
fixes this issue by adding an additional sanity check to _len_get.
Currently, asymcute only matches an MQTT-SN request to its
acknowledgement using the MsgId header. However, I strongly believe
this to be insufficient as asymcute would thus also match a SUBACK
to a prior PUBLISH message (for example) as long as the message ID
matches. To address this issue, this commit modifies _req_preprocess
to also compare the request message type in addition to the message id.
The _parse_reply function iterates over the DHCPv6 message options
twice but only performs sanity checks on the option length in the
first iteration. As such, both loop iterations need to be identical.
Unfortunately, there aren't without this commit as (1) they use
different maximum length values and (2) the first iteration stops
parsing as soon as it encounters a zero option while the second
doesn't. As such, it is possible for out-of-bounds read to be
performed by the second loop iteration. This commit fixes this.
The handlers for these MQTT message lock the connection mutex on
function entry. During automated testing of asymcute, I discovered
return paths for these function which do not unlock the connection
mutex. This results in a deadlock which prevents asymcute from
sending any further messages.
The assertion is a bit overeager.
In case of receiving a wrong message ID, we re-try receive without
entering the STATE_REQUEST_SEND state again, so it is expected that
we get a non-NULL ctx/response from sock_udp_recv_buf().
What this assert should actually check is that we don't get a non-NULL
ctx after calling sock_udp_recv_buf() with a non-NULL ctx.
So make this explicit to not falsely fail the assertion.
Most of the caching operation was moved to the client code. Since the
forward proxy is using that code for upstream messaging, interacting
with the cache directly is not necessary anymore.
The only cache-related thing necessary for the proxy is validating ETags
from upstream. However, that can be done by just looking at the ETags
from the upstream response (which may or may not have come from the
cache).
