Dan lagi tentang tertanam: mencari bug di proyek Embox

Gambar 2

Embox adalah sistem operasi real-time lintas platform, multi-tasking untuk sistem embedded. Ini dirancang untuk bekerja dalam sumber daya komputasi rendah dan memungkinkan Anda untuk menjalankan aplikasi berbasis Linux pada mikrokontroler tanpa menggunakan Linux itu sendiri. Tentu saja, seperti aplikasi lain, bug Embox juga tidak terhindar. Artikel ini dikhususkan untuk analisis kesalahan yang ditemukan dalam kode proyek Embox.

Beberapa bulan yang lalu, saya sudah menulis artikel tentang memeriksa FreeRTOS, OS lain untuk sistem embedded. Saya tidak menemukan kesalahan di dalamnya saat itu, tetapi saya menemukannya di perpustakaan yang ditambahkan oleh orang-orang dari Amazon ketika mengembangkan versi FreeRTOS mereka sendiri.

Artikel yang Anda lihat sekarang di depan Anda, dalam arti tertentu, melanjutkan tema yang sebelumnya. Kami sering menerima permintaan untuk memeriksa FreeRTOS, dan kami melakukannya. Kali ini, tidak ada permintaan untuk memeriksa proyek tertentu, tetapi saya mulai menerima surat dan komentar dari pengembang yang menyertainya dan yang menginginkan lebih.

Nah, edisi baru PVS-Studio untuk majalah Embedded telah matang dan ada di hadapan Anda. Selamat menonton!

Analisis


Analisis dilakukan dengan menggunakan PVS-Studio - penganalisa kode statis untuk C, C ++, C # dan Java. Sebelum analisis, proyek perlu dirakit - jadi kami akan yakin bahwa kode proyek berfungsi, dan kami juga akan memberikan kesempatan kepada penganalisa untuk mengumpulkan informasi perakitan yang dapat berguna untuk verifikasi kode yang lebih baik.

Instruksi dalam repositori resmi Embox menawarkan kemampuan untuk membangun di bawah sistem yang berbeda (Arch Linux, macOS, Debian) dan menggunakan Docker. Saya memutuskan untuk menambahkan beberapa variasi dalam hidup saya dan membangun serta menganalisis dari bawah Debian, yang baru-baru ini saya instal di mesin virtual saya.

Perakitan berjalan lancar. Sekarang perlu untuk melakukan analisis. Debian adalah salah satu sistem berbasis PVS-Studio Linux yang didukung. Cara mudah untuk memeriksa proyek dari Linux adalah dengan mengkompilasi jejak. Ini adalah mode khusus di mana penganalisa mengumpulkan semua informasi yang diperlukan tentang perakitan sehingga Anda dapat memulai analisis dengan satu klik. Yang perlu saya lakukan adalah:

1) Unduh dan instal PVS-Studio;

2) Luncurkan pelacakan perakitan dengan membuka folder dengan Embox dan mengetik di terminal

pvs-studio-analyzer analyze -- make

3) Setelah menunggu perakitan selesai, jalankan perintah:

pvs-studio-analyzer analyze -o /path/to/output.log

4) Konversi laporan mentah ke format apa pun yang nyaman bagi Anda. Alat analisis dilengkapi dengan utilitas khusus PlogConverter, yang dengannya Anda dapat melakukan ini. Misalnya, perintah untuk mengonversi laporan ke daftar tugas (untuk dilihat, misalnya, di QtCreator) akan terlihat seperti ini:

plog-converter -t tasklist -o /path/to/output.tasks /path/to/project

Semua! Butuh waktu tidak lebih dari 15 menit untuk menyelesaikan poin ini. Laporan sudah siap, sekarang Anda dapat melihat kesalahan. Baiklah, mari kita mulai :)

Siklus aneh


Salah satu kesalahan yang ditemukan oleh penganalisis adalah loop while aneh:

