Clever Geek Handbook

Vérification de QEMU avec PVS-Studio

image1.png


QEMU est une application d'émulation assez connue. L'analyse statique peut aider les développeurs de projets complexes comme QEMU à détecter les erreurs à un stade précoce et à améliorer généralement sa qualité et sa fiabilité. Dans cet article, le code source de l'application QEMU sera vérifié pour les vulnérabilités et erreurs potentielles à l'aide de l'outil d'analyse statique PVS-Studio.



QEMU est un logiciel gratuit conçu pour émuler du matériel sur plusieurs plates-formes. Il vous permet d'exécuter des applications et des systèmes d'exploitation sur des plates-formes matérielles différentes de la cible, par exemple, une application écrite pour MIPS à exécuter pour l'architecture x86. QEMU prend également en charge l'émulation d'une variété de périphériques tels que les cartes vidéo, USB, etc. Le projet est assez complexe et digne d'attention, ce sont ces projets qui présentent un intérêt pour l'analyse statique, il a donc été décidé de vérifier son code à l'aide de PVS-Studio.



À propos de l'analyse



Le code source du projet peut être obtenu à partir d'un miroir sur github . Le projet est assez vaste et peut être compilé pour diverses plates-formes. Pour faciliter la vérification du code, nous utiliserons le système de surveillance de compilation PVS-Studio . Ce système est conçu pour une intégration très facile de l'analyse statique dans presque toutes les plates-formes de construction. Le système est basé sur le suivi des appels du compilateur pendant la construction et vous permet de collecter toutes les informations pour l'analyse ultérieure des fichiers. En d'autres termes, nous commençons simplement la construction, PVS-Studio collecte les informations nécessaires, puis nous commençons l'analyse - tout est simple. Les détails peuvent être trouvés sur le lien ci-dessus.



Après vérification, l'analyseur a détecté de nombreux problèmes potentiels. Pour les diagnostics à usage général (analyse générale), il a été obtenu: 1940 élevé, 1996 moyen, 9596 faible. Après avoir examiné tous les avertissements, il a été décidé de se concentrer sur les diagnostics pour le premier niveau de confiance (élevé). Un grand nombre de ces avertissements ont été trouvés (1940), mais la plupart des avertissements sont soit du même type, soit associés à l'utilisation répétée d'une macro suspecte. Par exemple, considérons la macro g_new . 



#define g_new(struct_type, n_structs)
                        _G_NEW (struct_type, n_structs, malloc)

#define _G_NEW(struct_type, n_structs, func)       \
  (struct_type *) (G_GNUC_EXTENSION ({             \
    gsize __n = (gsize) (n_structs);               \
    gsize __s = sizeof (struct_type);              \
    gpointer __p;                                  \
    if (__s == 1)                                  \
      __p = g_##func (__n);                        \
    else if (__builtin_constant_p (__n) &&         \
             (__s == 0 || __n <= G_MAXSIZE / __s)) \
      __p = g_##func (__n * __s);                  \
    else                                           \
      __p = g_##func##_n (__n, __s);               \
    __p;                                           \
  }))


Pour chaque utilisation de cette macro, l'analyseur émet l'avertissement V773 (la portée de visibilité du pointeur '__p' a été fermée sans libérer la mémoire. Une fuite de mémoire est possible). La macro g_new est définie dans la bibliothèque glib , elle utilise la macro _G_NEW , et cette macro à son tour utilise une autre macro, G_GNUC_EXTENSION , qui indique au compilateur GCC d'ignorer les avertissements concernant le code non standard. C'est ce code non standard qui provoque l'avertissement de l'analyseur, faites attention à l'avant-dernière ligne. En général, la macro fonctionne. Il y a eu 848 avertissements de ce type, c'est-à-dire que près de la moitié des alertes se produisent à un seul endroit du code.



Tous ces avertissements inutiles sont faciles à supprimeren utilisant les paramètres de l'analyseur. Cependant, ce cas particulier, que nous avons rencontré lors de la rédaction de cet article, est la raison pour laquelle notre équipe a légèrement modifié la logique de l'analyseur pour de telles situations.



