Golded SIGABRT

Nil A написал(а) к All в Aug 23 03:22:20 по местному времени:

Нello, All!

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

Program received signal SIGABRT, Aborted.
gdb) bt
#0 0x00007ffff7099c37 in _GIraise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1 0x00007ffff709d028 in _GIabort () at abort.c:89
#2 0x00007ffff70d62a4 in _libc_message (do_abort=do_abort@entry=2, fmt=fmt@entry=0x7ffff71e5db0 "** %s **: %s terminated\n") at ../sysdeps/posix/libcfatal.c:175
#3 0x00007ffff717187c in _GI___fortify_fail (msg=<optimized out>, msg@entry=0x7ffff71e5d47 "buffer overflow detected") at fortifyfail.c:38
#4 0x00007ffff7170750 in _GI___chk_fail () at chkfail.c:28
#5 0x00007ffff716f8eb in _strcatchk (
dest=dest@entry=0xd20488 "\351 \323\324\301\314\311 \304\322\301\324\330\323\321 \351\314\330\321 \355\325\322\317\315\305\303 \323\317 \332\315\305\305\315 \347\317\322\331\316\331\336\305\315 \311 \317\324\322\325\302\311\314 \355\325\322\317\315\305\303 \317\304\316\325 \307\317\314\317\327\325 \372\315\305\300. \356\317 \327\331\322\317\323\314\317 \3257
#6 0x000000000047dad6 in strcat (src=<optimized out>,
dest=0xd20488 "\351 \323\324\301\314\311 \304\322\301\324\330\323\321 \351\314\330\321 \355\325\322\317\315\305\303 \323\317 \332\315\305\305\315 \347\317\322\331\316\331\336\305\315 \311 \317\324\322\325\302\311\314 \355\325\322\317\315\305\303 \317\304\316\325 \307\317\314\317\327\325 \372\315\305\300. \356\317 \327\331\322\317\323\314\317 \325 \324\3172
#7 ScanKludges (msg=msg@entry=0xd1ebec, getvalue=getvalue@entry=1) at /home/fido/src/golded-plus/golded3/geline.cpp:1743
#8 0x000000000047fe80 in MakeLineIndex (msg=0xd1ebec, margin=141, getvalue=true, header_recode=<optimized out>) at /home/fido/src/golded-plus/golded3/geline.cpp:3071
#9 0x00000000004817e9 in Area::LoadMsg (this=this@entry=0xbab570, msg=msg@entry=0xd1ebec, msgno=8388, margin=margin@entry=141, mode=mode@entry=0) at /home/fido/src/golded-plus/golded3/gelmsg.cpp:134
#10 0x000000000046fd9b in FindString (msg=0xd1ebec, prompt=<optimized out>, what=<optimized out>) at /home/fido/src/golded-plus/golded3/gmarea.h:1214
#11 0x00000000004701d1 in FindAll (msg=msg@entry=0xd1ebec, topline=@0x7ff8f4: 0, keyok=@0x7ff8f0: 0) at /home/fido/src/golded-plus/golded3/gmarea.h:985
#12 0x00000000004a7520 in Reader () at /home/fido/src/golded-plus/golded3/geread.cpp:872
#13 0x000000000040999e in main (argc=<optimized out>, argv=<optimized out>) at /home/fido/src/golded-plus/golded3/gemain.cpp:54

Чё-то, где-то. про Скан Клуджей. Я сейчас просто тут это оставлю, разбираться ночью с этим багом я не буду, не on-call я сегодня по голдеду.

Best Regards, Nil
--- GoldED+/LNX 1.1.5

Vitaliy Aksyonov написал(а) к Nil A в Aug 23 12:02:52 по местному времени:

Привет, Nil!

11 Aug 23 03:22, ты писал(а) All:

NA> Чё-то, где-то. про Скан Клуджей. Я сейчас просто тут это оставлю,
NA> разбираться ночью с этим багом я не буду, не on-call я сегодня по
NA> голдеду.

Так неспортивно. А Steps to reproduce? Ну или корку на крайняк.

Best regards,
Vitaliy Aksyonov.

... Вот такие загадочные эти зверьки, female'сы.
--- GoldED+/LNX 1.1.5-b20230221

Nil A написал(а) к Vitaliy Aksyonov в Aug 23 08:33:52 по местному времени:

Нello, Vitaliy!

Friday August 11 2023 12:02, from Vitaliy Aksyonov -> Nil A:

