Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

include: util: Add mem_xor functions #67159

Merged
merged 1 commit into from
Jan 5, 2024
Merged

include: util: Add mem_xor functions #67159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions include/zephyr/sys/util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,45 @@ char *utf8_lcpy(char *dst, const char *src, size_t n);
(((buflen) != 0) && \
((UINTPTR_MAX - (uintptr_t)(addr)) <= ((uintptr_t)((buflen) - 1))))

/**
* @brief XOR n bytes
*
* @param dst Destination of where to store result. Shall be @p len bytes.
* @param src1 First source. Shall be @p len bytes.
* @param src2 Second source. Shall be @p len bytes.
* @param len Number of bytes to XOR.
*/
static inline void mem_xor_n(uint8_t *dst, const uint8_t *src1, const uint8_t *src2, size_t len)
{
while (len--) {
*dst++ = *src1++ ^ *src2++;
}
}

/**
* @brief XOR 32 bits
*
* @param dst Destination of where to store result. Shall be 32 bits.
* @param src1 First source. Shall be 32 bits.
* @param src2 Second source. Shall be 32 bits.
*/
static inline void mem_xor_32(uint8_t dst[4], const uint8_t src1[4], const uint8_t src2[4])
{
mem_xor_n(dst, src1, src2, 4U);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One possible optimization here would be to cast to uint32_t and then xor directly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a bad suggestion. Since the code was copied from implementations from @cvinayak I'd like his input as to why it was not done like that in the first place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to support CPUs needing 4-byte aligned memory read/write operations (ARM cortex-m0 variants etc...).

Fun fact, was my very first contribution to Zephyr Project! de999a8

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably solvable by using UNALIGNED_PUT/UNALIGNED_GET macros.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry look at your comment on UNALIGNED_PUT/UNALIGNED_GET now. The implementation in our toolchain header has workarounds for compiler version, and my feeling goes towards sticking to simple old while loop for now.

}

/**
* @brief XOR 128 bits
*
* @param dst Destination of where to store result. Shall be 128 bits.
* @param src1 First source. Shall be 128 bits.
* @param src2 Second source. Shall be 128 bits.
*/
static inline void mem_xor_128(uint8_t dst[16], const uint8_t src1[16], const uint8_t src2[16])
{
mem_xor_n(dst, src1, src2, 16);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here have a small loop and call the 32 variant instead.

}

#ifdef __cplusplus
}
#endif
Expand Down
13 changes: 2 additions & 11 deletions subsys/bluetooth/audio/csip_crypto.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <tinycrypt/cmac_mode.h>
#include <tinycrypt/ccm_mode.h>
#include <zephyr/sys/byteorder.h>
#include <zephyr/sys/util.h>

#include "common/bt_str.h"

Expand Down Expand Up @@ -51,16 +52,6 @@ static int aes_cmac(const uint8_t key[BT_CSIP_CRYPTO_KEY_SIZE],
return 0;
}

static void xor_128(const uint8_t a[16], const uint8_t b[16], uint8_t out[16])
{
size_t len = 16;
/* TODO: Identical to the xor_128 from smp.c: Move to util */

while (len--) {
*out++ = *a++ ^ *b++;
}
}

int bt_csip_sih(const uint8_t sirk[BT_CSIP_SET_SIRK_SIZE], uint8_t r[BT_CSIP_CRYPTO_PRAND_SIZE],
uint8_t out[BT_CSIP_CRYPTO_HASH_SIZE])
{
Expand Down Expand Up @@ -229,7 +220,7 @@ int bt_csip_sef(const uint8_t k[BT_CSIP_CRYPTO_KEY_SIZE],
sys_mem_swap(k1_out, sizeof(k1_out));
}

xor_128(k1_out, sirk, out_sirk);
mem_xor_128(out_sirk, k1_out, sirk);
LOG_DBG("out %s", bt_hex(out_sirk, BT_CSIP_SET_SIRK_SIZE));

return 0;
Expand Down
1 change: 1 addition & 0 deletions subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv_iso.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <soc.h>
#include <zephyr/sys/byteorder.h>
#include <zephyr/sys/util.h>

#include "hal/cpu.h"
#include "hal/ccm.h"
Expand Down
1 change: 1 addition & 0 deletions subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync_iso.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <soc.h>
#include <zephyr/sys/byteorder.h>
#include <zephyr/sys/util.h>

#include "hal/cpu.h"
#include "hal/ccm.h"
Expand Down
1 change: 1 addition & 0 deletions subsys/bluetooth/controller/ll_sw/ull_conn_iso.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <zephyr/kernel.h>
#include <zephyr/sys/byteorder.h>
#include <zephyr/sys/util.h>
#include <zephyr/bluetooth/hci_types.h>

#include "util/util.h"
Expand Down
18 changes: 0 additions & 18 deletions subsys/bluetooth/controller/util/mem.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,24 +134,6 @@ uint8_t mem_nz(uint8_t *src, uint16_t len)
return 0;
}