Ainsi, un grand nombre d'avertissements ne signifie pas toujours une mauvaise qualité de code. Cependant, il existe des endroits vraiment suspects. Eh bien, passons aux avertissements.



Avertissement N1



V517 L'utilisation du modèle 'if (A) {...} else if (A) {...}' a été détectée. Il existe une probabilité de présence d'erreur logique. Vérifiez les lignes: 2395, 2397. megasas.c 2395 



#define MEGASAS_MAX_SGE 128             /* Firmware limit */
....
static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
{
  ....
  if (s->fw_sge >= MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE) {
    ....
  } else if (s->fw_sge >= 128 - MFI_PASS_FRAME_SIZE) {
    ....
  }
  ....
}


Toute utilisation de nombres «magiques» dans le code est toujours suspecte. Il y a deux conditions ici, et à première vue, elles semblent différentes, mais si vous regardez la valeur de la macro MEGASAS_MAX_SGE , il s'avère que les conditions se dupliquent. Très probablement, il y a une faute de frappe ici et au lieu de 128, il devrait y avoir un autre nombre. Bien sûr, c'est le problème de tous les nombres "magiques", il suffit de les sceller lors de leur utilisation. L'utilisation de macros et de constantes aide grandement le développeur dans ce cas.



Avertissement N2



V523 L' instruction «then» équivaut à l'instruction «else». cp0_helper.c 383 



target_ulong helper_mftc0_cause(CPUMIPSState *env)
{
  ....
  CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc);

  if (other_tc == other->current_tc) {
    tccause = other->CP0_Cause;
  } else {
    tccause = other->CP0_Cause;
  }
  ....
}


Dans le code considéré, les corps then et else de l' opérateur conditionnel sont identiques. Ici, probablement copier-coller. Il suffit de copier le corps puis la branche , et fixe oublié. On peut supposer que env aurait dû être utilisé à la place de l' autre objet . Un correctif pour cet endroit suspect pourrait ressembler à ceci: 



if (other_tc == other->current_tc) {
  tccause = other->CP0_Cause;
} else {
  tccause = env->CP0_Cause;
}


Seuls les développeurs de ce code peuvent dire sans équivoque comment il devrait réellement être. Un autre endroit similaire:



  • V523 L'instruction 'then' est équivalente à l'instruction 'else'. translate.c 641


Avertissement N3



V547 L' expression «ret <0» est toujours fausse. qcow2-cluster.c 1557 



static int handle_dependencies(....)
{
  ....
  if (end <= old_start || start >= old_end) {
    ....
  } else {

    if (bytes == 0 && *m) {
      ....
      return 0;           // <= 3
    }

    if (bytes == 0) {
      ....
      return -EAGAIN;     // <= 4
    }
  ....
  }
  return 0;               // <= 5
}

int qcow2_alloc_cluster_offset(BlockDriverState *bs, ....)
{
  ....
  ret = handle_dependencies(bs, start, &cur_bytes, m);
  if (ret == -EAGAIN) {   // <= 2
    ....
  } else if (ret < 0) {   // <= 1
    ....
  }
}


Ici, l'analyseur a constaté que la condition (commentaire 1) ne serait jamais remplie. La valeur de la variable ret est initialisée avec le résultat de l'exécution de la fonction handle_dependencies , cette fonction ne renvoie que 0 ou -EAGAIN (commentaires 3, 4, 5). Un peu plus haut, dans la première condition, nous avons vérifié la valeur de ret par rapport à -EAGAIN (commentaire 2), donc le résultat de l'expression ret <0 sera toujours faux. Peut-être que la fonction handle_dependencies servait à renvoyer des valeurs différentes, mais plus tard, à la suite, par exemple, du refactoring, le comportement a changé. Ici, il vous suffit de terminer le refactoring. Déclencheurs similaires:



  • L'expression V547 est toujours fausse. qcow2.c 1070
  • V547 Expression 's-> state! = MIGRATION_STATUS_COLO' est toujours faux. colo.c 595
  • L'expression V547 's-> metadata_entries.present & 0x20' est toujours fausse. vhdx.c 769


Avertissement Un dépassement de la



baie N4 V557 est possible. La fonction 'dwc2_glbreg_read' traite la valeur '[0..63]'. Inspectez le troisième argument. Vérifier les lignes: 667, 1040.hcd-dwc2.c 667 



