Jeder, der Mikrocontroller programmiert, kennt FreeRTOS wahrscheinlich oder hat zumindest von diesem Betriebssystem gehört. Die Mitarbeiter von Amazon haben beschlossen, die Funktionen dieses Betriebssystems für die Arbeit mit AWS Internet of Things-Diensten zu erweitern - so erschien Amazon FreeRTOS. Wir, die Entwickler des PVS-Studio-Code-Analysators, wurden gebeten, diese Projekte in der Mail und in den Kommentaren unter den Artikeln zu überprüfen. Nun, du hast gefragt - wir haben es getan. Was dabei herauskam - lesen Sie weiter.
Ein bisschen über Projekte
Zunächst erzähle ich Ihnen ein wenig über den "Vater" des geprüften Projekts - FreeRTOS (den Quellcode finden Sie
hier ). Wie Wikipedia sagt, ist FreeRTOS ein Multitasking-Echtzeitbetriebssystem für eingebettete Systeme.
Es wurde im guten alten C geschrieben, was nicht überraschend ist - dieses Betriebssystem sollte unter für Mikrocontroller typischen Bedingungen funktionieren: geringe Rechenleistung, wenig RAM und dergleichen. Die C-Sprache ermöglicht es Ihnen, mit Ressourcen auf niedrigem Niveau zu arbeiten und hat eine hohe Leistung. Daher ist sie am besten für die Entwicklung eines solchen Betriebssystems geeignet.
Nun zurück zu Amazon, das nicht still sitzt und sich in verschiedenen vielversprechenden Bereichen entwickelt. Zum Beispiel entwickelt Amazon die AAA-Engine des Amazon Lumberyard-Spiels, die wir
ebenfalls getestet haben .
Ein solcher Bereich ist das Internet der Dinge (Internet der Dinge, IoT). Um sich in diesem Bereich weiterzuentwickeln, hat Amazon beschlossen, ein eigenes Betriebssystem zu schreiben - und den FreeRTOS-Kernel als Grundlage genommen.
Das resultierende System - Amazon FreeRTOS - ist so positioniert, dass es "die Möglichkeit bietet, eine sichere Verbindung zu Amazon Web Services wie AWS IoT Core oder AWS IoT Greengrass herzustellen". Der Quellcode für dieses Projekt
wird auf Github
gespeichert .
In diesem Artikel werden wir untersuchen, ob es Fehler in FreeRTOS gibt und wie sicher das Betriebssystem von Amazon in Bezug auf die statische Code-Analyse ist.
Wie war der Scheck?
Der Code wurde mit einem automatischen Fehlersuchwerkzeug überprüft: PVS-Studio Static Code Analyzer. Es kann Fehler in Programmen erkennen, die in C, C ++, C # und Java geschrieben wurden.
Bevor Sie mit der Analyse beginnen, müssen Sie das Projekt zusammenstellen. Ich bin mir also sicher, dass ich alle erforderlichen Abhängigkeiten habe und alles mit dem Projekt in Ordnung ist. Es gibt verschiedene Möglichkeiten, ein Projekt zu überprüfen, z. B. mithilfe eines Kompilierungsüberwachungssystems. Ich habe die Analyse mit dem Plug-In für Visual Studio durchgeführt. Es ist gut, dass die Repositorys beider Projekte über eine Reihe von Projektdateien verfügen, die das Erstellen unter Windows vereinfachen.
Alles, was ich brauchte, war, die Projekte zu sammeln, um sicherzustellen, dass alles für die Überprüfung erforderlich war. Als nächstes startete ich die Analyse und voila! - Vor mir ein vorgefertigter Analysatorbericht.
In diesen Projekten enthaltene Bibliotheken von Drittanbietern können ebenfalls Fehler enthalten und natürlich auch den Betrieb des Programms beeinträchtigen. Ich habe sie jedoch aus Gründen der Reinheit der Erzählung von der Analyse ausgeschlossen.
So werden die Projekte analysiert, Berichte empfangen, interessante Fehler ausgeschrieben. Es ist Zeit, mit ihrer Analyse fortzufahren!
Was versteckt FreeRTOS
Anfangs hatte ich erwartet, zwei separate Artikel zu schreiben: einen für jedes Betriebssystem. Ich rieb mir bereits die Hände und bereitete mich darauf vor, einen guten Artikel über FreeRTOS zu schreiben. Ich erwartete die Entdeckung von mindestens ein paar saftigen Fehlern (wie
CWE-457 ) und schaute gespannt auf die wenigen Warnungen des Analysators und ... und nichts. Ich habe keinen interessanten Fehler gefunden.
Viele Warnungen, die der Analysator ausgegeben hat, waren für FreeRTOS nicht relevant. Solche Warnungen waren beispielsweise 64-Bit-Mängel wie das
Umwandeln von
size_t in
uint32_t . Dies liegt an der Tatsache, dass FreeRTOS für Geräte mit einer Zeigergröße von nicht mehr als 32 Bit ausgelegt ist.
Ich habe alle
V1027- Warnungen in Bezug auf
Umwandlungen zwischen Zeigern auf nicht verwandte Strukturen sorgfältig geprüft. Wenn reduzierbare Strukturen die gleiche Ausrichtung haben, ist ein solcher Guss kein Fehler. Und ich habe keine einzige gefährliche Besetzung gefunden!
Alle anderen verdächtigen Stellen bezogen sich entweder auf den Codierungsstil oder wurden mit einem Kommentar versehen, in dem erläutert wurde, warum dies hier genau geschieht und warum dies kein Fehler ist.
Im Allgemeinen möchte ich die Entwickler von FreeRTOS kontaktieren. Ihr seid wirklich großartig! Wir haben fast nie so saubere und qualitativ hochwertige Projekte wie Ihres getroffen. Und ich war sehr erfreut, sauberen, ordentlichen und gut dokumentierten Code zu lesen. Hut ab vor dir.
Obwohl ich an diesem Tag keine interessanten Fehler finden konnte, verstand ich, dass ich hier nicht aufhören würde. Ich ging mit der festen Überzeugung nach Hause, dass in der Version von Amazon 100% etwas Interessantes zu finden sein würde und dass ich morgen definitiv genug Fehler für den Artikel sammeln würde. Wie Sie wahrscheinlich vermutet haben, hatte ich recht.
Was verbirgt Amazon FreeRTOS
Die Amazon-Version des Systems erwies sich als ... gelinde gesagt etwas schlechter. Das Erbe von FreeRTOS ist genauso sauber geblieben, aber die neuen Revisionen erwiesen sich als recht interessant.
An einigen Stellen wurde die Logik des Programms verletzt, irgendwo wurde falsch mit Zeigern gearbeitet. An einigen Stellen könnte der Code zu undefiniertem Verhalten führen, aber irgendwo wusste der Programmierer einfach nichts über das von ihm erstellte Fehlermuster. Ich habe sogar einige ernsthafte potenzielle Schwachstellen gefunden.
Etwas, das ich mit der Einführung verzögert habe. Beginnen wir mit der Analyse der Fehler!
Verletzung der Programmlogik
Beginnen wir mit den Problembereichen, die deutlich machen, dass das Programm nicht genau so läuft, wie es der Programmierer erwartet hat. Der erste Ort dieser Art ist verdächtige Arbeit mit einem Array:
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 hat zwei Warnungen für diesen Code ausgegeben:
- V619 Das Array '_requestPool.pRequestDatas' wird als Zeiger auf ein einzelnes Objekt verwendet. iot_demo_https_s3_download_async.c 973
- V574 Der Zeiger '_requestPool.pRequestDatas' wird gleichzeitig als Array und als Zeiger auf ein einzelnes Objekt verwendet. Überprüfen Sie die Zeilen: 931, 973. iot_demo_https_s3_download_async.c 973
Für alle Fälle möchte ich Sie daran erinnern: Der Name des Arrays ist ein Zeiger auf sein erstes Element. Das heißt, wenn
_requestPool.pRequestDatas ein Array von Strukturen ist, ist
_requestPool.pRequestDatas [i] .scheduled der Zugriff auf das
geplante Mitglied der
i- ten Struktur des Arrays. Wenn Sie
_requestPool.pRequestDatas-> Scheduled schreiben, bedeutet dies den Zugriff auf das
geplante Mitglied der ersten Struktur des Arrays.
Dies geschieht im obigen Code-Snippet. Die letzte Zeile ändert den Wert immer nur für ein Mitglied der ersten Struktur des Arrays. An sich ist dieser Aufruf bereits verdächtig, aber die Situation hier ist noch offensichtlicher: Im gesamten Funktionskörper wird auf das Array
_requestPool.pRequestDatas per Index zugegriffen, und erst am Ende des Indizierungsvorgangs wurde vergessen, angewendet zu werden.
So wie ich es verstehe, sollte die letzte Zeile so aussehen:
_requestPool.pRequestDatas[reqIndex].scheduled = true;
Der folgende Fehler liegt in einer kleinen Funktion, daher werde ich ihn in seiner Gesamtheit angeben:
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 Warnung: V642 [CWE-197] Das Speichern des Funktionsergebnisses 'strncmp' in der Variablen 'short' ist unangemessen. Die signifikanten Bits könnten verloren gehen und die Logik des Programms brechen. aws_greengrass_discovery.c 637
Werfen wir einen Blick auf die Funktionsdefinition von strncmp:
int strncmp( const char *lhs, const char *rhs, size_t count );
Im Beispiel wird ein Ergebnis vom Typ
int mit einer Größe von 32 Bit in eine Variable vom Typ
int16_t konvertiert . Bei einer solchen "Verengungs" -Umwandlung gehen die höchstwertigen Bits des Rückgabewerts verloren. Wenn die Funktion
strncmp beispielsweise
0x00010000 zurückgibt,
geht die Funktion während der Konvertierung verloren und die Bedingung ist erfüllt.
In der Tat ist es seltsam, eine solche Besetzung in einem Zustand zu sehen. Warum überhaupt, wenn Sie normales
int mit Null vergleichen können? Wenn der Programmierer jedoch bewusst wollte, dass die Funktion manchmal
true zurückgibt , auch wenn dies nicht der
Fall sein sollte, warum wird dieses schwierige Verhalten dann nicht durch den Kommentar beschrieben? Aber dann ist das schon ein Lesezeichen. Im Allgemeinen neige ich dazu zu glauben, dass dies ein Fehler ist. Was denkst du?
Undefiniertes Verhalten und Zeiger
Jetzt wird es ein ziemlich großes Beispiel geben. Es verbirgt die mögliche Dereferenzierung eines Nullzeigers:
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 Warnung: V522 [CWE-690] Möglicherweise wird ein potenzieller Nullzeiger 'pCurrentHttpsResponse' dereferenziert. iot_https_client.c 1184
Problem-Dereferenzen sind ganz unten,
wenn . Mal sehen, was hier passiert.
Zu Beginn der Funktion werden die
Variablen pCurrentHttpsResponse und
pQItem auf
NULL initialisiert, und die Statusvariable wird auf
IOT_HTTPS_OK initialisiert , was bedeutet, dass alles reibungslos verläuft.
Als nächstes wird
pQItem der von der Funktion
IotDeQueue_PeekHead zurückgegebene Wert
zugewiesen , der einen Zeiger auf den Anfang einer doppelt verbundenen Warteschlange zurückgibt.
Was passiert, wenn die Warteschlange leer ist? In diesem Fall gibt die Funktion
IotDeQueue_PeekHead NULL zurück :
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; }
Als nächstes ist die Bedingung
pQItem == NULL erfüllt und der Steuerungsfluss
geht über
goto zum unteren Teil des Funktionskörpers. Zu diesem Zeitpunkt bleibt der
pCurrentHttpsResponse- Zeiger null und der
Status ist nicht mehr gleich
IOT_HTTPS_OK . Infolgedessen werden wir in
genau diesen Zweig fallen,
wenn und ... breit! Die Konsequenzen dieser Dereferenzierung kennen Sie selbst.
Okay Es war ein leicht verziertes Beispiel. Jetzt mache ich Sie auf eine sehr einfache und verständliche mögliche Dereferenzierung aufmerksam:
int PKI_mbedTLSSignatureToPkcs11Signature (uint8_t * pxSignaturePKCS, uint8_t * pxMbedSignature ) { int xReturn = 0; uint8_t * pxNextLength; uint8_t ucSigComponentLength = pxMbedSignature[ 3 ];
PVS-Studio Warnung: V595 [CWE-476] Der Zeiger 'pxMbedSignature' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 52, 54. iot_pki_utils.c 52
Diese Funktion erhält zwei Zeiger auf
uint8_t . Beide Zeiger werden auf
NULL geprüft, was eine gute Praxis ist - solche Situationen müssen sofort geklärt werden.
Aber hier ist das Pech:
Wenn pxMbedSignature überprüft wird, wird es bereits eine Zeile darüber buchstäblich dereferenziert. Ta-daa!
Ein weiteres Beispiel für spekulativen 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-Warnungen:- V1004 [CWE-628] Der Zeiger 'x51ByteHashOidBuffer' wurde unsicher verwendet, nachdem er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 275, 280. iot_pkcs11.c 280
- V1004 [CWE-628] Der Zeiger 'x32ByteHashedMessage' wurde unsicher verwendet, nachdem er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 275, 281. iot_pkcs11.c 281
Der Analysator warnt davor, dass Funktionsparameter, die Zeiger sind, unsicher verwendet werden, nachdem sie auf
NULL getestet wurden. In der Tat werden die Argumente überprüft, aber wenn sich herausstellt, dass eines von ihnen
NULL ist , werden keine Maßnahmen ergriffen, außer zum Schreiben in
xResult . Dieser Code scheint zu sagen: „Ja, das bedeutet, dass die Argumente schlecht waren. Wir werden es jetzt aufschreiben, während Sie fortfahren, fahren Sie fort. "
Fazit:
NULL wird an
memcpy übergeben . Was kann daraus werden? Wo und welche werden die Werte kopiert? In der Tat lohnt es sich nicht, darüber zu raten, weil Der Standard
besagt eindeutig, dass ein solcher Aufruf zu undefiniertem Verhalten führt (siehe Absatz 1).
Der Analysebericht enthält immer noch Beispiele für fehlerhafte Operationen mit Zeigern, die ich in Amazon FreeRTOS gefunden habe. Ich denke jedoch, dass die angegebenen Beispiele bereits ausreichen, um Ihnen die Möglichkeiten von PVS-Studio zur Erkennung solcher Fehler zu demonstrieren. Betrachten Sie etwas Neues.
WAHR! = 1
Einige Fehler, die ich gefunden habe, bezogen sich auf ein Muster, das leider oft vergessen wird.
Tatsache ist, dass sich der
Bool- Typ (aus C ++) vom
BOOL- Typ (normalerweise in C verwendet) unterscheidet. Die erste kann nur
wahr oder
falsch enthalten . Das zweite ist ein Typedef eines Integer-Typs (
int ,
long usw.). Für ihn ist „falsch“ der Wert
0 und „wahr“ ein anderer Wert als Null.
Da es in C keinen integrierten Booleschen Typ gibt, werden diese Konstanten der Einfachheit halber definiert:
#define FALSE 0 #define TRUE 1
Betrachten Sie nun ein Beispiel:
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-Warnungen:- V676 [CWE-253] Es ist falsch, die Variable vom Typ BOOL mit TRUE zu vergleichen. aws_entropy_hardware_poll.c 48
- V676 [CWE-253] Es ist falsch, die Variable vom Typ BOOL mit TRUE zu vergleichen. Der richtige Ausdruck lautet: 'FALSE! = CryptGenRandom (hProv, len, output)'. aws_entropy_hardware_poll.c 51
Einen Fehler gefunden? Und es ist :) Funktionen
CryptAcquireContextA und
CryptGenRandom sind Standardfunktionen aus dem Header
wincrypt.h . Bei Erfolg geben sie einen Wert ungleich Null zurück. Ich betone -
ungleich Null . Theoretisch kann dies also ein anderer Wert als Null sein:
1 ,
314 ,
42 ,
420 .
Anscheinend hat der Programmierer, der die Funktion aus dem Beispiel geschrieben hat, nicht darüber nachgedacht, und als Ergebnis werden die erhaltenen Werte mit der Einheit verglichen.
Mit welcher Wahrscheinlichkeit ist die Bedingung
TRUE == CryptGenRandom (....) nicht erfüllt? Es ist schwer zu sagen. Möglicherweise gibt
CryptGenRandom eine Einheit häufiger als andere Werte zurück, und möglicherweise wird immer nur eine zurückgegeben. Wir können nicht sicher wissen: Die Implementierung dieser kryptografischen Funktion ist vor den Augen sterblicher Programmierer verborgen :)
Es ist wichtig zu bedenken, dass solche Vergleiche potenziell gefährlich sind. Und statt:
if (TRUE == GetBOOL())
Verwenden Sie eine sicherere Option:
if (FALSE != GetBOOL())
Optimierungsprobleme
Mehrere Analysatorwarnungen wurden mit langsam laufenden Konstrukten in Verbindung gebracht. Zum Beispiel:
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strstr(contentRangeValStr, "/"); .... }
PVS-Studio Warnung: V817 Es ist effizienter, nach '/' als nach einer Zeichenfolge zu suchen. iot_demo_https_common.c 205
Kurz und klar, nicht wahr? Die
strstr- Funktion wird hier verwendet, um nach nur einem Zeichen zu suchen, das als Zeichenfolge an den Parameter übergeben wird (es steht in doppelten Anführungszeichen).
Dieser Ort kann möglicherweise optimiert werden, indem
strstr durch
strchr ersetzt wird :
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strchr(contentRangeValStr, '/'); .... }
Dann funktioniert die Suche etwas schneller. Eine Kleinigkeit, aber nett.
Solche Optimierungen sind natürlich gut, und der Analysator hat einen anderen Ort gefunden, der viel deutlicher optimiert werden kann:
void vRunOTAUpdateDemo(void) { .... for (; ; ) { .... xConnectInfo.cleanSession = true; xConnectInfo.clientIdentifierLength = (uint16_t)strlen(clientcredentialIOT_THING_NAME); xConnectInfo.pClientIdentifier = clientcredentialIOT_THING_NAME; .... } }
Warnung PVS-Studio: V814 Leistungsminderung. Die 'strlen'-Funktion wurde innerhalb des Körpers einer Schleife mehrmals aufgerufen. aws_iot_ota_update_demo.c 235
Hmmm ... Innerhalb der Schleife wird bei jeder Iteration
strlen aufgerufen, das jedes Mal die Länge derselben Zeile berechnet. Nicht die effizienteste Operation :)
Werfen wir einen Blick auf die Definition von
clientcredentialIOT_THING_NAME :
#define clientcredentialIOT_THING_NAME ""
Der Benutzer wird aufgefordert, hier den Namen seines Geräts einzugeben. Standardmäßig ist es leer und in diesem Fall ist alles in Ordnung. Aber was ist, wenn der Benutzer dort einen langen und schönen Namen eingeben möchte? Zum Beispiel würde ich meine Idee gerne "
Die leidenschaftliche und hoch entwickelte Kaffeemaschine BarBarista-N061E The Ultimate Edition " nennen. Können Sie sich vorstellen, was meine Überraschung wäre, wenn meine schöne Kaffeemaschine danach etwas langsamer arbeiten würde? Durcheinander!
Um den Fehler zu beheben, sollte
strlen aus dem Schleifenkörper genommen werden. Schließlich ändert sich der Gerätename nicht, während das Programm ausgeführt wird. Ehhh, hier wäre
constexpr von C ++ ...
Okay, okay, ich gebe zu: hier war ich etwas verdickt. Wie mein Kollege Andrei Karpov feststellte, wissen moderne Compiler, was
strlen ist, und er hat persönlich beobachtet, wie sie einfach eine Konstante im Binärcode verwenden, wenn sie verstehen, dass sich die Länge des Strings nicht ändern kann. Es besteht also eine hohe Wahrscheinlichkeit, dass im Build-Modus der Release-Version anstelle der tatsächlichen Berechnung der Zeichenfolgenlänge einfach ein zuvor berechneter Wert verwendet wird. Dies funktioniert jedoch nicht immer, so dass das Schreiben eines solchen Codes keine gute Praxis ist.
Ein paar Worte zu MISRA
Der PVS-Studio-Analysator verfügt über zahlreiche Regeln, mit denen Sie Ihren Code auf Übereinstimmung mit den Standards MISRA C und MISRA C ++ überprüfen können. Was sind diese Standards?
MISRA ist der Codierungsstandard für hochempfindliche eingebettete Systeme. Es enthält eine Reihe strenger Regeln und Richtlinien zum Schreiben von Code und zum Einrichten des Entwicklungsprozesses. Es gibt einige dieser Regeln, die nicht nur darauf abzielen, schwerwiegende Fehler zu beseitigen, sondern auch auf verschiedene „Code-Gerüche“ sowie den verständlichsten und lesbarsten Code zu schreiben.
Das Befolgen des MISRA-Standards hilft somit nicht nur, Fehler und Schwachstellen zu vermeiden, sondern auch erheblich - erheblich! - die Wahrscheinlichkeit ihres Auftretens in einem vorhandenen Code verringern.
MISRA wird in der Luft- und Raumfahrt, in der Medizin, in der Automobilindustrie und im Militär eingesetzt - wo Menschenleben von der Qualität der eingebetteten Software abhängen.
Anscheinend kennen Amazon FreeRTOS-Entwickler diesen Standard und befolgen ihn größtenteils. Das ist richtig: Wenn Sie ein breit angelegtes Betriebssystem für eingebettete Systeme schreiben, müssen Sie über Sicherheit nachdenken.
Ich habe jedoch einige Verstöße gegen den MISRA-Standard festgestellt. Ich werde hier keine Beispiele für Regeln wie "Keine Union verwenden" oder "Eine Funktion sollte nur eine Rückkehr am Ende des Körpers haben" nennen - leider sind sie nicht spektakulär, wie die meisten MISRA-Regeln. Ich gebe Ihnen besser Beispiele für Verstöße, die möglicherweise schwerwiegende Folgen haben können.
Beginnen wir mit den Makros:
#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-Warnungen:- V2546 [MISRA C 20.7] Das Makro und seine Parameter sollten in Klammern stehen. Überprüfen Sie den Parameter 'ms' des Makros 'FreeRTOS_ms_to_tick'. FreeRTOS_IP.h 201
- V2546 [MISRA C 20.7] Das Makro und seine Parameter sollten in Klammern stehen. Überprüfen Sie den Parameter 'ulIn' des Makros 'SOCKETS_htonl'. iot_secure_sockets.h 512
- V2546 [MISRA C 20.7] Das Makro und seine Parameter sollten in Klammern stehen. Überprüfen Sie die Parameter 'x', 'c' des Makros 'LEFT_ROTATE'. iot_device_metrics.c 90
Ja, genau das haben Sie gedacht. Die Parameter dieser Makros werden nicht in Klammern gesetzt. Wenn jemand versehentlich so etwas schreibt
val = LEFT_ROTATE(A[i] | 1, B);
dann öffnet sich ein solches "Aufruf" -Makro in:
val = ( ( A[i] | 1 << B ) | ( A[i] | 1 >> ( 32 - B ) ) );
Erinnern Sie sich an die Prioritäten der Operationen? Zuerst wird eine bitweise Verschiebung durchgeführt, und erst nachdem es ein bitweises "oder" ist. Daher wird die Logik des Programms verletzt. Ein einfacheres Beispiel: Was passiert, wenn der Ausdruck "
x + y " an das Makro
FreeRTOS_ms_to_tick übergeben wird ? Eines der Hauptziele von MISRA ist es, das Auftreten solcher Situationen zu verhindern.
Jemand könnte Einwände erheben: „Wenn Sie Programmierer haben, die nichts davon wissen, wird Sie kein Standard retten!“ Und ich werde dem nicht zustimmen. Programmierer sind auch Menschen, und egal wie erfahren ein Mensch ist, auch er kann müde werden und am Ende des Arbeitstages einen Fehler machen. Dies ist einer der Gründe, warum MISRA dringend die Verwendung automatisierter Analysetools empfiehlt, um ein Projekt auf Übereinstimmung mit dem Standard zu validieren.
Ich wende mich an die Entwickler von Amazon FreeRTOS: PVS-Studio hat 12 weitere unsichere Makros gefunden, daher sind Sie dort mit ihnen vorsichtiger :)
Ein weiterer interessanter MISRA-Verstoß:
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)); }
Können Sie den Fehler selbst finden?
PVS-Studio Warnung: V2537 [MISRA C 2.7] Funktionen sollten keine nicht verwendeten Parameter haben. Überprüfen Sie den Parameter: 'rc'. iot_demo_https_s3_upload_async.c 234
Schauen Sie genauer hin: Der Parameter
rc wird nirgendwo im Funktionskörper verwendet. Darüber hinaus wird im Kommentar zur Funktion ausdrücklich geschrieben, dass dieser Parameter der Rückkehrcode einer anderen Funktion ist und einen Fehler signalisieren kann. Warum wird dieser Parameter dann in keiner Weise verarbeitet? Hier stimmt eindeutig etwas nicht.
Selbst ohne solche Kommentare weisen nicht verwendete Parameter häufig auf eine fehlerhafte Programmlogik hin. Warum werden sie sonst in der Funktionssignatur benötigt?
Hier habe ich eine kleine Funktion angegeben, die für ein Beispiel im Artikel gut geeignet ist. Zusätzlich zu ihr habe ich 10 weitere unbenutzte Parameter gefunden. Viele von ihnen werden in größeren Funktionen verwendet, und es ist nicht einfach, sie zu finden.
Es ist verdächtig, dass sie vorher nicht gefunden wurden. In der Tat können Compiler solche Fälle leicht erkennen.
Fazit
Dies waren nicht alle Problembereiche, die der Analysator gefunden hatte, aber der Artikel war bereits ziemlich groß. Ich hoffe, dass die Entwickler von Amazon FreeRTOS dank dieser Probleme einige Mängel beheben können und
PVS-Studio vielleicht sogar selbst
ausprobieren möchten. Auf diese Weise können Warnungen eingehender untersucht werden, und die Arbeit mit einer praktischen Benutzeroberfläche ist in der Tat viel einfacher als das Betrachten eines Textberichts.
Vielen Dank für das Lesen unserer Artikel! Wir sehen uns in der nächsten Ausgabe: D.
PS Es ist einfach so passiert, dass dieser Artikel am 31. Oktober veröffentlicht wurde. Deshalb wünsche ich allen ein frohes Halloween!
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: George Gribkov.
Auf Anfrage eingebetteter Entwickler: Erkennen von Fehlern in Amazon FreeRTOS .