/**
* @brief XOR bytes
*/
inline void mem_xor_n(uint8_t *dst, uint8_t *src1, uint8_t *src2, uint16_t len)
{
while (len--) {
*dst++ = *src1++ ^ *src2++;
}
}

/**
* @brief XOR 32-bits
*/
void mem_xor_32(uint8_t *dst, uint8_t *src1, uint8_t *src2)
{
mem_xor_n(dst, src1, src2, 4U);
}

/**
* @brief Unit test
*/
Expand Down
2 changes: 0 additions & 2 deletions subsys/bluetooth/controller/util/mem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,5 @@ uint16_t mem_index_get(const void *mem, const void *mem_pool, uint16_t mem_size)

void mem_rcopy(uint8_t *dst, uint8_t const *src, uint16_t len);
uint8_t mem_nz(uint8_t *src, uint16_t len);
void mem_xor_n(uint8_t *dst, uint8_t *src1, uint8_t *src2, uint16_t len);
void mem_xor_32(uint8_t *dst, uint8_t *src1, uint8_t *src2);

uint32_t mem_ut(void);
13 changes: 2 additions & 11 deletions subsys/bluetooth/host/smp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1829,15 +1829,6 @@ static uint8_t smp_send_pairing_random(struct bt_smp *smp)
}

#if !defined(CONFIG_BT_SMP_SC_PAIR_ONLY)
static void xor_128(const uint8_t p[16], const uint8_t q[16], uint8_t r[16])
{
size_t len = 16;

while (len--) {
*r++ = *p++ ^ *q++;
}
}

static int smp_c1(const uint8_t k[16], const uint8_t r[16],
const uint8_t preq[7], const uint8_t pres[7],
const bt_addr_le_t *ia, const bt_addr_le_t *ra,
Expand All @@ -1864,7 +1855,7 @@ static int smp_c1(const uint8_t k[16], const uint8_t r[16],
/* c1 = e(k, e(k, r XOR p1) XOR p2) */

/* Using enc_data as temporary output buffer */
xor_128(r, p1, enc_data);
mem_xor_128(enc_data, r, p1);

err = bt_encrypt_le(k, enc_data, enc_data);
if (err) {
Expand All @@ -1878,7 +1869,7 @@ static int smp_c1(const uint8_t k[16], const uint8_t r[16],

LOG_DBG("p2 %s", bt_hex(p2, 16));

xor_128(enc_data, p2, enc_data);
mem_xor_128(enc_data, p2, enc_data);

return bt_encrypt_le(k, enc_data, enc_data);
}
Expand Down
71 changes: 71 additions & 0 deletions tests/unit/util/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,4 +618,75 @@ ZTEST(util, test_IF_DISABLED)
#undef test_IF_DISABLED_FLAG_B
}

ZTEST(util, test_mem_xor_n)
{
const size_t max_len = 128;
uint8_t expected_result[max_len];
uint8_t src1[max_len];
uint8_t src2[max_len];
uint8_t dst[max_len];

memset(expected_result, 0, sizeof(expected_result));
memset(src1, 0, sizeof(src1));
memset(src2, 0, sizeof(src2));
memset(dst, 0, sizeof(dst));

for (size_t i = 0U; i < max_len; i++) {
const size_t len = i;

for (size_t j = 0U; j < len; j++) {
src1[j] = 0x33;
src2[j] = 0x0F;
expected_result[j] = 0x3C;
}

mem_xor_n(dst, src1, src2, len);
zassert_mem_equal(expected_result, dst, len);
}
}

ZTEST(util, test_mem_xor_32)
{
uint8_t expected_result[4];
uint8_t src1[4];
uint8_t src2[4];
uint8_t dst[4];

memset(expected_result, 0, sizeof(expected_result));
memset(src1, 0, sizeof(src1));
memset(src2, 0, sizeof(src2));
memset(dst, 0, sizeof(dst));

for (size_t i = 0U; i < 4; i++) {
src1[i] = 0x43;
src2[i] = 0x0F;
expected_result[i] = 0x4C;
}

mem_xor_32(dst, src1, src2);
zassert_mem_equal(expected_result, dst, 4);
}

ZTEST(util, test_mem_xor_128)
{
uint8_t expected_result[16];
uint8_t src1[16];
uint8_t src2[16];
uint8_t dst[16];

memset(expected_result, 0, sizeof(expected_result));
memset(src1, 0, sizeof(src1));
memset(src2, 0, sizeof(src2));
memset(dst, 0, sizeof(dst));

for (size_t i = 0U; i < 16; i++) {
src1[i] = 0x53;
src2[i] = 0x0F;
expected_result[i] = 0x5C;
}

mem_xor_128(dst, src1, src2);
zassert_mem_equal(expected_result, dst, 16);
}

ZTEST_SUITE(util, NULL, NULL, NULL, NULL, NULL);
Loading