#define HSOTG_REG(x) (x)                                             // <= 5
....
struct DWC2State {
  ....
#define DWC2_GLBREG_SIZE    0x70
  uint32_t glbreg[DWC2_GLBREG_SIZE / sizeof(uint32_t)];              // <= 1
  ....
}
....
static uint64_t dwc2_glbreg_read(void *ptr, hwaddr addr, int index,
                                 unsigned size)
{
  ....
  val = s->glbreg[index];                                            // <= 2
  ....
}
static uint64_t dwc2_hsotg_read(void *ptr, hwaddr addr, unsigned size)
{
  ....
  switch (addr) {
    case HSOTG_REG(0x000) ... HSOTG_REG(0x0fc):                      // <= 4
        val = dwc2_glbreg_read(ptr, addr,
                              (addr - HSOTG_REG(0x000)) >> 2, size); // <= 3
    ....
  }
  ....
}


Il existe un problème potentiel avec les dépassements de tableau dans ce code. Dans la structure DWC2State défini le tableau glbreg , composé de 28 éléments (commentaire 1). Dans la fonction dwc2_glbreg_read , nous nous référons à notre tableau par index (commentaire 2). Maintenant, notez que l' expression ( addr - HSOTG_REG (0x000)) >> 2 (commentaire 3) est passée en tant qu'index à la fonction dwc2_glbreg_read , qui peut prendre une valeur dans l'intervalle [0..63]. Pour être convaincu de cela, faites attention aux commentaires 4 et 5. Peut-être, ici, il est nécessaire d'ajuster la plage de valeurs du commentaire 4. Déclencheurs plus similaires:







  • Un dépassement de la baie V557 est possible. La fonction 'dwc2_hreg0_read' traite la valeur '[0..63]'. Inspectez le troisième argument. Vérifier les lignes: 814, 1050.hcd-dwc2.c 814
  • Un dépassement de la baie V557 est possible. La fonction 'dwc2_hreg1_read' traite la valeur '[0..191]'. Inspectez le troisième argument. Vérifier les lignes: 927, 1053.hcd-dwc2.c 927
  • Un dépassement de la baie V557 est possible. La fonction 'dwc2_pcgreg_read' traite la valeur '[0..127]'. Inspectez le troisième argument. Vérifier les lignes: 1012, 1060.hcd-dwc2.c 1012


Avertissement N5



V575 La fonction 'strerror_s' traite les éléments '0'. Inspectez le deuxième argument. commandes-win32.c 1642 



void qmp_guest_set_time(bool has_time, int64_t time_ns, 
                        Error **errp)
{
  ....
  if (GetLastError() != 0) {
    strerror_s((LPTSTR) & msg_buffer, 0, errno);
    ....
  }
}


La fonction strerror_s renvoie une description textuelle du code d'erreur système. Sa signature ressemble à ceci: 



errno_t strerror_s( char *buf, rsize_t bufsz, errno_t errnum );


Le premier paramètre est un pointeur vers le tampon où la description textuelle sera copiée, le deuxième paramètre est la taille du tampon, le troisième est le code d'erreur. Dans le code, 0 est passé comme taille du tampon, c'est clairement une valeur erronée. D'ailleurs, il est possible de savoir à l'avance combien d'octets doivent être alloués: il suffit d'appeler strerrorlen_s , qui renvoie la longueur de la description textuelle de l'erreur. Cette valeur peut être utilisée pour allouer un tampon de taille suffisante.



Avertissement N6



V595 Le pointeur 'blen2p' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifier les lignes: 103, 106.dsound_template.h 103 



static int glue (
    ....
    DWORD *blen1p,
    DWORD *blen2p,
    int entire,
    dsound *s
    )
{
  ....
  dolog("DirectSound returned misaligned buffer %ld %ld\n",
        *blen1p, *blen2p);                         // <= 1
  glue(.... p2p ? *p2p : NULL, *blen1p,
                            blen2p ? *blen2p : 0); // <= 2
....
}


Dans ce code, la valeur de l'argument blen2p est d' abord utilisée (commentaire 1) puis vérifiée pour nullptr (commentaire 2). Cet endroit très suspect semble que vous venez d'oublier d'insérer le chèque avant la première utilisation (commentaire 1). Comme correctif, ajoutez simplement un chèque: 



dolog("DirectSound returned misaligned buffer %ld %ld\n",
      *blen1p, blen2p ? *blen2p : 0);


Il y a encore une question sur l'argument blen1p . Probablement, il peut également s'agir d'un pointeur nul, et ici vous devrez également ajouter une vérification. Plusieurs autres aspects positifs similaires:



  • V595 Le pointeur 'ref' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifier les lignes: 2191, 2193.uri.c 2191
  • V595 Le pointeur 'cmdline' a été utilisé avant d'être vérifié par rapport à nullptr. Lignes de contrôle: 420, 425.qemu-io.c 420
  • V595 Le pointeur 'dp' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 288, 294. onenand.c 288
  • V595 Le pointeur 'omap_lcd' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 81, 87. omap_lcdc.c 81


Avertissement N7



V597 Le compilateur pourrait supprimer l'appel de fonction 'memset', qui est utilisé pour vider l'objet 'op_info'. La fonction RtlSecureZeroMemory () doit être utilisée pour effacer les données privées. virtio-crypto.c 354 



static void virtio_crypto_free_request(VirtIOCryptoReq *req)
{
  if (req) {
    if (req->flags == CRYPTODEV_BACKEND_ALG_SYM) {
      ....
      /* Zeroize and free request data structure */
      memset(op_info, 0, sizeof(*op_info) + max_len); // <= 1
      g_free(op_info);
    }
    g_free(req);
  }
}


Dans ce fragment de code, la fonction memset est appelée pour l'objet op_info (commentaire 1), après quoi l' op_info est immédiatement supprimé, c'est-à-dire qu'après le nettoyage, cet objet n'est modifié nulle part ailleurs. C'est exactement le cas lorsque le compilateur peut supprimer l'appel memset pendant le processus d'optimisation . Pour éliminer ce comportement potentiel, vous pouvez utiliser des fonctions spéciales que le compilateur ne supprime jamais. Voir également l'article « Effacer les données privées en toute sécurité ».



Avertissement N8



V610 Comportement non spécifié. Vérifiez l'opérateur de décalage «>>». L'opérande de gauche est négatif ('nombre' = [-32768..2147483647]). cris.c 2111 



static void
print_with_operands (const struct cris_opcode *opcodep,
         unsigned int insn,
         unsigned char *buffer,
         bfd_vma addr,
         disassemble_info *info,
         const struct cris_opcode *prefix_opcodep,
         unsigned int prefix_insn,
         unsigned char *prefix_buffer,
         bfd_boolean with_reg_prefix)
{
  ....
  int32_t number;
  ....
  if (signedp && number > 127)
    number -= 256;            // <= 1
  ....
  if (signedp && number > 32767)
    number -= 65536;          // <= 2
  ....
  unsigned int highbyte = (number >> 24) & 0xff;
  ....
}


Étant donné que le nombre de variable peut être négatif, le décalage à droite au niveau du bit est un comportement non spécifié. Pour vous assurer que la variable en question peut prendre une valeur négative, regardez les commentaires 1 et 2. Pour éviter les différences de comportement de votre code sur différentes plates-formes, de tels cas doivent être évités.



Plus d'avertissements:



  • V610 Comportement indéfini. Vérifiez l'opérateur d'équipe «<<». L'opérande de gauche est négatif ('(hclk_div - 1)' = [-1..15]). aspeed_smc.c 1041
  • V610 Comportement indéfini. Vérifiez l'opérateur d'équipe «<<». L'opérande gauche '(target_long) - 1' est négatif. exec-varie.c 99
  • V610 Comportement indéfini. Vérifiez l'opérateur d'équipe «<<». L'opérande de gauche est négatif ('hex2nib (words [3] [i * 2 + 2])' = [-1..15]). qtest.c 561


Il existe également plusieurs avertissements du même type, seul -1 est utilisé comme opérande de gauche .



V610 Comportement indéfini . Vérifiez l'opérateur d'équipe «<<». L'opérande gauche '-1' est négatif. hppa.c 2702 



