2016-01-07

Поиск ошибок в Android ядре при помощи PVS-Studio v6.0

Поиск ошибок в Linux ядре уже проводился год назад создателями PVS-Studio. Теперь настала пора Android ядра. Но я не буду брать для исследования последнюю версию ядра для Nexus устройств, а возьму ядро, которое я использую в своём китайском смартфоне (разработка 2012 года). Кроме описания найденных интересных ошибок постараюсь подробно описать процесс получения plog-файла, который содержит все найденные ошибки и необходим для удобной обработки этих ошибок в специальной утилите PVS-Studio.


В качестве объекта исследования будет выступать ядро v3.4, которое я использую в прошивке CyanogenMod 13 для аппарата Innos D9 (в простонародье Highscreen Boost или DNS s4502). Это ядро является форком ядра chil360-kernel, которое является форком ядра для аппаратов Huawei Ascend Y300/G510. В свою очередь ядро от Huawei является форком Qualcomm ядра для платформы msm7x27a.

Для решения поставленной задачи нам понадобится Linux и Windows среда. Лично я использую две физические машины: Windows 7 и Ubuntu 14.04 (обе системы 64-битные). Но вполне допустимо использовать Ubuntu посредством эмуляции через VMWare, т.к. сборка Android ядра не требует много ресурсов.

Займёмся подготовкой обоих сред для выполнения задачи. На Windows нам потребуется только установленный пакет PVS-Studio (у меня в примерах будет фигурировать путь установки C:\PVS-Studio). А на Linux системе нужно для начала создать рабочую директорию (пусть будет называться dev) и скачать в неё необходимые исходники и средства сборки:
cd /home/acdev
mkdir dev
cd dev
git clone https://bitbucket.org/UBERTC/arm-eabi-4.9.git
git clone https://github.com/jsr-d9/android_kernel_jsr_msm8625.git

Теперь открываем файлик /home/acdev/dev/android_kernel_jsr_msm8625/scripts/gcc-wrapper.py и добавляем в него новый функционал:
def gen_pvs_data():
    if len(sys.argv) < 5 :
        return

    fn = sys.argv[-1]
    if fn[-2:] != ".c" :
        return
   
    alist = sys.argv[1:]
    for idx, item in enumerate(alist) :
        if item == '-E' :
            return

    sfn = sys.argv[0]
    klist = sfn.rsplit('/', 2)
    kdir = klist[0]
    pdir = os.environ['PVS_STUDIO_PRJ']
    ifn = fn[:-1]
    ifn += 'i'

    alist[-1] = '-E'
    alist.append(kdir + '/' + fn)

    for idx, item in enumerate(alist) :
        if item == '-o' :
            alist[idx+1] = kdir + '/' + ifn

    for idx, item in enumerate(alist) :
        if item[:2] == '-I' and item[2] != '/' :
            alist[idx] = '-I' + kdir + '/' + item[2:]

    try:
        proc = subprocess.Popen(alist)
        result = proc.wait()
    except OSError as e:
        result = e.errno

    if os.path.isfile(ifn) != True :
        return

    c = open(ifn, 'r')
    idata = c.read()
    c.close()
    idata = idata.replace(kdir, pdir.replace('\\', '/'))
    gcclnx = os.environ['PVS_STUDIO_GCC_LNX']
    gccwnd = os.environ['PVS_STUDIO_GCC_WND']
    idata = idata.replace(gcclnx, gccwnd.replace('\\', '/'))
    c = open(ifn, 'w')
    c.write(idata)
    c.close()

    c = open(os.environ['PVS_STUDIO_CFG'], 'r')
    cfg = c.read()
    c.close()
    cfg = cfg.replace('{pd}', pdir)
    cfg = cfg.replace('{fn}', fn.replace('/', '\\'))
    cfg = cfg.replace('{ifn}', ifn.replace('/', '\\'))

    cdir = os.path.dirname(fn)
    ext = os.environ['PVS_STUDIO_EXT']
    cfgfn = fn + ext
    c = open(cfgfn, 'w')
    c.write(cfg)
    c.close()   

    exe = os.environ['PVS_STUDIO_EXE']
    tcf = pdir + '\\' + fn.replace('/', '\\') + ext
    cmd = open(os.environ['PVS_STUDIO_CMD'], 'a')
    cmd.write(exe + ' --cfg ' + tcf + "\r\n")
    cmd.close()
