Виктор "Витус" Вагнер (vitus_wagner) wrote,
Виктор "Витус" Вагнер
vitus_wagner

Category:

О вреде valgrind

В последнее время среди многих Open Source разработчиков и мейнтейнеров распространилась мода брать какой-нибудь lint или valgrind и прогонять через него большой и толстый проект, исправляя все найденные warning-и.

В принципе, мода не новая . Питер ван дер Линден в "Expert C programming" описывает аналогичную lint party, предпринятую разработчиками ядра Solaris при переходе с BSD codebase на System V codebase.

И вообще warning-free code это хорошо. Если после модификации фигня полезла, можешь быть уверен, что именно ты её посадил.

Но есть одно большое но - для того чтобы корректно исправить warning, нужно разобраться что этот код делает, и почему там оно вылезает.

А то будет как было с Куртом Роексом, мейнтейнером OpenSSL в Debian.

В openssl есть функция ssleay_rand_add, которая выглядит примерно так

static void ssleay_rand_add(const void *buf, int num, double add)
............
 MD_Update(&m,buf,j)
....

Обратите внимание - в MD_Update передается буфер, переданный функции в качестве параметра.
Вызывается эта функция через обертку RAND_add которая обеспечивает работу со сменными реализациями ДСЧ (поэтому и static - мы положили её адрес в структуру RAND_METHOD, и позовут её через этот указатель.

Далее, где-то в функциях инициализации содержится вызов RAND_add, которому передается неинициализированный буфер. Собственно, почему бы и нет? Неинициализрованная память -тоже случайные данные. Среди прочих -несколько бит энтропии вполне добавить могут.

Но если напустить на это дело valgring, он, конечно, будет ругаться на доступ к неинициализированной памяти.

Почему-то Роекс отследил это дело до функции ssleay_rand_add, но не до того места, где этот код действительно вычзывается с неинициализированными данными. Впрочем, понятно почему. При статическом просмотре кода я этого места тоже сходу не нашел.
И радостно закомментарил вызов MD_Update. Отрезав таким образом ВСЕ источники энтропии, потому что ВСЕ они добавляются через RAND_add.

Что, естественно, привело к генерации плохих ключей. И целый год ошибка ходила незамеченной.

Впрочем, и в OpenSSL core team я сталкивался с подобными проблемами. Там, правда, был не valgrind, а всего лишь gcc -Werror -Wextra. Но тоже код пофиксили так, что ворнинги он выдавать перестал, но и результат - тоже. Вплоть до того, что в каком-то из вариантов сборки вообще то ли не компилировался, то ли не грузился - не помню уже.

Нет, всё-таки Кнут прав по поводу literate programming. Ежели бы этот код был написан в literate style (хотя как раз эта функция весьма близка - комментариев, объясняющих алгоритм там больше, чем самого кода), может быть до человека бы и дошло, что хвост по самую голову надо отрывать не в этом месте.

Правда есть еще добрый человек rse, rоторый в 1998 году поставил вокруг этого вызова #ifndef PURIFY, что, видимо, и натолкнуло Роекса на мысль, что комментарить надо именно этот вызов.

Похоже, он имел в виду "Ну кто же будет в production целях использовать код, собранный с -DPURIFY - это ж только для ловли утечек памяти".

А вот не надо так делать. Сколько я боролся с желанием besm6 собирать код для тестирования со спецдефайном. Тестироваться должен ровно тот код, который будет работать. Есть, конечно, особые случаи, и программные ДСЧ - как раз один из них. При тестировании нужно уметь получать из ДСЧ предсказуемый результат, а вот при реальной работе нужно добиваться того чтобы этого ни при каких обстоятельствах не случилось. Поэтому тесты, кода собранного в специальных режимах надо минимизировать и изолировать от основного test suite. А большая часть test suite должна быть способной выполниться с релизной сборкой.
Tags: open source, компьютерное, криптография
Subscribe
  • Post a new comment

    Error

    Anonymous comments are disabled in this journal

    default userpic

    Your reply will be screened

    Your IP address will be recorded 

  • 38 comments