alizar 4 мар 2014 в 03:23

История однострочных багов

2 мин

43K

Информационная безопасность*Криптография*Open source*

+94

Комментарии 90

dimkagfx 4 мар 2014 в 03:40

Never forget

+58

Snowly 4 мар 2014 в 05:49

Не всем думаю будет понятно, да и комит не в самом верху страницы.

-BUMBLEBEEVERSION=1.4.31
+BUMBLEBEEVERSION=1.4.32

@@ -348,7 +348,7 @@ case "$DISTRO" in
   ln -s /usr/lib/mesa/ld.so.conf /etc/alternatives/gl_conf
   rm -rf /etc/alternatives/xorg_extra_modules
   rm -rf /etc/alternatives/xorg_extra_modules-bumblebee
-  rm -rf /usr /lib/nvidia-current/xorg/xorg

Пробел после /usr. Меня тогда пронесло, не успел обновится.

+73

nomit 4 мар 2014 в 05:56

О да, а я тогда голову сломал чего это у меня система сломалась. И понять не мог, на вид как будто ядро не видит :)

j_wayne 4 мар 2014 в 09:25

ИМХО если бы не было дублирования префикса /usr/lib, вероятность такой опечатки была бы меньше… DRY

SovGVD 4 мар 2014 в 09:59

А мне тогда было лениво обновляться, а потом узнал что крупно повезло.

BalinTomsk 2 мая 2014 в 05:56

я вот всегда искренне неудомеваю — а что в open-source продуктах юнит тесты и функциональные тесты никто принципиально не пишет?

JIghtuse 2 мая 2014 в 06:09

Вы спрашиваете, чтобы потроллить или просто чтобы в поисковик не ходить? Каким образом связана открытость/закрытость проекта и его тестирование?
Вот вам пара open-source продуктов, которые тестируют так, как пожалуй ничто в мире.
SQLite
Chromium

linoleum 4 мар 2014 в 06:17

>>> Android <...> То есть один из передаваемых в функцию параметров вообще не используется.
Очень странно, что такой баг возник, учитывая, что большинство (если не все) IDE на такие вещи истерично ругаются.

ilmirus 4 мар 2014 в 06:23

Не только IDE, но и компиляторы.

eyeless_watcher 4 мар 2014 в 07:42

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

+23

НЛО прилетело и опубликовало эту надпись здесь

dalamber 4 мар 2014 в 09:55

Например, «замемсеттить» RGB24 буфер значением 255, чтобы получить белую канву. Но это я так, к слову — обычно канва нужна чёрная :)

splav_asv 4 мар 2014 в 09:59

В релизе — да, а для отладки забивают 0xCC или 0xDEADBEEF или еще чем-то в этом роде. Так что этот баг слегка затруднял отладку. Промахнулся на один уровень.

НЛО прилетело и опубликовало эту надпись здесь

splav_asv 4 мар 2014 в 17:09

Это был просто пример того, что не всегда пишут memset 0, иначе можно ыбло не объявлять bzero устаревшей. Для большинстве платфор никак нельзя memset'ами писать шаблоном шире одного байта, на сколько я знаю. Но на Mac OS X есть в libc функции memset_pattern[4,8,16], относящиеся к семейству memset.

НЛО прилетело и опубликовало эту надпись здесь

QtRoS 4 мар 2014 в 21:09

Тоже хотел написать про это.

Но раз уж Вы опередили, то вот другой вопрос — неужели memset там реализован через цикл? Небольшое гугление говорит, что

The underlying implementation might be in assembler specific to that architecture to take advantage of whatever its native properties are. It might also have internal special cases to handle the case of zeroing (versus setting some other value).

НЛО прилетело и опубликовало эту надпись здесь

Aux 4 мар 2014 в 13:02

Потому что vim. В исходниках Android очень много такой фигни.

ximaera 4 мар 2014 в 17:51

Использование vim какбэ не отменяет GCC-флаги -Wall -Wextra -Werror. Если человеку наплевать на предупреждения компилятора, то ему и на IDE будет наплевать.

Lol4t0 4 мар 2014 в 18:01

Работает, только если при компиляции у вас не генерится еще 1000 'unused parameter' warning-ов. Достаточно одного человека в команде, который допускает в коде предупреждения, чтобы проанализировать лог компилятора стало сложно.

-1

ximaera 4 мар 2014 в 18:26

Если у вас есть такой человек, то vim и warning'и — это ещё не самая большая из ваших бед. Устранять надо корень проблемы, а не следствия.