int main(int argc, char **argv) {
  ....

  while (dp.skip != 0 ) {
    n_read = read(ifd, tbuf, dp.bs);
    if (n_read < 0) {
      err = -errno;
      goto out_cmd;
    }
    if (n_read == 0) {
      goto out_cmd;
    }

    dp.skip --;
  } while (dp.skip != 0);       // <=

  do {
    n_read = read(ifd, tbuf, dp.bs);
    if (n_read < 0) {
      err = -errno;
      break;
    }

    if (n_read == 0) {
      break;
    }

    ....

    dp.count --;
  } while (dp.count != 0);
  ....
}

Peringatan PVS-Studio: V715 Operator 'while' memiliki tubuh kosong. Pola mencurigakan terdeteksi: 'while (expr) {...} while (dp.skip! = 0);'. dd.c 225 hm

. Lingkaran yang sangat aneh. Ekspresi while (dp.skip! = 0) ditulis dua kali, sekali tepat di atas loop, dan yang kedua tepat di bawahnya. Sebenarnya, sekarang ini adalah dua siklus yang berbeda: satu berisi ekspresi dalam kurung kurawal, dan yang kedua kosong. Dalam hal ini, siklus kedua tidak akan pernah dieksekusi.

Di bawah ini adalah ... do loop sementara dengan kondisi yang sama, yang membuat saya berpikir: loop aneh awalnya dimaksudkan sebagai do ... sementara, tetapi ada sesuatu yang salah. Saya pikir ini bagian dari kode dengan probabilitas tinggi mengandung kesalahan logis.

Memori bocor


Ya, bukan tanpa mereka.

int krename(const char *oldpath, const char *newpath) {
  
  char *newpatharg, *oldpatharg;

  ....

  oldpatharg =
    calloc(strlen(oldpath) + diritemlen + 2, sizeof(char));
  newpatharg =
    calloc(strlen(newpath) + diritemlen + 2, sizeof(char));
  if (NULL == oldpatharg || NULL == newpatharg) {
    SET_ERRNO(ENOMEM);
    return -1;
  }

  ....
}

Peringatan PVS-Studio:

  • V773 The function was exited without releasing the 'newpatharg' pointer. A memory leak is possible. kfsop.c 611
  • V773 The function was exited without releasing the 'oldpatharg' pointer. A memory leak is possible. kfsop.c 611

Fungsi ini menciptakan variabel lokal newpatharg dan oldpatharg di dalam dirinya sendiri . Pointer ini diberikan alamat lokasi memori baru yang dialokasikan secara internal menggunakan calloc . Jika masalah terjadi ketika mengalokasikan memori, calloc mengembalikan pointer nol.

Bagaimana jika hanya satu blok memori yang dialokasikan? Fungsi akan macet tanpa ada memori yang dibebaskan. Situs yang kebetulan dialokasikan akan tetap berada dalam memori tanpa ada kesempatan untuk mengaksesnya lagi dan membebaskannya untuk digunakan lebih lanjut.

Contoh lain dari kebocoran memori sedikit lebih cerah:

static int block_dev_test(....) {
  int8_t *read_buf, *write_buf;
  
  ....

  read_buf = malloc(blk_sz * m_blocks);
  write_buf = malloc(blk_sz * m_blocks);

  if (read_buf == NULL || write_buf == NULL) {
    printf("Failed to allocate memory for buffer!\n");

    if (read_buf != NULL) {
      free(read_buf);
    }

    if (write_buf != NULL) {
      free(write_buf);
    }

    return -ENOMEM;
  }

  if (s_block >= blocks) {
    printf("Starting block should be less than number of blocks\n");
    return -EINVAL;            // <=
  }

  ....
}

Peringatan PVS-Studio:

  • V773 The function was exited without releasing the 'read_buf' pointer. A memory leak is possible. block_dev_test.c 195
  • V773 The function was exited without releasing the 'write_buf' pointer. A memory leak is possible. block_dev_test.c 195

Di sini, programmer telah menunjukkan keakuratan dan memproses kasus dengan benar yang memungkinkan hanya mengalokasikan satu memori. Diproses dengan benar ... dan secara harfiah dalam ekspresi berikutnya membuat kesalahan lain.

Berkat cek yang ditulis dengan benar, kami dapat memastikan bahwa pada saat ekspresi dieksekusi, kembali -EINVAL; kami pasti akan memiliki memori yang dialokasikan untuk read_buf dan write_buf . Jadi, dengan kembalinya fungsi, kita akan mengalami dua kebocoran sekaligus.

