嵌入式 freertos

简要介绍项目 (Briefly about projects)

To begin with, I'll tell you a bit about the forerunner of the project being tested — FreeRTOS (the source code is available here by link). As Wikipedia states, FreeRTOS is a real-time multitasking operating system for embedded systems.

首先，我将向您介绍一下正在测试的项目的先驱者-FreeRTOS(源代码可在此处通过link获得 )。 如Wikipedia所述，FreeRTOS是嵌入式系统的实时多任务操作系统。

It is written in good old C, which is not surprising — this operating system should work in conditions typical of microcontrollers: low processing power, a small amount of RAM, and the like. The C language allows you to work with resources at a low level and has high performance, so it is best suited to develop such an OS.

它是用旧的C语言编写的，这并不奇怪-该操作系统应在微控制器的典型条件下工作：处理能力低，RAM量少等。 C语言允许您以较低的级别使用资源并具有高性能，因此最适合开发这样的OS。

Now back to Amazon, which is always on the move developing various promising directions. For example, Amazon is developing an Amazon Lumberyard AAA-engine, which we've also checked.

现在回到亚马逊，亚马逊一直在发展各种有前途的方向。 例如，亚马逊正在开发Amazon Lumberyard AAA引擎，我们也对此进行了检查 。

One of such directions is Internet of Things (IoT). To develop in this area, Amazon decided to write their own operating system — and they took the FreeRTOS core as a basis.

这样的方向之一是物联网(IoT)。 为了在这一领域发展，亚马逊决定编写自己的操作系统-他们以FreeRTOS核心为基础。

The resulting system, Amazon FreeRTOS, is positioned to «provide a secure connection to Amazon Web Services, such as AWS IoT Core or AWS IoT Greengrass». The source code of this project is available on Github.

最终的系统Amazon FreeRTOS的定位是“提供与Amazon Web Services的安全连接，例如AWS IoT Core或AWS IoT Greengrass”。 该项目的源代码可在Github上获得。

In this article, we'll find out if there are errors in FreeRTOS as well as how secure the Amazon operating system is in terms of static code analysis.

在本文中，我们将了解FreeRTOS中是否存在错误以及就静态代码分析而言，Amazon操作系统的安全性如何。

检查过程 (The course of the check)

The check was performed using the automatic error-finding tool — the PVS-Studio static code analyzer. It is able to detect errors in programs written in C, C++, C#, and Java.

该检查是使用自动错误查找工具PVS-Studio静态代码分析器执行的。 它能够检测用C，C ++，C＃和Java编写的程序中的错误。

Before the analysis, we have to build the project. This way, I'll be confident that I have all needed dependencies and the project is ready to be checked. One can check the project in a number of ways — for example, using a compilation monitoring system. In this case, I performed the analysis using the plugin for Visual Studio – it's good that repositories of both projects comprise the sets of project files that make it easy to build under Windows.

在分析之前，我们必须构建项目。 这样，我将确信我具有所有需要的依赖项，并且可以检查该项目了。 人们可以通过多种方式(例如，使用编译监视系统)检查项目。 在这种情况下，我使用Visual Studio插件进行了分析–很好的是，两个项目的存储库都包含可轻松在Windows下构建的项目文件集。

I just had to build projects to make sure that I had everything ready for the check. Next I ran the analysis and – voila! – I have a ready-made analyzer report in front of me.

我只需要构建项目以确保已准备好一切即可进行检查。 接下来，我进行分析，瞧！ –我面前有一份现成的分析仪报告。

Third-party libraries included in these projects can also contain errors, and they can, of course, also affect the program. However, I have excluded them from the analysis for the sake of the purity of the narrative.

这些项目中包含的第三方库也可能包含错误，它们当然也会影响程序。 但是，为了叙述的纯正，我从分析中排除了它们。

So, the projects are analyzed, reports are received, interesting errors are highlighted. It's time to get their review!

因此，分析项目，接收报告，突出显示有趣的错误。 现在该获得他们的评论！

FreeRTOS隐藏了什么 (What FreeRTOS hides)

Initially, I expected to write two separate articles: one for each operating system. I was already rubbing my hands? as I prepared to write a good article about FreeRTOS. Anticipating the discovery of at least a couple of juicy bugs (like CWE-457), I was looking through sparse warnings of the analyzer, and… found nothing. I didn't find any interesting error.

最初，我希望写两篇单独的文章：每个操作系统一篇。 我已经在搓手了吗？ 当我准备写一篇关于FreeRTOS的好文章时。 预期至少会发现两个多汁的错误(例如CWE-457 )，我正在查看分析仪的稀疏警告，却没有发现任何结果。 我没有发现任何有趣的错误。