Lol4t0 4 мар 2014 в 18:47

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

ximaera 4 мар 2014 в 18:56

__attribute__((__unused__))?

Lol4t0 4 мар 2014 в 19:02

Для этого надо хотя бы знать, что переменная не используется.

если есть код

int x = bar();
#ifdef EXT_LOG
ext_log(x);
#endif

И вы всегда собираете с EXT_LOG defined, а ваш товарищ — всегда с EXT_LOG undefined, то тут и возникает проблема.

ximaera 4 мар 2014 в 19:11

#ifndef EXT_LOG
__attribute__((__unused__)
#endif
int x = bar();

?

Хотя тогда уже проще само существование переменной x поставить в зависимость от EXT_LOG.

Наконец, если у вас действительно тысячелетний проект и вокруг все программисты — старцы с воооот такими бородами (бывает), то приведите в порядок тот исходник, над которым работаете, а остальные оставьте пока.

Lol4t0 5 мар 2014 в 11:44

Да вы поймите, что товарищ, который код правит — не получает предупреждения и потому не замечает ошибки. А вы тоже не хотите лезть со своими правками в чужой код.

ximaera 5 мар 2014 в 12:02

Практика code review у вас не распространена? Кроме того, можно попросить коллегу внести эти изменения в код, потому что это упростит вам работу и потенциально позволит избежать проблем в релизе. И вообще, не вижу ничего плохого в том, чтобы предложить другому человеку патч, тем более столь тривиальный и лаконичный.

burjui 5 мар 2014 в 00:46

_{С этими словами он бросил в сумку АКСУ и несколько магазинов к нему, после чего последовал к выходу.}

Aux 4 мар 2014 в 18:03

Регулярно собирая open source софт из исходников, могу сказать, что ВЕСЬ софт написанный в vim просто пестрит ворнингами.

В Android штука интересная в том, что большинство кода написано на Java, а компилятор не линтует код. При этом любая IDE для Java гоняет линт автоматом и сразу бьёт по рукам. Не сильно, но надоедливо. Люди, привыкшие к консоли линт не юзают в принципе, а потом их код зарастает тоннами навоза.

IDE очень сильно дисциплинирует и пресекает множество мелких косяков на корню. За использование текстовых редакторов в программировании надо выгонять из профессии. Это вам не посты в бложики писать.

-4

ximaera 4 мар 2014 в 18:34

Уважаемый, да вам с таким подходом в Госдуму надо идти вырабатывать требования к профессиональным программистам. Серьёзно, я даже не знаю, что тут ещё можно сказать

Если некто не умеет пользоваться неким инструментарием, это ещё не повод навязывать всем окружающим своё мнение. Казалось бы, это банальность, да?

Aux 4 мар 2014 в 18:36

Понимаете, если плотник не умеет пользоваться молотком, то это плохой плотник. А если программист не умеет пользоваться своими инструментами, то это уже личное мнение? Вы уж извините, но вы бред пишите.

ximaera 4 мар 2014 в 18:45

Плохая аналогия подобна котёнку с дверцей. Это раз.

Если ваш плотник достигнет феноменальной выработки при минимальных затратах, забивая гвозди микроскопом, то последнее, что вы должны делать — отбирать у него микроскоп и совать ему свой проклятый хлипкий молоток с зазубринами и неудобной ручкой. Оценивайте результаты, а не инструменты, чтобы не выяснилось как-нибудь, что вы ненароком половину разработчиков ядра Linux из профессии выгнали. Это два.

Aux 4 мар 2014 в 18:50

Работа над ядром Linux не такая уж и феноменальная работа. Мелкий патчик и в ноутпаде наклепать можно, и в общей архитектуре ядра для этого разбираться не надо.

-10

ximaera 4 мар 2014 в 18:57

То есть, на ваш взгляд, работа над ядром сводится к наклёпыванию мелких патчиков?

ilmirus 4 мар 2014 в 18:38

>Регулярно собирая open source софт из исходников, могу сказать, что ВЕСЬ >софт написанный в vim просто пестрит ворнингами.

Я вот специально скачал последнюю версию GCC из транка и собрал без ключа --disable-bootstrap. Предупреждения:
1) format not a string literal and no format arguments
2) unknown conversion type character ‘X’ in format
3) too many arguments for format

Все они -Wformat* и не ломают бутстрап, ибо признаны некритичными. Однако стоит объявить переменную и не использовать ее, сразу же ошибка бутстрапа.

