Clever Geek Handbook

Memeriksa QEMU dengan PVS-Studio

gambar1.png


QEMU adalah aplikasi emulasi yang cukup terkenal. Analisis statis dapat membantu pengembang proyek kompleks seperti QEMU menangkap kesalahan pada tahap awal dan secara umum meningkatkan kualitas dan keandalannya. Dalam artikel ini, kode sumber aplikasi QEMU akan diperiksa untuk potensi kerentanan dan kesalahan menggunakan alat analisis statis PVS-Studio.



QEMU adalah perangkat lunak gratis yang dirancang untuk meniru perangkat keras lintas platform. Ini memungkinkan Anda menjalankan aplikasi dan sistem operasi pada platform perangkat keras selain target, misalnya, aplikasi yang ditulis untuk MIPS agar dijalankan untuk arsitektur x86. QEMU juga mendukung emulasi berbagai periferal seperti kartu video, usb, dll. Proyek ini cukup kompleks dan layak untuk diperhatikan, ini adalah proyek yang menarik untuk analisis statis, jadi diputuskan untuk memeriksa kodenya menggunakan PVS-Studio.



Tentang analisis



Kode sumber proyek dapat diperoleh dari mirror di github . Proyek ini cukup besar dan dapat dikompilasi untuk berbagai platform. Untuk pemeriksaan kode yang lebih mudah, kami akan menggunakan sistem pemantauan kompilasi PVS-Studio . Sistem ini dirancang untuk integrasi analisis statis yang sangat mudah ke hampir semua platform build. Sistem ini didasarkan pada pelacakan panggilan compiler selama pembuatan dan memungkinkan Anda mengumpulkan semua informasi untuk analisis file selanjutnya. Dengan kata lain, kami baru saja memulai pembuatan, PVS-Studio mengumpulkan informasi yang diperlukan, dan kemudian kami memulai analisis - semuanya sederhana. Detail dapat ditemukan di tautan di atas.



Setelah diperiksa, penganalisis menemukan banyak potensi masalah. Untuk diagnostik tujuan umum (Analisis Umum) diperoleh: 1940 Tinggi, 1996 Sedang, 9596 Rendah. Setelah meninjau semua peringatan, diputuskan untuk fokus pada diagnostik untuk tingkat kepercayaan pertama (Tinggi). Cukup banyak peringatan seperti itu yang ditemukan (1940), tetapi sebagian besar peringatan memiliki tipe yang sama atau terkait dengan penggunaan berulang makro yang mencurigakan. Misalnya, pertimbangkan makro 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;                                           \
  }))


Untuk setiap penggunaan makro ini, penganalisis mengeluarkan peringatan V773 (Lingkup visibilitas penunjuk '__p' telah keluar tanpa melepaskan memori. Kebocoran memori mungkin terjadi). Makro g_new ditentukan di pustaka glib, makro ini menggunakan _G_NEW , dan makro ini selanjutnya menggunakan makro lain, G_GNUC_EXTENSION , untuk memberi tahu compiler GCC agar melewati peringatan tentang kode non-standar. Kode non-standar inilah yang menyebabkan peringatan penganalisis, perhatikan baris kedua dari belakang. Secara umum, makro berfungsi. Ada 848 peringatan jenis ini, artinya, hampir setengah dari peringatan terjadi hanya di satu tempat dalam kode.



Semua peringatan yang tidak perlu ini mudah dihapusmenggunakan pengaturan penganalisis. Namun, kasus khusus ini, yang kami temui saat menulis artikel ini, adalah alasan bagi tim kami untuk sedikit mengubah logika penganalisis untuk situasi seperti itu.



Jadi, sejumlah besar peringatan tidak selalu berarti kualitas kode yang buruk. Namun, ada beberapa tempat yang sangat mencurigakan. Nah, mari kita turun ke peringatannya.



Peringatan N1



V517 Penggunaan pola 'if (A) {...} else if (A) {...}' terdeteksi. Ada kemungkinan kehadiran kesalahan logis. Periksa baris: 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) {
    ....
  }
  ....
}


Setiap penggunaan nomor "ajaib" dalam kode selalu mencurigakan. Ada dua ketentuan di sini, dan sekilas terlihat berbeda, namun jika Anda melihat nilai makro MEGASAS_MAX_SGE , ternyata ketentuan tersebut saling duplikat. Kemungkinan besar, ada kesalahan ketik di sini dan alih-alih 128, seharusnya ada nomor lain. Tentu saja, ini adalah masalah dari semua nomor "ajaib", cukup disegel saja saat menggunakannya. Penggunaan macro dan konstanta sangat membantu developer dalam hal ini.



