Bug 21061 - Crashes right from the start
: Crashes right from the start
Status: CLOSED FIXED
: Sisyphus
(All bugs in Sisyphus/nickle)
: unstable
: all Linux
: P3 critical
Assigned To:
:
:
:
:
:
  Show dependency tree
 
Reported: 2009-08-14 02:41 by
Modified: 2009-08-24 01:45 (History)


Attachments
Use memcpy() instead of strcpy() (765 bytes, patch)
2009-08-15 11:43, Slava Semushin
no flags Details | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-08-14 02:41:41
Хотел посмотреть, что это за софтина для прототипирования, но меня ждал
жестокий облом:
$ nickle 
*** buffer overflow detected ***: nickle terminated
======= Backtrace: =========
/lib/libc.so.6(__fortify_fail+0x40)[0xb7e32f30]
/lib/libc.so.6[0xb7e31180]
/lib/libc.so.6(__strcpy_chk+0x44)[0xb7e304f4]
nickle(NewStrString+0x43)[0x807cc03]
nickle(StringInit+0x1b)[0x807cd0b]
nickle(ValueInit+0x7e)[0x808264e]
nickle(init+0x15)[0x806fb75]
nickle(main+0x11a)[0x806fe0a]
/lib/libc.so.6(__libc_start_main+0xe6)[0xb7d6bb26]
nickle[0x8050da1]
------- Comment #1 From 2009-08-15 11:43:12 -------
Created an attachment (id=3739) [details]
Use memcpy() instead of strcpy()

Хм, посмотрел на код. Почему-то не помогла замена на strncpy(). Не очень понял
всех тонкостей кода и как это работает. Но увидел как конкатенируются строки в
ф-ции strPlus() и сделал по аналогии -- использовал memcpy(). После этого
программа падать не стала.

Кстати, если собрать программу более старым компилятором -- gcc4.4-4.4.0-alt3,
то она не падает, а вот уже с 4.4.0-alt6 начинает падать.
------- Comment #2 From 2009-08-15 15:19:16 -------
После https://bugzilla.altlinux.org/show_bug.cgi?id=20842 мне этот вариант не
очень по душе. То что замена на strncpy не помогла, очень подозрительно.
------- Comment #3 From 2009-08-15 15:36:01 -------
М-да. Ну всё понятно. И почему strcpy обламывается, и strncpy не помогает.
Досточтимый апстримовый изобретатель велосипедов не осилил написать корректную
реализацию строк в стиле языка Паскаль. Даже не знаю за что хвататься, за
пистолет или за текстовый редактор. Могу лишь сказать, что приведённый патч не
работает. Почему - см. файл value.h, определение макроса StringChars.
------- Comment #4 From 2009-08-15 18:48:26 -------
(В ответ на комментарий №3)
> Почему - см. файл value.h, определение макроса StringChars.

Ты про s+1? Я так и не понял зачем это.

Кстати, автору я отписал и свой (пусть и неправильный патч) приложил.
Посмотрим, что он ответит и как будет это исправлять.
------- Comment #5 From 2009-08-15 23:05:39 -------
(В ответ на комментарий №4)
> (В ответ на комментарий №3)
> > Почему - см. файл value.h, определение макроса StringChars.
> 
> Ты про s+1? Я так и не понял зачем это.
Я ещё сам до конца не разобрался, что, собственно, происходит, но мне уже не
хочется продолжать.
s+1 - это такая чёрная магия. Товарищ хранит сначала структуру-дескриптор
строки (тип под названием String), а ПРЯМО следом за ней - байтики, из которых
состоит строка (см. реализацию NewString() ). Поскольку в StringChars
передаётся String*, то s+1 указывает на позицию ЗА дескриптором. Но вообще
говоря, s+1 вовсе необязательно будет указывать на непосредственно следующий за
дескрипотором _байт_. А памяти между тем выделяется впритык. Код, который
валится, всего-то навсего пытается разместить в такой структуре пустую строку.
strcpy и strncpy (в силу своих реализация) пытаются записать нулевой байт, но
передаваемый им указатель лежит за пределами выделенной области памяти, и
поэтому проверка границ благополучно сваливается. А memcpy никакого нулевого
байта не пишет, и в данном конкретном коде просто копирует ровно 0 байт, с
неизменно превосходным результатом. Зато когда memcpy попытается скопировать
больше 0 байт, произойдёт то же самое.