int print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
{
  ....
  disp = (-1 << 10) | imm10;
  ....
}


Autres avertissements similaires:



  • V610 Comportement indéfini. Vérifiez l'opérateur d'équipe «<<». L'opérande gauche '-1' est négatif. hppa.c 2718
  • V610 Comportement indéfini. Vérifiez l'opérateur d'équipe «<<». L'opérande gauche '-0x8000' est négatif. fmopl.c 1022
  • V610 Comportement indéfini. Vérifiez l'opérateur d'équipe «<<». L'opérande de gauche '(intptr_t) - 1' est négatif. sve_helper.c 889


Avertissement N9



V616 La constante nommée 'TIMER_NONE' avec la valeur 0 est utilisée dans l'opération au niveau du bit. sys_helper.c 179 



#define HELPER(name) ....

enum {
  TIMER_NONE = (0 << 30),        // <= 1
  ....
}

void HELPER(mtspr)(CPUOpenRISCState *env, ....)
{
  ....
  if (env->ttmr & TIMER_NONE) {  // <= 2
    ....
  }
}


Vous pouvez facilement vérifier que la valeur de la macro TIMER_NONE est zéro (commentaire 1). De plus, cette macro est utilisée dans une opération au niveau du bit, dont le résultat sera toujours 0. En conséquence, le corps de l'instruction conditionnelle if (env-> ttmr & TIMER_NONE) ne sera jamais exécuté.



Avertissement N10



V629 Pensez à inspecter l'expression 'n << 9'. Décalage de bits de la valeur 32 bits avec une extension ultérieure vers le type 64 bits. qemu-img.c 1839 



#define BDRV_SECTOR_BITS   9
static int coroutine_fn convert_co_read(ImgConvertState *s, 
                  int64_t sector_num, int nb_sectors, uint8_t *buf)
{
  uint64_t single_read_until = 0;
  int n;
  ....
  while (nb_sectors > 0) {
    ....
    uint64_t offset;
    ....
    single_read_until = offset + (n << BDRV_SECTOR_BITS);
    ....
  }
  ....
}


Dans ce fragment de code , une opération de décalage est effectuée sur la variable n , qui a un type signé 32 bits, puis ce résultat signé 32 bits est étendu à un type signé 64 bits, puis, en tant que type non signé, est ajouté au décalage de variable 64 bits non signé . Supposons qu'au moment où l'expression est exécutée, la variable n a quelques 9 bits les plus significatifs. Nous effectuons une opération de décalage de 9 bits ( BDRV_SECTOR_BITS), et cela, à son tour, est un comportement indéfini, alors nous pouvons obtenir le bit défini dans le bit le plus significatif. Rappelons que ce bit du type signé est responsable du signe, c'est-à-dire que le résultat peut devenir négatif. Puisque n est une variable signée, le signe sera pris en compte lors du développement. Le résultat est ensuite ajouté à la variable de décalage . À partir de ces considérations, il est facile de voir que le résultat de l'exécution de l'expression peut différer de celui prévu. Une des solutions possibles est de remplacer le type de la variable n par un type non signé 64 bits, c'est-à-dire par uint64_t .



Voici quelques déclencheurs plus similaires:



  • V629 Consider inspecting the '1 << refcount_order' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. qcow2.c 3204
  • V629 Consider inspecting the 's->cluster_size << 3' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. qcow2-bitmap.c 283
  • V629 Consider inspecting the 'i << s->cluster_bits' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. qcow2-cluster.c 983
  • V629 Consider inspecting the expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. vhdx.c 1145
  • V629 Consider inspecting the 'delta << 2' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. mips.c 4341


Avertissement N11



V634 La priorité de l'opération '*' est supérieure à celle de l'opération '<<'. Il est possible que des parenthèses soient utilisées dans l'expression. nand.c 310 



static void nand_command(NANDFlashState *s)
{
  ....
  s->addr &= (1ull << s->addrlen * 8) - 1;
  ....
}


