🌈 📗 ⚽️ التحقق من QEMU مع PVS-Studio 🧥 🔔 👺

QEMU هو تطبيق مضاهاة معروف إلى حد ما. يمكن أن يساعد التحليل الثابت مطوري المشاريع المعقدة مثل QEMU في اكتشاف الأخطاء في مرحلة مبكرة وتحسين الجودة والموثوقية بشكل عام. في هذه المقالة ، سيتم التحقق من التعليمات البرمجية المصدر لتطبيق QEMU بحثًا عن نقاط الضعف والأخطاء المحتملة باستخدام أداة التحليل الثابت PVS-Studio.

QEMU هو برنامج مجاني مصمم لمحاكاة الأجهزة عبر الأنظمة الأساسية. يسمح لك بتشغيل التطبيقات وأنظمة التشغيل على الأنظمة الأساسية للأجهزة بخلاف الهدف ، على سبيل المثال ، تطبيق مكتوب لـ MIPS للتشغيل مع هندسة x86. تدعم QEMU أيضًا محاكاة مجموعة متنوعة من الأجهزة الطرفية مثل بطاقات الفيديو و USB وما إلى ذلك. المشروع معقد للغاية ويستحق الاهتمام ، فهذه المشاريع ذات أهمية للتحليل الثابت ، لذلك تقرر التحقق من كودها باستخدام PVS-Studio.

حول التحليل

يمكن الحصول على الكود المصدري للمشروع من مرآة على جيثب . المشروع كبير جدًا ويمكن تجميعه لمنصات مختلفة. لتسهيل التحقق من الكود ، سوف نستخدم نظام مراقبة التجميع PVS-Studio . تم تصميم هذا النظام لتحقيق تكامل سهل للغاية للتحليل الثابت في أي نظام أساسي للبناء تقريبًا. يعتمد النظام على تتبع مكالمات المترجم أثناء الإنشاء ويسمح لك بجمع كل المعلومات للتحليل اللاحق للملفات. بعبارة أخرى ، بدأنا الإنشاء للتو ، حيث يقوم برنامج PVS-Studio بجمع المعلومات الضرورية ، ثم نبدأ التحليل - كل شيء بسيط. يمكن الاطلاع على التفاصيل على الرابط أعلاه.

بعد التحقق ، وجد المحلل العديد من المشاكل المحتملة. للتشخيصات ذات الأغراض العامة (التحليل العام) ، تم الحصول عليها: 1940 مرتفع ، 1996 متوسط ، 9596 منخفض. بعد مراجعة جميع التحذيرات ، تقرر التركيز على التشخيص لمستوى الثقة الأول (مرتفع). تم العثور على الكثير من هذه التحذيرات (1940) ، ولكن معظم التحذيرات إما من نفس النوع أو مرتبطة بالاستخدام المتكرر لوحدة ماكرو مشبوهة. على سبيل المثال ، ضع في اعتبارك الماكرو 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;                                           \
  }))

لكل استخدام لهذا الماكرو ، يصدر المحلل تحذيرًا V773 (تم الخروج من نطاق رؤية المؤشر "__p" بدون تحرير الذاكرة. من الممكن حدوث تسرب للذاكرة). يتم تعريف الماكرو g_new في مكتبة glib ، ويستخدم الماكرو _G_NEW ، ويستخدم هذا الماكرو بدوره ماكروًا آخر ، G_GNUC_EXTENSION ، والذي يخبر برنامج التحويل البرمجي GCC بتخطي التحذيرات حول التعليمات البرمجية غير القياسية. هذا هو الرمز غير القياسي الذي يسبب تحذير المحلل ، انتبه إلى السطر قبل الأخير. بشكل عام ، الماكرو يعمل. كان هناك 848 تحذيرًا من هذا النوع ، أي أن نصف التنبيهات تقريبًا تحدث في مكان واحد فقط في الكود.

كل هذه التحذيرات غير الضرورية سهلة الإزالةباستخدام إعدادات المحلل. ومع ذلك ، فإن هذه الحالة بالذات ، التي واجهناها أثناء كتابة هذا المقال ، هي سبب قيام فريقنا بإجراء تعديل طفيف على منطق المحلل لمثل هذه المواقف.

وبالتالي ، فإن عددًا كبيرًا من التحذيرات لا يعني دائمًا جودة الكود السيئة. ومع ذلك ، هناك بعض الأماكن المشبوهة حقًا. حسنًا ، دعنا ننتقل إلى التحذيرات.

تحذير N1

V517 استخدام "if (A) {...} وإلا إذا تم اكتشاف نمط (A) {...}". هناك احتمال وجود خطأ منطقي. فحص الأسطر: 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) {
    ....
  }
  ....
}

دائمًا ما يكون أي استخدام للأرقام "السحرية" في الشفرة أمرًا مريبًا. هناك شرطان هنا ، وللوهلة الأولى يبدو أنهما مختلفان ، ولكن إذا نظرت إلى قيمة الماكرو MEGASAS_MAX_SGE ، فقد اتضح أن الشروط تتكرر مع بعضها البعض. على الأرجح ، هناك خطأ مطبعي هنا وبدلاً من 128 يجب أن يكون هناك رقم آخر. بالطبع ، هذه مشكلة كل الأرقام "السحرية" ، يكفي فقط أن يتم ختمها عند استخدامها. إن استخدام وحدات الماكرو والثوابت يساعد المطور بشكل كبير في هذه الحالة.

تحذير N2

V523 عبارة "then" تكافئ جملة "else". 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;
  }
  ....
}

في الكود قيد النظر ، تتطابق أجسام then and else الخاصة بالمشغل الشرطي. هنا ، على الأرجح نسخ ولصق. ما عليك سوى نسخ الجثة ثم الفرع ، وإصلاح النسيان. يمكن افتراض أنه كان يجب استخدام env بدلاً من الكائن الآخر . قد يبدو إصلاح هذا المكان المريب على النحو التالي:

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

يمكن لمطوري هذا الكود فقط أن يقولوا بشكل لا لبس فيه كيف يجب أن يكون في الواقع. مكان آخر مشابه:

V523 عبارة "then" تعادل عبارة "else". ترجمة ج 641

تحذير N3

V547 التعبير "ret <0" خاطئ دائمًا. qcow2-الكتلة. 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
    ....
  }
}

هنا وجد المحلل أن الشرط (التعليق 1) لن يتحقق أبدًا. تتم تهيئة قيمة المتغير ret من خلال نتيجة تنفيذ وظيفة handle_dependencies ، هذه الدالة ترجع فقط 0 أو -EAGAIN (التعليقات 3 ، 4 ، 5). أعلى قليلاً ، في الشرط الأول ، تحققنا من قيمة ret مقابل -EAGAIN (التعليق 2) ، وبالتالي فإن نتيجة التعبير ret <0 ستكون دائمًا خاطئة. ربما تم استخدام وظيفة handle_dependencies لإرجاع قيم مختلفة ، ولكن لاحقًا ، نتيجة لإعادة البناء ، على سبيل المثال ، تغير السلوك. هنا تحتاج فقط إلى إكمال إعادة البناء. مشغلات مماثلة:

يكون التعبير V547 خاطئًا دائمًا. qcow2.c 1070
V547 Expression 's-> state! = MIGRATION_STATUS_COLO' دائما خطأ. 595 - علي
V547 Expression 's-> metadata_entries.present & 0x20' دائمًا خطأ. 769 - محل

تحذير N4

V557 تجاوز الصفيف ممكن. تعالج دالة "dwc2_glbreg_read" قيمة "[0..63]". افحص الوسيطة الثالثة. فحص السطور: 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
    ....
  }
  ....
}

هناك مشكلة محتملة في تجاوزات الصفيف في هذا الرمز. في بنية DWC2State المصفوفة المحددة glbreg ، تتكون من 28 عنصرًا (التعليق 1). في دالة dwc2_glbreg_read ، نشير إلى صفيفنا حسب الفهرس (التعليق 2). الآن ، لاحظ أن التعبير ( addr - HSOTG_REG (0x000)) >> 2 (التعليق 3) يتم تمريره كمؤشر إلى الدالة dwc2_glbreg_read ، والتي يمكن أن تأخذ قيمة في النطاق [0..63]. للاقتناع بهذا ، انتبه للتعليقين 4 و 5. ربما ، من الضروري هنا تعديل نطاق القيم من التعليق 4. المزيد من المشغلات المشابهة:

