Hugues de Valon | 0f4525a | 2019-03-20 16:26:57 +0000 | [diff] [blame] | 1 | ################################################ |
| 2 | Fixing implicit casting for C enumeration values |
| 3 | ################################################ |
| 4 | |
| 5 | :Authors: Hugues de Valon |
| 6 | :Organization: Arm Limited |
| 7 | :Contact: hugues.devalon@arm.com |
| 8 | :Status: Accepted |
| 9 | |
| 10 | ******** |
| 11 | Abstract |
| 12 | ******** |
| 13 | |
| 14 | C enumerations provide a nice way to increase readability by creating new |
| 15 | enumerated types but the developer should take extra care when mixing |
| 16 | enumeration and integer values. |
| 17 | This document investigates C enumerations safety and proposes strategies on how |
| 18 | to fix the implicit casting of the enumeration values of TF-M with other types. |
| 19 | |
| 20 | ************************** |
| 21 | C99 Standard point of view |
| 22 | ************************** |
| 23 | |
| 24 | In TF-M many implicit casts are done between integer types (``uint32_t``, |
| 25 | ``int32_t``, ``size_t``, etc), enumerated types (``enum foobar``) and |
| 26 | enumeration constants (``FOOBAR_ENUM_1``). |
| 27 | |
| 28 | According to the C99 standard [1]_: |
| 29 | |
| 30 | **§6.2.5, 16**: |
| 31 | An enumeration comprises a set of named integer constant values. Each |
| 32 | distinct enumeration constitutes a different numerated type. |
| 33 | |
| 34 | **§6.7.2.2, 2**: |
| 35 | The expression that defines the value of an enumeration constant shall be an |
| 36 | integer constant expression that has a value representable as an int. |
| 37 | |
| 38 | **§6.7.2.2, 3**: |
| 39 | The identifiers in an enumerator list are declared as constants that have |
| 40 | type int and may appear wherever such are permitted. |
| 41 | |
| 42 | **§6.7.2.2, 4**: |
| 43 | Each enumerated type shall be compatible with char, a signed integer type, |
| 44 | or an unsigned integer type. The choice of type is implementation-defined, |
| 45 | but shall be capable of representing the values of all the members of the |
| 46 | enumeration. |
| 47 | |
| 48 | From these four quotes from the C99 standard [1]_, the following conclusions can |
| 49 | be made: |
| 50 | |
| 51 | * an enumeration defines a new type and should be treated as such |
| 52 | * the enumeration constants must only contains value representable as an ``int`` |
| 53 | * the enumeration constants have type ``int`` |
| 54 | * the actual type of the enumeration can be between ``char``, signed and |
| 55 | unsigned ``int``. The compiler chooses the type it wants among those that can |
| 56 | represent all declared constants of the enumeration. |
| 57 | |
| 58 | Example:: |
| 59 | |
| 60 | enum french_cities { |
| 61 | MARSEILLE, |
| 62 | PARIS, |
| 63 | LILLE, |
| 64 | LYON |
| 65 | }; |
| 66 | |
| 67 | In that example, ``MARSEILLE``, ``PARIS``, ``LILLE`` and ``LYON`` are |
| 68 | enumeration constants of type ``int`` and ``enum french_cities`` is a enumerated |
| 69 | type which can be of actual type ``char``, ``unsigned int`` or ``int`` |
| 70 | (the compiler chooses!). |
| 71 | |
| 72 | For these reasons, doing an implicit cast between an enumeration and another |
| 73 | type is the same as doing an implicit cast between two different types. From a |
| 74 | defensive programming point of view, it should be checked that the destination |
| 75 | type can represent the values from the origin type. In this specific case it |
| 76 | means four things for enumerations: |
| 77 | |
| 78 | * it is always safe to assign an enumeration constant to an int, but might be |
| 79 | better to cast to show intent. |
| 80 | * when casting an enumeration constant to another type, it should be checked |
| 81 | that the constant can fit into the destination type. |
| 82 | * when casting from an integer type (``uint32_t``, ``int32_t``, etc) to an |
| 83 | enumeration type, it should be checked that the integer's value is one of the |
| 84 | enumeration constants. The comparison needs to be done on the biggest type of |
| 85 | the two so that no information is lost. C integer promotion should |
| 86 | automatically do that for the programmer (check §6.3.1.8, 1 for the rules). |
| 87 | * when casting from an enumeration type to an integer type, it should be checked |
| 88 | that the enumeration type value fits into the integer type. The value of a |
| 89 | variable which has the type of an enumeration type is not limited to the |
| 90 | enumeration constants of the type. An enumeration constant will always fit |
| 91 | into an ``int``. |
| 92 | |
| 93 | ***************** |
| 94 | Strategies to fix |
| 95 | ***************** |
| 96 | |
| 97 | 0. Replace the enumerated type with an integer type and replace the enumeration |
| 98 | constant with preprocessor constants. |
| 99 | 1. Whenever possible, try to use matching types to avoid implicit casting. |
| 100 | It happens, for example, for arithmetic operations, function calls and |
| 101 | function returns. This strategy always have the lowest performance impact. |
| 102 | 2. When using an enumeration constant in an arithmetic operation with another |
| 103 | type, verify that the constant can fit in the other type and cast it. |
| 104 | 3. When converting an integer to an enumeration type, use a conversion function |
| 105 | to check if the integer matches an enumeration constant. To not impact |
| 106 | performance too much, this function should be an inline function. If it does |
| 107 | not match, use (or add) the error constant or return an error value. |
| 108 | 4. When converting an enumeration type to an integer, use a conversion function |
| 109 | to check that the integer type can contain the enumeration value. |
| 110 | |
| 111 | ************************ |
| 112 | Design proposal for TF-M |
| 113 | ************************ |
| 114 | |
| 115 | In TF-M, an action will be taken for all enumerated types and enumeration |
| 116 | constants that are used for implicit casting. The goal of this proposal is to |
| 117 | remove all implicit casting of enumeration values in TF-M. |
| 118 | |
| 119 | The following enumerated types will be removed and replaced with preprocessor |
| 120 | constants (strategy 0). These enumerated types are not used in TF-M |
| 121 | but only the constants they declare. |
| 122 | |
| 123 | * ``enum spm_part_state_t`` |
| 124 | * ``enum spm_part_flag_mask_t`` |
| 125 | * ``enum tfm_partition_priority`` |
| 126 | |
| 127 | The following enumerated types will be kept because they are used in the |
| 128 | prototypes of functions and are useful for debugging. Whenever possible, |
| 129 | strategy 1 will be applied to remove implicit casting related with those |
| 130 | enumerations but dynamic conversions will be used if the first option would |
| 131 | create too much change in the code base. |
| 132 | |
| 133 | * ``enum tfm_status_e``: the return type of the following functions will be |
| 134 | changed to return the ``enum tfm_status_e`` type. These functions are already |
| 135 | returning the enumeration constants, but implicitly casted to an integer type |
| 136 | like ``int32_t``. |
| 137 | |
Hugues de Valon | 0f4525a | 2019-03-20 16:26:57 +0000 | [diff] [blame] | 138 | * ``int32_t check_address_range`` |
| 139 | * ``int32_t has_access_to_region`` |
| 140 | * ``int32_t tfm_core_check_sfn_parameters`` |
| 141 | * ``int32_t tfm_start_partition`` |
| 142 | * ``int32_t tfm_return_from_partition`` |
| 143 | * ``int32_t tfm_check_sfn_req_integrity`` |
| 144 | * ``int32_t tfm_core_check_sfn_req_rules`` |
Mingyang Sun | abb1aab | 2020-02-18 13:49:08 +0800 | [diff] [blame] | 145 | * ``int32_t tfm_spm_sfn_request_handler`` |
| 146 | * ``int32_t tfm_spm_sfn_request_thread_mode`` |
Hugues de Valon | 0f4525a | 2019-03-20 16:26:57 +0000 | [diff] [blame] | 147 | * ``enum tfm_buffer_share_region_e``: the following function prototypes will be |
| 148 | changed: |
| 149 | |
| 150 | * ``tfm_spm_partition_set_share(uint32_t partition_idx, uint32_t share)`` -> ``tfm_spm_partition_set_share(uint32_t partition_idx, enum tfm_buffer_share_region_e share)`` |
| 151 | * ``enum tfm_memory_access_e`` |
| 152 | * ``enum attest_memory_access_t`` |
| 153 | * ``enum engine_cipher_mode_t`` |
| 154 | * ``mbedtls_cipher_type_t`` |
| 155 | |
| 156 | The following enumerated types are used for error code values of Secure service |
| 157 | calls. They should be kept as they are part of the interface and might be used |
| 158 | by external parties in Non-Secure code. For the Initial Attestation service, |
| 159 | the enumeration is defined in the PSA Attestation API specifications. |
| 160 | |
| 161 | * ``enum psa_attest_err_t`` |
| 162 | * ``enum psa_audit_err`` |
| 163 | * ``enum tfm_platform_err_t`` |
| 164 | |
| 165 | Implicit casting related with these enumerations is happening in two locations |
| 166 | of TF-M and need conversion functions in those locations, because the types can |
| 167 | not be changed: |
| 168 | |
| 169 | * In the Non-Secure Client library, all of the Secure Service functions |
| 170 | implicitly cast the ``uint32_t`` returned by ``tfm_ns_lock_dispatch`` to |
| 171 | these enumerated types. Strategy 3 is needed here. |
| 172 | * In all of the veneer functions, there is an implicit cast from the ``int32_t`` |
| 173 | value returned by the SVC request function (``tfm_core_*_request``) to these |
| 174 | enumerated types. Strategy 3 is needed here as well. The implicit cast will |
| 175 | eventually be removed if all of the services are using the Uniform Signatures |
| 176 | Prototypes so that the veneer functions all return ``psa_status_t`` which is |
| 177 | an ``int32_t``. |
| 178 | |
| 179 | If the interface of those services can be changed, these enumerations could be |
| 180 | removed and replaced with the ``psa_status_t`` type to remove the implicit |
| 181 | casting. |
| 182 | |
| 183 | .. [1] C99 standard: http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf |
| 184 | |
| 185 | |
| 186 | -------------- |
| 187 | |
| 188 | |
Mingyang Sun | abb1aab | 2020-02-18 13:49:08 +0800 | [diff] [blame] | 189 | *Copyright (c) 2019-2020, Arm Limited. All rights reserved.* |
Hugues de Valon | 0f4525a | 2019-03-20 16:26:57 +0000 | [diff] [blame] | 190 | |