Примечание: В моём репозитории на github имеется соответствующий патч скрипта gcc-wrapper.py.
Функция gen_pvs_data создаёт рядом с каждым c-файлом препроцессорный i-файл и cfg-файл для PVS-Studio. Так же на выходе создаётся cmd-файл, который будет запускать обработку всех созданных файлов в анализаторе PVS-Studio. Т.е. функция gen_pvs_data создаёт всё необходимое для обработки исходников Android ядра в анализаторе PVS-Studio, запущенного в Windows среде.

В директории /home/acdev/dev создаём файл pvs.cfg с таким содержанием:
source-file = {pd}\{fn}
output-file = {pd}\pvs.log
new-output-format=yes
exclude-path = *\arm-eabi\*
exclude-path = *\include\asm\*
analysis-mode=4
independent=no
skip-cl-exe = yes
i-file = {pd}\{ifn}
timeout=600

platform = ARMV4
language = C
funsigned-char = no
stage=inprogress
remove-intermediate-files=no
preprocessor= gcc

Файл pvs.cfg является шаблоном для создания cfg-файла для каждого c-файла.

Теперь переходим в директорию /home/acdev/dev/android_kernel_jsr_msm8625 и создаём в ней bash-скрипт bld.sh, при помощи которого будем запускать сборку ядра и генерацию необходимых для PVS-Studio файлов:
#!/bin/bash

export PVS_STUDIO=1
export PVS_STUDIO_CMD='pvs.cmd'
export PVS_STUDIO_CFG='/home/acdev/dev/pvs.cfg'
export PVS_STUDIO_EXT='.PVS-Studio.cfg'
export PVS_STUDIO_EXE='c:\PVS-Studio\x64\PVS-Studio.exe'
export PVS_STUDIO_PRJ='d:\dev\android_kernel_jsr_msm8625'

export PVS_STUDIO_GCC_LNX='/home/acdev/dev/arm-eabi-4.9'
export PVS_STUDIO_GCC_WND='d:\dev\arm-eabi-4.9'

rm -f $PVS_STUDIO_CMD
echo "" > $PVS_STUDIO_CMD

make clean
find . -name '*.i' -delete
find . -name "*$PVS_STUDIO_EXT" -delete

make ARCH=arm jsr_d9_defconfig
make -j1 ARCH=arm CROSS_COMPILE=/home/acdev/dev/arm-eabi-4.9/bin/arm-eabi-

Можно заметить, что скрипт bld.sh кроме непосредственно запуска сборки содержит параметры, которые требуются для работы функции gen_pvs_data. Значения этих параметров следует откорректировать в соответствии с используемым Linux и Windows окружением.

Хочу отметить, что при запуске этого скрипта Android ядро будет собираться только одним потоком (опция -j1). Это ограничение вызвано тем, что при многопоточной сборке нужно синхронизировать запись строк в файл pvs.cmd, а эту задачу мне решать на питоне не хочется совсем (я впервые пишу на питоне).

Примечание: Файл pvs.cmd можно генерировать не в gcc-wrapper.py, а в скрипте bld.sh, т.к. после сборки ядра достаточно найти все файлы с расширением ".PVS-Studio.cfg". При этом можно будет смело использовать многопоточную сборку.

Настало время запускать скрипт bld.sh. Just do it!

После завершения сборки следует удалить все объектные файлы и упаковать директорию android_kernel_jsr_msm8625 в ZIP-архив:
cd /home/acdev/dev/android_kernel_jsr_msm8625
find . -name '*.o' -delete
cd ..
zip -r pvs-data.zip android_kernel_jsr_msm8625
Как мне кажется, перенести ядро на Windows проще и быстрее именно в ZIP-архиве.
После копирования полученного ZIP-архива на Windows его содержимое следует распаковать в директорию, которая указана в параметре PVS_STUDIO_PRJ. Затем следует стартануть pvs.cmd (содержится в ZIP-архиве), что запустит последовательную обработку каждого c-файла анализатором PVS-Studio. Данный анализ будет длится очень долго.

Примечание: Что бы ускорить анализ исходников ядра следует разделить файл pvs.cmd на несколько частей. Причём количество частей не должно превышать количества процессорных ядер, доступных на Windows машине. Тогда каждый запущенный pvsXXX.cmd будет выполняться параллельно.