Так ли вы уверены, что код, написанный без IDE плох?

>IDE очень сильно дисциплинирует и пресекает множество мелких косяков на корню

Посоветуете IDE, которая понимает ВЕСЬ GCC, со всей магией?

Aux 4 мар 2014 в 18:39

К сожалению, я очень давно не пишу на C/C++ и ничего вам посоветовать не могу.

-6

JIghtuse 4 мар 2014 в 19:01

Вот вам код из статьи в Vim с плагином YouCompleteMe.

Скрытый текст

WraithOW 4 мар 2014 в 19:54

Люди, привыкшие к консоли линт не юзают в принципе

Из консоли — это "javac *"? Мсье знает толк в несвежих извращениях.

любая IDE для Java гоняет линт автоматом

1) Не любая.
2) Инспекции для начала надо бы и настроить.
Так что не панацея.

ironsnake 4 мар 2014 в 22:19

Странно. Пишу код в Vim. И этот самый Vim выдает мне и ворнинги, и ошибки и все-все-все. Комплита нет, да, потому что не настраивал, ибо не люблю.
UPD. Да, gdb я к нему тоже прикручивал ради фана.

imwode 4 мар 2014 в 06:41

>Debian OpenSSL
При использовании нормального IDE это невозможно
А вот поделки типа IWARM — там еще и не такой хрени понаделаешь

-4

mayorovp 4 мар 2014 в 06:57

И что же нормальная IDE тут сделает-то?

imwode 4 мар 2014 в 07:05

высветит строку как коммент

mayorovp 4 мар 2014 в 07:07

Насколько я понял, строка была закоментирована намеренно, по причине "Don't add uninitialised data."

imwode 4 мар 2014 в 07:10

… во время отладки, а в релизе забыл раскомментить, потому что смотрелось ок

хотя кто знает

-2

allter 4 мар 2014 в 10:51

Вообще-то закомментировали намеренно, т.к. кому-то пришла в голову (или родилась на совещании в АНБ) гениальная мысль пройтись по исходникам профилировщиком valgrind, который высветил «использование не инициализированных данных», которое было бы ошибкой в любом проекте, кроме ГПСЧ, где это внезапно оказалось фичей.

amarao 4 мар 2014 в 12:53

Да, и был большой срач между дебиановцами и openssl'щиками. Потому что собирать энтропию из неинциализированной памяти — та ещё эвристика.

khim 4 мар 2014 в 13:09

Там эта переменная частично инициализирована. Они берут большой буфер и начинают класть в него всё подряд: информацию о процессах, что-то из /dev/random, etc. Но до конца буфер так и не заполняют, в результате чего часть буфера так и остаётся неинициализированной. После чего весь буфер скармливается в ГПСЧ.

Так что неинициализированные данные — это далеко не основной источник энтропии для ГПСЧ — но основной источник ругани valgrind'а. Уж сколько раз твердили миру, что все автоматические тулы — это лишь помощники, для интерпретации их сообщений голова нужна.

amarao 4 мар 2014 в 15:23

А что мешало им этот буффер занулить и складывать туда всё остальное? Срач-то был по поводу того, что «не-иницализация буффера» — это дурная-дурная практика.

-1

alexac 4 мар 2014 в 21:30

Вообще-то эта строка вообще является undefined behaviour, gcc и clang могут такую строку вовсе выкинуть из исходников, перенести в другое место с нарушением сайд эффектов и натворить прочие радости.

-1

udachnik 4 мар 2014 в 07:17

Я целую минуту всматривался в строчку
if (getuid() != 0 && geteuid() == 0)
пытаясь понять ее смысл. Потом заметил.

+34

ploop 4 мар 2014 в 07:57

Вот что значит неудачный идентификатор…

+12

Borz 4 мар 2014 в 10:47

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

-1

WerewolfPrankster 4 мар 2014 в 11:05

Да нет. Тут беда в том, что имена функций различаются в одну букву. Я тоже тупил секунд 10. А не знал бы — и не заметил бы.

+10

Borz 4 мар 2014 в 11:08

эм. а разве разница не в том, что после «geteuid» не было скобок?

ploop 4 мар 2014 в 11:18

Ну это как-бы явный баг, описанный в посте.
В общем проблема в другом: getuid() и geteuid() не мудрено попутать в любом месте кода, и IDE ничем не поможет, так как функции однотипные. И тупить можно не один час, вылавливая такой баг.

