company_banner

Поиск и исправление багов в исходниках PHP

https://www.sammyk.me/how-to-find-and-patch-a-bug-in-php-source-php-internals
  • Перевод


Честно предупреждаю: воспринимайте этот текст с определённой долей скептицизма. Я лишь недавно начал знакомство с внутренностями PHP, но хотел бы рассказать вам о том, что творится за кулисами бага #75237.


Обнаружение бага


Всё началось с поиска строк кода для покрытия на митапе Chicago PHP UG, который был частью PHP TestFest 2017.


Я нашёл непокрытую строку с помощью сайта gcov для ext/json/json_encoder.c. Попытался написать код на PHP, который обработал бы эту строку.



Попытался и так и эдак, но ничто не сработало, и внезапно я посредством этого кода ненароком инициировал segfault:


<?php
class Foo implements JsonSerializable {
  public function jsonSerialize() {
    return new self;
  }
}

# Segfault!
var_dump(json_encode(new Foo));

Но в PHP никогда не должно быть segfault. Основной код PHP должен бросать исключение и предоставлять для пользовательского пространства красивое сообщение об ошибке, прежде чем произойдёт segfault.


Чтобы узнать, какие версии PHP подвержены этому багу, я создал сниппет 3v4l и увидел, что все активно поддерживаемые версии генерируют segfault.


Это случается только тогда, когда jsonSerialize() возвращает новый экземпляр самого себя. Если я возвращаю с помощью $this тот же экземпляр, код работает как нужно.


<?php
class Foo implements JsonSerializable {
  public function jsonSerialize() {
    return $this;
  }
}

# Works fine
var_dump(json_encode(new Foo));

Отправка отчёта о баге


Обо всех багах в PHP нужно сообщать посредством bugs.php.net. Хотя я и пытался патчить баг самостоятельно, мне всё же пришлось создать отчёт, на который можно ссылаться в патче.


Так что я заполнил форму — и возник баг #75237.


Написание патча


Теперь самое трудное. Как начать патчить эту ошибку?


Обновление php-src и перекомпилирование


Сначала я получил все предыдущие изменения из основного Git-репозитория, чтобы быть уверенным, что я работаю с наиболее свежей версией php-src. Я удалённо вызвал команду upstream, указывающую на основной репозиторий php-src на GitHub.


$ git fetch upstream
$ git checkout master
$ git rebase upstream/master master

Затем создал новую ветку master для патча и назвал её в честь ID бага.


$ git checkout -b bug75237

Теперь нужно перекомпилироваться из исходного кода. Не буду вдаваться в подробности, у меня есть другая статья, где подробно описано, как компилировать PHP из исходного кода.


Когда я перекомпилирую PHP, я запускаю bash-скрипт под названием config, который я храню вне основной папки php-src. Он удаляет все существующие скомпилированные бинарники и прочие не находящиеся под версионным контролем файлы, собирает конфигурационный скрипт и запускает конфигурирование с нужными мне флагами, включёнными по умолчанию.


Вот мой скрипт, если захотите сделать для себя что-то подобное:


#!/bin/sh

make distclean
./vcsclean
./buildconf
./configure --enable-maintainer-zts \
        --enable-debug \
        --enable-cli \
        --with-zlib \
        --enable-mbstring \
        --with-openssl=/usr/local/opt/openssl \
        --with-libxml-dir=/usr/local/opt/libxml2 \
        --with-sodium \
        "$@"

Почему здесь такие странные пути? Я работаю на Mac с некоторыми зависимостями, установленными с Homebrew, поэтому кое-где ради некоторых расширений пути ведут в /usr/local/opt.


Так что я просто запустил свой конфигурационный скрипт, а затем скомпилировал всё с помощью make.


$ ../config && make -j9

По адресу sapi/cli/php создался бинарник, после чего я проверил версию PHP и запустил приводящий к segfault код, чтобы удостовериться, что мастер-ветка php-src still всё ещё содержит баг.


$ sapi/cli/php --version
$ sapi/cli/php json_encode.php