> Кстати, автору я отписал и свой (пусть и неправильный патч) приложил.
> Посмотрим, что он ответит и как будет это исправлять.
------- Comment #6 From 2009-08-16 11:38:13 -------
(В ответ на комментарий №5)
> (В ответ на комментарий №4)
> > (В ответ на комментарий №3)
> > > Почему - см. файл value.h, определение макроса StringChars.
> > 
> > Ты про s+1? Я так и не понял зачем это.
> Я ещё сам до конца не разобрался, что, собственно, происходит, но мне уже не
> хочется продолжать.

Ты мог бы объяснить автору на нормальном английском в чем тут проблема? Потому
что:

>> May be you can apply my patch or made better fix for this?

>I don't understand why this works though -- strcpy should write
>precisely the same sequence of bytes as memcpy, along with the trailing
>'\0' character into a buffer which is allocated to be the length of the
>string plus one bytes long.

>Sounds like there's some kind of compiler bug we've uncovered here, and
>I'm afraid it will affect more than just nickle.

Это цитата Keith Packard, автора nickle.

> Зато когда memcpy попытается скопировать больше 0 байт, произойдёт то же самое.

У меня не происходило. Наверное, повезло?
------- Comment #7 From 2009-08-16 12:50:14 -------
(В ответ на комментарий №6)
> Ты мог бы объяснить автору на нормальном английском в чем тут проблема?
Для начала неплохо бы на самом деле разобраться, в чём проблема. Пока я вижу
проблему в голове автора, но вряд ли он это признает :)

> > Зато когда memcpy попытается скопировать больше 0 байт, произойдёт то же самое.
> 
> У меня не происходило. Наверное, повезло?
Не знаю. Может, и я где-то ошибся с анализом. Отлаживать надо. Но я не
мейнтейнер этого пакета и мне лень с ним возиться.
------- Comment #8 From 2009-08-16 22:13:43 -------
Лёш, с удовольствием тебе его отдам :)
------- Comment #9 From 2009-08-17 00:57:55 -------
??? Он мне на фиг не сдался. Лучше вон php-coder'у, он даже патчи к нему пишет
:)
------- Comment #10 From 2009-08-17 01:17:17 -------
Тогда проще сразу в orphaned отправить...  (/me runs!!!)
------- Comment #11 From 2009-08-17 07:07:29 -------
(В ответ на комментарий №9)
> ??? Он мне на фиг не сдался. Лучше вон php-coder'у, он даже патчи к нему пишет
> :)

Мне эта программа интересна только из-за того, что она падает. Когда это будет
исправлено она мне станет не интересна. (Впрочем, если мне выдадут права на
NMU, то я самолично исправлю это падение.)
------- Comment #12 From 2009-08-17 23:01:28 -------
добавил nmu на php-coder
------- Comment #13 From 2009-08-19 21:56:52 -------
(В ответ на комментарий №3)
> Могу лишь сказать, что приведённый патч не работает.

Алексей, а ты можешь попробовать пересобрать nickle с последней версией gcc?
(4.4.1-alt1) После пересборки с ним у меня не падает, как раньше, и даже патчи
не понадобились. (Похоже, что это был действительно баг компилятора...)
------- Comment #14 From 2009-08-24 01:45:34 -------
nickle-2.68-alt1.1 -> sisyphus:

* Sun Aug 23 2009 Slava Semushin <php-coder@altlinux> 2.68-alt1.1

- NMU
- Rebuild with gcc 4.4.1-alt1 to fix crash after start (Closes: #21061)