NA>> Чё-то, где-то. про Скан Клуджей. Я сейчас просто тут это оставлю,
NA>> разбираться ночью с этим багом я не буду, не on-call я сегодня по
NA>> голдеду.
VA> Так неспортивно. А Steps to reproduce? Ну или корку на крайняк.

В hobbit.test эхе хулиганят, написали вот такой вот длинный ориджин

> * Origin: И стали драться Илья Муромец со змеем Горынычем и отрубил
> Муромец одну голову Змею. Но выросло у того 2. И отрубил он у змея 2
> головы, но выросло 4... и отрубил он у Горыныча 65535

и фсё, голдед на свою голую попу сел. Причём, там есть нюанс, когда билд -O0 и -O1,.. может в кору упасть, а может и не упасть, точнее в другом месте упадёт ;-)

Так то строчка выглядит норм
> strxcpy(msg->origin, line->txt.c_str()+11, sizeof(msg->origin));
Чертовски длинный ориджин лежит в стринге line->txt, msg->origin это char[160] просто. Я уж не буду комментировать +11 отступ, чтобы пропустить " * Origin: " ;-)
Внутри strxcpy() делает strcpy() размер -1, и закрывает \0 буфер - всё вроде хорошо.



Короче, вот что я точно знаю, что Одинн Соренсен (царство ему небесное), он был кулхацкером в 90е года, когда плюсы были "Си с классами". Его хакерский стиль - это переиспользовать структурки, просто memset() затираем все поля, и дальше едем. Конструкторы/декструкторы - это для слабаков, мы сейчас сэкономим на new/delete. Окей, всё будет работать, пока у тебя структурка просто POD (типа std::istrivialv), но когда там добавили стринги..

А потом пришли чувачки, невкурили они идею Одинна, и втащили местами std::string с std::vector, и структурки стали уже не POD. Я как-то под valgrind запустил, и там потерянная память, потому что кулхацкер Одинн memset() затирает структурки, а стринги и вектора никто не деструктирует как положено. Короче, там где стринги и вектора, то там новодел - инфа 100%.



Да говно код вашего голдеда, вот чесно. Думал, сейчас голыми руками голдеда поймаю, собрал АСАН билд

> -fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all
> -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow
> -fno-sanitize=null -fno-sanitize=alignment

и что? До списка эх не дошёл, уже арифметика указателей нитаво

> goldlib/gall/gmemdbg.cpp:130:53: runtime error: pointer index expression
> with base 0x000000000000 overflowed to 0xffffffffffffffd4
Вот тут хитро прячут структурку Throw за адресом буфера - тот ещё хак. Ну блин, не свой же кастомный маллок пишем.
> (Throw)((const char)ptr-sizeof(Throw)+1);

Ну окей, мне сейчас надо чисто санитайзер на адреса сделать, просто один -fsanitize=address. Опять до списка эх не дошёл.
> AddressSanitizer: alloc-dealloc-mismatch (operator new [] vs operator
> delete)

Да блин, там десятилетиями эти баги сидят, и всем пофиг. Надо прораваться дальше, запускаем с ASANOPTIONS=alloc_deallocmismatch=0 -у нас список эх уже, нажимаем клавишу названия эхи, и...
> AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffded765a05
> at pc 0x00000071d98d bp 0x7ffded765910 sp 0x7ffded765908

Стрёмненько как-то вобщем.

Best Regards, Nil
--- GoldED+/LNX 1.1.5

Nil A написал(а) к Vitaliy Aksyonov в Aug 23 07:03:34 по местному времени:

Нello, Vitaliy!

Friday August 11 2023 12:02, from Vitaliy Aksyonov -> Nil A:

NA>> Чё-то, где-то. про Скан Клуджей. Я сейчас просто тут это оставлю,
NA>> разбираться ночью с этим багом я не буду, не on-call я сегодня по
NA>> голдеду.
VA> Так неспортивно. А Steps to reproduce? Ну или корку на крайняк.

Ларчик легко открывался.
TLDR; надо бы все strcat() в коде на strncat() заменить, а иначе классический buffer overflow.

Сейчас будут детали бага, заодно расскажу, какие тулы мне в такие моменты приходят на ум.

Имеется - хорошее воспроизведение бага. При поиске "Enter Searchstring (Нeader+Text)", по hobbit.test, у меня shift+f, или option+f кнопка, вводим пофиг что. Как оказалось, не пофиг размер окна терминала, т.е. $COLUMNS у меня сейчас 148, если совсем маленький, или совсем широкий, то не воспроизводится баг.