После завершения анализа в директории PVS_STUDIO_PRJ появится файл pvs.log, содержащий все найденные предупреждения. Этот файл следует открывать при помощи Standalone.exe (находится в директории PVS-Studio). При этом в диалоге открытия файла следует обязательно указать тип файла "Unparsed output". Теперь уже можно приступить к просмотру всех найденных предупреждений (при этом присутствует привязка к исходникам).

В процессе ручного анализа предупреждений следует не забывать помечать реальные ошибки звёздочкой. Результаты работы следует сохранить в plog-файл и в дальнейшем использовать его для исследования.

В моём случае количественные результаты анализа таковы:
  • проанализировано 1556 файлов
  • найдено 1465 предупреждений первого уровня
  • найдено 238 предупреждений второго уровня
  • найдено 711 предупреждений третьего уровня
  • возникло 4 ошибки анализа кода (V002)  
Т.е. всего найдено 2414 предупреждения. И среди всего этого многообразия предупреждений следует вручную выискивать реальные ошибки. В моём случае таких ошибок оказалось менее 40 штук (менее 2%). Такое соотношение предупреждений и ошибок могу объяснить тем, что анализатор PVS-Studio совсем не заточен под "сишную стилистику кода", которая используется в Linux-ядре.

Ну а теперь разберём самые интересные ошибки.


Предупреждение PVS-Studio: V512 A call of the 'memcpy' function will lead to overflow of the buffer '& vout->fb.fmt'
Файл: drivers/media/video/msm/msm_v4l2_video.c
static int
msm_v4l2_overlay_open(struct file *file)
{
    struct msm_v4l2_overlay_device *vout = NULL;
    struct v4l2_pix_format *pix = NULL;
    struct msm_v4l2_overlay_fh *fh;

    vout = saved_vout0;
    ....
    pix    = &vout->pix;
    pix->width   = vout->screen_width;
    pix->height  = vout->screen_height;
    pix->pixelformat = V4L2_PIX_FMT_RGB32;
    ....
    memcpy(&vout->fb.fmt, pix, sizeof(struct v4l2_format));
    ....
    return 0;
}
Размер структуры v4l2_format намного больше, чем размер v4l2_pix_format.
Для исправления бага следует заметить v4l2_format на v4l2_pix_format.


Предупреждение PVS-Studio: V547 Expression 'i >= 0' is always true. Unsigned type value is always >= 0
Файл: arch/arm/mach-msm/qdsp5/audio_amrnb_in.c
static void audamrnb_in_flush(struct audio_amrnb_in *audio)
{
    uint8_t i;
    ....
    for (i = FRAME_NUM-1; i >= 0; i--) {
        audio->in[i].size = 0;
        audio->in[i].read = 0;
    }
    ....
}
Тип переменной i явно должен быть int32_t.

Аналогичная ошибка присутвует и в функции audamrnb_out_flush того же файла.


Предупреждение PVS-Studio: V562 It's odd to compare 0 or 1 with a value of 3: !pdata->target_hw == MSM_8x25
Файл: drivers/hwmon/msm_adc.c
static int __devinit msm_rpc_adc_device_init(struct platform_device *pdev)
{
    struct msm_adc_platform_data *pdata = pdev->dev.platform_data;
    struct adc_dev *adc_dev;
    int i, rc;
    ....
        if (!pdata->target_hw == MSM_8x25) {
            /* DAL device lookup */
            rc = msm_adc_getinputproperties(msm_adc, adc_dev->name, &target);
            if (rc) {
                dev_err(&pdev->dev, "No such DAL device[%s]\n", adc_dev->name);
                goto dev_init_err;
            }

            adc_dev->transl.dal_dev_idx = target.dal.dev_idx;
            adc_dev->nchans = target.dal.chan_idx;
        } else {
            /* On targets prior to MSM7x30 the remote driver has
               only the channel list and no device id. */
            adc_dev->transl.dal_dev_idx = MSM_8x25_ADC_DEV_ID;
            adc_dev->nchans = MSM_8x25_CHAN_ID;
        }
    ....
    return 0;

dev_init_err:
    msm_rpc_adc_teardown_devices(pdev);
    return rc;
}
Здесь явно используется некорректное условие.