Many of the warnings issued by the analyzer were not relevant to FreeRTOS. For example, such warnings were 64-bit flaws such as casting size_t to uint32_t. It is related to the fact that FreeRTOS is meant to be working on devices with a pointer size no larger than 32 bits.

分析仪发出的许多警告与FreeRTOS不相关。 例如，此类警告是64位缺陷，例如将size_t转换为uint32_t 。 这与FreeRTOS旨在在指针大小不大于32位的设备上工作有关。

I have thoroughly checked all V1027 warnings indicating castings between pointers to unrelated structures. If casted structures have the same alignment, then such a casting is a mistake. And I haven't found a single dangerous casting!

我已经彻底检查了所有V1027警告，这些警告指示在指向不相关结构的指针之间进行强制转换 。 如果铸造结构具有相同的对齐方式，则这种铸造是错误的。 而且我还没有发现任何危险的铸件！

All other suspicious places were either associated with coding style or were staffed with a comment explaining why it was done that way and why it wasn't a mistake.

所有其他可疑位置都与编码风格相关联，或者配备了注释，以解释为什么这样做和为什么不是错误。

So I'd like to appeal to FreeRTOS developers. Guys, you're awesome! We have hardly seen such clean and high-quality projects as yours. And it was a pleasure to read the clean, neat, and well-documented code. Hats off to you, guys.

因此，我想吸引FreeRTOS开发人员。 伙计们，您真棒！ 我们几乎看不到像您这样的干净优质的项目。 很高兴阅读干净，整洁且有据可查的代码。 伙计们，向您致敬。

Even though I couldn't find any interesting bugs that day, I knew I wouldn't stop there. I was going home with the firm confidence that Amazon's version would 100% have something interesting, and that tomorrow I'd definitely pick up enough bugs for the article. As you may have guessed, I was right.

即使那天我找不到任何有趣的错误，但我知道我不会止步于此。 我怀着坚定的信心回到家，因为亚马逊的版本将100％具有有趣之处，而且明天我肯定会为本文找到足够的bug。 您可能已经猜到了，我是对的。

Amazon FreeRTOS隐藏的内容 (What Amazon FreeRTOS hides)

Amazon's version of the system turned out to be… to put it mildly, a little worse. The legacy of FreeRTOS remained as clean whereas the new improvements concealed a lot of interesting issues.

事实证明，亚马逊的系统版本是……稍微说得更糟。 FreeRTOS的遗产仍然很干净，而新的改进隐藏了许多有趣的问题。

Some fragments had the program logic broken, some handled pointers incorrectly. In some places, the code could lead to undefined behavior, and there were cases in which the programmer simply did not know about the pattern of an error he made. I even found several serious potential vulnerabilities.

一些片段破坏了程序逻辑，一些错误地处理了指针。 在某些地方，代码可能导致未定义的行为，并且在某些情况下，程序员根本不了解他所犯错误的模式。 我什至发现了几个严重的潜在漏洞。

Seems like I've tightened up with the introduction. Let's start figure out errors!

似乎我已经加强了介绍。 让我们开始找出错误！

程序逻辑中断 (Breaking of the program logic)

Let's start with problem places that obviously indicate that the program works not in the way the programmer expected. Suspicious array handling will come first:

让我们从有问题的地方开始，这些地方显然表明程序的工作方式与程序员预期的不同。 可疑数组处理将首先出现：