Peringatan N2



V523 Pernyataan 'kemudian' sama dengan pernyataan '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;
  }
  ....
}


Dalam kode yang dipertimbangkan, badan operator bersyarat lalu dan lainnya identik. Di sini, kemungkinan besar salin-tempel. Cukup salin badannya lalu cabangnya , dan perbaiki dilupakan. Dapat diasumsikan bahwa env seharusnya digunakan sebagai pengganti objek lainnya . Perbaikan untuk tempat yang mencurigakan ini mungkin terlihat seperti ini: 



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


Hanya pengembang kode ini yang dapat dengan tegas mengatakan bagaimana sebenarnya seharusnya. Tempat serupa lainnya:



  • V523 Pernyataan 'then' setara dengan pernyataan 'else'. translate.c 641


Peringatan N3



V547 Ekspresi 'ret <0' selalu salah. 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
    ....
  }
}


Di sini penganalisis menemukan bahwa kondisi (komentar 1) tidak akan pernah terpenuhi. Nilai dari variabel ret diinisialisasi oleh hasil dari menjalankan fungsi handle_dependencies , fungsi ini hanya mengembalikan 0 atau -EAGAIN (komentar 3, 4, 5). Sedikit lebih tinggi, pada kondisi pertama, kita memeriksa nilai ret terhadap -EAGAIN (comment 2), sehingga hasil ekspresi ret <0 akan selalu salah. Mungkin fungsi handle_dependencies digunakan untuk mengembalikan nilai yang berbeda, tetapi kemudian, sebagai hasil dari, misalnya, pemfaktoran ulang, perilaku berubah. Di sini Anda hanya perlu menyelesaikan pemfaktoran ulang. Pemicu serupa:



  • Ekspresi V547 selalu salah. qcow2.c 1070
  • V547 Expression 's-> state! = MIGRATION_STATUS_COLO' selalu salah. colo.c 595
  • V547 Expression 's-> metadata_entries.present & 0x20' selalu salah. vhdx.c 769


Peringatan N4



V557 Array overrun dimungkinkan. Fungsi 'dwc2_glbreg_read' memproses nilai '[0..63]'. Periksa argumen ketiga. Periksa baris: 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
    ....
  }
  ....
}


Ada kemungkinan masalah dengan kelebihan array dalam kode ini. Dalam struktur DWC2State didefinisikan array glbreg , yang terdiri dari 28 elemen (komentar 1). Dalam fungsi dwc2_glbreg_read , kami merujuk ke array kami dengan indeks (komentar 2). Sekarang, perhatikan bahwa ekspresi ( addr - HSOTG_REG (0x000)) >> 2 (comment 3) diteruskan sebagai indeks ke fungsi dwc2_glbreg_read , yang dapat mengambil nilai dalam kisaran [0..63]. Untuk meyakinkan hal ini, perhatikan komentar 4 dan 5. Mungkin, di sini perlu untuk menyesuaikan rentang nilai dari komentar 4. Pemicu yang lebih mirip:







  • V557 Array overrun dimungkinkan. Fungsi 'dwc2_hreg0_read' memproses nilai '[0..63]'. Periksa argumen ketiga. Periksa baris: 814, 1050.hcd-dwc2.c 814
  • V557 Array overrun dimungkinkan. Fungsi 'dwc2_hreg1_read' memproses nilai '[0..191]'. Periksa argumen ketiga. Periksa jalur: 927, 1053.hcd-dwc2.c 927
  • V557 Array overrun dimungkinkan. Fungsi 'dwc2_pcgreg_read' memproses nilai '[0..127]'. Periksa argumen ketiga. Periksa baris: 1012, 1060.hcd-dwc2.c 1012


Peringatan N5



V575 Fungsi 'strerror_s' memproses elemen '0'. Periksa argumen kedua. commands-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);
    ....
  }
}


Fungsi strerror_s mengembalikan deskripsi tekstual dari kode kesalahan sistem. Tanda tangannya terlihat seperti ini: 



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


Parameter pertama adalah pointer ke buffer tempat deskripsi teks akan disalin, parameter kedua adalah ukuran buffer, yang ketiga adalah kode kesalahan. Dalam kode, 0 dilewatkan sebagai ukuran buffer, ini jelas merupakan nilai yang salah. Ngomong-ngomong, dimungkinkan untuk mengetahui sebelumnya berapa banyak byte yang harus dialokasikan: Anda hanya perlu memanggil strerrorlen_s , yang mengembalikan panjang deskripsi teks dari kesalahan tersebut. Nilai ini dapat digunakan untuk mengalokasikan buffer dengan ukuran yang memadai.



