Skip to content

libckteec: add and use {ADD|SUB|MUL}_OVERFLOW() macros #421

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

Closed
etienne-lms wants to merge 2 commits into from
Closed

libckteec: add and use {ADD|SUB|MUL}_OVERFLOW() macros #421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd3a3a0
libckteec: add {ADD|SUB|MUL}_OVERFLOW() helper macros
etienne-lms May 13, 2026
fb181f3
libckteec: consider overflows in multiplication and additions
etienne-lms May 13, 2026
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
186 changes: 186 additions & 0 deletions libckteec/src/local_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,190 @@
switch (0) { case 0: case ((x) ? 1: 0) : default : break; } \
} while (0)

/*
* Checking overflow for addition, subtraction and multiplication. Result
* of operation is stored in res which is a pointer to some kind of
* integer.
*
* The macros return true if an overflow occurred and *res is undefined.
*/
#define ADD_OVERFLOW(a, b, res) __compiler_add_overflow((a), (b), (res))
#define SUB_OVERFLOW(a, b, res) __compiler_sub_overflow((a), (b), (res))
#define MUL_OVERFLOW(a, b, res) __compiler_mul_overflow((a), (b), (res))

#define __GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + \
__GNUC_PATCHLEVEL__)

#if __GCC_VERSION >= 50100 && !defined(__CHECKER__)
#define __HAVE_BUILTIN_OVERFLOW 1
#endif

#if __GCC_VERSION >= 90100 && !defined(__CHECKER__)
#define __HAVE_SINGLE_ARGUMENT_STATIC_ASSERT 1
#endif

#ifdef __HAVE_BUILTIN_OVERFLOW
#define __compiler_add_overflow(a, b, res) \
__builtin_add_overflow((a), (b), (res))

#define __compiler_sub_overflow(a, b, res) \
__builtin_sub_overflow((a), (b), (res))

#define __compiler_mul_overflow(a, b, res) \
__builtin_mul_overflow((a), (b), (res))
#else /*!__HAVE_BUILTIN_OVERFLOW*/

/*
* Copied/inspired from https://www.fefe.de/intof.html
*/

#define __INTOF_ASSIGN(dest, src) (__extension__({ \
typeof(src) __intof_x = (src); \
typeof(dest) __intof_y = __intof_x; \
(((uintmax_t)__intof_x == (uintmax_t)__intof_y) && \
((__intof_x < 1) == (__intof_y < 1)) ? \
(void)((dest) = __intof_y) , 0 : 1); \
}))

#define __INTOF_ADD(c, a, b) (__extension__({ \
typeof(a) __intofa_a = (a); \
typeof(b) __intofa_b = (b); \
intmax_t __intofa_a_signed = __intofa_a; \
uintmax_t __intofa_a_unsigned = __intofa_a; \
intmax_t __intofa_b_signed = __intofa_b; \
uintmax_t __intofa_b_unsigned = __intofa_b; \
\
__intofa_b < 1 ? \
__intofa_a < 1 ? \
((INTMAX_MIN - __intofa_b_signed <= \
__intofa_a_signed)) ? \
__INTOF_ASSIGN((c), __intofa_a_signed + \
__intofa_b_signed) : 1 \
: \
((__intofa_a_unsigned >= (uintmax_t)-__intofa_b) ? \
__INTOF_ASSIGN((c), __intofa_a_unsigned + \
__intofa_b_signed) \
: \
__INTOF_ASSIGN((c), \
(intmax_t)(__intofa_a_unsigned + \
__intofa_b_signed))) \
: \
__intofa_a < 1 ? \
((__intofa_b_unsigned >= (uintmax_t)-__intofa_a) ? \
__INTOF_ASSIGN((c), __intofa_a_signed + \
__intofa_b_unsigned) \
: \
__INTOF_ASSIGN((c), \
(intmax_t)(__intofa_a_signed + \
__intofa_b_unsigned))) \
: \
((UINTMAX_MAX - __intofa_b_unsigned >= \
__intofa_a_unsigned) ? \
__INTOF_ASSIGN((c), __intofa_a_unsigned + \
__intofa_b_unsigned) : 1); \
}))