Saya pikir mendapatkan kebocoran memori pada perangkat yang disematkan bisa lebih menyakitkan daripada pada PC klasik. Dalam kondisi ketika sumber daya sangat terbatas, Anda perlu memantaunya dengan cermat.

Pointer salah penanganan


Kode kesalahan berikut singkat dan cukup sederhana:

static int scsi_write(struct block_dev *bdev, char *buffer,
                      size_t count, blkno_t blkno) {
  struct scsi_dev *sdev;
  int blksize;

  ....

  sdev = bdev->privdata;
  blksize = sdev->blk_size; // <=

  if (!sdev) {              // <=
    return -ENODEV;
  }

  ....
}

Peringatan PVS-Studio: V595 Pointer 'sdev' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 116, 118. scsi_disk.c 116

The sdev pointer adalah dereferenced sebelum itu diperiksa NULL. Adalah logis untuk berasumsi bahwa jika seseorang menulis cek semacam itu, maka penunjuk ini mungkin nol. Dalam kasus ini, kita memiliki potensi referensi dari pointer nol di string blksize = sdev-> blk_size .

Kesalahannya adalah bahwa cek tidak terletak di tempat yang diperlukan. Seharusnya muncul setelah baris " sdev = bdev-> privdata; ", tetapi sebelum baris " blksize = sdev-> blk_size; ". Maka potensi banding ke alamat nol dapat dihindari.

PVS-Studio menemukan dua kesalahan lagi dalam kode berikut:

void xdrrec_create(....)
{
  char *buff;

  ....

  buff = (char *)malloc(sendsz + recvsz);
  assert(buff != NULL);

  ....

  xs->extra.rec.in_base = xs->extra.rec.in_curr = buff;
  xs->extra.rec.in_boundry 
    = xs->extra.rec.in_base + recvsz;                    // <=

  ....
  xs->extra.rec.out_base
    = xs->extra.rec.out_hdr = buff + recvsz;             // <= 
  xs->extra.rec.out_curr 
    = xs->extra.rec.out_hdr + sizeof(union xdrrec_hdr);

  ....
}

Peringatan PVS-Studio:

  • V769 Pointer 'xs-> extra.rec.in_base' dalam ekspresi 'xs-> extra.rec.in_base + recvsz' bisa berupa nullptr. Dalam kasus seperti itu, nilai yang dihasilkan akan menjadi tidak masuk akal dan tidak boleh digunakan. Periksa baris: 56, 48. xdr_rec.c 56
  • V769 Pointer 'buff' di ekspresi 'buff + recvsz' bisa berupa nullptr. Dalam kasus seperti itu, nilai yang dihasilkan akan menjadi tidak masuk akal dan tidak boleh digunakan. Periksa baris: 61, 48. xdr_rec.c 61

Pointer buf diinisialisasi dengan malloc , dan kemudian nilainya digunakan untuk menginisialisasi pointer lain. Fungsi malloc dapat mengembalikan pointer nol, dan ini harus selalu diperiksa. Akan terlihat bahwa di sini adalah menegaskan memeriksa buf untuk NULL , dan segala sesuatu harus bekerja dengan baik.

Tapi tidak! Faktanya adalah bahwa menegaskan 's digunakan untuk debugging, dan ketika membangun proyek dalam konfigurasi Rilis, ini menegaskan akan dihapus. Ternyata ketika bekerja di Debug, program akan bekerja dengan benar, dan ketika membangun dalam Release, null pointer "berjalan" lebih jauh.

Gunakan noldalam operasi aritmatika salah, karena hasil operasi semacam itu tidak masuk akal, dan hasil seperti itu tidak dapat digunakan. Inilah yang diperingatkan oleh penganalisa kita.

Beberapa orang mungkin berpendapat bahwa tidak memeriksa pointer setelah malloc / realloc / calloc tidak kenal takut. Seperti, pada akses pertama oleh pointer nol, sinyal / pengecualian akan terjadi dan tidak akan ada yang salah. Dalam praktiknya, semuanya jauh lebih rumit, dan jika kurangnya verifikasi tampaknya tidak berbahaya bagi Anda, saya sarankan Anda melihat artikel " Mengapa penting untuk memeriksa apa fungsi malloc dikembalikan ".

