Embox是用于嵌入式系统的跨平台,多任务实时操作系统。它旨在在低计算资源下工作,并允许您在微控制器上运行基于Linux的应用程序,而无需使用Linux本身。当然,像其他任何应用程序一样,Embox错误也无法幸免。本文专门分析Embox项目代码中发现的错误。几个月前,我已经写了一篇有关检查FreeRTOS 的文章,FreeRTOS是另一个用于嵌入式系统的OS。那时我没有发现错误,但是在开发他们自己的FreeRTOS版本时,我在亚马逊的家伙添加的库中找到了它们。从某种意义上讲,您现在看到的这篇文章延续了上一篇的主题。我们经常收到检查FreeRTOS的请求,而我们做到了。这次,没有要求检查特定项目的请求,但是我开始收到喜欢它并想要更多东西的嵌入式开发人员的来信和评论。好吧,《 PVS-Studio for Embedded》杂志的新一期已经成熟,摆在您面前。喜欢看!分析
使用PVS-Studio(用于C,C ++,C#和Java的静态代码分析器)进行了分析。在进行分析之前,需要对项目进行组装-因此,我们将确保项目代码正在运行,并且还将为分析人员提供收集组装信息的机会,这些信息对于更好的代码验证很有用。在指令正式 Embox 仓库报价的能力,不同的系统(Arch Linux的,MacOS的,Debian的),并使用泊坞窗下构建。我决定为自己的生活增添些变化,并从Debian(我最近安装在虚拟机上)进行构建和分析。大会进行得很顺利。现在有必要进行分析。Debian是受支持的基于Linux的PVS-Studio系统之一。在Linux下检查项目的一种便捷方法是编译跟踪。这是一种特殊模式,在该模式下,分析仪将收集有关装配的所有必要信息,以便您一键即可开始分析。我需要做的就是:1)下载并安装PVS-Studio;2)通过使用Embox转到文件夹并在终端中键入来启动程序集跟踪pvs-studio-analyzer analyze -- make
3)等待组装完成后,运行命令:pvs-studio-analyzer analyze -o /path/to/output.log
4)将原始报告转换为任何方便的格式。该分析仪带有一个特殊的实用程序PlogConverter,您可以使用它执行此操作。例如,将报告转换为任务列表的命令(例如,在QtCreator中进行查看)将如下所示:plog-converter -t tasklist -o /path/to/output.tasks /path/to/project
所有!完成这些步骤只花了我不到15分钟的时间。报告已准备就绪,现在您可以查看错误了。好吧,让我们开始吧:)怪周期
分析器发现的错误之一是奇怪的while循环: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);
....
}
PVS-Studio 警告:V715 “ while”操作员的身体空了。检测到可疑模式:“ while(expr){...} while(dp.skip!= 0);”。dd.c 225 hm。真是怪异的循环。while表达式(dp.skip!= 0)被写入两次,一次在循环的上方,第二次在循环的下方。实际上,现在有两个不同的循环:一个循环包含花括号,而第二个循环为空。在这种情况下,将永远不会执行第二个循环。下面是具有类似条件的do ... while循环,这使我想到:奇怪的循环原本是指do ... while,但出了点问题。我认为这段代码很可能包含逻辑错误。内存泄漏
是的,不是没有他们。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;
}
....
}
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
该函数在其内部创建局部变量newpatharg和oldpatharg。这些指针被分配了使用calloc在内部分配的新内存位置的地址。如果分配内存时出现问题,则calloc返回空指针。如果只分配了一个内存块怎么办?该函数将崩溃,而不会释放任何内存。恰好已分配的站点将保留在内存中,而没有任何机会再次访问它并释放它以供将来使用。内存泄漏的另一个示例更亮一些: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;
}
....
}
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
在这里,程序员已经显示出准确性,并且正确地处理了仅分配一块内存的情况。正确处理了……在下一个表达式中,从字面上看,这又犯了一个错误。通过正确编写的检查,我们可以确保在执行表达式时返回-EINVAL;。我们肯定会为read_buf和write_buf分配内存。因此,从该函数返回后,我们将同时发生两次泄漏。我认为,与传统PC相比,嵌入式设备上的内存泄漏可能会更加痛苦。在资源严重受限的情况下,您需要特别仔细地监视它们。指针处理不当
以下错误代码简洁明了: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;
}
....
}
PVS-Studio警告: V595在针对nullptr进行验证之前,已使用了'sdev'指针。检查行:116,118。scsi_disk.c 116在检查sdev指针是否为NULL之前就已取消对其的引用。逻辑上假设,如果有人写了这样的支票,则该指针可能为空。在这种情况下,我们有可能在字符串blksize = sdev-> blk_size中取消引用空指针。错误是支票不在需要的地方。它应该在“ sdev = bdev-> privdata; ”行之后,但应在“ blksize = sdev-> blk_size; ” 行之前。这样就可以避免对零地址的潜在吸引力。PVS-Studio在以下代码中发现了另外两个错误: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);
....
}
PVS-Studio警告:- V769'xs- > extra.rec.in_base + recvsz'表达式中的'xs-> extra.rec.in_base'指针可以为nullptr。在这种情况下,结果值将毫无意义,因此不应使用。检查行:56,48。xdr_rec.c 56
- V769'buff + recvsz'表达式中的'buff'指针可以为nullptr。在这种情况下,结果值将毫无意义,因此不应使用。检查行:61,48。xdr_rec.c 61
buf指针使用malloc初始化,然后其值用于初始化其他指针。malloc函数可以返回空指针,应始终检查该指针。似乎这是一个断言检查buf是否为NULL的断言,并且一切正常。但不是!事实是使用assert进行调试,并且在Release配置中构建项目时,该assert将被删除。事实证明,在Debug中工作时,程序将正确运行,而在Release中进行构建时,空指针将进一步“运行”。使用空值算术运算中的错误是不正确的,因为这样的运算结果将毫无意义,并且无法使用这种结果。这就是分析仪警告我们的内容。有人可能会争辩说,在malloc / realloc / calloc之后不检查指针是无所畏惧的。就像,在第一次由空指针访问时,将发生信号/异常,并且不会有任何错误。在实践中,一切都更加复杂,如果缺少验证对您而言似乎并不危险,我建议您看一下文章“ 为什么检查malloc函数返回什么很重要 ”。数组处理不正确
以下错误与上一个示例非常相似:
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警告: V781使用“ offt”索引后,将对其进行检查。程序逻辑中可能有一个错误。fat_common.c 1813 首先在索引操作中使用变量offt,然后才检查其值是否大于零。但是,如果name变成空字符串,会发生什么?strlen()函数将返回0,然后在腿部大声射击。该程序将以负索引访问,这将导致未定义的行为。任何事情都可能发生,包括程序崩溃。乱!可疑情况
没有他们的地方呢?从字面上看,我们在检查的每个项目中都会发现此类错误。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;
}
PVS-Studio警告: V617考虑检查状况。 “ |”的“ 0x0010”参数按位运算包含一个非零值。 index_descriptor.c 55要了解错误所在,请查看FD_CLOEXEC常量的定义:#define FD_CLOEXEC 0x0010
事实证明,在表达式中,如果按位“或”右边的if(cloexec | FD_CLOEXEC)始终存在一个非零常数。这样的运算结果将始终为非零数。因此,此表达式将始终等同于if(true)表达式,并且我们将始终仅处理if-expression的then分支。我怀疑此宏常量用于预配置Embox操作系统,但是即使这样,始终为true的条件看起来还是很奇怪。也许他们想在这里使用&运算符,但是他们打错了字。整数除法
以下错误与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);
....
}
PVS-Studio 警告: V636'1024 / dev_bsize'表达式已从'int'类型隐式转换为'float'类型。考虑使用显式类型转换以避免丢失小数部分。例如:double A =(double)(X)/ Y;。 ext2.c 777此功能如下:如果我们将两个整数值相除,则相除的结果将为整数。因此,将进行除法运算而没有余数,换句话说,将从除法结果中舍弃小数部分。有时程序员会忘记它,并且会出现这样的错误。 SBSIZE常量和变量dev_bsize具有整数类型(分别为int和size_t)。因此,SBSIZE / dev_bsize表达式的结果也将是整数类型。但请稍等。变量dev_factor的类型为float!显然,程序员希望得到分数除法结果。如果您注意此变量的进一步使用,则可以进一步验证。例如,ext2_dflt_sb函数(其中dev_factor作为第三个参数传递)具有以下签名:static void ext2_dflt_sb(struct ext2sb *sb, size_t dev_size, float dev_factor);
类似地,在其他使用dev_factor变量的地方:一切都表明需要浮点数。要更正此错误,仅将除法操作数之一转换为实型就足够了。例如:dev_factor = float(SBSIZE) / dev_bsize;
然后除法的结果将是一个分数。未验证的输入
以下错误与使用从程序外部接收的未经验证的数据有关。int main(int argc, char **argv) {
int ret;
char text[SMTP_TEXT_LEN + 1];
....
if (NULL == fgets(&text[0], sizeof text - 2,
stdin)) { ret = -EIO; goto error; }
text[strlen(&text[0]) - 1] = '\0';
....
}
PVS-Studio警告: V1010索引“ strlen(&text [0])”中使用了未经检查的污染数据。sendmail.c 102首先,考虑fgets函数返回什么。如果成功读取一行,该函数将返回指向该行的指针。如果在读取至少一个元素之前读取遇到文件结尾,或者发生输入错误,则fgets函数将返回NULL。因此表达式为NULL == fgets(....)检查收到的输入是否正确。但是有一个警告。如果将空终端作为要读取的第一个字符进行传输(例如,可以通过在Windows命令行的Legacy模式下按Ctrl + 2来完成),则fgets函数将其视为不返回NULL。在这种情况下,要进行记录的行将只有一个元素-' \ 0 '。接下来会发生什么?表达式strlen(&text [0])将返回0。结果,我们得到一个负索引调用:text[ 0 - 1 ] = '\0';
结果,我们可以通过简单地将行终止符传递给输入来“翻转”程序。这很尴尬,可能会被用来攻击使用Embox的系统。正在开发此诊断规则的我的同事甚至以NcFTP项目受到这种攻击的示例作了记录:我建议您看看您是否仍然不相信这样的机会:)此外,分析仪还发现了另外两个具有相同错误的地方:- V1010未检查的污染数据用于索引:“ strlen(&from [0])”。sendmail.c 55
- V1010未检查的污染数据用于索引:“ strlen(&到[0])”。sendmail.c 65
米斯拉
MISRA是用于为高度负责的嵌入式系统编写安全的C和C ++代码的一组准则和准则。从某种意义上讲,这是一本培训手册,在此手册之后,您不仅可以摆脱所谓的“代码异味”,而且还可以保护程序免受漏洞侵害。MISRA用于医疗,汽车,飞机和军工行业中人们生活取决于嵌入式系统质量的场合。PVS-Studio具有广泛的诊断规则,使您可以检查代码是否符合MISRA C和MISRA C ++标准。默认情况下,具有这些诊断的模式是关闭的,但是由于我们正在寻找嵌入式系统项目中的错误,因此没有MISRA我无能为力。这是我设法找到的:
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;
}
PVS-Studio警告: V2548 [MISRA C 18.6]本地阵列'namebuf'的地址不应存储在该阵列范围之外。 ext2.c 298分析仪检测到可疑的分配,有可能导致不确定的行为。让我们仔细看一下代码。在这里,namebuf是在函数的本地范围内创建的数组,并且cp指针通过指针传递给函数。根据C语法,数组的名称是指向存储该数组的存储区中第一个元素的指针。事实证明,表达式* cp = namebuf会将数组namebuf的地址分配给cp指向的变量。由于cp是通过指针传递给函数的,因此它指向的值的更改将反映在调用函数的位置。事实证明,在ext2_read_symlink函数完成其工作之后,其第三个参数将指示namebuf数组曾经占据的区域。只有一个“但是”:由于namebuf是在堆栈上保留的数组,因此在函数退出时将被删除。因此,存在于函数外部的指针将指向已释放的内存。该地址将是什么?无法预测。一段时间后,阵列的内容可能会继续保留在内存中,或者程序可能会立即使用其他内容立即擦除该区域。通常,访问此类地址将返回未定义的值,并且使用此类值是严重错误。分析仪还发现了另一个警告相同的错误:- V2548 [MISRA C 18.6]本地变量'dst_haddr'的地址不应存储在该变量范围之外。net_tx.c 82
结论
我喜欢与Embox项目一起工作。尽管我没有写出本文中发现的所有错误,但警告的总数却相对较少,并且总的来说,项目代码的编写质量很高。因此,我对开发人员以及代表社区为该项目做出贡献的开发人员表示感谢。你很棒!我借此机会向图拉的开发人员致以问候。我会相信在圣彼得堡现在还不是很冷:)这就是我的文章结束的地方。我希望您喜欢阅读它,并为自己找到了一些新东西。如果您对PVS-Studio感兴趣,并想独立验证使用它的某些项目,请下载并尝试。这将不超过15分钟。
如果您想与讲英语的读者分享本文,请使用翻译链接:George Gribkov。关于再次嵌入:在Embox项目中搜索错误。