#define __INTOF_SUB(c, a, b) (__extension__({ \
typeof(a) __intofs_a = a; \
typeof(b) __intofs_b = b; \
intmax_t __intofs_a_signed = __intofs_a; \
uintmax_t __intofs_a_unsigned = __intofs_a; \
intmax_t __intofs_b_signed = __intofs_b; \
uintmax_t __intofs_b_unsigned = __intofs_b; \
\
__intofs_b < 1 ? \
__intofs_a < 1 ? \
((INTMAX_MAX + __intofs_b_signed >= \
__intofs_a_signed) ? \
__INTOF_ASSIGN((c), __intofs_a_signed - \
__intofs_b_signed) : 1) \
: \
(((uintmax_t)(UINTMAX_MAX + __intofs_b_signed) >= \
__intofs_a_unsigned) ? \
__INTOF_ASSIGN((c), __intofs_a - \
__intofs_b) : 1) \
: \
__intofs_a < 1 ? \
(((intmax_t)(INTMAX_MIN + __intofs_b) <= \
__intofs_a_signed) ? \
__INTOF_ASSIGN((c), \
(intmax_t)(__intofs_a_signed - \
__intofs_b_unsigned)) : 1) \
: \
((__intofs_b_unsigned <= __intofs_a_unsigned) ? \
__INTOF_ASSIGN((c), __intofs_a_unsigned - \
__intofs_b_unsigned) \
: \
__INTOF_ASSIGN((c), \
(intmax_t)(__intofs_a_unsigned - \
__intofs_b_unsigned))); \
}))

/*
* Dealing with detecting overflow in multiplication of integers.
*
* First step is to remove two corner cases with the minum signed integer
* which can't be represented as a positive integer + sign.
* Multiply with 0 or 1 can't overflow, no checking needed of the operation,
* only if it can be assigned to the result.
*
* After the corner cases are eliminated we convert the two factors to
* positive unsigned values, keeping track of the original in another
* variable which is used at the end to determine the sign of the product.
*
* The two terms (a and b) are divided into upper and lower half (x1 upper
* and x0 lower), so the product is:
* ((a1 << hshift) + a0) * ((b1 << hshift) + b0)
* which also is:
* ((a1 * b1) << (hshift * 2)) + (T1)
* ((a1 * b0 + a0 * b1) << hshift) + (T2)
* (a0 * b0) (T3)
*
* From this we can tell and (a1 * b1) has to be 0 or we'll overflow, that
* is, at least one of a1 or b1 has to be 0. Once this has been checked the
* addition: ((a1 * b0) << hshift) + ((a0 * b1) << hshift)
* isn't an addition as one of the terms will be 0.
*
* Since each factor in: (a0 * b0)
* only uses half the capicity of the underlaying type it can't overflow
*
* The addition of T2 and T3 can overflow so we use __INTOF_ADD() to
* perform that addition. If the addition succeeds without overflow the
* result is assigned the required sign and checked for overflow again.
*/

#define __intof_mul_negate ((__intof_oa < 1) != (__intof_ob < 1))
#define __intof_mul_hshift (sizeof(uintmax_t) * 8 / 2)
#define __intof_mul_hmask (UINTMAX_MAX >> __intof_mul_hshift)
#define __intof_mul_a0 ((uintmax_t)(__intof_a) >> __intof_mul_hshift)
#define __intof_mul_b0 ((uintmax_t)(__intof_b) >> __intof_mul_hshift)
#define __intof_mul_a1 ((uintmax_t)(__intof_a) & __intof_mul_hmask)
#define __intof_mul_b1 ((uintmax_t)(__intof_b) & __intof_mul_hmask)
#define __intof_mul_t (__intof_mul_a1 * __intof_mul_b0 + \
__intof_mul_a0 * __intof_mul_b1)