Penanganan array yang salah


Kesalahan berikut ini sangat mirip dengan contoh sebelum terakhir:


int fat_read_filename(struct fat_file_info *fi,
                      void *p_scratch,
                      char *name) {
  int offt = 1;

  ....

  offt = strlen(name);
  while (name[offt - 1] == ' ' && offt > 0) { // <=
    name[--offt] = '\0';
  }
  log_debug("name(%s)", name);

  return DFS_OK;
}

PVS-Studio Warning: V781 Nilai indeks 'offt' diperiksa setelah digunakan. Mungkin ada kesalahan dalam logika program. fat_common.c 1813

Variabel offt pertama kali digunakan di dalam operasi pengindeksan, dan hanya kemudian diperiksa bahwa nilainya lebih besar dari nol. Tetapi apa yang terjadi jika nama ternyata berupa string kosong? Fungsi strlen () akan mengembalikan 0 , kemudian bidikan keras di kaki. Program akan mengakses pada indeks negatif, yang akan mengarah pada perilaku yang tidak terdefinisi. Apa pun bisa terjadi, termasuk crash program. Kekacauan!

Gambar 1

Kondisi mencurigakan


Dan di mana tanpa mereka? Kami menemukan kesalahan seperti itu di setiap proyek yang kami periksa.

int index_descriptor_cloexec_set(int fd, int cloexec) {
  struct idesc_table *it;

  it = task_resource_idesc_table(task_self());
  assert(it);

  if (cloexec | FD_CLOEXEC) {
    idesc_cloexec_set(it->idesc_table[fd]);
  } else {
    idesc_cloexec_clear(it->idesc_table[fd]);
  }
  return 0;
}

Peringatan PVS-Studio: V617 Pertimbangkan untuk memeriksa kondisi. Argumen '0x0010' dari '|' operasi bitwise mengandung nilai bukan nol. index_descriptor.c 55

Untuk memahami apa kesalahannya, lihat definisi konstanta FD_CLOEXEC :

#define FD_CLOEXEC 0x0010

Ternyata dalam ekspresi if (cloexec | FD_CLOEXEC) di sebelah kanan bitwise “atau” selalu ada konstanta bukan nol. Hasil dari operasi semacam itu akan selalu berupa angka yang bukan nol. Dengan demikian, ungkapan ini akan selalu setara dengan ekspresi if (true) , dan kami akan selalu memproses hanya cabang if-expression.

Saya menduga bahwa konstanta makro ini digunakan untuk pra-konfigurasi sistem operasi Embox, tetapi meskipun demikian kondisi yang selalu benar ini terlihat aneh. Mungkin mereka ingin menggunakan operator & di sini , tetapi mereka membuat kesalahan ketik.

Divisi integer


Kesalahan berikut ini terkait dengan satu fitur bahasa C:

#define SBSIZE    1024

static int ext2fs_format(struct block_dev *bdev, void *priv) {
  size_t dev_bsize;
  float dev_factor;

  ....

  dev_size = block_dev_size(bdev);
  dev_bsize = block_dev_block_size(bdev);
  dev_factor = SBSIZE / dev_bsize;            // <=

  ext2_dflt_sb(&sb, dev_size, dev_factor);
  ext2_dflt_gd(&sb, &gd);

  ....
}

Peringatan PVS-Studio: V636 Ekspresi '1024 / dev_bsize' secara implisit dilemparkan dari tipe 'int' ke tipe 'float'. Pertimbangkan untuk menggunakan pemeran tipe eksplisit untuk menghindari hilangnya bagian fraksional. Contoh: double A = (double) (X) / Y; ext2.c 777

Fitur ini adalah sebagai berikut: jika kita membagi dua nilai integer, maka hasil pembagian akan menjadi integer. Dengan demikian, pembagian akan terjadi tanpa sisa, atau, dengan kata lain, bagian fraksional akan dibuang dari hasil pembagian.

Terkadang programmer melupakannya, dan kesalahan seperti ini terjadi. SBSIZE konstan dan variabel dev_bsize masing - masing memiliki tipe integer (int, dan size_t). Oleh karena itu, hasil dari ekspresi SBSIZE / dev_bsizejuga akan bertipe integer.

