Jeder, der Mikrocontroller programmiert, kennt FreeRTOS wahrscheinlich oder hat zumindest von diesem Betriebssystem gehört. Amazon-Entwickler haben beschlossen, die Funktionen dieses Betriebssystems für die Arbeit mit AWS Internet of Things-Diensten zu verbessern. So erschien Amazon FreeRTOS. Wir, Entwickler des statischen Code-Analysators PVS-Studio, wurden per E-Mail und in Kommentaren gebeten, diese Projekte zu überprüfen. Nun, jetzt holen Sie sich, wonach Sie gefragt haben. Lesen Sie weiter, um herauszufinden, was dabei herausgekommen ist.
Kurz über Projekte
Zunächst erzähle ich Ihnen etwas über den Vorläufer des getesteten Projekts - FreeRTOS (der Quellcode ist hier per
Link verfügbar). Laut Wikipedia ist FreeRTOS ein Echtzeit-Multitasking-Betriebssystem für eingebettete Systeme.
Es ist in gutem alten C geschrieben, was nicht überraschend ist - dieses Betriebssystem sollte unter für Mikrocontroller typischen Bedingungen funktionieren: geringe Verarbeitungsleistung, 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 geeignet, um ein solches Betriebssystem zu entwickeln.
Nun zurück zu Amazon, das immer in Bewegung ist und verschiedene vielversprechende Richtungen entwickelt. Zum Beispiel entwickelt Amazon eine AAA-Engine von Amazon Lumberyard, die wir
ebenfalls überprüft haben .
Eine dieser Richtungen ist das Internet der Dinge (IoT). Um sich in diesem Bereich weiterzuentwickeln, hat Amazon beschlossen, ein eigenes Betriebssystem zu schreiben - und den FreeRTOS-Kern zugrunde gelegt.
Das resultierende System, Amazon FreeRTOS, ist so positioniert, dass es "eine sichere Verbindung zu Amazon Web Services wie AWS IoT Core oder AWS IoT Greengrass bereitstellt". Der Quellcode dieses Projekts
ist auf Github
verfügbar .
In diesem Artikel erfahren Sie, ob FreeRTOS fehlerhaft ist und wie sicher das Amazon-Betriebssystem in Bezug auf die statische Code-Analyse ist.
Der Verlauf der Prüfung
Die Überprüfung wurde mit dem Tool zur automatischen Fehlersuche durchgeführt - dem statischen Code-Analysator PVS-Studio. Es kann Fehler in Programmen erkennen, die in C, C ++, C # und Java geschrieben wurden.
Vor der Analyse müssen wir das Projekt erstellen. Auf diese Weise bin ich sicher, dass ich alle erforderlichen Abhängigkeiten habe und das Projekt zur Überprüfung bereit ist. Das Projekt kann auf verschiedene Arten überprüft werden - beispielsweise mithilfe eines Kompilierungsüberwachungssystems. In diesem Fall habe ich die Analyse mit dem Plugin für Visual Studio durchgeführt. Es ist gut, dass die Repositorys beider Projekte die Sätze von Projektdateien enthalten, die das Erstellen unter Windows vereinfachen.
Ich musste nur Projekte erstellen, um sicherzustellen, dass ich alles für die Überprüfung bereit hatte. Als nächstes führte ich die Analyse durch und - voila! - Ich habe einen vorgefertigten Analysatorbericht vor mir.
In diesen Projekten enthaltene Bibliotheken von Drittanbietern können ebenfalls Fehler enthalten und sich natürlich auch auf das Programm auswirken. 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 hervorgehoben. Es ist Zeit, ihre Bewertung zu erhalten!
Was FreeRTOS versteckt
Anfangs hatte ich erwartet, zwei separate Artikel zu schreiben: einen für jedes Betriebssystem. Ich habe mir schon die Hände gerieben? als ich mich darauf vorbereitete, einen guten Artikel über FreeRTOS zu schreiben. Als ich die Entdeckung von mindestens ein paar saftigen Fehlern (wie
CWE-457 ) erwartete, sah ich mir die spärlichen Warnungen des Analysators an und ... fand nichts. Ich habe keinen interessanten Fehler gefunden.
Viele der vom Analysegerät ausgegebenen Warnungen waren für FreeRTOS nicht relevant. Solche Warnungen waren beispielsweise 64-Bit-Fehler wie das
Umwandeln von
size_t in
uint32_t . Dies hängt mit der Tatsache zusammen, dass FreeRTOS für Geräte mit einer Zeigergröße von nicht mehr als 32 Bit vorgesehen ist.
Ich habe alle
V1027- Warnungen, die auf Abgüsse zwischen Zeigern auf nicht verwandte Strukturen hinweisen, gründlich überprüft. Wenn gegossene Strukturen die gleiche Ausrichtung haben, ist ein solches Gießen ein Fehler. Und ich habe kein einziges gefährliches Casting gefunden!
Alle anderen verdächtigen Stellen waren entweder mit dem Codierungsstil verbunden oder mit einem Kommentar versehen, der erklärte, warum dies so gemacht wurde und warum es kein Fehler war.
Daher möchte ich FreeRTOS-Entwickler ansprechen. Leute, du bist großartig! Wir haben kaum so saubere und qualitativ hochwertige Projekte gesehen wie Ihre. Und es war eine Freude, den sauberen, ordentlichen und gut dokumentierten Code zu lesen. Hut ab vor euch.
Obwohl ich an diesem Tag keine interessanten Fehler finden konnte, wusste ich, dass ich hier nicht aufhören würde. Ich ging mit der festen Zuversicht nach Hause, dass die Amazon-Version zu 100% etwas Interessantes haben würde und dass ich morgen definitiv genug Fehler für den Artikel finden würde. Wie Sie vielleicht erraten haben, hatte ich recht.
Was Amazon FreeRTOS verbirgt
Die Amazon-Version des Systems erwies sich als ... gelinde gesagt etwas schlechter. Das Erbe von FreeRTOS blieb ebenso sauber, während die neuen Verbesserungen viele interessante Probleme verdeckten.
Bei einigen Fragmenten war die Programmlogik fehlerhaft, bei anderen wurden Zeiger falsch behandelt. An einigen Stellen konnte der Code zu undefiniertem Verhalten führen, und es gab Fälle, in denen der Programmierer einfach nicht über das Muster eines von ihm gemachten Fehlers Bescheid wusste. Ich habe sogar einige schwerwiegende potenzielle Sicherheitslücken gefunden.
Scheint, als hätte ich mich mit der Einführung verschärft. Lassen Sie uns anfangen, Fehler herauszufinden!
Unterbrechung der Programmlogik
Beginnen wir mit Problemstellen, die offensichtlich darauf hinweisen, dass das Programm nicht so funktioniert, wie es der Programmierer erwartet hat. Die Behandlung verdächtiger Arrays steht an erster Stelle:
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 Array-Name ist der Zeiger auf das erste Element. Das heißt, wenn
_requestPool.pRequestDatas ein Array von Strukturen ist, ist
_requestPool.pRequestDatas [i] .scheduled eine Bewertung für das
geplante Mitglied der
i- Array-Struktur.
Und wenn wir
_requestPool.pRequestDatas-> Scheduled schreiben, stellt sich heraus, dass dies der
Fall ist Auf das Mitglied der ersten Array-Struktur wird zugegriffen.
Im obigen Auszug des Codes passiert genau das. In der letzten Zeile wird nur der Wert des Mitglieds der ersten Array-Struktur geändert. An sich ist ein solcher Zugriff bereits verdächtig, aber hier ist der Fall noch deutlicher:
Das Array
_requestPool.pRequestDatas wird im gesamten Funktionskörper anhand des Index bewertet. Am Ende wurde der Indizierungsvorgang jedoch vergessen.
So wie ich es verstehe, sollte die letzte Zeile so aussehen:
_requestPool.pRequestDatas[reqIndex].scheduled = true;
Der nächste Fehler liegt in einer kleinen Funktion, also werde ich es komplett geben:
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 Definition der Funktion strncmp:
int strncmp( const char *lhs, const char *rhs, size_t count );
Im Beispiel wird das Ergebnis des 32-Bit-Typs
int in eine Variable vom Typ
int16_t konvertiert. Bei dieser "Verengungs" -Konvertierung gehen die älteren Bits des zurückgegebenen Werts verloren. Wenn die Funktion
strncmp beispielsweise
0x00010000 zurückgibt, geht die Einheit während der Konvertierung verloren und die Bedingung wird ausgeführt.
Es ist tatsächlich seltsam, ein solches Casting in diesem Zustand zu sehen. Warum wird es hier jemals benötigt, wenn ein gewöhnliches
int mit Null verglichen werden kann? Auf der anderen Seite, wenn ein Programmierer wollte, dass diese Funktion manchmal
true zurückgibt, auch wenn dies nicht der
Fall sein sollte, warum nicht solch kniffliges Verhalten mit einem Kommentar unterstützen? Aber auf diese Weise ist es eine Art Hintertür. Wie auch immer, ich neige dazu zu denken, dass es ein Fehler ist. Was denkst du?
Undefiniertes Verhalten und Zeiger
Hier kommt ein großes Beispiel. Es deckt eine mögliche Nullzeiger-Dereferenzierung ab:
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
Der letzte
if- Block enthält problematische Dereferenzen. Lassen Sie uns herausfinden, was hier los ist.
Die Funktion beginnt mit den Variablen
pCurrentHttpsResponse und
pQItem , die durch den Wert
NULL initialisiert wurden, und die Statusvariable wird durch den Wert
IOT_HTTPS_OK initialisiert, was bedeutet, dass alles korrekt ist.
Weiterem pQItem wird der Wert zugewiesen, der von der Funktion
IotDeQueue_PeekHead zurückgegeben wird, die den Zeiger auf den Anfang der doppelt verknüpften 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; }
Ferner wird die Bedingung
pQItem == NULL wahr und der Kontrollfluss wird von
goto an den unteren Teil der Funktion übergeben. Zu diesem Zeitpunkt bleibt der
pCurrentHttpsResponse- Zeiger null, während der
Status nicht gleich
IOT_HTTPS_OK ist . Am Ende kommen wir zum selben Zweig und ... boom! Nun, Sie kennen die Konsequenzen einer solchen Dereferenzierung.
Okay Es war ein etwas kniffliges Beispiel. Nun schlage ich vor, dass Sie sich eine sehr einfache und verständliche mögliche Dereferenzierung ansehen:
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 empfängt Zeiger auf
uint8_t . Beide Zeiger werden auf
NULL überprüft, was eine gute Vorgehensweise ist. Solche Situationen sollten sofort geklärt werden.
Aber hier ist das Problem:
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, nach ihrer Prüfung auf
NULL nicht sicher verwendet werden. In der Tat werden die Argumente überprüft. Falls jedoch einer von ihnen nicht
NULL ist , werden keine Maßnahmen ergriffen, außer in
xResult zu schreiben
. In diesem Abschnitt des Codes heißt es: "Ja, die Argumente haben sich als schlecht herausgestellt. Wir werden es jetzt bemerken, und Sie - machen Sie weiter, machen Sie weiter. “
Ergebnis:
NULL wird an
memcpy übergeben. Was kann daraus werden? Wo und welche werden die Werte kopiert? In der Tat hilft das Erraten nicht, da der Standard
eindeutig besagt, dass ein solcher Aufruf zu undefiniertem Verhalten führt (siehe Abschnitt 1).
Es gibt andere Beispiele für die falsche Behandlung von Zeigern im Analysebericht in Amazon FreeRTOS, aber ich denke, dass gegebene Beispiele ausreichen, um die Fähigkeiten von PVS-Studio bei der Erkennung solcher Fehler zu demonstrieren. Schauen wir uns etwas Neues an.
WAHR! = 1
Es gab mehrere Fehler im Zusammenhang mit dem Muster, die leider oft übersehen werden.
Tatsache ist, dass sich der
Bool- Typ (aus C ++) vom
BOOL- Typ (üblicherweise in C verwendet) unterscheidet. Der erste kann nur einen
wahren oder
falschen Wert enthalten. Der zweite ist der Typedef eines Integer-Typs (
int ,
long und andere). Der
0- Wert ist dafür "falsch", und jeder andere von Null abweichende Wert ist "wahr".
Da es in C keinen integrierten Booleschen Typ gibt, werden diese Konstanten der Einfachheit halber definiert:
#define FALSE 0 #define TRUE 1
Schauen wir uns das Beispiel an.
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
Haben Sie einen Fehler gefunden? Zweifeln Sie nicht, es ist hier :) Die Funktionen
CryptAcquireContextA und
CryptGenRandom sind Standardfunktionen aus dem Header
wincrypt.h . Bei Erfolg geben sie den Wert ungleich Null zurück. Lassen Sie mich betonen, dass es
nicht Null ist . Theoretisch könnte es sich also um einen anderen Wert als Null handeln:
1 ,
314 ,
42 ,
420 .
Anscheinend hat der Programmierer, der die Funktion aus dem Beispiel geschrieben hat, nicht darüber nachgedacht, und am Ende werden die resultierenden Werte mit einem verglichen.
Wie wahrscheinlich ist es, dass die
TRUE == CryptGenRandom (....) Bedingung nicht erfüllt wird? Es ist schwer zu sagen. Vielleicht gibt
CryptGenRandom 1 häufiger zurück als andere Werte, aber vielleicht gibt es nur 1 zurück. Wir können dies 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. Anstelle von:
if (TRUE == GetBOOL())
Verwenden Sie eine sicherere Version des Codes:
if (FALSE != GetBOOL())
Optimierungsprobleme
Mehrere Warnungen des Analysators bezogen sich auf langsam arbeitende Strukturen. 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
Es ist kurz und einfach, nicht wahr? Die Funktion
strstr wird hier verwendet, um nur ein Zeichen zu suchen, das im Parameter als Zeichenfolge übergeben wird (in doppelten Anführungszeichen).
Dieser Ort kann möglicherweise optimiert werden, indem
strstr durch
strchr ersetzt wird :
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strchr(contentRangeValStr, '/'); .... }
Auf diese Weise funktioniert die Suche etwas schneller. Eine kleine, aber feine Sache.
Nun, solche Optimierungen sind gut, aber der Analysator hat auch einen anderen Ort gefunden, der viel deutlicher optimiert werden könnte:
void vRunOTAUpdateDemo(void) { .... for (; ; ) { .... xConnectInfo.cleanSession = true; xConnectInfo.clientIdentifierLength = (uint16_t)strlen(clientcredentialIOT_THING_NAME); xConnectInfo.pClientIdentifier = clientcredentialIOT_THING_NAME; .... } }
PVS-Studio-Warnung: V814 Leistungsminderung. Die 'strlen'-Funktion wurde innerhalb des Körpers einer Schleife mehrmals aufgerufen. aws_iot_ota_update_demo.c 235
Hmm ... Innerhalb der Schleife wird bei jeder Iteration
strlen aufgerufen, das jedes Mal die Länge derselben Zeile auswertet. Nicht die effektivste Operation :)
Schauen wir uns die Definition von
clientcredentialIOT_THING_NAME an :
#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. Was ist, wenn ein 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, wie meine Überraschung wäre, wenn meine schöne Kaffeemaschine danach etwas langsamer arbeiten würde? Ärgernis!
Um den Fehler zu korrigieren, lohnt es sich,
strlen außerhalb der Körperschleife zu nehmen. Schließlich ändert sich der Name des Geräts während der Programmarbeit nicht. Oh,
constexpr aus C ++ würde hier perfekt passen ...
Okay, lass uns die Lilie nicht vergolden. Wie mein Kollege Andrey Karpov feststellte, wissen moderne Compiler, was
strlen ist, und er beobachtete sie persönlich mit einer Konstante im Binärcode, wenn sie feststellen, dass sich die Länge der Zeile nicht ändern kann. Es besteht also eine gute Chance, dass im Release-Erstellungsmodus anstelle einer tatsächlichen Zeilenlängenbewertung der vorab bewertete 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 verantwortungsbewusste eingebettete Systeme. Es enthält eine Reihe strenger Regeln und Richtlinien zum Schreiben von Code und zum Einrichten eines Entwicklungsprozesses. Diese Regeln sind ziemlich umfangreich und zielen nicht nur darauf ab, schwerwiegende Fehler zu beseitigen, sondern auch auf verschiedene "Code-Gerüche". Es zielt auch darauf ab, den verständlichsten und lesbarsten Code zu schreiben.
Das Befolgen des MISRA-Standards hilft somit nicht nur, Fehler und Schwachstellen zu vermeiden, sondern auch die Wahrscheinlichkeit ihres Auftretens in bereits vorhandenem Code erheblich zu verringern.
MISRA wird in der Luft- und Raumfahrt, in der Medizin, in der Automobilindustrie und im Militär eingesetzt, wo das Leben von Menschen von der Qualität eingebetteter Software abhängt.
Anscheinend kennen Amazon FreeRTOS-Entwickler diesen Standard und befolgen ihn größtenteils. Ein solcher Ansatz ist absolut vernünftig: Wenn Sie ein breit angelegtes Betriebssystem für eingebettete Systeme schreiben, müssen Sie über Sicherheit nachdenken.
Ich habe jedoch ziemlich viele Verstöße gegen den MISRA-Standard festgestellt. Ich werde keine Beispiele für Regeln wie "Union nicht verwenden" oder "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 möchte Ihnen lieber Beispiele für Verstöße nennen, die möglicherweise schwerwiegende Folgen haben können.
Beginnen wir mit 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 denkst du. Die Parameter dieser Makros sind nicht in Klammern angegeben. Wenn jemand versehentlich so etwas schreibt
val = LEFT_ROTATE(A[i] | 1, B);
Ein solcher "Aufruf" eines Makros wird erweitert 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 vorgenommen, und erst danach - ein bitweises "oder". Daher wird die Logik des Programms unterbrochen. Ein einfacheres Beispiel: Was würde passieren, wenn der Ausdruck "
x + y " im Makro
FreeRTOS_ms_to_tick übergeben wird ? Eines der Hauptziele von MISRA ist es, solche Situationen zu verhindern.
Einige könnten argumentieren: "Wenn Sie Programmierer haben, die nichts davon wissen, kann Ihnen kein Standard helfen!" Dem werde ich nicht zustimmen. Programmierer sind auch Menschen, und egal wie erfahren eine Person ist, sie können auch müde werden und am Ende des Tages einen Fehler machen. Dies ist einer der Gründe, warum MISRA dringend empfiehlt, automatische Analysetools zu verwenden, um ein Projekt auf Konformität zu testen.
Lassen Sie mich die Entwickler von Amazon FreeRTOS ansprechen: PVS-Studio hat 12 weitere unsichere Makros gefunden, daher müssen Sie vorsichtig mit ihnen sein :)
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)); }
Kannst du 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. Während der Kommentar der Funktion eindeutig besagt, dass dieser Parameter ein Rückkehrcode einer anderen Funktion ist und einen Fehler signalisieren kann. Warum wird dieser Parameter in keiner Weise behandelt? Hier stimmt eindeutig etwas nicht.
Selbst ohne solche Kommentare weisen nicht verwendete Parameter häufig auf die fehlerhafte Logik des Programms hin. Warum brauchen Sie sie sonst in der Funktionssignatur?
Hier habe ich eine kleine Funktion angegeben, die für ein Beispiel im Artikel gut ist. Außerdem habe ich 10 weitere nicht verwendete Parameter gefunden. Viele von ihnen werden in größeren Funktionen verwendet, und es ist nicht einfach, sie zu erkennen.
Verdächtigerweise wurden sie noch nie gefunden. Schließlich können Compiler solche Fälle leicht erkennen.
Fazit
Dies waren nicht alle vom Analysegerät gefundenen Probleme, aber der Artikel erwies sich bereits als ziemlich groß. Ich hoffe, dass die Entwickler von amazon FreeRTOS dank dieser Funktion einige Mängel beheben können und
PVS-Studio möglicherweise sogar selbst
ausprobieren möchten. Auf diese Weise ist es bequemer, Warnungen gründlich zu untersuchen. Tatsächlich ist das Arbeiten mit einer praktischen Benutzeroberfläche viel einfacher als das Betrachten eines Textberichts.
Vielen Dank für das Lesen unserer Artikel! Wir sehen uns in der nächsten Veröffentlichung: D.
PS Es ist einfach so passiert, dass dieser Artikel am 31. Oktober veröffentlicht wurde. Fröhliches Halloween, Jungs und Mädels!