Да! Опять segfault. Теперь воспользуемся отладчиком, чтобы пройти по С-коду, который строка за строкой исполняет наш PHP-скрипт.


Выполнение GDB


Для отладки кода я использовал GDB. Он спроектирован для компилируемых языков вроде С и не работает с обычными PHP-файлами. Но PHP написан на С, так что с помощью gdb --args можно заставить GDB выполнить скомпилированный бинарник с PHP-скриптом.


$ gdb --args sapi/cli/php json_encode.php

Запустив GDB, я установил контрольную точку для паузы исполнения программы, а затем прошёл по каждой строке С-кода. О подробностях я рассказал в вышеприведённом видео, но если вкратце: оказалось, что всё дело в функции php_json_encode_serializable_object(), поэтому я установил здесь контрольную точку и запустил программу с помощью run.


> break php_json_encode_serializable_object
> run

Когда GDB доходит до контрольной точки и ставит исполнение кода на паузу, можно написать:


  • n и перепрыгнуть на следующую строку,
  • s и перейти в область видимости / фрейм следующей строки.
  • c и продолжить выполнение до следующей контрольной точки.

После ввода команд я понял, что код входит в бесконечную рекурсию между php_json_encode_zval() и php_json_encode_serializable_object(). Это приводит к переполнению стека и segfault, потому что у программы кончается память.


Меня интересовало выражение if внутри php_json_encode_serializable_object().


if ((Z_TYPE(retval) == IS_OBJECT) &&
        (Z_OBJ(retval) == Z_OBJ_P(val))) {
        //
} else {
        //
}

Там обрабатывался случай, когда возвращаемым jsonSerialize() значением является тот же экземпляр его самого, но никто не ловил ситуацию, когда возвращался новый экземпляр.


Ещё раз упомяну, что подробности описываются в видео, а здесь я приведу финальную часть кода, который я добавил для бросания исключения, когда jsonSerialize() возвращает новый экземпляр самого себя.


if ((Z_TYPE(retval) == IS_OBJECT) &&
        // Make sure the objects are not the same instance
        (Z_OBJ(retval) != Z_OBJ_P(val)) &&
        // Check that the class names of the objects are the same
        zend_string_equals(ce->name, Z_OBJCE(retval)->name)) {
        // Throw an exception
        zend_throw_exception_ex(NULL, 0, "%s::jsonSerialize() cannot return a new instance of itself", ZSTR_VAL(ce->name));

        // Garbage collection
        zval_ptr_dtor(&retval);
        zval_ptr_dtor(&fname);
        PHP_JSON_HASH_APPLY_PROTECTION_DEC(myht);

        // Return fail
        return FAILURE;
} else if ((Z_TYPE(retval) == IS_OBJECT) &&
        (Z_OBJ(retval) == Z_OBJ_P(val))) {
        //
} else {
        //
}

Я сделал много проб и ошибок, чтобы прийти к этому варианту, и мне очень помог сайт phpinternalsbook.com, особенно при работе со zval.


Обновление: Сара Голмон, долгое время являющаяся ключевым разработчиком PHP и одним из релиз-менеджеров PHP 7.2, дала замечательный комментарий к моему патчу: «Если пытаешься просто сопоставить класс, то можешь сравнить только ce. Так будет корректнее, чем сравнивать имена, и (номинально) более эффективно».


Благодаря предложению Сары мы можем изменить строку с zend_string_equals() в выражении if:


if ((Z_TYPE(retval) == IS_OBJECT) &&
        (Z_OBJ(retval) != Z_OBJ_P(val)) &&
        // This line changes
        ce == Z_OBJCE(retval)) {
        // Same as above
} else if ((Z_TYPE(retval) == IS_OBJECT) &&
        (Z_OBJ(retval) == Z_OBJ_P(val))) {
        //
} else {
        //
}

Тестирование


Прежде чем применить патч, нужно написать тест, подтверждающий, что ошибка действительно исправлена.


Опять же без подробностей, но если вам нужно больше информации, то я создал руководство из шести частей по написанию тестов для исходного кода PHP.


$ vi ext/json/tests/bug75237.phpt
--TEST--
Bug #75237 (jsonSerialize() - Returning new instance of self causes segfault)
--SKIPIF--
<?php if (!extension_loaded("json")) die("skip ext/json required"); ?>
--FILE--
<?php
class Foo implements JsonSerializable {
  public function jsonSerialize() {
    return new self;
  }
}

try {
  var_dump(json_encode(new Foo));
} catch (Exception $e) {
  echo $e->getMessage();
}
?>
--EXPECT--
Foo::jsonSerialize() cannot return a new instance of itself

Я запустил тест и проверил его прохождение, а затем прогнал все тесты в ext/json, чтобы удостовериться, что ничего не сломал.


$ make test TESTS=ext/json/tests/bug75237.phpt
$ make test TESTS=ext/json/tests/

Все тесты были пройдены, так что я был готов к применению патча.


Отправка PR и обновление бага


Затем я залил патч в свой форк на GitHub...


$ git add ext/json/json_encoder.c ext/json/tests/bug75237.phpt
$ git commit -m "Fix bug 75237 - (jsonSerialize() - Returning new instance of self causes segfault)"
$ git push origin 

… и создал PR в основном репозитории php-src.


Наконец, я вернулся к своему отчёту об ошибке и обновил его ссылкой на только что созданный PR.


Ожидание отклика и внесение изменений


После отправки PR прошло немного времени, и я получил отклик от нескольких сотрудников, работающих над PHP. Один из них помог понять, что я некорректно сравниваю имена классов (я это исправил в видео и кусках кода в статье, но оставил неизменённым исходный PR, так что вы можете проанализировать первый вариант).


В конце концов Niki P закрыл PR, потому что мы выявили более глубокую, более общую проблему, связанную с PHP. У этого языка нет общей защиты от переполнения стека, а значит, segfault может быть создан во многих других контекстах.


Вот как об этом высказалась Сара:



Удачи!


Хотя мой патч не был слит с кодовой базой, всё же считаю его успешным, потому что в процессе не только многому научился, но и привлёк внимание разработчиков к главной проблеме с переполнением стека, что может поспособствовать её решению.


Надеюсь, вам понравился рассказ о моём приключении с багом. Желаю удачи в поисках и исправлении собственных багов в исходном коде PHP!

Mail.Ru Group 812,25
Строим Интернет
Поделиться публикацией
Похожие публикации
Комментарии 11
  • +7

    В коде патча ничего не понял, но предполагаю что он завалится на вот этом тесте:


    class Foo implements JsonSerializable {
      public function jsonSerialize() {
        return new Bar;
      }
    }
    class Bar implements JsonSerializable {
      public function jsonSerialize() {
        return new Foo;
      }
    }
  • –4

    Есть вполне понятный debug json, когда он возвращает ошибку. Она имеет документацию http://php.net/manual/ru/function.json-last-error.php

    • 0

      Не понял как ваш комментарий относится к теме статьи…
      Segfault же.…

    • –3
      Потер, не гоже.
      • +2
        В комментарии к PR совершенно верно заявили что это не баг а фича, главная же проблема это переполнение стека.
        • –1
          Chicago PHP UG represent!
          • 0
            Основной баг как раз в вашем рнр коде, как вы сами и написали рекурсивный вызов, который и так очевиден, без анализа исходников.
            По хорошему метод jsonSerialize должен возвращать строку, а не новый экземпляр какого-то объекта и не массив как тут некоторые предлагают :)
            Забавно, что на винхп на рнр версии 5.2.4 нет никаких segfault, а на экране печатается
            string(2) "{}"

            Правда класс объявлен без
            implements JsonSerializable
            т.к. моя сбока рнр не знает о существовании JsonSerializable интерфейса.
            Но в целом вы получили разносторонний опыт…
            • +1
              JsonSerializable появился в 5.4.
              > Забавно, что на винхп на рнр версии 5.2.4 нет никаких segfault, а на экране печатается
              Да ничего забавного. Так вы не реализуете интерфейс, указанная функция просто не вызывается.

            Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

            Самое читаемое