对微控制器进行编程的每个人都可能了解FreeRTOS，或者至少听说过该操作系统。 亚马逊公司决定扩展此操作系统的功能以与AWS物联网服务一起使用-这就是Amazon FreeRTOS出现的方式。 我们，PVS-Studio代码分析器的开发人员，被要求检查邮件和文章下方的注释中的这些项目。 好吧，你问-我们做到了。 这是怎么回事-请继续阅读。
关于项目的一点
首先，我将向您介绍已检查项目的“爸爸”-FreeRTOS（您可以在
此处找到源代码）。 如Wikipedia所述，FreeRTOS是嵌入式系统的多任务实时操作系统。
它是用很好的旧C语言编写的，这并不奇怪-该操作系统应在微控制器的典型条件下工作：低计算能力，少量RAM等。 C语言允许您以较低的级别使用资源并具有高性能，因此它最适合开发此类OS。
现在回到亚马逊，它并没有停滞不前，而是在各个有前途的领域中发展。 例如，亚马逊正在开发Amazon Lumberyard游戏AAA引擎，我们
也对其进行了测试 。
物联网（Internet of Things，IoT）就是这样的领域。 为了在这一领域发展，亚马逊决定编写自己的操作系统-他们以FreeRTOS内核为基础。
最终的系统Amazon FreeRTOS被定位为“提供安全连接到Amazon Web Services（例如AWS IoT Core或AWS IoT Greengrass）的能力”。 该项目的源代码
存储在Github上。
在本文中，我们将检查FreeRTOS中是否存在错误，以及就静态代码分析而言，该操作系统相对于Amazon的安全性如何。
支票怎么样
使用自动错误搜索工具PVS-Studio静态代码分析器检查代码。 它能够检测用C，C ++，C＃和Java编写的程序中的错误。
在开始分析之前，有必要组装项目-因此我将确保我具有所有必要的依赖关系，并且一切与该项目一致。 有几种验证项目的方法-例如，使用编译监视系统。 我使用Visual Studio插件进行了分析-很好的是，在两个项目的存储库中都有一组项目文件，可轻松在Windows下构建。
对我而言，所需要做的就是收集项目，以确保有必要进行验证。 接下来，我开始分析，瞧！ -在我眼前是现成的分析仪报告。
这些项目中包含的第三方库也可能包含错误，它们当然也可能影响程序的运行。 但是，为了叙述的纯正，我从分析中排除了它们。
因此，分析项目，接收报告，写出有趣的错误。 现在该继续进行他们的分析了！
什么隐藏了FreeRTOS
最初，我希望写两篇单独的文章：每个操作系统一篇。 我已经揉了揉双手，准备写一篇关于FreeRTOS的好文章。 预期甚至会检测到几个多汁的错误（例如
CWE-457 ），我热切地看着分析仪的一些警告，而……什么也没有。 我没有发现任何有趣的错误。
分析仪发出的许多警告与FreeRTOS无关。 例如，此类警告是64位缺陷，例如将
size_t强制转换为
uint32_t 。 这是由于FreeRTOS设计为可在指针大小不超过32位的设备上工作。
我仔细检查了所有与不相关结构的指针之间的强制转换有关的
V1027警告。 如果可还原结构具有相同的对齐方式，则这种转换不是错误。 而且我没有找到一个危险的演员！
所有其他可疑位置要么与编码样式相关，要么带有注释，以解释为什么在此处准确执行此操作以及为什么这不是错误。
通常，我想联系FreeRTOS的开发人员。 你们真的很棒！ 我们几乎从未遇到过像您这样的干净优质的项目。 我很高兴阅读干净，整洁且有据可查的代码。 向您致敬。
尽管那天我找不到任何有趣的错误，但我知道我不会就此止步。 我怀着坚定的信念回到家，将在Amazon 100％的版本中找到一些有趣的东西，并且明天我一定会为本文收集足够的错误。 您可能猜到了，我是对的。
什么隐藏了Amazon FreeRTOS
事实证明，亚马逊的系统版本是...稍微说得有点差。 FreeRTOS的遗留物仍然很干净，但是新修订版却非常有趣。
在某些地方，程序的逻辑被违反，在某些地方指针不能正确使用。 在某些地方，代码可能导致未定义的行为，但是程序员在某些地方根本不了解他所犯的错误模式。 我什至发现了一些严重的潜在漏洞。
我在介绍时有所延迟。 让我们开始解析错误！
程序逻辑违规
让我们从有问题的区域开始，这些问题区域清楚地表明程序运行不完全符合程序员的预期。 第一个这样的地方将是对数组的可疑工作：
static _requestPool_t _requestPool = { 0 }; .... static int _scheduleAsyncRequest(int reqIndex, uint32_t currentRange) { .... _requestPool.pRequestDatas[reqIndex].pConnHandle = &_connHandle; _requestPool.pRequestDatas[reqIndex].pConnConfig = &_connConfig; _requestPool.pRequestDatas[reqIndex].reqNum = reqIndex; _requestPool.pRequestDatas[reqIndex].currRange = currentRange; _requestPool.pRequestDatas[reqIndex].currDownloaded = 0; _requestPool.pRequestDatas[reqIndex].numReqBytes = numReqBytes; .... _requestPool.pRequestDatas->scheduled = true; .... }
PVS-Studio为此段代码发出了两个警告：
- V619数组'_requestPool.pRequestDatas'被用作指向单个对象的指针。 iot_demo_https_s3_download_async.c 973
- V574'_requestPool.pRequestDatas'指针同时用作数组和指向单个对象的指针。 检查行：931、973。iot_demo_https_s3_download_async.c 973
为了以防万一，让我提醒您：数组的名称是指向其第一个元素的指针。 也就是说，如果
_requestPool.pRequestDatas是结构数组，则
_requestPool.pRequestDatas [i] .scheduled可以访问数组第
i个结构的
计划成员。 而且，如果您编写
_requestPool.pRequestDatas-> cheduled ，这将意味着访问数组第一个结构的
Scheduled成员。
这就是上面的代码片段中发生的情况。 最后一行始终仅更改数组第一个结构的成员的值。 就其本身而言，这样的调用已经是可疑的，但是这里的情况更加明显：在函数的整个主体中，
_requestPool.pRequestDatas数组都由索引访问，并且只有在索引操作结束时才被忘记使用。
据我了解，最后一行应如下所示：
_requestPool.pRequestDatas[reqIndex].scheduled = true;
以下错误在于一个小的函数，因此，我将完整介绍它：
static BaseType_t prvGGDJsoneq( const char * pcJson, const jsmntok_t * const pxTok, const char * pcString ) { uint32_t ulStringSize = ( uint32_t ) pxTok->end - ( uint32_t ) pxTok->start; BaseType_t xStatus = pdFALSE; if( pxTok->type == JSMN_STRING ) { if( ( uint32_t ) strlen( pcString ) == ulStringSize ) { if( ( int16_t ) strncmp( &pcJson[ pxTok->start ],
PVS-Studio警告： V642 [CWE-197]将'strncmp'函数结果保存在'short'类型变量中是不合适的。 有效位可能会丢失，从而破坏程序的逻辑。 第637章
让我们看一下strncmp函数定义：
int strncmp( const char *lhs, const char *rhs, size_t count );
在该示例中，大小为32位的
int类型的结果被转换为
int16_t类型的变量。 通过这种“缩小”转换，将丢失返回值的最高有效位。 例如，如果
strncmp函数返回
0x00010000 ，则在转换过程中将
丢失 一个 ，并满足条件。
实际上，看到这样的演员处于某种状态是很奇怪的。 如果您可以将正常
int与零进行比较，为什么还要这样做呢？ 另一方面，如果程序员有意识地希望函数有时返回
true ，即使它不应该返回
true ，那么为什么注释中没有描述这种棘手的行为呢？ 但是，这已经是一个书签。 总的来说，我倾向于认为这是一个错误。 你觉得呢
未定义的行为和指针
现在将有一个很大的例子。 它隐藏了空指针的潜在取消引用：
static void _networkReceiveCallback(....) { IotHttpsReturnCode_t status = IOT_HTTPS_OK; _httpsResponse_t* pCurrentHttpsResponse = NULL; IotLink_t* pQItem = NULL; .... IotMutex_Lock(&(pHttpsConnection->connectionMutex)); pQItem = IotDeQueue_PeekHead(&(pHttpsConnection->respQ)); IotMutex_Unlock(&(pHttpsConnection->connectionMutex)); if (pQItem == NULL) { IotLogError(....); fatalDisconnect = true; status = IOT_HTTPS_NETWORK_ERROR; goto iotCleanup; } .... iotCleanup : if (status != IOT_HTTPS_OK) { if ( pCurrentHttpsResponse->isAsync && pCurrentHttpsResponse->pCallbacks->errorCallback) { pCurrentHttpsResponse->pCallbacks->errorCallback(....); } pCurrentHttpsResponse->syncStatus = status; } .... }
PVS-Studio警告： V522 [CWE-690]可能会取消引用潜在的空指针'pCurrentHttpsResponse'。 iot_https_client.c 1184
如果取消引用，则问题最底层。 让我们看看这里发生了什么。
在函数开始时，
pCurrentHttpsResponse和
pQItem变量被初始化为
NULL ，
状态变量被
初始化为
IOT_HTTPS_OK ，这意味着一切顺利。
接下来，为
pQItem分配
IotDeQueue_PeekHead函数返回的值，该函数返回指向双连接队列开头的指针。
如果队列为空会怎样？ 在这种情况下，
IotDeQueue_PeekHead函数将返回
NULL ：
static inline IotLink_t* IotDeQueue_PeekHead (const IotDeQueue_t* const pQueue) { return IotListDouble_PeekHead(pQueue); } .... static inline IotLink_t* IotListDouble_PeekHead (const IotListDouble_t* const pList) { IotLink_t* pHead = NULL; if (pList != NULL) { if (IotListDouble_IsEmpty(pList) == false) { pHead = pList->pNext; } } return pHead; }
接下来，满足条件
pQItem == NULL ，并且控制流程通过
goto 进入功能主体的下部。 到这时，
pCurrentHttpsResponse指针将保持为空，并且
状态将不再等于
IOT_HTTPS_OK 。 结果，
如果和...更广泛，我们将属于那个分支！ 贬低您自己的后果是您自己知道的。
好吧 这是一个华丽的例子。 现在，我提请您注意一个非常简单且可以理解的潜在取消引用：
int PKI_mbedTLSSignatureToPkcs11Signature (uint8_t * pxSignaturePKCS, uint8_t * pxMbedSignature ) { int xReturn = 0; uint8_t * pxNextLength; uint8_t ucSigComponentLength = pxMbedSignature[ 3 ];
PVS-Studio警告： V595 [CWE-476]在针对nullptr进行验证之前，已使用“ pxMbedSignature”指针。 检查行：52，54。iot_pki_utils.c 52
该函数获得两个指向
uint8_t的指针 。 两个指针都检查为
NULL ，这是一个好习惯-必须立即解决这种情况。
但是，这是不幸的事情：选中
pxMbedSignature时 ，它实际上已经在上面的一行中被取消引用。 塔达！
投机代码的另一个示例：
CK_RV vAppendSHA256AlgorithmIdentifierSequence ( uint8_t * x32ByteHashedMessage, uint8_t * x51ByteHashOidBuffer ) { CK_RV xResult = CKR_OK; uint8_t xOidSequence[] = pkcs11STUFF_APPENDED_TO_RSA_SIG; if( ( x32ByteHashedMessage == NULL ) || ( x51ByteHashOidBuffer == NULL ) ) { xResult = CKR_ARGUMENTS_BAD; } memcpy( x51ByteHashOidBuffer, xOidSequence, sizeof( xOidSequence ) ); memcpy( &x51ByteHashOidBuffer[ sizeof( xOidSequence ) ], x32ByteHashedMessage, 32 ); return xResult; }
PVS-Studio警告：- V1004 [CWE-628]在针对nullptr对其进行验证后，不安全地使用了x51ByteHashOidBuffer指针。 检查行：275、280。iot_pkcs11.c 280
- V1004 [CWE-628]在针对nullptr进行验证之后，不安全地使用了x32ByteHashedMessage指针。 检查行：275、281。iot_pkcs11.c 281
分析器警告说，在测试了
NULL之后，不能安全地使用作为指针的函数参数。 实际上，已经检查了参数，但是如果其中任何一个结果为
NULL ，则除了写入
xResult之外，不执行任何操作。 这段代码似乎在说：“是的，这意味着论点证明是不好的。 我们现在将其记录下来，在您继续的同时，继续。”
底线：
NULL将传递给
memcpy 。 这能带来什么？ 值将复制到哪里以及哪些值被复制？ 实际上，对此进行猜测是不值得的，因为 该标准
明确指出 ，这样的调用会导致未定义的行为（请参见第1段）。
分析器报告仍然包含我在Amazon FreeRTOS中发现的带有指针的不正确操作的示例，但是我认为给出的示例已经足以向您展示PVS-Studio检测此类错误的功能。 考虑一些新事物。
TRUE！= 1
我发现的几种错误与一种模式有关，但不幸的是，这种错误经常被忘记。
事实是，
布尔类型（来自C ++）不同于
布尔类型（通常用于C语言）。 第一个只能包含
true或
false 。 第二个是某种整数类型（
int ，
long等）的typedef。 对他来说，“ false”是值
0 ，“ true”是除零以外的任何值。
由于C中没有内置的布尔类型，为方便起见定义了以下常量：
#define FALSE 0 #define TRUE 1
现在考虑一个示例：
int mbedtls_hardware_poll(void* data, unsigned char* output, size_t len, size_t* olen) { int lStatus = MBEDTLS_ERR_ENTROPY_SOURCE_FAILED; HCRYPTPROV hProv = 0; (void)data; if (TRUE == CryptAcquireContextA( &hProv, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT)) { if (TRUE == CryptGenRandom(hProv, len, output)) { lStatus = 0; *olen = len; } CryptReleaseContext(hProv, 0); } return lStatus; }
PVS-Studio警告：- V676 [CWE-253]比较BOOL类型的变量和TRUE是不正确的。 aws_entropy_hardware_poll.c 48
- V676 [CWE-253]比较BOOL类型的变量和TRUE是不正确的。 正确的表达式是：“ FALSE！= CryptGenRandom（hProv，len，输出）”。 aws_entropy_hardware_poll.c 51
发现错误？ 它是:)函数
CryptAcquireContextA和
CryptGenRandom是
wincrypt.h标头中的标准函数。 如果成功，它们将返回非零值。 我强调-
非零 。 因此，从理论上讲，它可以是除零以外的任何值：
1、314、42、420 。
显然，从示例中编写函数的程序员没有考虑过它，因此，将获得的值与1进行比较。
满足条件
TRUE == CryptGenRandom（....）的概率是多少？ 很难说。 也许
CryptGenRandom比其他值更经常返回一个单位，也许总是只返回一个。 我们不能肯定地知道：这个加密功能的实现对凡人程序员是隐藏的：)
重要的是要记住，这样的比较有潜在的危险。 而不是：
if (TRUE == GetBOOL())
使用更安全的选项：
if (FALSE != GetBOOL())
优化问题
一些分析器警告已与运行缓慢的构造相关联。 例如：
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strstr(contentRangeValStr, "/"); .... }
PVS-Studio警告： V817查找'/'字符而不是字符串更有效。 iot_demo_https_common.c 205
简要清楚地说，不是吗？ 此处的
strstr函数仅用于搜索一个字符，该字符作为字符串传递给参数（用双引号引起来）。
可以通过将
strstr替换为
strchr来优化该位置：
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strchr(contentRangeValStr, '/'); .... }
然后搜索将更快一些。 有点琐事，但很好。
这样的优化当然是好的，而且分析仪发现了另一个可以更明显地优化的地方：
void vRunOTAUpdateDemo(void) { .... for (; ; ) { .... xConnectInfo.cleanSession = true; xConnectInfo.clientIdentifierLength = (uint16_t)strlen(clientcredentialIOT_THING_NAME); xConnectInfo.pClientIdentifier = clientcredentialIOT_THING_NAME; .... } }
警告PVS-Studio： V814性能下降。 在循环体内多次调用了“ strlen”函数。 aws_iot_ota_update_demo.c 235
嗯……在循环内部，每次迭代都会调用
strlen ，每次都计算同一行的长度。 不是最有效的操作:)
让我们看看
clientcredentialIOT_THING_NAME的定义：
#define clientcredentialIOT_THING_NAME ""
提示用户在此处输入其设备的名称。 默认情况下，它是空的，在这种情况下，一切都很好。 但是，如果用户想要在其中输入一些长而优美的名称怎么办？ 例如，我想称自己的想法为“
热情而精致的BarBarista-N061E终极咖啡机 ”。 您能想象如果我那台漂亮的咖啡机开始工作慢一点后会感到惊讶吗？ 乱！
要解决该错误，应从循环体中取出
strlen 。 毕竟，程序运行时设备的名称不会更改。 恩，这是C ++的
constexpr ...
好吧，好吧，我承认：在这里我有点胖了。 正如我的同事安德烈·卡波夫（Andrei Karpov）指出的那样，现代编译器知道
strlen是什么
，并且他本人观察到，如果他们知道字符串的长度不能改变，他们将如何简单地在二进制代码中使用常量。 因此，在发布版本的构建模式中，很有可能会使用以前计算出的值，而不是实际计算字符串长度，而是很有可能。 但是，这并不总是可行的，因此编写这样的代码不是一个好习惯。
关于MISRA的几句话
PVS-Studio分析仪具有大量规则，可让您检查代码是否符合MISRA C和MISRA C ++标准。 这些标准是什么？
MISRA是高响应嵌入式系统的编码标准。 它包含一组用于编写代码和设置开发过程的严格规则和准则。 这些规则中有很多，它们不仅旨在消除严重错误，而且还针对各种“代码味道”，以及编写最易懂和易读的代码。
因此，遵循MISRA标准不仅有助于避免错误和漏洞，而且还可以显着-显着！ -降低它们在现有代码中出现的可能性。
MISRA用于航空航天，医疗，汽车和军事行业-人们的生活取决于嵌入式软件的质量。
显然，Amazon FreeRTOS开发人员知道此标准，并且在大多数情况下都遵循它。 没错：如果您要为嵌入式系统编写基础广泛的OS，则必须考虑安全性。
但是，我发现很多违反MISRA标准的行为。 在这里，我将不提供“不使用并集”或“一个函数在主体末端仅应有一个收益”之类的规则示例-不幸的是，它们与大多数MISRA规则一样并不引人注目。 我最好举几个例子，说明可能会导致严重后果的违规行为。
让我们从宏开始：
#define FreeRTOS_ms_to_tick(ms) ( ( ms * configTICK_RATE_HZ + 500 ) / 1000 )
#define SOCKETS_htonl( ulIn ) ( ( uint32_t ) \ ( ( ( ulIn & 0xFF ) << 24 ) | ( ( ulIn & 0xFF00 ) << 8 ) \ | ( ( ulIn & 0xFF0000 ) >> 8 ) | ( ( ulIn & 0xFF000000 ) >> 24 ) ) )
#define LEFT_ROTATE( x, c ) ( ( x << c ) | ( x >> ( 32 - c ) ) )
PVS-Studio警告：- V2546 [MISRA C 20.7]宏及其参数应放在括号中。 考虑检查“ FreeRTOS_ms_to_tick”宏的“ ms”参数。 FreeRTOS_IP.h 201
- V2546 [MISRA C 20.7]宏及其参数应放在括号中。 考虑检查“ SOCKETS_htonl”宏的“ ulIn”参数。 iot_secure_sockets.h 512
- V2546 [MISRA C 20.7]宏及其参数应放在括号中。 考虑检查“ LEFT_ROTATE”宏的参数“ x”，“ c”。 iot_device_metrics.c 90
是的，这正是您的想法。 这些宏的参数未括在方括号中。 如果有人不小心写了类似
val = LEFT_ROTATE(A[i] | 1, B);
那么这样的“调用”宏将在以下位置打开：
val = ( ( A[i] | 1 << B ) | ( A[i] | 1 >> ( 32 - B ) ) );
还记得运营的重点吗？ 首先，仅在按位“或”之后才执行按位移位。 因此，将违反程序的逻辑。 一个简单的例子：如果将表达式“
x + y ”传递给
FreeRTOS_ms_to_tick宏会怎样？ MISRA的主要目标之一是防止此类情况的发生。
有人可能会反对：“如果您有对此一无所知的程序员，那么没有任何标准可以拯救您！”我也不同意。 程序员也是人，无论一个人有多经验，他也会在工作结束时感到疲倦并犯错。 这是MISRA强烈建议使用自动分析工具来验证项目是否符合标准的原因之一。
我转向Amazon FreeRTOS的开发人员：PVS-Studio发现了另外12个不安全的宏，因此您在使用它们时要格外小心:)
另一个有趣的MISRA违规行为：
static void _responseCompleteCallback(void* pPrivData, IotHttpsResponseHandle_t respHandle, IotHttpsReturnCode_t rc, uint16_t status) { bool* pUploadSuccess = (bool*)pPrivData; if (status == IOT_HTTPS_STATUS_OK) { *pUploadSuccess = true; } else { *pUploadSuccess = false; } IotSemaphore_Post(&(_uploadFinishedSem)); }
您可以自己找到错误吗？
PVS-Studio警告： V2537 [MISRA C 2.7]功能中不应包含未使用的参数。 考虑检查参数：“ rc”。 iot_demo_https_s3_upload_async.c 234
仔细看看：
rc参数未在函数主体中的任何位置使用。 此外，在函数注释中明确指出该参数是另一个函数的返回码，并且它可以表示错误。 那么为什么不以任何方式处理此参数？ 显然这里有问题。
但是，即使没有这些注释，未使用的参数也经常表示程序逻辑已损坏。 否则，为什么在功能签名中需要它们？
在这里，我给出了一个非常适合本文中的示例的小函数。 除了她，我还发现了10个未使用的参数。 它们中的许多都用于较大的功能，而找到它们并不是最容易的事情。
怀疑之前没有找到它们。 确实，编译器可以轻松检测到这种情况。
结论
这些并不是分析仪发现的所有问题区域，但是文章已经很大了。 我希望借此，Amazon FreeRTOS开发人员将能够解决一些缺陷，甚至可能希望自己
尝试PVS-Studio 。 这样，就可以更彻底地研究警告，并且确实，使用便捷的界面比查看文本报告要容易得多。
感谢您阅读我们的文章！ 下期再见：D
PS恰好在10月31日发表了这篇文章。 因此，祝大家万圣节快乐！
如果您想与讲英语的读者分享这篇文章，请使用翻译链接：George Gribkov。
应嵌入式开发人员的要求：在Amazon FreeRTOS中检测错误 。