Quiconque programme des microcontrôleurs connaît probablement FreeRTOS, ou du moins a entendu parler de ce système d'exploitation. Les développeurs d'Amazon ont décidé d'améliorer les capacités de ce système d'exploitation à fonctionner avec les services AWS Internet of Things. C'est ainsi qu'Amazon FreeRTOS est apparu. Nous, développeurs de l'analyseur de code statique PVS-Studio, avons été invités par mail et dans les commentaires à vérifier ces projets. Eh bien, obtenez maintenant ce que vous avez demandé. Continuez à lire pour découvrir ce qui en est ressorti.
En bref sur les projets
Pour commencer, je vais vous parler un peu du précurseur du projet en cours de test - FreeRTOS (le code source est disponible ici par
lien ). Comme le précise Wikipedia, FreeRTOS est un système d'exploitation multitâche en temps réel pour les systèmes embarqués.
Il est écrit en bon vieux C, ce qui n'est pas surprenant - ce système d'exploitation devrait fonctionner dans des conditions typiques des microcontrôleurs: faible puissance de traitement, petite quantité de RAM, etc. Le langage C vous permet de travailler avec des ressources à un faible niveau et a des performances élevées, il est donc le mieux adapté pour développer un tel système d'exploitation.
Revenons maintenant à Amazon, qui est toujours en mouvement et qui développe diverses directions prometteuses. Par exemple, Amazon développe un moteur Amazon Lumberyard AAA, que nous avons
également vérifié .
L'une de ces directions est l'Internet des objets (IoT). Pour se développer dans ce domaine, Amazon a décidé d'écrire son propre système d'exploitation - et ils ont pris le noyau FreeRTOS comme base.
Le système résultant, Amazon FreeRTOS, est positionné pour «fournir une connexion sécurisée à Amazon Web Services, comme AWS IoT Core ou AWS IoT Greengrass». Le code source de ce projet
est disponible sur Github.
Dans cet article, nous verrons s'il y a des erreurs dans FreeRTOS ainsi que la sécurité du système d'exploitation Amazon en termes d'analyse de code statique.
Le déroulement du chèque
La vérification a été effectuée à l'aide de l'outil de recherche d'erreur automatique - l'analyseur de code statique PVS-Studio. Il est capable de détecter les erreurs dans les programmes écrits en C, C ++, C # et Java.
Avant l'analyse, nous devons construire le projet. De cette façon, je serai sûr d'avoir toutes les dépendances nécessaires et le projet est prêt à être vérifié. On peut vérifier le projet de plusieurs façons - par exemple, en utilisant un système de surveillance de compilation. Dans ce cas, j'ai effectué l'analyse en utilisant le plugin pour Visual Studio - il est bon que les référentiels des deux projets contiennent les ensembles de fichiers de projet qui facilitent la construction sous Windows.
Je devais juste construire des projets pour m'assurer que tout était prêt pour le contrôle. Ensuite, j'ai exécuté l'analyse et - le tour est joué! - J'ai un rapport d'analyseur prêt à l'emploi devant moi.
Les bibliothèques tierces incluses dans ces projets peuvent également contenir des erreurs et, bien entendu, elles peuvent également affecter le programme. Cependant, je les ai exclus de l'analyse par souci de pureté du récit.
Ainsi, les projets sont analysés, des rapports sont reçus, des erreurs intéressantes sont mises en évidence. Il est temps d'obtenir leur avis!
Ce que FreeRTOS cache
Au départ, je m'attendais à écrire deux articles distincts: un pour chaque système d'exploitation. Je me frottais déjà les mains? alors que je me préparais à écrire un bon article sur FreeRTOS. Anticipant la découverte d'au moins quelques bogues juteux (comme
CWE-457 ), je regardais les avertissements clairsemés de l'analyseur et ... je n'ai rien trouvé. Je n'ai trouvé aucune erreur intéressante.
De nombreux avertissements émis par l'analyseur n'étaient pas pertinents pour FreeRTOS. Par exemple, ces avertissements étaient des défauts 64 bits tels que la
conversion de
size_t en
uint32_t . Cela est lié au fait que FreeRTOS est censé fonctionner sur des appareils dont la taille du pointeur ne dépasse pas 32 bits.
J'ai soigneusement vérifié tous les avertissements
V1027 indiquant des pièces moulées entre des pointeurs vers des structures non liées. Si les structures coulées ont le même alignement, une telle coulée est une erreur. Et je n'ai trouvé aucun casting dangereux!
Tous les autres endroits suspects étaient soit associés au style de codage, soit dotés d'un commentaire expliquant pourquoi cela avait été fait de cette façon et pourquoi ce n'était pas une erreur.
Je voudrais donc faire appel aux développeurs de FreeRTOS. Les gars, vous êtes génial! Nous avons à peine vu des projets aussi propres et de haute qualité que le vôtre. Et ce fut un plaisir de lire le code propre, soigné et bien documenté. Chapeau à vous les gars.
Même si je n'ai pas trouvé de bogues intéressants ce jour-là, je savais que je ne m'arrêterais pas là. Je rentrais chez moi avec la ferme conviction que la version d'Amazon aurait 100% quelque chose d'intéressant, et que demain je prendrais certainement assez de bugs pour l'article. Comme vous l'avez peut-être deviné, j'avais raison.
Ce qu'Amazon FreeRTOS cache
La version d'Amazon du système s'est avérée être ... pour le dire légèrement, un peu pire. L'héritage de FreeRTOS est resté aussi propre alors que les nouvelles améliorations ont caché beaucoup de problèmes intéressants.
Certains fragments ont cassé la logique du programme, certains ont mal géré les pointeurs. À certains endroits, le code pouvait conduire à un comportement indéfini, et il y avait des cas où le programmeur ignorait tout simplement le motif d'une erreur qu'il avait commise. J'ai même trouvé plusieurs vulnérabilités potentielles graves.
On dirait que je me suis resserré avec l'introduction. Commençons à comprendre les erreurs!
Rupture de la logique du programme
Commençons par les endroits problématiques qui indiquent évidemment que le programme ne fonctionne pas de la manière attendue par le programmeur. La gestion des tableaux suspects viendra en premier:
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 a émis deux avertissements pour ce morceau de code:
- V619 Le tableau '_requestPool.pRequestDatas' est utilisé comme pointeur vers un seul objet. iot_demo_https_s3_download_async.c 973
- V574 Le pointeur '_requestPool.pRequestDatas' est utilisé simultanément comme tableau et comme pointeur vers un seul objet. Vérifiez les lignes: 931, 973. iot_demo_https_s3_download_async.c 973
Au cas où, permettez-moi de vous rappeler: le nom du tableau est le pointeur sur son premier élément. Autrement dit, si
_requestPool.pRequestDatas est un tableau de structures,
_requestPool.pRequestDatas [i] .scheduled est une évaluation du membre
planifié de la structure du tableau
i. Et si nous écrivons
_requestPool.pRequestDatas-> planifié , il se révélera que cela le membre à savoir la première structure de tableau sera accessible.
Dans l'extrait du code ci-dessus, c'est ce qui se passe. Dans la dernière ligne, seule la valeur du membre de la première structure de tableau est modifiée. En soi, un tel accès est déjà suspect, mais ici le cas est encore plus clair: le tableau
_requestPool.pRequestDatas est évalué par index dans tout le corps de la fonction. Mais à la fin l'opération d'indexation a été oubliée.
Si je comprends bien, la dernière ligne devrait ressembler à ceci:
_requestPool.pRequestDatas[reqIndex].scheduled = true;
L'erreur suivante réside dans une petite fonction, donc je vais la donner complètement:
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 ],
Avertissement PVS-Studio: V642 [CWE-197] L'enregistrement du résultat de la fonction 'strncmp' dans la variable de type 'short' est inapproprié. Les bits significatifs pourraient être perdus en brisant la logique du programme. aws_greengrass_discovery.c 637
Jetons un coup d'œil à la définition de la fonction strncmp:
int strncmp( const char *lhs, const char *rhs, size_t count );
Dans l'exemple, le résultat du type
int , d'une taille de 32 bits, est converti en une variable de type
int16_t . Avec cette conversion de "rétrécissement", les bits les plus anciens de la valeur retournée seront perdus. Par exemple, si la fonction
strncmp renvoie
0x00010000 , l'unité sera perdue pendant la conversion et la condition sera exécutée.
C'est en fait étrange de voir un tel casting dans l'état. Pourquoi est-il jamais nécessaire ici, si un
int ordinaire peut être comparé à zéro? D'un autre côté, si un programmeur voulait que cette fonction retourne parfois
true même si ce n'était pas le cas, pourquoi ne pas supporter un comportement aussi délicat avec un commentaire? Mais de cette façon, c'est une sorte de porte dérobée. Quoi qu'il en soit, j'ai tendance à penser que c'est une erreur. Qu'en penses-tu?
Comportement et pointeurs indéfinis
Voici un grand exemple. Il recouvre une déréférence potentielle de pointeur nul:
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; } .... }
Avertissement PVS-Studio: V522 [CWE-690] Il peut y avoir un déréférencement d'un pointeur nul potentiel 'pCurrentHttpsResponse'. iot_https_client.c 1184
Le dernier bloc
if contient des déréférences problématiques. Voyons ce qui se passe ici.
La fonction commence par les variables
pCurrentHttpsResponse et
pQItem initialisées par la valeur
NULL et la variable d'
état est initialisée par la valeur
IOT_HTTPS_OK , ce qui signifie que tout est correct.
En outre,
pQItem se voit attribuer la valeur, renvoyée par la fonction
IotDeQueue_PeekHead , qui renvoie le pointeur au début de la file d'attente à double liaison.
Que se passe-t-il si la file d'attente est vide? Dans ce cas, la fonction
IotDeQueue_PeekHead retournera
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; }
De plus, la condition
pQItem == NULL deviendra vraie et le flux de contrôle sera passé par
goto à la partie inférieure de la fonction. À ce moment, le pointeur
pCurrentHttpsResponse restera nul, tandis que le
statut ne sera pas égal à
IOT_HTTPS_OK . En fin de compte, nous arriverons au même
si la branche, et ... boum! Eh bien, vous connaissez les conséquences d'une telle déréférence.
Ok C'était un exemple un peu délicat. Maintenant, je vous suggère de jeter un œil à un déréférencement potentiel très simple et compréhensible:
int PKI_mbedTLSSignatureToPkcs11Signature (uint8_t * pxSignaturePKCS, uint8_t * pxMbedSignature ) { int xReturn = 0; uint8_t * pxNextLength; uint8_t ucSigComponentLength = pxMbedSignature[ 3 ];
Avertissement PVS-Studio: V595 [CWE-476] Le pointeur 'pxMbedSignature' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 52, 54. iot_pki_utils.c 52
Cette fonction reçoit des pointeurs vers
uint8_t . Les deux pointeurs sont vérifiés pour
NULL , ce qui est une bonne pratique - de telles situations doivent être résolues immédiatement.
Mais voici le problème: au moment où
pxMbedSignature est vérifié, il sera déjà déréférencé littéralement une ligne au-dessus. Ta-daa!
Un autre exemple de code spéculatif:
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; }
Avertissements de PVS-Studio:- V1004 [CWE-628] Le pointeur 'x51ByteHashOidBuffer' a été utilisé de manière non sécurisée après avoir été vérifié par rapport à nullptr. Vérifiez les lignes: 275, 280. iot_pkcs11.c 280
- V1004 [CWE-628] Le pointeur 'x32ByteHashedMessage' a été utilisé de manière non sécurisée après avoir été vérifié par rapport à nullptr. Vérifiez les lignes: 275, 281. iot_pkcs11.c 281
L'analyseur avertit que les paramètres de fonction qui sont des pointeurs, ne sont pas utilisés en toute sécurité après leur vérification de
NULL . En effet, les arguments sont vérifiés. Mais dans le cas où l'un d'entre eux n'est pas
NULL , aucune action n'est prise, sauf pour l'écriture dans
xResult. Cette section du code dit en quelque sorte: "Ouais, donc les arguments se sont avérés mauvais. Nous allons le noter maintenant, et vous - continuez, continuez. »
Résultat:
NULL sera transmis à
memcpy. Que peut-il en résulter? Où les valeurs seront-elles copiées et lesquelles? En fait, deviner n'aidera pas, car la norme
stipule clairement qu'un tel appel conduit à un comportement indéfini (voir la section 1).
Il existe d'autres exemples de gestion de pointeurs incorrects dans le rapport d'analyseur trouvé dans Amazon FreeRTOS, mais je pense que les exemples donnés sont suffisants pour montrer les capacités de PVS-Studio à détecter de telles erreurs. Jetons un œil à quelque chose de nouveau.
VRAI! = 1
Il y avait plusieurs erreurs liées au modèle, qui, malheureusement, sont souvent négligées.
Le fait est que le type
bool (de C ++) est différent du type
BOOL (couramment utilisé en C). Le premier ne peut contenir qu'une valeur
vraie ou
fausse . Le second est le typedef d'un type entier (
int ,
long et autres). La valeur
0 est "fausse" pour elle, et toute autre valeur différente de zéro est "vraie".
Puisqu'il n'y a pas de type booléen intégré en C, ces constantes sont définies par commodité:
#define FALSE 0 #define TRUE 1
Regardons l'exemple.
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; }
Avertissements de PVS-Studio:- V676 [CWE-253] Il est incorrect de comparer la variable de type BOOL avec TRUE. aws_entropy_hardware_poll.c 48
- V676 [CWE-253] Il est incorrect de comparer la variable de type BOOL avec TRUE. L'expression correcte est: «FAUX! = CryptGenRandom (hProv, len, output)». aws_entropy_hardware_poll.c 51
Avez-vous trouvé une erreur? Ne doutez pas, c'est ici :) Les fonctions
CryptAcquireContextA et
CryptGenRandom sont des fonctions standard de l'en
- tête
wincrypt.h . En cas de succès, ils renvoient la valeur non nulle. Permettez-moi de souligner qu'il est
non nul . Donc, théoriquement, cela pourrait être n'importe quelle valeur différente de zéro:
1 ,
314 ,
42 ,
420 .
Apparemment, le programmeur qui écrivait la fonction de l'exemple ne pensait pas à cela, et à la fin, les valeurs résultantes sont comparées à une.
Quelle est la probabilité que la condition
VRAI == CryptGenRandom (....) ne soit pas remplie? C'est difficile à dire. Peut-être,
CryptGenRandom pourrait renvoyer 1 plus souvent que les autres valeurs, mais peut-être qu'il n'en retournera que 1. Nous ne pouvons pas le savoir avec certitude: la mise en œuvre de cette fonction cryptographique est cachée aux yeux des programmeurs mortels :)
Il est important de se rappeler que de telles comparaisons sont potentiellement dangereuses. Au lieu de:
if (TRUE == GetBOOL())
Utilisez une version plus sûre du code:
if (FALSE != GetBOOL())
Problèmes d'optimisation
Plusieurs avertissements de l'analyseur étaient liés à des structures à fonctionnement lent. Par exemple:
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strstr(contentRangeValStr, "/"); .... }
Avertissement PVS-Studio: V817 Il est plus efficace de rechercher un caractère '/' plutôt qu'une chaîne. iot_demo_https_common.c 205
C'est court et simple, non? La fonction
strstr est utilisée ici pour rechercher un seul caractère, passé dans le paramètre sous forme de chaîne (c'est entre guillemets).
Cet endroit peut potentiellement être optimisé en remplaçant
strstr par
strchr :
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strchr(contentRangeValStr, '/'); .... }
De cette façon, la recherche fonctionnera un peu plus rapidement. Une petite mais jolie chose.
Eh bien, ces optimisations sont bonnes, mais l'analyseur a également trouvé un autre endroit, qui pourrait être optimisé de manière beaucoup plus notable:
void vRunOTAUpdateDemo(void) { .... for (; ; ) { .... xConnectInfo.cleanSession = true; xConnectInfo.clientIdentifierLength = (uint16_t)strlen(clientcredentialIOT_THING_NAME); xConnectInfo.pClientIdentifier = clientcredentialIOT_THING_NAME; .... } }
Avertissement PVS-Studio: V814 Baisse des performances. La fonction 'strlen' a été appelée plusieurs fois à l'intérieur du corps d'une boucle. aws_iot_ota_update_demo.c 235
Hmm ... A l'intérieur de la boucle, à chaque itération, un
strlen est appelé qui évalue la longueur de la même ligne à chaque fois. Pas l'opération la plus efficace :)
Regardons à l'intérieur de la définition de
clientcredentialIOT_THING_NAME :
#define clientcredentialIOT_THING_NAME ""
L'utilisateur est invité à saisir le nom de son appareil ici. Par défaut, il est vide, et dans ce cas, tout va bien. Et si un utilisateur veut y entrer un nom long et beau? Par exemple, j'aimerais appeler mon idée originale "
La machine à café passionnée et sophistiquée BarBarista-N061E The Ultimate Edition ". Pouvez-vous imaginer ce que serait ma surprise si ma belle machine à café commençait à fonctionner un peu plus lentement après cela? Nuisance!
Pour corriger l'erreur, il vaut la peine de sortir
strlen hors de la boucle du corps. Après tout, le nom de l'appareil ne change pas pendant le fonctionnement du programme. Oh,
constexpr de C ++ conviendrait parfaitement ici ...
Bon, eh bien, ne dorons pas le lis. Comme mon collègue Andrey Karpov l'a noté, les compilateurs modernes savent ce qui est
strlen et il les a personnellement regardés en utilisant une constante en code binaire s'ils constatent que la longueur de la ligne ne peut pas changer. Il y a donc de fortes chances que dans le mode de génération de version, au lieu d'une évaluation de la longueur de ligne réelle, la valeur pré-évaluée soit utilisée. Cependant, cela ne fonctionne pas toujours, donc l'écriture d'un tel code n'est pas une bonne pratique.
Quelques mots sur MISRA
L'analyseur PVS-Studio dispose d'un large ensemble de règles pour vérifier la conformité de votre code avec les normes MISRA C et MISRA C. Quelles sont ces normes?
MISRA est la norme de codage pour les systèmes embarqués hautement responsables. Il contient un ensemble de règles et de directives strictes pour l'écriture de code et la mise en place d'un processus de développement. Ces règles sont nombreuses et visent non seulement à éliminer les erreurs graves, mais aussi à diverses "odeurs de code". Il vise également à écrire le code le plus compréhensible et lisible.
Ainsi, la norme MISRA suivante permet non seulement d'éviter les erreurs et les vulnérabilités, mais également de réduire considérablement la probabilité de leur apparition dans le code déjà existant.
MISRA est utilisé dans les industries aérospatiale, médicale, automobile et militaire, où la vie humaine dépend de la qualité des logiciels embarqués.
Apparemment, les développeurs d'Amazon FreeRTOS connaissent cette norme et la suivent pour la plupart. Une telle approche est absolument raisonnable: si vous écrivez un système d'exploitation étendu pour les systèmes embarqués, vous devez alors penser à la sécurité.
Cependant, j'ai trouvé pas mal de violations de la norme MISRA. Je ne vais pas donner d'exemples de règles comme "ne pas utiliser l'union" ou "la fonction ne devrait avoir qu'un seul retour à la fin du corps" - malheureusement, elles ne sont pas spectaculaires, comme le sont la plupart des règles MISRA. Je préfère vous donner des exemples de violations qui pourraient potentiellement entraîner de graves conséquences.
Commençons par les 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 ) ) )
Avertissements de PVS-Studio:- V2546 [MISRA C 20.7] La macro et ses paramètres doivent être placés entre parenthèses. Pensez à inspecter le paramètre «ms» de la macro «FreeRTOS_ms_to_tick». FreeRTOS_IP.h 201
- V2546 [MISRA C 20.7] La macro et ses paramètres doivent être placés entre parenthèses. Pensez à inspecter le paramètre 'ulIn' de la macro 'SOCKETS_htonl'. iot_secure_sockets.h 512
- V2546 [MISRA C 20.7] La macro et ses paramètres doivent être placés entre parenthèses. Pensez à inspecter les paramètres «x», «c» de la macro «LEFT_ROTATE». iot_device_metrics.c 90
Oui, c'est exactement ce que tu penses. Les paramètres de ces macros ne sont pas entre crochets. Si quelqu'un écrit accidentellement quelque chose comme
val = LEFT_ROTATE(A[i] | 1, B);
un tel "appel" d'une macro se développera en:
val = ( ( A[i] | 1 << B ) | ( A[i] | 1 >> ( 32 - B ) ) );
Rappelez-vous les priorités des opérations? Tout d'abord, un décalage au niveau du bit est effectué, et seulement après - un "ou" au niveau du bit. Par conséquent, la logique du programme sera rompue. Un exemple plus simple: que se passerait-il si l'expression "
x + y " était passée dans la macro
FreeRTOS_ms_to_tick ? L'un des principaux objectifs de MISRA est de prévenir de telles situations.
Certains pourraient dire: "Si vous avez des programmeurs qui ne connaissent pas cela, aucune norme ne peut vous aider!" Je ne suis pas d'accord avec ça. Les programmeurs sont aussi des gens, et peu importe l'expérience d'une personne, ils peuvent aussi se fatiguer et faire une erreur à la fin de la journée. C'est l'une des raisons pour lesquelles MISRA recommande fortement d'utiliser des outils d'analyse automatique pour tester la conformité d'un projet.
Permettez-moi de m'adresser aux développeurs d'Amazon FreeRTOS: PVS-Studio a trouvé 12 autres macros dangereuses, vous devez donc faire attention avec elles :)
Une autre violation intéressante de la 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)); }
Pouvez-vous trouver le bug vous-même?
Avertissement PVS-Studio: V2537 [MISRA C 2.7] Les fonctions ne doivent pas avoir de paramètres inutilisés. Pensez à inspecter le paramètre: «rc». iot_demo_https_s3_upload_async.c 234
Regardez de plus près: le paramètre
rc n'est utilisé nulle part dans le corps de la fonction. Alors que le commentaire de la fonction indique clairement que ce paramètre est un code retour d'une autre fonction, et qu'il peut signaler une erreur. Pourquoi ce paramètre n'est-il géré d'aucune façon? Quelque chose cloche clairement ici.
Cependant, même sans de tels commentaires, les paramètres inutilisés indiquent souvent la logique brisée du programme. Sinon, pourquoi en avez-vous besoin dans la signature de fonction?
Ici, j'ai donné une petite fonction qui est bonne pour un exemple dans l'article. En plus de cela, j'ai trouvé 10 autres paramètres inutilisés. Beaucoup d'entre eux sont utilisés dans des fonctions plus importantes et il n'est pas facile de les détecter.
Curieusement, ils n'ont jamais été trouvés auparavant. Après tout, les compilateurs détectent facilement de tels cas.
Conclusion
Ce ne sont pas tous les problèmes trouvés par l'analyseur, mais l'article s'est déjà révélé assez volumineux. J'espère que grâce à cela, les développeurs d'Amazon FreeRTOS seront en mesure de corriger certaines lacunes et pourraient même vouloir
essayer PVS-Studio par eux-mêmes. De cette façon, il sera plus pratique d'enquêter de manière approfondie sur les avertissements. Et en fait - travailler avec une interface pratique est beaucoup plus facile que de regarder un rapport texte.
Merci d'avoir lu nos articles! Rendez-vous dans la prochaine publication: D
PS Il se trouve que cet article a été publié le 31 octobre. Joyeux Halloween, les gars et les filles!