#define __INTOF_MUL(c, a, b) (__extension__({ \
typeof(a) __intof_oa = (a); \
typeof(a) __intof_a = __intof_oa < 1 ? -__intof_oa : __intof_oa; \
typeof(b) __intof_ob = (b); \
typeof(b) __intof_b = __intof_ob < 1 ? -__intof_ob : __intof_ob; \
typeof(c) __intof_c; \
\
__intof_oa == 0 || __intof_ob == 0 || \
__intof_oa == 1 || __intof_ob == 1 ? \
__INTOF_ASSIGN((c), __intof_oa * __intof_ob) : \
(__intof_mul_a0 && __intof_mul_b0) || \
__intof_mul_t > __intof_mul_hmask ? 1 : \
__INTOF_ADD((__intof_c), __intof_mul_t << __intof_mul_hshift, \
__intof_mul_a1 * __intof_mul_b1) ? 1 : \
__intof_mul_negate ? __INTOF_ASSIGN((c), -__intof_c) : \
__INTOF_ASSIGN((c), __intof_c); \
}))

#define __compiler_add_overflow(a, b, res) __INTOF_ADD(*(res), (a), (b))
#define __compiler_sub_overflow(a, b, res) __INTOF_SUB(*(res), (a), (b))
#define __compiler_mul_overflow(a, b, res) __INTOF_MUL(*(res), (a), (b))

#endif /*!__HAVE_BUILTIN_OVERFLOW*/

#endif /*LIBCKTEEC_LOCAL_UTILS_H*/
66 changes: 49 additions & 17 deletions libckteec/src/pkcs11_processing.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Copyright (c) 2017-2018, Linaro Limited
*/

#include <limits.h>

@jforissier jforissier Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe <stdint.h> should be included in local_utils.h instead

@etienne-lms etienne-lms Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Would make sense.
That said, limits.h is not needed here, can be removed.

#include <pkcs11.h>
#include <pkcs11_ta.h>
#include <stdio.h>
Expand All @@ -12,6 +13,7 @@

#include "pkcs11_processing.h"
#include "invoke_ta.h"
#include "local_utils.h"
#include "serializer.h"
#include "serialize_ck.h"