V557 تجاوز الصفيف ممكن. تعالج الدالة "dwc2_hreg0_read" القيمة "[0..63]". افحص الوسيطة الثالثة. فحص الأسطر: 814، 1050.hcd-dwc2.c 814
V557 تجاوز الصفيف ممكن. تعالج الدالة "dwc2_hreg1_read" القيمة "[0..191]". افحص الوسيطة الثالثة. فحص الأسطر: 927، 1053.hcd-dwc2.c 927
V557 تجاوز الصفيف ممكن. تعالج الدالة "dwc2_pcgreg_read" القيمة "[0..127]". افحص الوسيطة الثالثة. فحص الأسطر: 1012، 1060.hcd-dwc2.c 1012

تحذير N5

V575 تعالج وظيفة "strerror_s" العناصر "0". افحص الوسيطة الثانية. أوامر 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);
    ....
  }
}

ترجع الدالة strerror_s وصفًا نصيًا لرمز خطأ النظام. يبدو توقيعه كالتالي:

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

المعلمة الأولى هي مؤشر إلى المخزن المؤقت حيث سيتم نسخ وصف النص ، والثاني هو حجم المخزن المؤقت ، والثالث هو رمز الخطأ. في الكود ، يتم تمرير 0 كحجم المخزن المؤقت ، ومن الواضح أن هذه قيمة خاطئة. بالمناسبة ، من الممكن أن تعرف مسبقًا عدد البايتات التي يجب تخصيصها: ما عليك سوى استدعاء strerrorlen_s ، والتي تُرجع طول الوصف النصي للخطأ. يمكن استخدام هذه القيمة لتخصيص مخزن مؤقت بحجم كافٍ.

تحذير N6

V595 تم استخدام المؤشر "blen2p" قبل أن يتم التحقق منه مقابل nullptr. فحص السطور: 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
....
}

في هذا الكود ، يتم استخدام قيمة وسيطة blen2p أولاً (التعليق 1) ثم التحقق من القيمة nullptr (التعليق 2). يبدو هذا المكان المريب للغاية وكأنك نسيت إدخال الشيك قبل الاستخدام الأول (تعليق 1). كإصلاح ، ما عليك سوى إضافة شيك:

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

لا يزال هناك سؤال حول حجة blen1p . على الأرجح ، يمكن أن يكون أيضًا مؤشرًا فارغًا ، وهنا ستحتاج أيضًا إلى إضافة تحقق. عدة إيجابيات مماثلة:

V595 تم استخدام مؤشر "المرجع" قبل أن يتم التحقق منه مقابل nullptr. فحص الأسطر: 2191، 2193.uri.c 2191
V595 تم استخدام مؤشر "cmdline" قبل أن يتم التحقق منه مقابل nullptr. فحص الأسطر: 420 ، 425.qemu-io.c 420
V595 تم استخدام المؤشر "dp" قبل أن يتم التحقق منه مقابل nullptr. فحص السطور: 288 ، 294. onenand.c 288
V595 تم استخدام المؤشر "omap_lcd" قبل أن يتم التحقق منه مقابل nullptr. فحص السطور: 81 ، 87. omap_lcdc.c 81

تحذير N7

V597 يمكن للمترجم أن يحذف استدعاء دالة "memset" ، والذي يستخدم لمسح كائن "op_info". يجب استخدام الدالة RtlSecureZeroMemory () لمسح البيانات الخاصة. 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);
  }
}

في هذا جزء التعليمات البرمجية، و memset يتم استدعاء الدالة ل op_info الكائن (تعليق 1)، وبعد ذلك يتم op_info حذف فورا، وهذا هو، وبعبارة أخرى، بعد لا يتم تعديل هذا الكائن تنظيف أي مكان آخر. هذا هو الحال بالضبط عندما يتمكن المترجم من إزالة استدعاء memset أثناء عملية التحسين . للقضاء على هذا السلوك المحتمل ، يمكنك استخدام وظائف خاصة لا يزيلها المترجم مطلقًا. راجع أيضًا مقالة " محو البيانات الخاصة بأمان ".