amarao 4 мар 2014 в 12:55

Для любого юниксоида разница между euid и uid понятна, и путать их — всё равно, что путать slip() и sleep().

ploop 4 мар 2014 в 15:16

get_uid()/get_euid(), getUid()/getEuid() в зависимости от принятых стандартов, и ещё масса вариантов.
Дело не в разнице или её знании/не знаии, дело просто в зрительном восприятии.

amarao 4 мар 2014 в 15:29

Ну так функций, которые на одну букву отличаются в названии очень много (vnsprintf vs vnprintf). Это вполне стандартная практика в Си, и с учётом того внимания, которое надо проявлять при работе с памятью, внимание к имени функции просто неощутимо.

Lol4t0 4 мар 2014 в 18:05

Стандартная-то она может и стандартная, но она ужасна.

aryeh 4 мар 2014 в 11:11

Минимум 2 из 5 ошибок были бы найдены самым простым статическим анализатором кода.

Andrey2008 4 мар 2014 в 11:15

У меня тут таких ляпов.... Другое дело, что это просто ошибки, а не известные epic fail.
Используйте варнинги компилятора и статический анализ. Часто помогает.

m08pvv 4 мар 2014 в 13:11

Писать с упором на MISRA-C, собирать с -Wall -Wextra -Wpedantic — да, некоторые так делают (и даже больше).

valplo 4 мар 2014 в 11:18

А как же the most expencive overbar in history?

poige 4 мар 2014 в 14:20

Баг у xserver вообще некритичный, ни разу. Более того, эта проверка ещё и не нужна, ибо лучше не проверять на рутовость, а просто обламываться там, где не хватает прав, желательно с адекватной диагностикой. Это гораздо более гибкий подход, ведь иногда пользователь может входить в нужную группу/обладать достаточными привилегиями, но при этом EUID не будет root'овым.

mayorovp 4 мар 2014 в 16:37

Этот код не имеет отношения к проверке прав. Это — защита на тот случай, если админ поставил программе флаг setuid. Смысл защиты в том, что в режиме setuid программа может работать так, как была настроена — но не может быть перенастроена заного. Лично я не понимаю, зачем вообще так делать — но, видимо, юзкейсы были.

-1

poige 4 мар 2014 в 17:06

О чём я и говорю — при -configure сервер запишет новый конфиг-файл. Проверка почему-то предполагает, что не-root-пользователь этого сделать не должен, но, что если именно этот пользователь является владельцем конфиг-файла, и, соответственно, по-логике вещей, должен был бы суметь воспользоваться этой опцией. Типичный случай, когда assumption is the mother of all f*ckups.

mayorovp 4 мар 2014 в 17:09