/*** @brief Pool of request and associated response buffers, *  handles, and configurations.*/
static _requestPool_t _requestPool = { 0 };....static int _scheduleAsyncRequest(int reqIndex,uint32_t currentRange)
{..../* Set the user private data to use in the asynchronous callback context. */_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 issued two warnings for this piece of code:

PVS-Studio针对此代码段发出了两个警告：

• V619 The array '_requestPool.pRequestDatas' is being utilized as a pointer to single object. iot_demo_https_s3_download_async.c 973
  V619数组'_requestPool.pRequestDatas'被用作指向单个对象的指针。 iot_demo_https_s3_download_async.c 973
• V574 The '_requestPool.pRequestDatas' pointer is used simultaneously as an array and as a pointer to single object. Check lines: 931, 973. iot_demo_https_s3_download_async.c 973
  V574'_requestPool.pRequestDatas'指针同时用作数组和指向单个对象的指针。 检查行：931、973。iot_demo_https_s3_download_async.c 973

Just in case, let me remind you: the array name is the pointer to its first element. That is, if _requestPool.pRequestDatas is an array of structures, _requestPool.pRequestDatas[i].scheduled is an assess to the scheduled member of the i array structure.And if we write _requestPool.pRequestDatas->scheduled, it will turn out that themember of namely the first array structure will beaccessed.

以防万一，让我提醒您：数组名称是指向其第一个元素的指针。 也就是说，如果_requestPool.pRequestDatas是结构数组，则_requestPool.pRequestDatas [i] .scheduled是对i数组结构的已调度成员的评估。如果我们编写_requestPool.pRequestDatas-> scheduled ，结果将是第一个数组结构的成员将被访问。

In the excerpt of the code above, that's what happens. In the last line, the value of only the member of the first array structure is changed. In itself, such an accessing is already suspicious, but here the case is even more clear: the _requestPool.pRequestDatas array is assessed by index throughout the body of the function. But at the end the indexing operation was forgotten.

在上面的代码摘录中，就是这样。 在最后一行中，仅更改第一个数组结构的成员的值。 就其本身而言，这样的访问已经是可疑的，但是在这种情况下更加清楚： _requestPool.pRequestDatas数组由整个函数主体中的索引评估。 但是最后，索引操作被遗忘了。

As I understand it, the last line should look like this:

据我了解，最后一行应如下所示：

_requestPool.pRequestDatas[reqIndex].scheduled = true;

The next error lies in a small function, so I'll give it completely:

下一个错误在于一个小的函数，因此我将对其进行完整介绍：

/* Return true if the string " pcString" is found* inside the token pxTok in JSON file pcJson. */
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 ], // <=pcString,ulStringSize ) == 0 ){xStatus = pdTRUE;}}}return xStatus;
}

PVS-Studio警告： (PVS-Studio warning:)

V642 [CWE-197] Saving the 'strncmp' function result inside the 'short' type variable is inappropriate. The significant bits could be lost breaking the program's logic. aws_greengrass_discovery.c 637

V642 [CWE-197]将'strncmp'函数结果保存在'short'类型变量中是不合适的。 有效位可能会丢失，从而破坏程序的逻辑。 第637章

Let's take a look at the definition of the strncmp function:

让我们看一下strncmp函数的定义：

int strncmp( const char *lhs, const char *rhs, size_t count );

In the example, the result of the int type, which is 32 bits in size is converted in a variable of the int16_t type. With this «narrowing» converting, the older bits of the returned value will be lost. For example, if the strncmp function returns 0x00010000, unit will be lost during the conversion, and the condition will be executed.

在该示例中，大小为32位的int类型的结果将转换为int16_t类型的变量。 通过这种“缩小”转换，返回值的旧位将丢失。 例如，如果strncmp函数返回0x00010000 ，则在转换过程中单元将丢失，条件将被执行。

It's actually strange to see such a casting in the condition. Why is it ever needed here, if an ordinary int can be compared with zero? On the other hand, if a programmer wanted this function to sometimes return true even if it shouldn't, why not support such tricky behavior with a comment? But this way it's some kind of a backdoor. Anyway, I'm inclined to think it's an error. What do you think?

看到这种情况真的很奇怪。 如果普通int可以与零进行比较，为什么在这里需要它？ 另一方面，如果程序员希望该函数有时即使不应该返回true ，为什么不通过注释支持这种棘手的行为呢？ 但这是一种后门。 无论如何，我倾向于认为这是一个错误。 你怎么看？

未定义的行为和指针 (Undefined behavior and pointers)

Here comes a large example. It covers up a potential null pointer dereference:

这是一个大例子。 它掩盖了潜在的空指针取消引用：

static void _networkReceiveCallback(....)
{IotHttpsReturnCode_t status = IOT_HTTPS_OK;_httpsResponse_t* pCurrentHttpsResponse = NULL;IotLink_t* pQItem = NULL;..../* Get the response from the response queue. */IotMutex_Lock(&(pHttpsConnection->connectionMutex));pQItem = IotDeQueue_PeekHead(&(pHttpsConnection->respQ));IotMutex_Unlock(&(pHttpsConnection->connectionMutex));/* If the receive callback is invoked * and there is no response expected,* then this a violation of the HTTP/1.1 protocol. */if (pQItem == NULL){IotLogError(....);fatalDisconnect = true;status = IOT_HTTPS_NETWORK_ERROR;goto iotCleanup;}....iotCleanup :/* Report errors back to the application. */if (status != IOT_HTTPS_OK){if ( pCurrentHttpsResponse->isAsync&& pCurrentHttpsResponse->pCallbacks->errorCallback){pCurrentHttpsResponse->pCallbacks->errorCallback(....);}pCurrentHttpsResponse->syncStatus = status;}....
}

PVS-Studio警告： (PVS-Studio warning: )

V522 [CWE-690] There might be dereferencing of a potential null pointer 'pCurrentHttpsResponse'. iot_https_client.c 1184

V522 [CWE-690]可能会取消引用潜在的空指针“ pCurrentHttpsResponse”。 iot_https_client.c 1184

The last if block contains problematic dereferences. Let's find out what's going on here.

最后一个if块包含有问题的取消引用。 让我们找出这里发生了什么。

The function begins with pCurrentHttpsResponse and pQItem variables initialized by the NULL value and the status variable is initialized by the IOT_HTTPS_OK value, which means that it's all correct.

该函数以pCurrentHttpsResponse和pQItem变量(由NULL值初始化)以及status变量(由IOT_HTTPS_OK值)初始化开始 ，这意味着它们都是正确的。

Further pQItem is assigned the value, returned from the IotDeQueue_PeekHead function, which returns the pointer to the beginning of the double-linked queue.

进一步为pQItem分配了从IotDeQueue_PeekHead函数返回的值，该函数返回指向双向链接队列开头的指针。

What happens if the queue is empty? In this case, the IotDeQueue_PeekHead function will return NULL:

如果队列为空会怎样？ 在这种情况下， 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)
/* @[declare_linear_containers_list_double_peekhead] */
{IotLink_t* pHead = NULL;if (pList != NULL){if (IotListDouble_IsEmpty(pList) == false){pHead = pList->pNext;}}return pHead;
}

Further the condition pQItem == NULL will become true and the control flow will be passed by goto to the lower part of the function. By this time, the pCurrentHttpsResponse pointer will remain null, while status will not be equal to IOT_HTTPS_OK. In the end, we'll get to the same if branch, and ...boom! Well, you know about consequences of such a dereference.

此外，条件pQItem == NULL将变为true，并且控制流将通过goto传递到函数的下部。 到此时， pCurrentHttpsResponse指针将保持为空，而状态将不等于IOT_HTTPS_OK 。 最后，我们将进入相同的if分支，然后……轰！ 好吧，您知道这种取消引用的后果。

Okay. It was a slightly tricky example. Now I suggest that you take a look at a very simple and understandable potential dereferencing:

好的。 这是一个有点棘手的例子。 现在，我建议您看一下一个非常简单且可以理解的潜在取消引用：

int PKI_mbedTLSSignatureToPkcs11Signature(uint8_t * pxSignaturePKCS, uint8_t * pxMbedSignature )
{int xReturn = 0;uint8_t * pxNextLength;/* The 4th byte contains the length of the R component */uint8_t ucSigComponentLength = pxMbedSignature[ 3 ]; // <=if(  ( pxSignaturePKCS == NULL )|| ( pxMbedSignature == NULL ) ){xReturn = FAILURE;}....
}

PVS-Studio警告： (PVS-Studio warning:)

V595 [CWE-476] The 'pxMbedSignature' pointer was utilized before it was verified against nullptr. Check lines: 52, 54. iot_pki_utils.c 52

V595 [CWE-476]在针对nullptr对其进行验证之前，已使用'pxMbedSignature'指针。 检查行：52，54。iot_pki_utils.c 52

This function receives to pointers to uint8_t. Both pointers are checked for NULL, which is a good practice — such situations should be worked out immediately.

该函数接收指向uint8_t的指针 。 两个指针都检查是否为NULL ，这是一个好习惯-应该立即解决这种情况。

But here's the problem: by the time pxMbedSignature is checked, it will already be dereferenced literally one line above. Ta-daa!

但这是问题所在：选中pxMbedSignature时 ，它实际上已经在上面的一行中被取消引用。 塔达！

Another example of speculative code:

投机代码的另一个示例：

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警告： (PVS-Studio warnings: )

• V1004 [CWE-628] The 'x51ByteHashOidBuffer' pointer was used unsafely after it was verified against nullptr. Check lines: 275, 280. iot_pkcs11.c 280
  V1004 [CWE-628]在针对nullptr进行验证之后，不安全地使用了x51ByteHashOidBuffer指针。 检查行：275、280。iot_pkcs11.c 280
• V1004 [CWE-628] The 'x32ByteHashedMessage' pointer was used unsafely after it was verified against nullptr. Check lines: 275, 281. iot_pkcs11.c 281
  V1004 [CWE-628]在针对nullptr对其进行验证之后，不安全地使用了x32ByteHashedMessage指针。 检查行：275、281。iot_pkcs11.c 281

The analyzer warns that function parameters that are pointers, are unsafely used after their check for NULL. Indeed, the arguments are checked. But in case if any of them isn't NULL, no action is taken except for writing in xResult. This section of the code kind of says: «Yeah, so the arguments turned out to be bad. We're going to note it now, and you — keep going, keep going.»