Начинаем ковырять, собираем из гитхаб мастера с -O0 -g. Напускаем gdb, видим, что в разных функциях, где передаётся *Gmg msg, и потом, когда достают msg->lin, то там оказывается всегда одинаковое фиксированное значение, и это явно за пределами памяти.

(gdb) up 2
#2 0x00000000004852e9 in MsgLineReIndex (msg=0xd893cc, viewhidden=0, viewkludge=0, viewquote=1)
at /home/fido/src/golded-plus/golded3/geline.cpp:3111
3111 throw_xrelease(msg->line);
(gdb) p msg->lin
$1 = (Line *) 0xd9ced9d2cfe720d5
(gdb) p *msg->lin
Cannot access memory at address 0xd9ced9d2cfe720d5

Сначала я в gdb пытался watch разные ставить, чтобы отловить, кто туда пишет. Оказалось, что мой линукс в виртуалке, которая на OpenVZ, ещё и плохо с хардварными вочпоинтами дружит, типа не пермишен.

Потом я думал, что отловлю на ASAN билде, кто в какую память пишет не так. Но там всё совсем плохо оказалось, я уже писал ранее. Хотя, сейчас понимаю, что рано сдался, надо было с ASANOPTIONS=halt_onerror=0 пускать, и он бы прошагал дальше.

Потом я запустил под valgrind, который, сначала обругался, что у нас в if выражениях используются значения неинициализованных переменных, в районе Initialize->ReadKeysCfg. Потом ругань на несоответствия free() / delete / delete [], которые я уже видел на ASAN билде. Кстати, может кто-нибудь это всё вычистит?

И вот тут звёзды сошлись. Valgrind сказал, что пишем за пределы вот тут
by 0x48156C: ScanKludges(GMsg*, int) (geline.cpp:1743)
Там вот такая строчка
strcat(msg->origin, line->next->txt.c_str());

И, как и ожидалось, в структурке GMsg, за "char origin[160]" (который оказался чуть-чуть длинноватым в сообщении), следующее поле "Line* lin". Таким образом этот указатель съезжает на левый адрес, а дальше разыменование указателя это UB.

Best Regards, Nil
--- GoldED+/LNX 1.1.5

Nil A написал(а) к All в Aug 23 07:26:14 по местному времени:

Нello, All!

Friday August 11 2023 03:22, from Nil A -> All:

NA> Я имею устойчивый репродакшен баг, на голдеде, построенном из гитхаб
NA> мастер.

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

-+- a/golded3/geline.cpp
+++ b/golded3/geline.cpp
@@ -1740,7 +1740,7 @@ void ScanKludges(GMsg* msg, int getvalue)
strxcpy(msg->origin, line->txt.c_str()+11, sizeof(msg->origin));
if(nextor) // Get the next line too
{
- strcat(msg->origin, line->next->txt.c_str());
+ strxcat(msg->origin, line->next->txt.c_str(), sizeof(msg->origin));
line->next->color = C_READO;
line->next->type |= GLINE_ORIG; // Mark next line as Origin too
}

P.S. Бонусом расскажу прикол заодно. Здесь sizeof(msg->origin) можно написать без скобок, потому sizeof не функция, а оператор. Если sizeof с именем переменной, то скобки можно не ставить, а если с типом надо писать. Казалось бы, кого это волнует? А вот, у меня на работе такие умники есть, на код-ревью мне такие комментарии пишут, просто в остальном сложно по коду докопаться. :-)