Вглядитесь еще раз в эту строчку: if (getuid() != 0 && geteuid() == 0) {
Если админ не выставлял программе флага setuid — любой пользователь без проблем настроит свой личный конфиг.

-1

poige 4 мар 2014 в 17:38

Что ж так упорно понимать не хотите?

www.wikihow.com/Configure-X11-in-Linux:

A new file has been created in /etc/X11/ called xorg.conf

+ man:

-configure
When this option is specified, the Xorg server loads all video driver modules, probes for available hardware, and writes out an initial xorg.conf(5) file based on what was detected. This option currently has some problems on some platforms, but in most cases it is a good way to bootstrap the configuration process. This option is only available when the server is run as root (i.e, with real-uid 0).

Суть моего коммента в том, что пользователи могут что-то вполне административно-легально (права-членства в группах, или собственность) делать вещи, которые обычно может делать root. Данная проверка этой возможности его лишает. Именно поэтому нужно «… не проверять на рутовость, а просто обламываться там, где не хватает прав, желательно с адекватной диагностикой. …».

mayorovp 4 мар 2014 в 17:42

Данная проверка — не лишает! Что же вы вчитаться в булево условие не желаете так упорно?..

PS и да, хотелось бы посмотреть на пользователя, имеющего права на загрузку драйверов…

-2

poige 4 мар 2014 в 17:46

То есть выдержка из man'а, где просто по-английски написано, что -configure можно выполнить только будучи root'ом, не смущает. Я даже курсивом выделил.

> PS и да, хотелось бы посмотреть на пользователя, имеющего права на загрузку драйверов…

Наслаждайтесь, чё…

mayorovp 4 мар 2014 в 17:47

Меня смущает не выдержка из мана, а тот факт, что обвиняют совершенно постороннюю проверку.

PS спасибо.

-1

poige 4 мар 2014 в 17:56

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

-configure даёт возможность получить текстовое представление рабочего конфига сервера. Автоматически. root-пользователь просто запускает с этим ключом сервер, тот сохраняет рабочий конфиг.

Проверка «если root не настоящий» (а только SETUID'ный) -configure делать запрещает

ErrorF("The '-configure' option can only be used by root.\n");

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

А теперь — что если Вася Пупкин как раз и есть владелец /etc/X11/xorg.conf? Почему Вася, не должен смочь записать новый конфиг?

mayorovp 4 мар 2014 в 18:00

Потому что для этого нужна более сложная проверка. Зачем тратить силы на те юзкейсы, которыми никто не пользуется?

Васе же Пупкину никто не запрещает сделать так (если у него есть права грузить драйвера — то и куда исполнимый файл записать найдется):

cp /path/to/Xorg ./Xorg
chmod +x ./Xorg
./Xorg -configure

И еще есть тот вариант, в котором рут снимает флаг setuid с файла Xorg — и Васе Пупкину ничего не надо никуда копировать.

-1

poige 4 мар 2014 в 18:12

Потому что для этого нужна более сложная проверка. Зачем тратить силы на те юзкейсы, которыми никто не пользуется?

Во-1-х, потому что идеологически правильнее проверять должна операционная система. Например, из-за появления тех же capabilites в Linux, кучу таких проверок пришлось найти, и выбросить…
Во-2-х, «никто не пользуется» — да-да. Расскажите UNIX-администраторам, что они не должны больше пользоваться стандартной моделью прав доступа, а должны каждому Васе сразу выдавать права root.

если у него есть права грузить драйвера — то и куда исполнимый файл записать найдется):

Если SETUID на данной платформе был реально необходим для штатной работы X-сервера, то после копирования, файл X превратится в тыкву.

(Думаю теперь можно поставить точку в этом диалоге.)

mayorovp 4 мар 2014 в 18:27

Вы сами себе противоречите. Все-таки, есть у Васи Пупкина необходимые права — или нет?

Во-1-х, потому что идеологически правильнее проверять должна операционная система. Например, из-за появления тех же capabilites в Linux, кучу таких проверок пришлось найти, и выбросить…

Скажите мне, КАК должна операционная система проверять права Васи Пупкина, если программа работает под euid 0? Проблема именно в том, что setuid root выключает все проверки со стороны операционной системы — и прикладная программа должна проводить их самостоятельно.

Конечно же, можно вместо сообщения об ошибке написать следующее:

if (getuid() != 0 && geteuid() == 0)
    setuid(getuid());

Это вернет проверку прав на уровень ОС — но такое действие эквивалентно флага setuid у файла. То есть возвращаемся к варианту

Если SETUID на данной платформе был реально необходим для штатной работы X-сервера, то после копирования, файл X превратится в тыкву.

PS хорошие админы UNIX давно уже усвоили, что setuid — зло. Поэтому хорошим админам эта проверка совсем не мешает.

~~(Думаю, да, теперь можно и точку поставить)~~

-1

poige 4 мар 2014 в 18:32

Скажите мне, КАК должна операционная система проверять права Васи Пупкина, если программа работает под euid 0?

В UNIX это называется Saved UID. На этом позволю себе таки окончательно закончить сеанс вашего бесплатного образования.

IamKarlson 4 мар 2014 в 17:17

Автор, а почему вы не указываете что это перевод?

+11

mayorovp 4 мар 2014 в 17:25

Потому что он никогда не указывает этого.

kolpeex 4 мар 2014 в 20:11

Забавно, как раз сегодня отловил такой баг:
$a = true;
$b = false;
$c = $a and $b;
var_dump($c); // bool (true)

valplo 4 мар 2014 в 20:32

Прошу прощения за мое невежество, но это на каком языке?

resurtm 4 мар 2014 в 20:47

PHP. Дополнение:

$a = true;
$b = false;
$c = $a and $b;
$d = $a && $b;
var_dump($c); // bool (true)
var_dump($d); // bool (false)

galaxy 4 мар 2014 в 21:13

В чем баг?

bohdan-shulha 4 мар 2014 в 22:36

Это не баг. Это, немного неожиданное для рядового программиста, нормальное поведение.

$c = $a and $b;

читается как

($c = $a) and $b;

ua1.php.net/operators.precedence