C'est juste un endroit suspect. Ce que le programmeur voulait faire au début n'est pas clair: décalage ou multiplication. Même s'il n'y a pas d'erreur ici, vous devez toujours revoir le code et placer les crochets correctement. Ce n'est qu'un de ces endroits que les développeurs devraient examiner pour s'assurer que leur algorithme est correct. D'autres tels endroits:



  • V634 La priorité de l'opération '*' est supérieure à celle de l'opération '<<'. Il est possible que des parenthèses soient utilisées dans l'expression. exynos4210_mct.c 449
  • V634 La priorité de l'opération '*' est supérieure à celle de l'opération '<<'. Il est possible que des parenthèses soient utilisées dans l'expression. exynos4210_mct.c 1235
  • V634 La priorité de l'opération '*' est supérieure à celle de l'opération '<<'. Il est possible que des parenthèses soient utilisées dans l'expression. exynos4210_mct.c 1264


Avertissement N12



V646 Pensez à inspecter la logique de l'application. Il est possible que le mot-clé «else» soit manquant. pl181.c 400 



static void pl181_write(void *opaque, hwaddr offset,
                        uint64_t value, unsigned size)
{
  ....
  if (s->cmd & PL181_CMD_ENABLE) {
    if (s->cmd & PL181_CMD_INTERRUPT) {
      ....
    } if (s->cmd & PL181_CMD_PENDING) { // <= else if
      ....
    } else {
      ....
    }
    ....
  }
  ....
}