Tapi tunggu sebentar. Variabel dev_factor adalah tipe float ! Jelas, programmer diharapkan mendapatkan hasil pembagian fraksional. Ini dapat diverifikasi lebih lanjut jika Anda memperhatikan penggunaan lebih lanjut dari variabel ini. Misalnya, fungsi ext2_dflt_sb , di mana dev_factor dilewatkan sebagai parameter ketiga, memiliki tanda tangan berikut:

static void ext2_dflt_sb(struct ext2sb *sb, size_t dev_size, float dev_factor);

Demikian pula, di tempat lain di mana variabel dev_factor digunakan : semuanya menunjukkan bahwa angka floating-point diharapkan.

Untuk memperbaiki kesalahan ini, cukup dengan melemparkan salah satu operan divisi ke tipe nyata. Contohnya:

dev_factor = float(SBSIZE) / dev_bsize;

Maka hasil pembagian akan menjadi angka pecahan.

Input tidak terverifikasi


Kesalahan berikut ini terkait dengan penggunaan data yang tidak diverifikasi yang diterima dari luar program.

int main(int argc, char **argv) {
  int ret;
  char text[SMTP_TEXT_LEN + 1];

  ....

  if (NULL == fgets(&text[0], sizeof text - 2, /* for \r\n */
      stdin)) { ret = -EIO; goto error; }
    text[strlen(&text[0]) - 1] = '\0'; /* remove \n */    // <=

  ....
}

Peringatan PVS-Studio: V1010 Data tercemar yang tidak dicentang digunakan dalam indeks: 'strlen (& teks [0])'. sendmail.c 102

Pertama, pertimbangkan apa fungsi pengembalian fgets . Jika berhasil membaca suatu baris, fungsi mengembalikan sebuah pointer ke baris ini. Jika pembacaan menemui akhir-file sebelum setidaknya satu elemen dibaca, atau jika terjadi kesalahan input, fungsi fgets mengembalikan NULL .

Jadi ungkapannya adalah NULL == fgets (....)memeriksa apakah input yang diterima benar. Tapi ada satu peringatan. Jika Anda mentransfer terminal nol sebagai karakter pertama yang akan dibaca (ini dapat dilakukan, misalnya, dengan menekan Ctrl + 2 dalam mode Legacy dari baris perintah Windows), fungsi gadget menganggapnya tanpa mengembalikan NULL . Dalam hal ini, garis tempat rekaman dibuat hanya akan memiliki satu elemen - ' \ 0 '.

Apa yang akan terjadi selanjutnya? Ekspresi strlen (& teks [0]) akan mengembalikan 0 . Akibatnya, kami mendapat panggilan indeks negatif:

text[ 0 - 1 ] = '\0';

Akibatnya, kita dapat "membalikkan" program dengan hanya meneruskan karakter terminasi baris ke input. Ini aneh, dan berpotensi digunakan untuk menyerang sistem menggunakan Embox.

Kolega saya, yang mengembangkan aturan diagnostik ini, bahkan membuat catatan dengan contoh serangan seperti itu pada proyek NcFTP:


Saya sarankan untuk melihat apakah Anda masih tidak percaya pada kesempatan seperti itu :)

Juga, penganalisa menemukan dua tempat lagi dengan kesalahan yang sama:

  • V1010 Data tercemar yang tidak diperiksa digunakan dalam indeks: 'strlen (& from [0])'. sendmail.c 55
  • V1010 Data tercemar yang tidak dicentang digunakan dalam indeks: 'strlen (& to [0])'. sendmail.c 65

Misra


MISRA adalah seperangkat pedoman dan pedoman untuk menulis kode C dan C ++ yang aman untuk sistem tertanam yang sangat bertanggung jawab. Dalam arti tertentu, ini adalah manual pelatihan, yang mengikuti yang akan memungkinkan Anda untuk menyingkirkan tidak hanya apa yang disebut "bau kode", tetapi juga melindungi program Anda dari kerentanan.

MISRA digunakan di mana kehidupan manusia bergantung pada kualitas sistem tertanam Anda: dalam industri medis, otomotif, pesawat terbang, dan militer.

