libckteec: add and use {ADD|SUB|MUL}_OVERFLOW() macros - #421
Conversation
Add ADD_OVERFLOW(), SUB_OVERFLOW() and MUL_OVERFLOW() helper macros dumped from optee_os util.h [1] and compiler.h [3] where released under the same BSD-2-Clause license terms used in libckteec implementation. Link: https://github.com/OP-TEE/optee_os/blob/4.10.0/lib/libutils/ext/include/util.h [1] Link: https://github.com/OP-TEE/optee_os/blob/4.10.0/lib/libutils/ext/include/compiler.h [2] Signed-off-by: Etienne Carriere <etienne.carriere@st.com>
Guard against overflow in buffer size before they are allocated. Guard against overflow in when serializing data send to the TA. Use *_OVERFLOW() macros where applicable instead of less intuitive implementation. Signed-off-by: Etienne Carriere <etienne.carriere@st.com>
There was a problem hiding this comment.
Hi @etienne-lms,
Please see one comment below, and also:
s/Guard against overflow in when serializing/Guard against overflow when serializing/
With that fixed:
Reviewed-by: Jerome Forissier <jerome.forissier@arm.com>
Thanks!
| * Copyright (c) 2017-2018, Linaro Limited | ||
| */ | ||
|
|
||
| #include <limits.h> |
There was a problem hiding this comment.
I believe <stdint.h> should be included in local_utils.h instead
There was a problem hiding this comment.
Would make sense.
That said, limits.h is not needed here, can be removed.
| * Copyright (c) 2017-2018, Linaro Limited | ||
| */ | ||
|
|
||
| #include <limits.h> |
There was a problem hiding this comment.
Would make sense.
That said, limits.h is not needed here, can be removed.
| /* 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; |
There was a problem hiding this comment.
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.
|
This pull request has been marked as a stale pull request because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this pull request will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time. |
2 participants
Add
ADD_OVERFLOW(),SUB_OVERFLOW()andMUL_OVERFLOW()helper macros dumped from optee_os util.h [1] and compiler.h [2] and use them to guard against index/buffer overflows.[1] https://github.com/OP-TEE/optee_os/blob/4.10.0/lib/libutils/ext/include/util.h
[2] https://github.com/OP-TEE/optee_os/blob/4.10.0/lib/libutils/ext/include/compiler.h