Предупреждение PVS-Studio: V579 The strlcpy function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument.
Файл: drivers/mmc/core/sdio_cis.c
static int cistpl_vers_1(struct mmc_card *card, struct sdio_func *func,
             const unsigned char *buf, unsigned size)
{
    unsigned i, nr_strings;
    char **buffer, *string;
    ....
    buffer = kzalloc(sizeof(char*) * nr_strings + size, GFP_KERNEL);
    if (!buffer)
        return -ENOMEM;

    string = (char*)(buffer + nr_strings);

    for (i = 0; i < nr_strings; i++) {
        buffer[i] = string;
        strlcpy(string, buf, sizeof(string));
        string += strlen(string) + 1;
        buf += strlen(buf) + 1;
    }
    ....
    return 0;
}
Тут длинные строки будут обрезаны до 3 символов. Что бы этого избежать, нужно копирование строк производить таким образом:
strcpy(string, buf);


Предупреждение PVS-Studio: V595 The 'mfd' pointer was utilized before it was verified against nullptr. Check lines: 663, 665
Файл: drivers/video/msm/msm_fb.c
static int msm_fb_remove(struct platform_device *pdev)
{
    struct msm_fb_data_type *mfd;

    MSM_FB_DEBUG("msm_fb_remove\n");

    mfd = (struct msm_fb_data_type *)platform_get_drvdata(pdev);

    msm_fb_pan_idle(mfd);

    msm_fb_remove_sysfs(pdev);

    pm_runtime_disable(mfd->fbi->dev);

    if (!mfd)
        return -ENODEV;
    ....
    return 0;
}
Тут проверка на NULL должна производиться сразу после вызова platform_get_drvdata.


Предупреждение PVS-Studio: V519 The 'ret' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 4549, 4551
Файл: drivers/video/msm/msm_fb.c
static int msm_fb_ioctl(struct fb_info *info, unsigned int cmd,
            unsigned long arg)
{
    struct msm_fb_data_type *mfd = (struct msm_fb_data_type *)info->par;
    void __user *argp = (void __user *)arg;
    ....
    int ret = 0;
    msm_fb_pan_idle(mfd);

    switch (cmd) {
    ....
    case MSMFB_METADATA_SET:
        ret = copy_from_user(&mdp_metadata, argp, sizeof(mdp_metadata));
        if (ret)
            return ret;
        ret = msmfb_handle_metadata_ioctl(mfd, &mdp_metadata);
    case MSMFB_DISPLAY_COMMIT:
        ret = msmfb_display_commit(info, argp);
        break;

    case MSMFB_METADATA_GET:
        ret = copy_from_user(&mdp_metadata, argp, sizeof(mdp_metadata));
    ....
    }

    return ret;
}
Хоть анализатор и указал на двойное присвоение значения переменной ret, но на самом деле тут иная ошибка: отсутствие неоходимого оператора break.
Есть большая вероятность, что break был потерян в результате поспешного "мержинга" веток в CAF репозитории.


Предупреждение PVS-Studio: V640 The code's operational logic does not correspond with its formatting. The statement is indented to the right, but it is always executed. It is possible that curly brackets are missing
Файл: arch/arm/mach-msm/qdsp5/audio_aac_in.c
static long audaac_in_ioctl(struct file *file,
                unsigned int cmd, unsigned long arg)
{
    struct audio_aac_in *audio = file->private_data;
    int rc = 0;
    ....
    mutex_lock(&audio->lock);
    switch (cmd) {
    ....
    case AUDIO_SET_STREAM_CONFIG: {
        struct msm_audio_stream_config cfg;
        if (copy_from_user(&cfg, (void *) arg, sizeof(cfg))) {
            rc = -EFAULT;
            break;
        }
        /* Allow only single frame */
        if (audio->mode == MSM_AUD_ENC_MODE_TUNNEL) {
            if (cfg.buffer_size != (FRAME_SIZE - 8))
                rc = -EINVAL;
                break;
        } else {
            if (cfg.buffer_size != (AAC_FRAME_SIZE + 14))
                rc = -EINVAL;
                break;
        }
        audio->buffer_size = cfg.buffer_size;
        break;
    }
    ....
    default:
        rc = -EINVAL;
    }
    mutex_unlock(&audio->lock);
    return rc;
}
Тут можно предположить, что код дописывали вечером после допиливания большого Python-скрипта.
Явно забыли расставить фигурные скобки.


Все найденные мною ошибки постараюсь исправить и результаты исправлений закоммитить в репозиторий на github.

Комментариев нет:

Отправить комментарий