分析器警告在检查NULL后不安全地使用作为指针的函数参数。 实际上，已检查了参数。 但是，如果其中任何一个都不为NULL ，则除了写入xResult之外，不会采取任何其他措施。 代码的这一部分说：«是的，因此论据证明是不好的。 我们现在要注意它，您-继续前进，继续前进。»

Result: NULL will be passed to memcpy. What can come of it? Where will the values be copied and which ones? In fact, guessing won't help, as the standard clearly states that such a call leads to undefined behavior (see the section 1).

结果： NULL将传递给memcpy。 它能带来什么？ 值将复制到哪里以及哪些值被复制？ 实际上，猜测将无济于事，因为该标准明确指出此类调用会导致未定义的行为(请参见第1节)。

There are other examples of incorrect pointers handling in the analyzer report found in Amazon FreeRTOS, but I think, given examples are enough to show capabilities of PVS-Studio in detecting such errors. Let's take a look at something new.

Amazon FreeRTOS的分析器报告中还有其他不正确的指针处理示例，但我认为，给出的示例足以显示PVS-Studio检测此类错误的功能。 让我们来看看一些新东西。

是！= 1 (TRUE != 1)

There were several errors related to the pattern, which, unfortunately, is often overlooked.

与模式有关的错误有很多，但不幸的是，这些错误经常被忽略。

The fact is that the bool type (from C++) is different from the BOOL type (commonly used in C). The first can only contain a true or false value. The second one is the typedef of an integer type (int, long, and others). The 0 value is «false» for it, and any other value different from zero is «true».

事实是， 布尔类型(来自C ++)不同于布尔类型(通常用于C语言)。 第一个只能包含true或false值。 第二个是整数类型( int ， long和其他类型)的typedef。 它的0值是«false»，而其他任何不同于零的值都是«true»。

Since there is no built-in Boolean type in C, these constants are defined for convenience:

由于C中没有内置的布尔类型，为方便起见定义了以下常量：

#define FALSE 0
#define TRUE 1

Let's look at the example.

让我们来看一个例子。

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;/* Unferenced parameter. */(void)data;/** This is port-specific for the Windows simulator,* so just use Crypto API.*/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警告： (PVS-Studio warnings: )

• V676 [CWE-253] It is incorrect to compare the variable of BOOL type with TRUE. aws_entropy_hardware_poll.c 48
  V676 [CWE-253]比较BOOL类型的变量和TRUE是不正确的。 aws_entropy_hardware_poll.c 48
• V676 [CWE-253] It is incorrect to compare the variable of BOOL type with TRUE. Correct expression is: 'FALSE != CryptGenRandom(hProv, len, output)'. aws_entropy_hardware_poll.c 51
  V676 [CWE-253]比较BOOL类型的变量和TRUE是不正确的。 正确的表达式是：“ FALSE！= CryptGenRandom(hProv，len，输出)”。 aws_entropy_hardware_poll.c 51

Did you find an error? Don't doubt, it is here :) The CryptAcquireContextA and CryptGenRandom functions are standard functions from the wincrypt.h header. If successful, they return the non-zero value. Let me emphasize that it is non-zero. So, theoretically, it could be any value different from zero: 1, 314, 42, 420.

发现错误了吗？ 毫无疑问，它在这里：) CryptAcquireContextA和CryptGenRandom函数是wincrypt.h标头中的标准函数。 如果成功，它们将返回非零值。 让我强调一下，它是非零的 。 所以，从理论上讲，它可以是从零的任何值不同：1，314，42，420。

Apparently, the programmer who was writing the function from the example, wasn't thinking about that, and in the end, resulting values are compared to one.

显然，从示例中编写函数的程序员没有考虑这一点，最后，将结果值与一个值进行了比较。

How likely is that the TRUE == CryptGenRandom(....) condition will not be fulfilled? It's hard to say. Perhaps, CryptGenRandom might return 1 more often than other values, but maybe it might return only 1. We can't know this for sure: the implementation of this cryptographic function is hidden from the eyes of mortal programmers :)

TRUE == CryptGenRandom(....)条件将不被满足的可能性有多大 ？ 很难说。 也许， CryptGenRandom可能比其他值返回的次数更多，但是也许只能返回1。我们不能确定这一点：加密函数的实现对凡人程序员是隐藏的：)

It is important to remember that such comparisons are potentially dangerous. Instead of:

重要的是要记住，这样的比较有潜在的危险。 代替：

if (TRUE == GetBOOL())

Use a safer version of the code:

使用更安全的代码版本：

if (FALSE != GetBOOL())

优化问题 (Optimization problems)

Several warnings of the analyzer were related to slowly operating structures. For example:

分析仪的一些警告与运行缓慢的结构有关。 例如：

int _IotHttpsDemo_GetS3ObjectFileSize(....)
{....pFileSizeStr = strstr(contentRangeValStr, "/");....
}

PVS-Studio警告： (PVS-Studio warning: )

V817 It is more efficient to seek '/' character rather than a string. iot_demo_https_common.c 205

V817查找'/'字符而不是字符串更有效。 iot_demo_https_common.c 205

It's short and simple, isn't it? The strstr function is used here for searching only one character, passed in the parameter as a string (it's in double quotes).

简短而简单，不是吗？ 在这里， strstr函数仅用于搜索一个字符，该字符作为字符串传入参数中(用双引号引起来)。

This place can potentially be optimized by replacing strstr with strchr:

可以通过将strstr替换为strchr来优化该位置：

int _IotHttpsDemo_GetS3ObjectFileSize(....)
{....pFileSizeStr = strchr(contentRangeValStr, '/');....
}

This way, searching will work a bit faster. A small, but nice thing.

这样，搜索将工作得更快。 一件很小但是很好的事情。

Well, such optimizations are good, but the analyzer has also found another place, which could be optimized in a much more noticeable way:

好的，这样的优化是好的，但是分析仪还找到了另一个地方，可以用更加明显的方式对其进行优化：

void vRunOTAUpdateDemo(void)
{....for (; ; ){....xConnectInfo.cleanSession = true;xConnectInfo.clientIdentifierLength = (uint16_t)strlen(clientcredentialIOT_THING_NAME);xConnectInfo.pClientIdentifier = clientcredentialIOT_THING_NAME;....}
}

PVS-Studio警告： (PVS-Studio warning: )

V814 Decreased performance. The 'strlen' function was called multiple times inside the body of a loop. aws_iot_ota_update_demo.c 235

V814性能下降。 在循环体内多次调用了“ strlen”函数。 235 aws_iot_ota_update_demo.c

Hmm… Inside the loop, with each iteration strlen is called that evaluates the length of the same line each time. Not the most effective operation :)

嗯……在循环内，每次迭代都会调用strlen ，每次都评估同一行的长度。 不是最有效的操作:)

Let's look inside the definition of clientcredentialIOT_THING_NAME:

让我们看一下clientcredentialIOT_THING_NAME的定义：

/** @brief Host name.** @todo Set this to the unique name of your IoT Thing.*/
#define clientcredentialIOT_THING_NAME               ""

The user is asked to enter the name of their device here. By default, it's empty, and in this case, everything is fine. What if a user wants to enter a long and beautiful name there? For example, I'd love to call my brainchild "The Passionate And Sophisticated Coffee Machine BarBarista-N061E The Ultimate Edition." Can you imagine what my surprise would be like if my beautiful coffee machine started working a little slower after that? Nuisance!

要求用户在此处输入其设备的名称。 默认情况下，它是空的，在这种情况下，一切都很好。 如果用户想在此处输入一个长而优美的名称怎么办？ 例如，我很想将自己的想法称为“ 热情而精致的咖啡机BarBarista-N061E Ultimate Edition” 。 您能想象如果我那台漂亮的咖啡机在那之后开始变慢了些，我会感到惊讶吗？ 讨厌

To correct the error, it is worth taking strlen outside the body loop. After all, the name of the device does not change during the program working. Oh, constexpr from C++ would suit perfectly here…

要更正错误，值得在体循环之外大吃一惊 。 毕竟，程序运行期间设备的名称不会更改。 哦，C ++的constexpr非常适合这里……

Okay, well, let's not gild the lily. As my colleague Andrey Karpov noted, modern compilers know what is strlen and he personally watched them using a constant in binary code if they get that the length of the line can't change. So there's a good chance that in the release build mode, instead of a real line length evaluation, the pre-evaluated value will be used. However, this does not always work, so writing such code is not a good practice.

好吧，好吧，我们不要为百合花镀金。 正如我的同事安德烈·卡尔波夫(Andrey Karpov)所指出的那样，现代编译器知道什么是复杂的 ，如果它们知道行的长度不能改变，他本人会使用二进制代码中的常量亲自观察它们。 因此，很有可能在发行版本构建模式中，将使用预先评估的值来代替实际的行长评估。 但是，这并不总是可行的，因此编写此类代码不是一个好习惯。

关于MISRA的几句话 (A few words about MISRA)

The PVS-Studio analyzer has a large set of rules to check your code for compliance with MISRA C and MISRA C standards. What are these standards?

PVS-Studio分析仪具有大量规则，可检查您的代码是否符合MISRA C和MISRA C标准。 这些标准是什么？

MISRA is the coding standard for highly responsible embedded systems. It contains a set of strict rules and guidelines for writing code and setting up a development process. These rules are quite a lot, and they are aimed not only at eliminating serious errors, but also at various «code smells». It is also aimed at writing the most comprehensible and readable code.

MISRA是高度负责的嵌入式系统的编码标准。 它包含一组用于编写代码和建立开发过程的严格规则和准则。 这些规则很多，它们不仅旨在消除严重的错误，而且还针对各种“代码气味”。 它还旨在编写最易懂和易读的代码。

Thus, following the MISRA standard not only helps to avoid mistakes and vulnerabilities, but also to significantly reduce the likelihood of their appearing in already existing code.

因此，遵循MISRA标准不仅有助于避免错误和漏洞，而且还可以大大减少错误和漏洞在已经存在的代码中出现的可能性。

MISRA is used in the aerospace, medical, automotive and military industries, where human lives depend on the quality of embedded software.

MISRA用于航空，医疗，汽车和军事行业，在这些行业中，人的生命取决于嵌入式软件的质量。

Apparently, Amazon FreeRTOS developers know about this standard, and for the most part follow it. Such approach is absolutely reasonable: if you write a broad-based OS for embedded systems, then you must think about security.

显然，Amazon FreeRTOS开发人员了解此标准，并且在大多数情况下都遵循它。 这种方法是绝对合理的：如果为嵌入式系统编写基础广泛的OS，则必须考虑安全性。

However, I have found quite a lot of violations of the MISRA standard. I'm not going to give examples of rules like «don't use union» or «function should only have one return at the end of the body» — unfortunately, they are not spectacular, as are most of MISRA rules. I'd rather give you examples of violations that could potentially lead to serious consequences.

但是，我发现很多违反MISRA标准的行为。 我将不提供“不要使用联合”或“功能在身体末端只能有一个回报”之类的规则示例，但不幸的是，它们不像大多数MISRA规则那样引人注目。 我想举几个例子，说明可能会导致严重后果的违规行为。

Let's start with macros:

让我们从宏开始：

#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警告： (PVS-Studio warnings:)

• V2546 [MISRA C 20.7] The macro and its parameters should be enclosed in parentheses. Consider inspecting the 'ms' parameter of the 'FreeRTOS_ms_to_tick' macro. FreeRTOS_IP.h 201
  V2546 [MISRA C 20.7]宏及其参数应放在括号中。 考虑检查“ FreeRTOS_ms_to_tick”宏的“ ms”参数。 FreeRTOS_IP.h 201
• V2546 [MISRA C 20.7] The macro and its parameters should be enclosed in parentheses. Consider inspecting the 'ulIn' parameter of the 'SOCKETS_htonl' macro. iot_secure_sockets.h 512
  V2546 [MISRA C 20.7]宏及其参数应放在括号中。 考虑检查“ SOCKETS_htonl”宏的“ ulIn”参数。 iot_secure_sockets.h 512
• V2546 [MISRA C 20.7] The macro and its parameters should be enclosed in parentheses. Consider inspecting parameters 'x', 'c' of the 'LEFT_ROTATE' macro. iot_device_metrics.c 90
  V2546 [MISRA C 20.7]宏及其参数应放在括号中。 考虑检查“ LEFT_ROTATE”宏的参数“ x”，“ c”。 iot_device_metrics.c 90

Yes, that's exactly what you're thinking. The parameters of these macros are not bracketed. If someone accidentally writes something like

是的，这正是您的想法。 这些宏的参数未括在方括号中。 如果有人不小心写了类似

val = LEFT_ROTATE(A[i] | 1, B);

such a «call» of a macro will expand into:

这样的宏“调用”将扩展为：

val = ( ( A[i] | 1 << B ) | ( A[i] | 1 >> ( 32 - B ) ) );

Remember the priorities of operations? First, a bitwise shift is made, and only after it — a bitwise «or». Therefore, the logic of the program will be broken. A simpler example: what would happen if the expression "x + y" is passed in the macro FreeRTOS_ms_to_tick? One of the main objectives of MISRA is to prevent such situations.

还记得运营的重点吗？ 首先，进行按位移位，并且仅在此之后进行按位“或”。 因此，该程序的逻辑将被破坏。 一个简单的例子：如果在FreeRTOS_ms_to_tick宏中传递表达式“ x + y ”，将会发生什么？ MISRA的主要目标之一就是预防这种情况。