Expand All @@ -36,7 +38,9 @@ CK_RV ck_create_object(CK_SESSION_HANDLE session, CK_ATTRIBUTE_PTR attribs,
goto out;

/* Shm io0: (i/o) [session-handle][serialized-attributes] / [status] */
ctrl_size = sizeof(session_handle) + obj.size;
if (ADD_OVERFLOW(sizeof(session_handle), obj.size, &ctrl_size))
return CKR_ARGUMENTS_BAD;

@etienne-lms etienne-lms Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My mistake. serialize_ck_attributes() ( a few lines above) allocates memory that should be free before returning from this function. goto out would better apply here.

Note: I missed #417 that does the jobs nicely. I'll close my P-R.


ctrl = ckteec_alloc_shm(ctrl_size, CKTEEC_SHM_INOUT);
if (!ctrl) {
rv = CKR_HOST_MEMORY;
Expand Down Expand Up @@ -132,7 +136,9 @@ CK_RV ck_encdecrypt_init(CK_SESSION_HANDLE session,
* (in) [session-handle][key-handle][serialized-mechanism-blob]
* (out) [status]
*/
ctrl_size = sizeof(session_handle) + sizeof(key_handle) + obj.size;
ctrl_size = sizeof(session_handle) + sizeof(key_handle);
if (ADD_OVERFLOW(ctrl_size, obj.size, &ctrl_size))
return CKR_ARGUMENTS_BAD;

ctrl = ckteec_alloc_shm(ctrl_size, CKTEEC_SHM_INOUT);
if (!ctrl) {
Expand Down Expand Up @@ -388,7 +394,9 @@ CK_RV ck_digest_init(CK_SESSION_HANDLE session, CK_MECHANISM_PTR mechanism)
* (in) [session-handle][serialized-mechanism-blob]
* (out) [status]
*/
ctrl_size = sizeof(session_handle) + obj.size;
if (ADD_OVERFLOW(sizeof(session_handle), obj.size, &ctrl_size))
return CKR_ARGUMENTS_BAD;

ctrl = ckteec_alloc_shm(ctrl_size, CKTEEC_SHM_INOUT);
if (!ctrl) {
rv = CKR_HOST_MEMORY;
Expand Down Expand Up @@ -656,7 +664,10 @@ CK_RV ck_signverify_init(CK_SESSION_HANDLE session,
* (in) [session-handle][key-handle][serialized-mechanism-blob]
* (out) [status]
*/
ctrl_size = sizeof(session_handle) + sizeof(key_handle) + obj.size;
ctrl_size = sizeof(session_handle) + sizeof(key_handle);
if (ADD_OVERFLOW(ctrl_size, obj.size, &ctrl_size))
return CKR_ARGUMENTS_BAD;

ctrl = ckteec_alloc_shm(ctrl_size, CKTEEC_SHM_INOUT);
if (!ctrl) {
rv = CKR_HOST_MEMORY;
Expand Down Expand Up @@ -902,7 +913,9 @@ CK_RV ck_generate_key(CK_SESSION_HANDLE session,
* (in) [session-handle][serialized-mecha][serialized-attributes]
* (out) [status]
*/
ctrl_size = sizeof(session_handle) + smecha.size + sattr.size;
if (ADD_OVERFLOW(sizeof(session_handle), smecha.size, &ctrl_size) ||
ADD_OVERFLOW(ctrl_size, sattr.size, &ctrl_size))
return CKR_ARGUMENTS_BAD;

ctrl = ckteec_alloc_shm(ctrl_size, CKTEEC_SHM_INOUT);
if (!ctrl) {
Expand Down Expand Up @@ -970,7 +983,9 @@ CK_RV ck_find_objects_init(CK_SESSION_HANDLE session,
* (in) [session-handle][headed-serialized-attributes]
* (out) [status]
*/
ctrl_size = sizeof(session_handle) + obj.size;
if (ADD_OVERFLOW(sizeof(session_handle), obj.size, &ctrl_size))
return CKR_ARGUMENTS_BAD;

ctrl = ckteec_alloc_shm(ctrl_size, CKTEEC_SHM_INOUT);
if (!ctrl) {
rv = CKR_HOST_MEMORY;
Expand Down Expand Up @@ -1004,14 +1019,17 @@ CK_RV ck_find_objects(CK_SESSION_HANDLE session,
TEEC_SharedMemory *out_shm = NULL;
uint32_t session_handle = session;
uint32_t *handles = NULL;
size_t handles_size = max_count * sizeof(uint32_t);
size_t handles_size = 0;
CK_ULONG n = 0;
CK_ULONG last = 0;
size_t out_size = 0;

if (!count || (max_count && !obj))
return CKR_ARGUMENTS_BAD;

if (MUL_OVERFLOW(max_count, sizeof(uint32_t), &handles_size))
return CKR_ARGUMENTS_BAD;

/* Shm io0: (in/out) ctrl = [session-handle] / [status] */
ctrl = ckteec_alloc_shm(sizeof(session_handle), CKTEEC_SHM_INOUT);
if (!ctrl) {
Expand Down Expand Up @@ -1154,7 +1172,9 @@ CK_RV ck_get_attribute_value(CK_SESSION_HANDLE session,
goto bail;

/* Shm io0: (in/out) [session][obj-handle][attributes] / [status] */
ctrl_size = sizeof(session_handle) + sizeof(obj_handle) + sattr.size;
ctrl_size = sizeof(session_handle) + sizeof(obj_handle);
if (ADD_OVERFLOW(ctrl_size, sattr.size, &ctrl_size))
return CKR_ARGUMENTS_BAD;

ctrl = ckteec_alloc_shm(ctrl_size, CKTEEC_SHM_INOUT);
if (!ctrl) {
Expand Down Expand Up @@ -1219,7 +1239,9 @@ CK_RV ck_set_attribute_value(CK_SESSION_HANDLE session,
goto bail;

/* Shm io0: (in/out) [session][obj-handle][attributes] / [status] */
ctrl_size = sizeof(session_handle) + sizeof(obj_handle) + sattr.size;
ctrl_size = sizeof(session_handle) + sizeof(obj_handle);
if (ADD_OVERFLOW(ctrl_size, sattr.size, &ctrl_size))
return CKR_ARGUMENTS_BAD;

ctrl = ckteec_alloc_shm(ctrl_size, CKTEEC_SHM_INOUT);
if (!ctrl) {
Expand Down Expand Up @@ -1271,7 +1293,9 @@ CK_RV ck_copy_object(CK_SESSION_HANDLE session,
goto bail;

/* Shm io0: (in/out) [session][obj-handle][attributes] / [status] */
ctrl_size = sizeof(session_handle) + sizeof(obj_handle) + sattr.size;
ctrl_size = sizeof(session_handle) + sizeof(obj_handle);
if (ADD_OVERFLOW(ctrl_size, sattr.size, &ctrl_size))
return CKR_ARGUMENTS_BAD;

ctrl = ckteec_alloc_shm(ctrl_size, CKTEEC_SHM_INOUT);
if (!ctrl) {
Expand Down Expand Up @@ -1350,8 +1374,10 @@ CK_RV ck_derive_key(CK_SESSION_HANDLE session,
* (in) [session-handle][obj-handle][serialized-mecha][serialized-attributes]
* (out) [status]
*/
ctrl_size = sizeof(session_handle) + sizeof(obj_handle) + smecha.size +
sattr.size;
ctrl_size = sizeof(session_handle) + sizeof(obj_handle);
if (ADD_OVERFLOW(ctrl_size, smecha.size, &ctrl_size) ||
ADD_OVERFLOW(ctrl_size, sattr.size, &ctrl_size))
return CKR_ARGUMENTS_BAD;

ctrl = ckteec_alloc_shm(ctrl_size, CKTEEC_SHM_INOUT);
if (!ctrl) {
Expand Down Expand Up @@ -1474,8 +1500,10 @@ CK_RV ck_generate_key_pair(CK_SESSION_HANDLE session,
* [serialized-priv_attribs]
* (out) [status]
*/
ctrl_size = sizeof(session_handle) + smecha.size + pub_sattr.size +
priv_sattr.size;
if (ADD_OVERFLOW(sizeof(session_handle), smecha.size, &ctrl_size) ||
ADD_OVERFLOW(ctrl_size, pub_sattr.size, &ctrl_size) ||
ADD_OVERFLOW(ctrl_size, priv_sattr.size, &ctrl_size))
return CKR_ARGUMENTS_BAD;

ctrl = ckteec_alloc_shm(ctrl_size, CKTEEC_SHM_INOUT);
if (!ctrl) {
Expand Down Expand Up @@ -1557,7 +1585,9 @@ CK_RV ck_wrap_key(CK_SESSION_HANDLE session, CK_MECHANISM_PTR mechanism,
* (out) [status]
*/
ctrl_size = sizeof(session_handle) + sizeof(wrp_key_handle) +
sizeof(key_handle) + smecha.size;
sizeof(key_handle);
if (ADD_OVERFLOW(ctrl_size, smecha.size, &ctrl_size))
return CKR_ARGUMENTS_BAD;

ctrl = ckteec_alloc_shm(ctrl_size, CKTEEC_SHM_INOUT);
if (!ctrl) {
Expand Down Expand Up @@ -1647,8 +1677,10 @@ CK_RV ck_unwrap_key(CK_SESSION_HANDLE session, CK_MECHANISM_PTR mechanism,
* [serialized-attributes]
* (out) [status]
*/
ctrl_size = sizeof(session_handle) + sizeof(unwrapping_key_handle) +
smecha.size + sattr.size;
ctrl_size = sizeof(session_handle) + sizeof(unwrapping_key_handle);
if (ADD_OVERFLOW(ctrl_size, smecha.size, &ctrl_size) ||
ADD_OVERFLOW(ctrl_size, sattr.size, &ctrl_size))
return CKR_ARGUMENTS_BAD;

ctrl = ckteec_alloc_shm(ctrl_size, CKTEEC_SHM_INOUT);
if (!ctrl) {
Expand Down
Loading
Loading