Dans ce code, à en juger par le formatage, l'utilisation de else if au lieu de if se suggère directement . Peut-être ont-ils oublié d'ajouter autre chose ici . Ensuite, l'option de correction pourrait être comme ceci: 



} else if (s->cmd & PL181_CMD_PENDING) { // <= else if


Cependant, il est possible que tout soit en ordre avec ce code et qu'il y ait un formatage incorrect du texte du programme, ce qui est déroutant. Ensuite, le code pourrait ressembler à ceci: 



if (s->cmd & PL181_CMD_INTERRUPT) {
  ....
}
if (s->cmd & PL181_CMD_PENDING) { // <= if
  ....
} else {
  ....
}


Avertissement N13



V773 La fonction a été quittée sans relâcher le pointeur «règle». Une fuite de mémoire est possible. blkdebug.c 218 



static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
{
  ....
  struct BlkdebugRule *rule;
  ....
  rule = g_malloc0(sizeof(*rule));                   // <= 1
  ....
  if (local_error) {
    error_propagate(errp, local_error);
    return -1;                                       // <= 2
  }
  ....
  /* Add the rule */
  QLIST_INSERT_HEAD(&s->rules[event], rule, next);   // <= 3
  ....
}


Dans ce code, l'objet règle (commentaire 1) est sélectionné et ajouté à la liste pour une utilisation ultérieure (commentaire 3), mais en cas d'erreur, la fonction retourne sans supprimer l'objet règle précédemment créé (commentaire 2). Ici, il vous suffit de gérer correctement l'erreur: supprimez l'objet précédemment créé, sinon il y aura une fuite de mémoire.



Avertissement N14



V781 La valeur de l'index 'ix' est vérifiée après son utilisation. Il y a peut-être une erreur dans la logique du programme. uri.c 2110 



char *uri_resolve_relative(const char *uri, const char *base)
{
  ....
  ix = pos;
  if ((ref->path[ix] == '/') && (ix > 0)) {
  ....
}


Ici, l'analyseur a détecté un potentiel hors limites. Tout d'abord, l'élément du tableau ref-> path à l'index ix est lu , puis ix est vérifié pour l'exactitude ( ix> 0 ). La bonne solution ici est d'inverser ces actions: 



if ((ix > 0) && (ref->path[ix] == '/')) {


Il y avait plusieurs de ces endroits:



  • V781 La valeur de l'index 'ix' est vérifiée après son utilisation. Il y a peut-être une erreur dans la logique du programme. uri.c 2112
  • V781 La valeur de l'index 'offset' est vérifiée après son utilisation. Il y a peut-être une erreur dans la logique du programme. keymaps.c 125
  • V781 La valeur de la variable 'qualité' est vérifiée après son utilisation. Il y a peut-être une erreur dans la logique du programme. Vérifier les lignes: 326, 335.vnc-enc-tight.c 326
  • V781 La valeur de l'index 'i' est vérifiée après son utilisation. Il y a peut-être une erreur dans la logique du programme. mem_helper.c 1929


Avertissement N15



V784 La taille du masque de bits est inférieure à la taille du premier opérande. Cela entraînera la perte de bits plus élevés. cadence_gem.c 1486 



typedef struct CadenceGEMState {
  ....
  uint32_t regs_ro[CADENCE_GEM_MAXREG];
}
....
static void gem_write(void *opaque, hwaddr offset, uint64_t val,
        unsigned size)
{
  ....
  val &= ~(s->regs_ro[offset]);
  ....
}


Ce code effectue une opération au niveau du bit sur des objets de différents types. L'opérande de gauche est l'argument val , qui est un type non signé 64 bits. La valeur reçue de l'élément de tableau s-> regs_ro au niveau de l'index de décalage , qui a un type non signé de 32 bits, est utilisée comme opérande de droite . Le résultat de l'opération sur le côté droit (~ (s-> regs_ro [offset])) est un type 32 bits non signé, et avant la multiplication au niveau du bit, il sera étendu au type 64 bits avec des zéros, c'est-à-dire qu'après le calcul de l'expression entière, tous les bits de poids fort de la variable val seront remis à zéro . De tels endroits ont toujours l'air suspect. Ici, nous ne pouvons que recommander aux développeurs de réviser à nouveau ce code. Plus similaire:



  • V784 La taille du masque de bits est inférieure à la taille du premier opérande. Cela entraînera la perte de bits plus élevés. xlnx-zynq-devcfg.c 199
  • V784 La taille du masque de bits est inférieure à la taille du premier opérande. Cela entraînera la perte de bits plus élevés. soc_dma.c 214
  • V784 La taille du masque de bits est inférieure à la taille du premier opérande. Cela entraînera la perte de bits plus élevés. fpu_helper.c 418


Avertissement N16



V1046 Utilisation non sécurisée des types 'bool' et 'unsigned int' ensemble dans l'opération '& ='. helper.c 10821 



static inline uint32_t extract32(uint32_t value, int start, int length);
....
static ARMVAParameters aa32_va_parameters(CPUARMState *env, uint32_t va,
                                          ARMMMUIdx mmu_idx)
{
  ....
  bool epd, hpd;
  ....
  hpd &= extract32(tcr, 6, 1);
}


Dans ce fragment de code, une opération AND au niveau du bit est effectuée sur la variable hpd de type bool et le résultat de l'exécution de la fonction extract32 , qui a le type uint32_t . Étant donné que la valeur de bit d'une variable booléenne ne peut être que 0 ou 1, le résultat de l'expression sera toujours faux si le bit le moins significatif renvoyé par la fonction extract32 est zéro. Regardons cela avec un exemple. Supposons que hpd soit vrai et que la fonction renvoie 2, c'est-à-dire qu'en représentation binaire, l'opération ressemblera à 01 & 10 = 0, et le résultat de l'expression sera faux . Très probablement, le programmeur voulait définir la valeur sur truesi la fonction renvoie quelque chose de différent de zéro. Apparemment, le code doit être corrigé pour que le résultat de la fonction soit converti en type booléen , par exemple, comme ceci: 



hpd = hpd && (bool)extract32(tcr, 6, 1);


Conclusion



Comme vous pouvez le voir, l'analyseur a trouvé de nombreux endroits suspects. Peut-être que les problèmes potentiels rencontrés ne se sont pas encore manifestés, mais leur présence ne peut qu'être alarmante, car ils sont capables de tirer au moment le plus inattendu. Il vaut mieux voir tous les endroits suspects à l'avance et les corriger que de corriger un flot infini de bogues. Évidemment, pour des projets complexes comme celui-ci, l'analyse statique peut apporter des avantages tangibles, surtout si vous organisez une revue de projet régulière. Si vous souhaitez essayer PVS-Studio pour votre projet, vous pouvez télécharger l'analyseur et obtenir une clé d'essai gratuite sur cette page.





Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien de traduction: Evgeniy Ovsannikov. Vérification de QEMU à l'aide de PVS-Studio .


More articles:


All Articles