تحذير N8

V610 سلوك غير محدد. تحقق من عامل النقل ">>". المعامل الأيسر سالب ('number' = [-32768..2147483647]). كريس ج 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;
  ....
}

نظرًا لأن الرقم المتغير يمكن أن يكون سالبًا ، فإن التحول الأيمن باتجاه أحادي هو سلوك غير محدد. للتأكد من أن المتغير المعني يمكن أن يأخذ قيمة سالبة ، راجع التعليقين 1 و 2. لإزالة الاختلافات في سلوك التعليمات البرمجية الخاصة بك على الأنظمة الأساسية المختلفة ، يجب تجنب مثل هذه الحالات.

المزيد من التحذيرات:

V610 سلوك غير معرف. تحقق من عامل النقل "<<". المعامل الأيسر سالب ('(hclk_div - 1)' = [-1..15]). 1041
V610 سلوك غير معرف. تحقق من عامل النقل "<<". المعامل الأيسر '(target_long) - 1' سالب. تنوع exec.c 99
V610 سلوك غير معرف. تحقق من عامل النقل "<<". المعامل الأيسر سالب ('hex2nib (الكلمات [3] [i * 2 + 2])' = [-1..15]). 561

هناك أيضًا عدة تحذيرات من نفس النوع ، يتم استخدام -1 فقط كمعامل أيسر .

V610 سلوك غير معرف. تحقق من عامل النقل "<<". المعامل الأيسر "-1" سلبي. hppa.c 2702

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

تحذيرات أخرى مماثلة:

V610 سلوك غير معرف. تحقق من عامل النقل "<<". المعامل الأيسر "-1" سلبي. hppa.c 2718
V610 سلوك غير معرف. تحقق من عامل النقل "<<". المعامل الأيسر "-0x8000" سلبي. fmopl.c 1022
V610 سلوك غير معرف. تحقق من عامل النقل "<<". المعامل الأيسر '(intptr_t) - 1' سالب. 889

تحذير N9

V616 يتم استخدام الثابت المسمى "TIMER_NONE" بقيمة 0 في عملية البت. 179

#define HELPER(name) ....

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

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

يمكنك بسهولة التحقق من أن قيمة الماكرو TIMER_NONE هي صفر (التعليق 1). علاوة على ذلك ، يتم استخدام هذا الماكرو في عملية أحادي الاتجاه ، وستكون النتيجة دائمًا 0. ونتيجة لذلك ، لن يتم تنفيذ نص العبارة الشرطية إذا (env-> ttmr & TIMER_NONE) .

تحذير N10

V629 ضع في اعتبارك فحص التعبير 'n << 9'. تحويل البت لقيمة 32 بت مع التوسيع اللاحق لنوع 64 بت. 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);
    ....
  }
  ....
}

في جزء الكود هذا ، يتم إجراء عملية إزاحة على المتغير n ، الذي له نوع موقّع 32 بت ، ثم يتم توسيع هذه النتيجة الموقعة 32 بت إلى نوع موقّع 64 بت ، ثم ، كنوع غير موقّع ، تتم إضافته إلى إزاحة متغير 64 بت غير الموقعة . افترض أنه في الوقت الذي يتم فيه تنفيذ التعبير ، يحتوي المتغير n على بعض أهم 9 بتات. نحن نجري عملية إزاحة 9 بت ( BDRV_SECTOR_BITS) ، وهذا بدوره سلوك غير محدد ، ونتيجة لذلك يمكننا الحصول على البت المحدد في البت الأكثر أهمية. تذكر أن هذا البت في النوع الموقع مسؤول عن العلامة ، أي أن النتيجة يمكن أن تصبح سلبية. نظرًا لأن n متغير موقع ، فستؤخذ العلامة في الاعتبار عند التوسع. ثم تضاف النتيجة إلى متغير الإزاحة . من هذه الاعتبارات ، من السهل ملاحظة أن نتيجة تنفيذ التعبير قد تختلف عن النتيجة المقصودة. أحد الحلول الممكنة هو استبدال نوع المتغير n بنوع 64 بت بدون إشارة ، أي بـ uint64_t .