Some might argue, «If you have programmers who don't know about this, no standard can help you!». I won't agree with that. Programmers are also people, and no matter how experienced a person is, they also can get tired and make a mistake at the end of the day. This is one of the reasons why MISRA strongly recommends using automatic analysis tools to test a project for compliance.

有人可能会争辩说：“如果您有对此一无所知的程序员，那么没有任何标准可以帮助您！”。 我不同意。 程序员也是人，无论一个人的经验如何，他们最终都会感到疲倦并犯错。 这就是MISRA强烈建议使用自动分析工具来测试项目符合性的原因之一。

Let me address the developers of Amazon FreeRTOS: PVS-Studio has found 12 more unsafe macros, so you kind of need to be careful with them :)

让我向Amazon FreeRTOS的开发人员致辞：PVS-Studio发现了12个更多不安全的宏，因此您需要谨慎对待它们：)

Another interesting MISRA violation:

另一个有趣的MISRA违规行为：

/*** @brief Callback for an asynchronous request to notify *        that the response is complete.** @param[in] 0pPrivData - User private data configured *            with the HTTPS Client library request configuration.* @param[in] respHandle - Identifier for the current response finished.* @param[in] rc - Return code from the HTTPS Client Library*            signaling a possible error.* @param[in] status - The HTTP response status.*/static void _responseCompleteCallback(void* pPrivData,IotHttpsResponseHandle_t respHandle,IotHttpsReturnCode_t rc,uint16_t status)
{bool* pUploadSuccess = (bool*)pPrivData;/* When the remote server response with 200 OK,the file was successfully uploaded. */if (status == IOT_HTTPS_STATUS_OK){*pUploadSuccess = true;}else{*pUploadSuccess = false;}/* Post to the semaphore that the upload is finished. */IotSemaphore_Post(&(_uploadFinishedSem));
}

Can you find the bug yourself?

您可以自己找到错误吗？

PVS-Studio警告： (PVS-Studio warning: )

V2537 [MISRA C 2.7] Functions should not have unused parameters. Consider inspecting the parameter: 'rc'. iot_demo_https_s3_upload_async.c 234

V2537 [MISRA C 2.7]函数不应具有未使用的参数。 考虑检查参数：“ rc”。 iot_demo_https_s3_upload_async.c 234

Take a closer look: the rc parameter isn't used anywhere in the function body. While the comment of the function clearly says that this parameter is a return code of another function, and that it can signal an error. Why isn't this parameter handled in any way? Something is clearly wrong here.

仔细看看： rc参数未在函数主体中的任何地方使用。 尽管该函数的注释清楚地表明此参数是另一个函数的返回码，但它可能表示错误。 为什么不以任何方式处理此参数？ 显然这里有些错误。

However, even without such comments, unused parameters often point to the broken logic of the program. Otherwise, why do you need them in the function signature?

但是，即使没有这些注释，未使用的参数也经常指向程序的逻辑中断。 否则，为什么在功能签名中需要它们？

Here I've given a small function that's good for an example in the article. In addition to it, I found 10 other unused parameters. Many of them are used in larger functions, and it is not easy to detect them.

在这里，我给出了一个小的函数，该函数对于本文中的示例非常有用。 除此之外，我发现了10个其他未使用的参数。 它们中的许多都用于较大的功能，因此检测起来并不容易。

Suspiciously, they haven't been found before. After all, compilers easily detect such cases.

可疑的是，以前没有找到它们。 毕竟，编译器很容易检测到这种情况。

结论 (Conclusion)

These were not all the issues found by the analyzer, but the article already turned out to be quite large. I hope that thanks to it, amazon FreeRTOS developers will be able to correct some of shortcomings, and may even want to try PVS-Studio on their own. This way it will be more convenient to thoroughly investigate warnings. And as a matter of fact — working with a convenient interface is much easier than looking at a text report.

这些并不是分析仪发现的所有问题，但文章原来已经很大了。 我希望借此，亚马逊FreeRTOS开发人员能够纠正一些缺陷，甚至可能希望自己尝试使用PVS-Studio 。 这样，彻底调查警告将更加方便。 实际上，与查看文本报告相比，使用便捷的界面要容易得多。

Thanks for reading our articles! See you in the next publication :D

感谢您阅读我们的文章！ 下次见：D

P.S. It just so happened that this article was published on October 31. Happy Halloween, guys and girls!

PS恰巧，这篇文章于10月31日发表。万圣节快乐，男孩和女孩！

翻译自: https://habr.com/en/company/pvs-studio/blog/473966/

嵌入式 freertos