NA> Program received signal SIGABRT, Aborted.
NA> gdb) bt
NA> #0 0x00007ffff7099c37 in _GIraise (sig=sig@entry=6) at
NA> ../nptl/sysdeps/unix/sysv/linux/raise.c:56 #1 0x00007ffff709d028 in
NA> _GI_abort () at abort.c:89 #2 0x00007ffff70d62a4 in __libcmessage
NA> (doabort=doabort@entry=2, fmt=fmt@entry=0x7ffff71e5db0 "** %s **:
NA> %s terminated\n") at ../sysdeps/posix/libc_fatal.c:175 #3
NA> 0x00007ffff717187c in _GI___fortifyfail (msg=<optimized out>,
NA> msg@entry=0x7ffff71e5d47 "buffer overflow detected") at
NA> fortifyfail.c:38 #4 0x00007ffff7170750 in __GI___chkfail () at
NA> chkfail.c:28 #5 0x00007ffff716f8eb in __strcatchk (
NA> dest=dest@entry=0xd20488 "\351 \323\324\301\314\311
NA> \304\322\301\324\330\323\321 \351\314\330\321
NA> \355\325\322\317\315\305\303 \323\317 \332\315\305\305\315
NA> \347\317\322\331\316\331\336\305\315 \311 \317\324\322\325\302\311\314
NA> \355\325\322\317\315\305\303 \317\304\316\325 \307\317\314\317\327\325
NA> \372\315\305\300. \356\317 \327\331\322\317\323\314\317 \3257 #6
NA> 0x000000000047dad6 in strcat (src=<optimized out>,
NA> dest=0xd20488 "\351 \323\324\301\314\311
NA> \304\322\301\324\330\323\321 \351\314\330\321
NA> \355\325\322\317\315\305\303 \323\317 \332\315\305\305\315
NA> \347\317\322\331\316\331\336\305\315 \311 \317\324\322\325\302\311\314
NA> \355\325\322\317\315\305\303 \317\304\316\325 \307\317\314\317\327\325
NA> \372\315\305\300. \356\317 \327\331\322\317\323\314\317 \325 \324\3172
NA> #7 ScanKludges (msg=msg@entry=0xd1ebec, getvalue=getvalue@entry=1) at
NA> /home/fido/src/golded-plus/golded3/geline.cpp:1743 #8
NA> 0x000000000047fe80 in MakeLineIndex (msg=0xd1ebec, margin=141,
NA> getvalue=true, header_recode=<optimized out>) at
NA> /home/fido/src/golded-plus/golded3/geline.cpp:3071 #9
NA> 0x00000000004817e9 in Area::LoadMsg (this=this@entry=0xbab570,
NA> msg=msg@entry=0xd1ebec, msgno=8388, margin=margin@entry=141,
NA> mode=mode@entry=0) at
NA> /home/fido/src/golded-plus/golded3/gelmsg.cpp:134 #10
NA> 0x000000000046fd9b in FindString (msg=0xd1ebec, prompt=<optimized
NA> out>, what=<optimized out>) at
NA> /home/fido/src/golded-plus/golded3/gmarea.h:1214 #11
NA> 0x00000000004701d1 in FindAll (msg=msg@entry=0xd1ebec,
NA> topline=@0x7ff8f4: 0, keyok=@0x7ff8f0: 0) at
NA> /home/fido/src/golded-plus/golded3/gmarea.h:985 #12 0x00000000004a7520
NA> in Reader () at /home/fido/src/golded-plus/golded3/geread.cpp:872 #13
NA> 0x000000000040999e in main (argc=<optimized out>, argv=<optimized
NA> out>) at /home/fido/src/golded-plus/golded3/gemain.cpp:54

Best Regards, Nil
--- GoldED+/LNX 1.1.5

Sergey Myasoedov написал(а) к Nil A в Aug 23 22:04:32 по местному времени:

Нi Nil!

13 Aug 23 07:26, you wrote to All:

NA> Вот такой простой фикс чинит проблему, с которой я сталкнулся, когда перцы
NA> делают Ориджин длиннее 160 символов.

Чего ж ты коммит не залил?


--- cut here ---

Nil A написал(а) к Sergey Myasoedov в Aug 23 06:55:20 по местному времени:

Нello, Sergey!

Tuesday August 22 2023 22:04, from Sergey Myasoedov -> Nil A:

NA>> Вот такой простой фикс чинит проблему, с которой я сталкнулся,
NA>> когда перцы делают Ориджин длиннее 160 символов.
SM> Чего ж ты коммит не залил?

Вкуда комиит залил? Мне чё щас ещё на гитхабе, может быть, зарегистрироваться (или где его там хостят)?
Ага, потом ещё в телегремме зарегаться, потом ещё во вконтекте, потом ещё воцапе, потом ещё фейсбуки, тиндеры-шминдеры...

Фидо самодостаточное, или таковым должно быть, ибо сеть целая!
Я, кстати, прям готов даже кодревью делать, вот прям вот так вот, используя эхомейл, с минимальным форматированием. Не Маркдаун, конечно, но могу вот так, и вообще по
> всякому могу

Между прочим. Все линуксо-кернеловские патчи летали всегда просто в мейл-листе, там же и на них комменты писали. А тут, видишь ли, фидоНЕТ не самодостаточен, оказывается!

Best Regards, Nil
--- GoldED+/LNX 1.1.5