Peringatan N6



V595 Penunjuk 'blen2p' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 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
....
}


Dalam kode ini, nilai argumen blen2p pertama kali digunakan (komentar 1) dan kemudian diperiksa untuk nullptr (komentar 2). Tempat yang sangat mencurigakan ini sepertinya Anda lupa memasukkan cek sebelum digunakan pertama kali (komentar 1). Sebagai perbaikan, tambahkan saja cek: 



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


Masih ada pertanyaan tentang argumen blen1p . Mungkin, ini juga bisa menjadi penunjuk nol, dan di sini Anda juga perlu menambahkan tanda centang. Beberapa hal positif lainnya yang serupa:



  • V595 Penunjuk 'ref' digunakan sebelum diverifikasi terhadap nullptr. Periksa jalur: 2191, 2193. uri.c 2191
  • V595 Penunjuk 'cmdline' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 420, 425.qemu-io.c 420
  • V595 Penunjuk 'dp' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 288, 294. onenand.c 288
  • V595 Penunjuk 'omap_lcd' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 81, 87. omap_lcdc.c 81


Peringatan N7



V597 Kompilator dapat menghapus panggilan fungsi 'memset', yang digunakan untuk membersihkan objek 'op_info'. Fungsi RtlSecureZeroMemory () harus digunakan untuk menghapus data pribadi. 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);
  }
}


Dalam fragmen kode ini, fungsi memset dipanggil untuk objek op_info (komentar 1), setelah itu op_info segera dihapus, dengan kata lain, setelah dibersihkan, objek ini tidak diubah di tempat lain. Ini adalah kasus yang tepat ketika kompilator dapat menghapus panggilan memset selama proses pengoptimalan . Untuk menghilangkan perilaku potensial ini, Anda dapat menggunakan fungsi khusus yang tidak pernah dihapus oleh compiler. Lihat juga artikel " Menghapus Data Pribadi dengan Aman ".



N8



V610 Peringatan perilaku tidak ditentukan. Periksa operator shift '>>'. Operan kiri negatif ('number' = [-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;
  ....
}


Karena bilangan variabel bisa negatif, pergeseran ke kanan bitwise adalah perilaku yang tidak ditentukan. Untuk memastikan bahwa variabel yang dimaksud dapat mengambil nilai negatif, lihat komentar 1 dan 2. Untuk menghilangkan perbedaan perilaku kode Anda pada platform yang berbeda, kasus seperti itu harus dihindari.



Lebih banyak peringatan:



  • V610 Perilaku tidak ditentukan. Periksa operator shift '<<'. Operan kiri negatif ('(hclk_div - 1)' = [-1..15]). aspeed_smc.c 1041
  • V610 Perilaku tidak ditentukan. Periksa operator shift '<<'. Operan kiri '(target_long) - 1' negatif. exec -variasi.c 99
  • V610 Perilaku tidak ditentukan. Periksa operator shift '<<'. Operan kiri negatif ('hex2nib (words [3] [i * 2 + 2])' = [-1..15]). qtest.c 561


Ada juga beberapa peringatan dengan tipe yang sama, hanya -1 yang digunakan sebagai operan kiri .



V610 Perilaku tidak ditentukan. Periksa operator shift '<<'. Operan kiri '-1' negatif. hppa.c 2702 



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


Peringatan serupa lainnya:



  • V610 Perilaku tidak ditentukan. Periksa operator shift '<<'. Operan kiri '-1' negatif. hppa.c 2718
  • V610 Perilaku tidak ditentukan. Periksa operator shift '<<'. Operan kiri '-0x8000' negatif. fmopl.c 1022
  • V610 Perilaku tidak ditentukan. Periksa operator shift '<<'. Operan kiri '(intptr_t) - 1' negatif. sve_helper.c 889


Peringatan N9



V616 Konstanta bernama 'TIMER_NONE' dengan nilai 0 digunakan dalam operasi bitwise. sys_helper.c 179 



#define HELPER(name) ....

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

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


Anda dapat dengan mudah memverifikasi bahwa nilai makro TIMER_NONE adalah nol (komentar 1). Lebih lanjut, makro ini digunakan dalam operasi bitwise, yang hasilnya akan selalu 0. Hasilnya, isi pernyataan bersyarat if (env-> ttmr & TIMER_NONE) tidak akan pernah dijalankan.



Peringatan N10



V629 Pertimbangkan untuk memeriksa ekspresi 'n << 9'. Pergeseran bit dari nilai 32-bit dengan perluasan berikutnya ke tipe 64-bit. 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);
    ....
  }
  ....
}