فيما يلي بعض المحفزات المشابهة:

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

تحذير N11

V634 أولوية العملية "*" أعلى من أولوية العملية "<<". من الممكن أن يتم استخدام الأقواس في التعبير. ناند.ج 310

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

إنه مجرد مكان مريب. ليس من الواضح ما الذي أراد المبرمج فعله في البداية: التحول أو الضرب. حتى إذا لم يكن هناك خطأ هنا ، فلا تزال بحاجة إلى إلقاء نظرة على الكود مرة أخرى ووضع الأقواس بشكل صحيح. هذا مجرد مكان واحد من تلك الأماكن التي يجب على المطورين النظر إليها للتأكد من صحة الخوارزمية الخاصة بهم. أماكن أخرى من هذا القبيل:

V634 أولوية العملية "*" أعلى من أولوية العملية "<<". من الممكن أن يتم استخدام الأقواس في التعبير. 449
V634 أولوية العملية "*" أعلى من أولوية العملية "<<". من الممكن أن يتم استخدام الأقواس في التعبير. 1235
V634 أولوية العملية "*" أعلى من أولوية العملية "<<". من الممكن أن يتم استخدام الأقواس في التعبير. 1264

تحذير N12

V646 جرب فحص منطق التطبيق. من المحتمل أن تكون الكلمة الرئيسية "else" مفقودة. 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 {
      ....
    }
    ....
  }
  ....
}

في هذا الكود ، بناءً على التنسيق ، فإن استخدام else if بدلاً من if يقترح نفسه بشكل مباشر . ربما نسوا إضافة أخرى هنا . ثم يمكن أن يكون خيار التصحيح كما يلي:

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

ومع ذلك ، هناك احتمال أن يكون كل شيء على ما يرام مع هذا الرمز ، وهناك تنسيق غير صحيح لنص البرنامج ، وهو أمر محير. ثم قد يبدو الرمز كما يلي:

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

تحذير N13

V773 تم إنهاء الوظيفة بدون تحرير مؤشر "القاعدة". من الممكن حدوث تسرب للذاكرة. 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
  ....
}

في هذا الرمز ، يتم تحديد كائن القاعدة (التعليق 1) وإضافته إلى القائمة للاستخدام اللاحق (التعليق 3) ، ولكن في حالة حدوث خطأ ، يتم إرجاع الوظيفة دون حذف كائن القاعدة الذي تم إنشاؤه مسبقًا (التعليق 2). هنا تحتاج فقط إلى معالجة الخطأ بشكل صحيح: احذف الكائن الذي تم إنشاؤه مسبقًا ، وإلا سيكون هناك تسرب للذاكرة.

تحذير N14

V781 يتم فحص قيمة الفهرس "ix" بعد استخدامه. ربما هناك خطأ في منطق البرنامج. uri.c 2110

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

هنا اكتشف المحلل صفيفًا محتملاً خارج الحدود. أولاً ، تتم قراءة عنصر المصفوفة ref-> path في الفهرس ix ، ثم يتم فحص ix للتحقق من صحتها ( ix> 0 ). الحل الصحيح هنا هو عكس هذه الإجراءات:

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

كان هناك العديد من هذه الأماكن:

V781 يتم فحص قيمة الفهرس "ix" بعد استخدامه. ربما هناك خطأ في منطق البرنامج. uri.c 2112
V781 يتم التحقق من قيمة فهرس "الإزاحة" بعد استخدامه. ربما هناك خطأ في منطق البرنامج. خرائط المفاتيح. c 125
V781 يتم التحقق من قيمة متغير "الجودة" بعد استخدامه. ربما هناك خطأ في منطق البرنامج. فحص الأسطر: 326 ، 335.vnc-enc-tight.c 326
V781 يتم التحقق من قيمة الفهرس "i" بعد استخدامه. ربما هناك خطأ في منطق البرنامج. mem_helper.c 1929