PVS-Studio memiliki seperangkat aturan diagnostik yang luas yang memungkinkan Anda memeriksa kode Anda untuk kesesuaian dengan standar MISRA C dan MISRA C ++. Secara default, mode dengan diagnostik ini dimatikan, tetapi karena kami mencari kesalahan dalam proyek untuk sistem embedded, saya tidak bisa melakukannya tanpa MISRA.

Inilah yang berhasil saya temukan:

/* find and read symlink file */
static int ext2_read_symlink(struct nas *nas,
                             uint32_t parent_inumber,
                             const char **cp) {
  char namebuf[MAXPATHLEN + 1];

  ....

  *cp = namebuf;              // <=
  if (*namebuf != '/') {
    inumber = parent_inumber;
  } else {
    inumber = (uint32_t) EXT2_ROOTINO;
  }
  rc = ext2_read_inode(nas, inumber);

  return rc;
} 

Peringatan PVS-Studio: V2548 [MISRA C 18.6] Alamat dari array lokal 'namebuf' tidak boleh disimpan di luar lingkup array ini. ext2.c 298

Analyzer mendeteksi tugas mencurigakan yang berpotensi menyebabkan perilaku tidak terdefinisi.

Mari kita lihat lebih dekat kode ini. Di sini, namebuf adalah array yang dibuat dalam lingkup fungsi lokal, dan pointer cp diteruskan ke fungsi dengan pointer.

Menurut sintaks C, nama array adalah penunjuk ke elemen pertama di area memori di mana array disimpan. Ternyata ekspresi * cp = namebuf akan menetapkan alamat array namebuf ke variabel yang ditunjukkan oleh cp. Karena cp dilewatkan ke fungsi oleh pointer, perubahan nilai yang ditunjukkannya akan tercermin di tempat di mana fungsi dipanggil.

Ternyata setelah fungsi ext2_read_symlink menyelesaikan pekerjaannya, parameter ketiga akan menunjukkan area yang pernah ditempati array namebuf .

Hanya ada satu "tetapi": karena namebuf adalah array yang dicadangkan di stack, itu akan dihapus ketika fungsi keluar. Dengan demikian, pointer yang ada di luar fungsi akan menunjuk ke memori yang dibebaskan.

Apa yang akan ada di alamat itu? Tidak mungkin untuk diprediksi. Ada kemungkinan bahwa untuk beberapa waktu isi array akan terus berada dalam memori, atau ada kemungkinan bahwa program akan segera menghapus area ini dengan sesuatu yang lain. Secara umum, mengakses alamat seperti itu akan mengembalikan nilai yang tidak ditentukan, dan menggunakan nilai seperti itu adalah kesalahan besar.

Penganalisa juga menemukan kesalahan lain dengan peringatan yang sama:

  • V2548 [MISRA C 18.6] Alamat variabel lokal 'dst_haddr' tidak boleh disimpan di luar ruang lingkup variabel ini. net_tx.c 82

Gambar 6

Kesimpulan


Saya suka bekerja dengan proyek Embox. Terlepas dari kenyataan bahwa saya tidak menuliskan semua kesalahan yang ditemukan dalam artikel, jumlah total peringatan relatif kecil, dan secara umum, kode proyek ditulis dengan kualitas tinggi. Oleh karena itu, saya mengucapkan terima kasih kepada para pengembang, serta kepada mereka yang berkontribusi pada proyek atas nama komunitas. Kamu hebat!

Saya mengambil kesempatan ini untuk menyampaikan salam kepada para pengembang dari Tula. Saya akan percaya bahwa di St. Petersburg saat ini tidak terlalu dingin :)

Di sinilah artikel saya berakhir. Saya harap Anda menikmati membacanya, dan Anda menemukan sesuatu yang baru untuk diri Anda sendiri.

Jika Anda tertarik pada PVS-Studio dan ingin memverifikasi sendiri beberapa proyek yang menggunakannya, unduh dan coba . Ini akan memakan waktu tidak lebih dari 15 menit.



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: George Gribkov. Tentang tertanam lagi: mencari bug di proyek Embox .

All Articles