Dalam fragmen kode ini , operasi shift dilakukan pada variabel n , yang memiliki tipe bertanda 32-bit, kemudian hasil bertanda 32-bit ini diperluas ke tipe bertanda 64-bit, dan kemudian, sebagai tipe tak bertanda, ditambahkan ke offset variabel 64-bit tak bertanda . Misalkan pada saat ekspresi dieksekusi, variabel n memiliki 9 bit paling signifikan. Kami melakukan operasi shift 9-bit ( BDRV_SECTOR_BITS), dan ini, pada gilirannya, adalah perilaku tidak terdefinisi, maka sebagai hasilnya kita bisa mendapatkan bit set dalam bit yang paling signifikan. Ingatlah bahwa bit dalam tipe bertanda ini bertanggung jawab atas tanda tersebut, yaitu, hasilnya bisa menjadi negatif. Karena n adalah variabel bertanda, tanda tersebut akan diperhitungkan saat memperluas. Hasilnya kemudian ditambahkan ke variabel offset . Dari pertimbangan ini, mudah untuk melihat bahwa hasil mengeksekusi ekspresi mungkin berbeda dari yang dimaksudkan. Salah satu solusi yang mungkin adalah mengganti tipe variabel n dengan tipe unsigned 64-bit, yaitu dengan uint64_t .



Berikut beberapa pemicu serupa lainnya:



  • 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


Peringatan N11



V634 Prioritas operasi '*' lebih tinggi daripada operasi '<<'. Ada kemungkinan bahwa tanda kurung harus digunakan dalam ekspresi. nand.c 310 



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


Itu hanya tempat yang mencurigakan. Tidak jelas apa yang ingin dilakukan programmer pada awalnya: shift atau perkalian. Meskipun tidak ada kesalahan di sini, Anda masih perlu melihat kode lagi dan menempatkan tanda kurung dengan benar. Ini hanyalah salah satu tempat yang harus dilihat developer untuk memastikan algoritme mereka benar. Tempat lain seperti:



  • V634 Prioritas operasi '*' lebih tinggi daripada operasi '<<'. Mungkin saja tanda kurung harus digunakan dalam ekspresi. exynos4210_mct.c 449
  • V634 Prioritas operasi '*' lebih tinggi daripada operasi '<<'. Ada kemungkinan bahwa tanda kurung harus digunakan dalam ekspresi. exynos4210_mct.c 1235
  • V634 Prioritas operasi '*' lebih tinggi daripada operasi '<<'. Ada kemungkinan bahwa tanda kurung harus digunakan dalam ekspresi. exynos4210_mct.c 1264


Peringatan N12



V646 Pertimbangkan untuk memeriksa logika aplikasi. Mungkin kata kunci 'lain' tidak ada. 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 {
      ....
    }
    ....
  }
  ....
}


Dalam kode ini, dilihat dari pemformatannya, penggunaan else if bukannya jika secara langsung menyarankan dirinya sendiri . Mungkin mereka lupa menambahkan yang lain di sini . Maka opsi koreksi bisa seperti ini: 



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


Namun, ada kemungkinan semuanya beres dengan kode ini, dan ada format yang salah dari teks program, yang membingungkan. Maka kodenya akan terlihat seperti ini: 



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


Peringatan N13



V773 Fungsi itu keluar tanpa melepaskan penunjuk 'aturan'. Kebocoran memori mungkin terjadi. 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
  ....
}


Dalam kode ini, objek aturan (komentar 1) dipilih dan ditambahkan ke daftar untuk digunakan nanti (komentar 3), tetapi jika terjadi kesalahan, fungsi kembali tanpa menghapus objek aturan yang dibuat sebelumnya (komentar 2). Di sini Anda hanya perlu menangani kesalahan dengan benar: hapus objek yang dibuat sebelumnya, jika tidak maka akan terjadi kebocoran memori.



Peringatan N14



V781 Nilai indeks 'ix' diperiksa setelah digunakan. Mungkin ada kesalahan dalam logika program. uri.c 2110 



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


Di sini penganalisis telah mendeteksi potensi out of bounds array. Pertama, elemen ref-> path array pada indeks ix dibaca , dan kemudian ix diperiksa kebenarannya ( ix> 0 ). Solusi yang tepat di sini adalah membalikkan tindakan ini: 



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


Ada beberapa tempat seperti itu:



  • V781 Nilai indeks 'ix' diperiksa setelah digunakan. Mungkin ada kesalahan dalam logika program. uri.c 2112
  • V781 Nilai indeks 'offset' diperiksa setelah digunakan. Mungkin ada kesalahan dalam logika program. keymaps.c 125
  • V781 Nilai variabel 'kualitas' diperiksa setelah digunakan. Mungkin ada kesalahan dalam logika program. Periksa baris: 326, 335.vnc-enc-tight.c 326
  • V781 Nilai indeks 'i' diperiksa setelah digunakan. Mungkin ada kesalahan dalam logika program. mem_helper.c 1929


Peringatan N15



V784 Ukuran topeng bit kurang dari ukuran operan pertama. Ini akan menyebabkan hilangnya bit yang lebih tinggi. 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]);
  ....
}


Kode ini melakukan operasi bitwise pada objek dengan tipe berbeda. Operan kiri adalah argumen val , yang merupakan tipe unsigned 64-bit. Nilai yang diterima dari elemen array s-> regs_ro pada indeks offset , yang memiliki tipe unsigned 32-bit, digunakan sebagai operan kanan . Hasil dari operasi di sisi kanan (~ (s-> regs_ro [offset])) adalah tipe unsigned 32-bit, dan sebelum perkalian bitwise akan diperluas ke tipe 64-bit dengan nol, yaitu, setelah mengevaluasi seluruh ekspresi, semua bit orde tinggi dari variabel val akan menjadi nol . Tempat-tempat seperti itu selalu terlihat mencurigakan. Di sini kami hanya dapat merekomendasikan pengembang untuk merevisi kode ini lagi. Lebih mirip:



  • V784 Ukuran bit mask lebih kecil dari ukuran operan pertama. Ini akan menyebabkan hilangnya bit yang lebih tinggi. xlnx-zynq-devcfg.c 199
  • V784 Ukuran bit mask lebih kecil dari ukuran operan pertama. Ini akan menyebabkan hilangnya bit yang lebih tinggi. soc_dma.c 214
  • V784 Ukuran bit mask lebih kecil dari ukuran operan pertama. Ini akan menyebabkan hilangnya bit yang lebih tinggi. fpu_helper.c 418


Peringatan N16



V1046 Penggunaan yang tidak aman dari tipe 'bool' dan 'unsigned int' bersamaan dalam operasi '& ='. 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);
}


Dalam fragmen kode ini, operasi AND bitwise dilakukan pada variabel hpd tipe bool dan hasil dari menjalankan fungsi extract32 , yang memiliki tipe uint32_t . Karena nilai bit variabel boolean hanya bisa 0 atau 1, hasil ekspresi akan selalu salah jika bit paling tidak signifikan yang dikembalikan oleh fungsi extract32 adalah nol. Mari kita lihat ini dengan sebuah contoh. Misalkan hpd benar dan fungsi mengembalikan 2, yaitu, dalam representasi biner, operasi akan terlihat seperti 01 & 10 = 0, dan hasil ekspresi akan salah . Kemungkinan besar, pemrogram ingin menyetel nilainya menjadi truejika fungsi mengembalikan sesuatu yang bukan nol. Ternyata, kode tersebut perlu dikoreksi agar hasil fungsinya dilemparkan ke tipe bool , misalnya seperti ini: 



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


Kesimpulan



Seperti yang Anda lihat, penganalisis menemukan banyak tempat yang mencurigakan. Mungkin masalah potensial yang ditemukan belum termanifestasi dengan cara apa pun, tetapi kehadiran mereka tidak bisa tidak mengkhawatirkan, karena mereka mampu menembak pada saat yang paling tidak terduga. Melihat semua tempat mencurigakan sebelumnya dan memperbaikinya lebih baik daripada memperbaiki aliran bug yang tak ada habisnya. Jelas, untuk proyek kompleks seperti ini, analisis statis dapat memberikan manfaat nyata, terutama jika Anda mengatur tinjauan proyek secara teratur. Jika Anda ingin mencoba PVS-Studio untuk proyek Anda, Anda dapat mengunduh penganalisis dan mendapatkan kunci uji coba gratis di halaman ini .





Jika Anda ingin berbagi artikel ini dengan audiens berbahasa Inggris, silakan gunakan tautan terjemahan: Evgeniy Ovsannikov. Memeriksa QEMU menggunakan PVS-Studio .


More articles:


All Articles