تحذير N15

V784 حجم قناع البت أقل من حجم المعامل الأول. سيؤدي هذا إلى فقدان بتات أعلى. رحمه الله 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]);
  ....
}

ينفذ هذا الرمز عملية بت على كائنات من أنواع مختلفة. المعامل الأيسر هو معامل val ، وهو نوع 64 بت بدون إشارة. يتم استخدام القيمة المستلمة لعنصر المصفوفة s-> regs_ro في فهرس الإزاحة ، والذي يحتوي على نوع بدون إشارة 32 بت ، كمعامل صحيح . نتيجة العملية على الجانب الأيمن (~ (s-> regs_ro [offset])) هي نوع 32 بت بدون إشارة ، وقبل الضرب بالبت ، سيتم توسيعه إلى نوع 64 بت مع أصفار ، أي بعد حساب التعبير بالكامل ، سيتم صفير جميع وحدات البت عالية الترتيب لمتغير val . تبدو مثل هذه الأماكن دائمًا مشبوهة. هنا لا يسعنا إلا أن نوصي المطورين بمراجعة هذا الرمز مرة أخرى. أكثر تشابهًا:

V784 حجم قناع البت أقل من حجم المعامل الأول. سيؤدي هذا إلى فقدان بتات أعلى. 199
V784 حجم قناع البت أقل من حجم المعامل الأول. سيؤدي هذا إلى فقدان بتات أعلى. 214
V784 حجم قناع البت أقل من حجم المعامل الأول. سيؤدي هذا إلى فقدان بتات أعلى. 418

تحذير N16

V1046 من الاستخدام غير الآمن لأنواع "bool" و "int غير الموقعة" معًا في العملية '& ='. المساعد. ج 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);
}

في جزء الكود هذا ، يتم تنفيذ عملية AND على مستوى بت على متغير hpd من النوع bool ونتيجة تنفيذ وظيفة extract32 ، التي لها النوع uint32_t . نظرًا لأن قيمة البت للمتغير المنطقي يمكن أن تكون 0 أو 1 فقط ، فإن نتيجة التعبير ستكون دائمًا خاطئة إذا كانت أقل قيمة بت ناتجة عن دالة extract32 هي صفر. لنلق نظرة على هذا بمثال. افترض أن hpd صحيح وأن الدالة ترجع 2 ، أي في التمثيل الثنائي ، ستبدو العملية مثل 01 & 10 = 0 ، وستكون نتيجة التعبير خاطئة . على الأرجح ، أراد المبرمج ضبط القيمة على صحيحإذا كانت الوظيفة ترجع شيئًا غير صفري. على ما يبدو ، يجب تصحيح الكود بحيث يتم إرسال نتيجة الوظيفة إلى النوع المنطقي ، على سبيل المثال ، مثل هذا:

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

خاتمة

كما ترى ، وجد المحلل العديد من الأماكن المشبوهة. ربما لم تظهر المشاكل المحتملة التي تم العثور عليها بعد بأي شكل من الأشكال ، لكن وجودها لا يمكن إلا أن يكون مقلقًا ، حيث إنها قادرة على إطلاق النار في أكثر اللحظات غير المتوقعة. يعد عرض جميع الأماكن المشبوهة مسبقًا وإصلاحها أفضل من إصلاح دفق لا نهاية له من الأخطاء. من الواضح ، بالنسبة للمشاريع المعقدة مثل هذه ، يمكن للتحليل الثابت أن يجلب فوائد ملموسة ، خاصة إذا كنت تنظم مراجعة منتظمة للمشروع. إذا كنت ترغب في تجربة PVS-Studio لمشروعك ، يمكنك تنزيل المحلل والحصول على مفتاح تجريبي مجاني في هذه الصفحة.

إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فيرجى استخدام رابط الترجمة: Evgeniy Ovsannikov. التحقق من QEMU باستخدام PVS-Studio .

التحقق من QEMU مع PVS-Studio

حول التحليل

خاتمة

More articles: