🈺 🙊 🌶️ 使用PVS-Studio检查QEMU 📨 🚋 🚴🏽

QEMU是一个相当知名的仿真应用程序。静态分析可以帮助QEMU等复杂项目的开发人员尽早发现错误，并总体上提高质量和可靠性。在本文中，将使用PVS-Studio静态分析工具检查QEMU应用程序的源代码是否存在潜在的漏洞和错误。

QEMU是旨在模拟跨平台硬件的免费软件。它使您可以在不同于目标的硬件平台上运行应用程序和操作系统，例如，为MIPS编写的应用程序可以在x86体系结构上运行。QEMU还支持仿真各种外设，例如视频卡，USB等。该项目非常复杂且值得关注，静态分析对此类项目很感兴趣，因此决定使用PVS-Studio检查其代码。

关于分析

该项目的源代码可以从github上的镜像中获取。该项目相当大，可以针对各种平台进行编译。为了简化代码检查，我们将使用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）{...} else if（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'语句。 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;
  }
  ....
}

在所考虑的代码中，条件运算符的then和else主体是相同的。在这里，很可能是复制粘贴。只需复制主体，然后复制分支，然后修复遗忘的内容即可。可以假定应该使用env代替其他对象。此可疑位置的修复程序可能如下所示：

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

只有该代码的开发人员才能明确地说出实际情况。另一个类似的地方：

V523'then'语句等效于'else'语句。翻译.c 641

警告N3

V547表达式'ret <0'始终为false。第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）。在更高的条件下，在第一个条件下，我们针对-EAGAIN检查了ret的值（注释2），因此表达式ret <0的结果将始终为false。也许handle_dependencies函数用于返回不同的值，但是后来由于重构等原因，行为发生了变化。在这里，您只需要完成重构。类似的触发器：

V547表达式始终为假。qcow2.c 1070
V547表达式's-> state！= MIGRATION_STATUS_COLO'始终为false。彩色595
V547表达式's-> metadata_entries.present＆0x20'始终为false。vhdx.c 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在针对nullptr对其进行验证之前，已使用了'blen2p'指针。检查行：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进行验证之前，已使用了'ref'指针。检查行：2191，2193.uri.c 2191
V595在对nullptr进行验证之前，已使用“ cmdline”指针。检查线：420，425.qemu-io.c 420
V595在针对nullptr进行验证之前，已使用了'dp'指针。检查行：288，294。onenand.c 288
V595在对nullptr进行验证之前，已使用'omap_lcd'指针。检查行：81，87。omap_lcdc.c 81

警告N7

V597编译器可能会删除“ memset”函数调用，该函数调用用于刷新“ op_info”对象。RtlSecureZeroMemory（）函数应用于擦除私有数据。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);
  }
}

在此代码段中，将为op_info对象（注释1）调用memset函数，此后立即删除op_info，也就是说，在清除该对象后，不会在其他任何地方修改此对象。当编译器可以在优化过程中删除memset调用时，就是这种情况。为了消除这种潜在的行为，您可以使用编译器从不删除的特殊功能。另请参阅文章“安全清除私有数据”。N8 V610未指定行为警告。检查移位运算符“ >>”。左操作数为负（“数字” = [-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;
  ....
}

由于变量号可以为负，因此按位右移是未指定的行为。为确保所讨论的变量可以取负值，请参见注释1和2。为消除代码在不同平台上的行为差异，应避免这种情况。

更多警告：

V610未定义的行为。检查移位运算符“ <<”。左操作数为负（'（（hclk_div-1）'= [-1..15]））。aspeed_smc.c 1041
V610未定义的行为。检查移位运算符“ <<”。左操作数'（target_long）-1'为负。可执行文件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'为负。sve_helper.c 889

警告N9

V616在按位运算中使用值为0的名为“ TIMER_NONE”的常量。sys_helper.c 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。因此，将永远不会执行条件语句if（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);
    ....
  }
  ....
}

在此代码段中，对具有32位带符号类型的变量n执行移位操作，然后将此32位带符号结果扩展为64位带符号类型，然后将其作为无符号类型添加到无符号64位变量offset中。假设在执行表达式时，变量n具有一些最高有效9位。我们正在执行9位移位操作（BDRV_SECTOR_BITS），这又是未定义的行为，因此，我们可以将最高有效位设为置位。回想一下，有符号类型中的该位负责符号，即结果可能变为负数。由于n是一个有符号变量，因此在扩展时将考虑该符号。然后将结果添加到offset变量中。从这些考虑，很容易看出执行表达式的结果可能与预期的结果不同。一种可能的解决方案是用64位无符号类型（即uint64_t）替换变量n的类型。以下是一些类似的触发器：

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 '*'操作的优先级高于'<<'操作的优先级。表达式中可能应使用括号。nand.c 310

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

这只是一个可疑的地方。程序员一开始想做什么是不清楚的：移位或乘法。即使这里没有错误，您仍然需要再次查看代码并正确放置括号。这只是开发人员应确保其算法正确的地方之一。其他此类地方：

V634'*'操作的优先级高于'<<'操作的优先级。表达式中可能应使用括号。exynos4210_mct.c 449
V634'*'操作的优先级高于'<<'操作的优先级。表达式中可能应使用括号。exynos4210_mct.c 1235
V634'*'操作的优先级高于'<<'操作的优先级。表达式中可能应使用括号。exynos4210_mct.c 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)) {
  ....
}

在此，分析仪已检测到电位超出范围阵列。首先，读取索引ix处ref->路径数组的元素，然后检查ix的正确性（ix> 0）。正确的解决方案是撤销这些操作：

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

有几个这样的地方：

V781使用后，检查“ ix”索引的值。程序逻辑中可能有一个错误。uri.c 2112
V781使用“ offset”索引后，将对其进行检查。程序逻辑中可能有一个错误。keymaps.c 125
V781使用“品质”变量后，将对其进行检查。程序逻辑中可能有一个错误。检查线：326，335.vnc-enc-tight.c 326
V781使用“ i”索引后，对其进行检查。程序逻辑中可能有一个错误。mem_helper.c 1929

警告N15

V784位掩码的大小小于第一个操作数的大小。这将导致丢失更高的位。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]);
  ....
}

此代码对不同类型的对象执行按位运算。左侧的操作数是参数val，它是64位无符号类型。偏移量索引处的数组元素s-> regs_ro的接收值（具有32位无符号类型）用作右操作数。右侧的运算结果（〜（s-> regs_ro [offset]））为32位无符号类型，在按位乘法运算之前，它将扩展为零的64位类型，也就是说，在对整个表达式求值之后，val变量的所有高阶位将被清零。这样的地方看起来总是可疑的。在这里，我们只能建议开发人员再次修改此代码。更多类似：

V784位掩码的大小小于第一个操作数的大小。这将导致丢失更高的位。第199章
V784位掩码的大小小于第一个操作数的大小。这将导致丢失更高的位。soc_dma.c 214
V784位掩码的大小小于第一个操作数的大小。这将导致丢失更高的位。418章

警告N16

V1046在操作'＆='中不安全地同时使用'bool'和'unsigned int'类型。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);
}

在此代码片段中，对类型为bool的hpd变量和执行类型为uint32_t的extract32函数的结果进行按位与运算。由于布尔变量的位值只能为0或1，因此如果extract32函数返回的最低有效位为零，则表达式的结果将始终为false。让我们来看一个例子。假设hpd为true，并且该函数返回2，即以二进制表示，该操作将看起来像01＆10 = 0，并且表达式的结果为false。程序员最有可能希望将值设置为true如果函数返回非零值。显然，需要对代码进行更正，以便将函数的结果强制转换为bool类型，例如，如下所示：

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

结论

如您所见，分析仪发现了许多可疑的地方。也许发现的潜在问题尚未显现出来，但是它们的出现令人震惊，因为它们能够在最意外的时刻引发火灾。事先查看并修复所有可疑位置比修复无尽的错误流要好。显然，对于像这样的复杂项目，静态分析可以带来明显的好处，特别是如果您组织定期的项目审查。如果要为项目尝试使用PVS-Studio，则可以下载分析仪并在此页面上获得免费的试用版密钥。

如果您想与说英语的读者分享这篇文章，请使用翻译链接：Evgeniy Ovsannikov。使用PVS-Studio检查QEMU。

使用PVS-Studio检查QEMU

关于分析

结论

More articles: