LINUX.ORG.RU

Оказывается, snprintf() таки небезопасна

 ,


2

5

Не люблю я Си-шные строки, не люблю эти низкоуровневые пляски со строками. Но вот понадобилось сделать интерфейс с такими строками. На первый взгляд все выглядело кошерно. Но полезли баги, и я чего только не передумал, пока не додебажился до проблемы в snprintf().

Так вот, оказывается snprintf() не умеет писать строку саму в себя. Зачем это нужно? Ну вот например код:

// connectUrl - имя/IP сервера (поле класса)
// connectPort - порт сервера (поле класса)

bool TcpChannel::open(const char *url, int port)
{
    // Запоминаются URL и порт
    snprintf(connectUrl, TCP_CHANNEL_MAX_URL_LEN, url);
    connectPort=port;

    // Устанавливается соединение (возвращает void)
    tcpSocket->connectToHost(connectUrl, connectPort);
    ...
}

void TcpChannel::reConnect()
{
    ... всякая логика ...

    // Переустанавливается соединение
    open(connectUrl, connectPort);
}

Тут поле connectUrl из reConnect() передается в open() и должно записаться само в себя, то есть, вообще ничего не должно измениться по-хорошему. А вот болт. Первый байт строки будет обнулен и получится пустая строка. Не ожидал я такого. Как люди пишут на сях с такими строками - не понимаю.

★★★★★

Последнее исправление: cetjs2 (всего исправлений: 1)
Ответ на: комментарий от asyncron2018

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

saibogo ★★★★
()
Ответ на: комментарий от Xintrea

это не строки, это набор байт

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

Deleted
()
Ответ на: комментарий от Deleted

на то есть свои причины и они всем известны.

Тонны легаси, большое количество библиотек, наличие только компилятора С под некоторые платформы и достаточный пул программистов на С?

red75prim ★★★
()
Ответ на: комментарий от anonymous

Можно ссылочку?

Из последних: Вызов никогда не вызываемой функции О неопределённом поведении и багах оптимизатора clang на примере разбора цикла статей Криса Латтнера, разработчика clang. Читать - про абстрактную семантику, есть даже в стандарте C.

Правда, я не понял как всё это относится к вопросу ТС.

anonymous
()
Ответ на: комментарий от red75prim

Я бы ещё скорость выполнения программ, написанных на С добавил к списку.

Deleted
()
Ответ на: комментарий от vromanov

Аааа, это хэлп (который выводится при вызове без параметров) В man zip все адекватно же. Каки в гугле.

Dudraug ★★★★★
()
Ответ на: комментарий от Xintrea

Ты просто неосилятор и нытик.

anonymous
()
Ответ на: комментарий от Xintrea

Семантически это все что угодно, но не строки.

Да, это просто массивы байтов, завершающиеся 0. Если нужны настоящие строки, используй класс std::string в крестах или что-то аналогичное типа QString в тех же крестах в Qt и т. д.

И это для какого-нибудь элементарного действия.

Это си, который, напомню, был изначально написан в качестве замены ассемблеру для написания ОС Unix (первый вариант Unix был написан на ассемблере, но у них в Bell Labs были 2 машины с разной архитектурой, и им показалось проще создать язык и транслятор и переписать всё на нём для обеих машин, чем писать ещё один Unix на другом ассемблере). Отсюда все вытекающие особенности си. Если для твоих задач эти особенности не нужны, значит тебе и си не нужен. Просто для каждого класса задач надо выбирать наиболее подходящий язык. Си нужен там, где нужна высокая производительность и/или простой доступ к произвольным участкам памяти + разные трюки ценой снижения безопасности кода и усложнения разработки и отладки.

Так можно и проверку на NULL/nul_ptr из delete убрать. Типа, пусть будет UB, а кому нужно, пусть в своем коде проверяют.

Тоже не вижу большого смысла в автоматизации этой проверки, особенно в свете того, что обнулять освобождённый указатель всё равно нужно вручную, а если этого не сделать, то повторное освобождение памяти приведёт к ub (скорее всего это будет сегфолт или немедленно, или через некоторое время, при попытке повторного обращения к куче). Но зачем-то обязанность обнулять указатель возложили на программиста, а проверку на 0 осуществляет оперетор delete.

aureliano15 ★★
()
Ответ на: комментарий от aureliano15

обнулять освобождённый указатель всё равно нужно

Зачем?

а если этого не сделать, то повторное освобождение памяти приведёт к ub

Повторные освобождения надо находить и справлять, а не делать их «безопасными», обнуляя указатель. Лол.

anonymous
()
Ответ на: комментарий от anonymous

а если этого не сделать, то повторное освобождение памяти приведёт к ub

Повторные освобождения надо находить и справлять, а не делать их «безопасными», обнуляя указатель. Лол.

Вообще-то и я о том же. Речь шла о том, что попытка освобождения 0 указателя проверяется оператором delete и поэтому безопасна. А я говорю о том, что при освобождении указатель сам по себе не обнуляется, поэтому безопасность эта мнимая, и могли бы и не делать этого, тратя несколько тактов на ненужную проверку при каждом освобождении. Но коли сделали, то не страшно, не умрём от потери нескольких тактов. :-)

aureliano15 ★★
()
Ответ на: комментарий от aureliano15

Вообще-то и я о том же.

Очевидно, нет.

Речь шла о том, что попытка освобождения 0 указателя проверяется оператором delete и поэтому безопасна.

Ага.

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

delete f(); // the return type of `f` is `T*`

И как delete мог бы обнулить этот указатель? Лол.

безопасность эта мнимая

Мнимая безопасность — это обнуление указателя после delete. (Ты так и не ответил, зачем обнулять освобождённый указатель.)

могли бы и не делать этого, тратя несколько тактов на ненужную проверку при каждом освобождении

Тогда бы эту проверку нужно было бы копипастить в пользовательском коде. Т.к. проверка так или иначе есть, проще иметь её в одном место. Don’t repeat yourself.

anonymous
()
Ответ на: комментарий от anonymous

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

И как delete мог бы обнулить этот указатель? Лол.

О том, как delete мог бы обнулить этот указатель, я вообще не говорил, т. к. мне это неинтересно. Я говорил о том, что он не обнуляется. Но если тебе интересно, как можно было бы реализовать обнуление, то копай в сторону передачи по ссылке.

(Ты так и не ответил, зачем обнулять освобождённый указатель.)

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

могли бы и не делать этого, тратя несколько тактов на ненужную проверку при каждом освобождении

Тогда бы эту проверку нужно было бы копипастить в пользовательском коде.

Как это утверждение согласуется с цитируемым ниже

Повторные освобождения надо находить и справлять, а не делать их «безопасными», обнуляя указатель. Лол.

?

aureliano15 ★★
()
Ответ на: комментарий от aureliano15

(Ты так и не ответил, зачем обнулять освобождённый указатель.)

А почему я должен отвечать, зачем делать то, чего делать по моему мнению в общем случае не нужно?

Как это утверждение согласуется с цитируемым ниже

обнулять освобождённый указатель всё равно нужно

?

могли бы и не делать этого, тратя несколько тактов на ненужную проверку при каждом освобождении

Тогда бы эту проверку нужно было бы копипастить в пользовательском коде.

Как это утверждение согласуется с цитируемым ниже

Повторные освобождения надо находить и справлять, а не делать их «безопасными», обнуляя указатель. Лол.

?

Отлично согласуется. Ты опять думаешь в своёй манере: что нулевой указатель — признак освобождённой памяти. А я так не думаю.

anonymous
()
Ответ на: комментарий от aureliano15

Но коли сделали, то не страшно, не умрём от потери нескольких тактов. :-)

Вы натурально не понимаете. Потеря тактов не в проверке у free/delete на 0 аргумента, а именно в занулении там, где надо просто аккуратно выйти без излишних вызовов функций, которые по потерям превосходят if(ptr==0). А проверка сделана не для безопасности, а для возможности работ алгоритмов, где есть ветки, когда память не выделялась вообще, и 0 означает не освобождено, а начальное значение.

vodz ★★★★★
()
Ответ на: комментарий от aureliano15

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

И как delete мог бы обнулить этот указатель? Лол.

О том, как delete мог бы обнулить этот указатель, я вообще не говорил, т. к. мне это неинтересно. Я говорил о том, что он не обнуляется. Но если тебе интересно, как можно было бы реализовать обнуление, то копай в сторону передачи по ссылке.

void delete(T*& p) { delete p; p = nullptr; }

delete(f()) // не работает т.к. это временный объект и нужно const T*&.

anonymous
()
Ответ на: комментарий от anonymous

А почему я должен отвечать, зачем делать то, чего делать по моему мнению в общем случае не нужно?

Как это утверждение согласуется с цитируемым ниже

обнулять освобождённый указатель всё равно нужно

?

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

Ты опять думаешь в своёй манере: что нулевой указатель — признак освобождённой памяти. А я так не думаю.

А чем это ещё может быть в контексте new/delete, где по дефолту в случае неудачи new бросает исключение, а не возвращает 0? И ловить надо это исключение, а не равенство указателя 0.

Но даже в случае с malloc/free и со старыми версиями new, возвращавшими 0, память обычно выделяется не для того, чтобы её сразу освободить, а для того, чтобы сначала в неё что-то записать, прочитать и т. д., короче, поработать с нею. Но перед работой её по-хорошему надо проверить на 0. Т. е. безопасный код для работы с malloc/free будет выглядеть примерно так:

if((ptr=malloc(N))!=0)
{
  // bla-bla-bla.
  free(ptr);
}

И мы всё равно получаем 2 проверки на 0: одну в пользовательском коде, а другую — во free. Если же написать так:

ptr=malloc(N);
ptr[0] = 100500;
// bla-bla-bla
free(ptr);

то сегфолт в случае неудачи мы получим, ещё не дойдя до free.

Но если тебе интересно, как можно было бы реализовать обнуление, то копай в сторону передачи по ссылке.

delete(f()) // не работает т.к. это временный объект и нужно const T*&.

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

vodz написал:

Вы натурально не понимаете. Потеря тактов не в проверке у free/delete на 0 аргумента, а именно в занулении там, где надо просто аккуратно выйти без излишних вызовов функций, которые по потерям превосходят if(ptr==0).

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

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

Так в таких алгоритмах ничто не мешает явно проверить ptr на 0 перед вызовом delete или free. Зато в других алгоритмах этого не придётся делать дважды. С другой стороны, что, если я, используя такой алгоритм, забыл изначально явно инициализировать локальный нестатический указатель 0? Тогда там может оказаться всё что угодно, и проверка на 0 окажется бесполезной. Вы скажете, что в этом случае я сам себе злобный буратина. Но ведь речь как раз о защите таких ссзб от собственных ошибок. И мы видим, что не защищает эта проверка на 0 на 100% даже в этом случае.

aureliano15 ★★
()
Ответ на: комментарий от aureliano15

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

Я не про сравнение тактов if(ptr==0) vs ptr=0, а про то, что писать алгоритм надо так, чтобы вообще этого ничего не надо было делать.

Так в таких алгоритмах ничто не мешает явно проверить ptr на 0

Теоретически, должно проверяться не на 0, на на валидность указателя, выделенного менеджером памяти. Следовательно, проверка на 0 можно считать что там может вообще получаться автоматом. Отсюда, задокументировать поведение, когда проверка на 0 выполняется внутри библиотечной функции/языка — наоборот более правильно.

Зато в других алгоритмах этого не придётся делать дважды.

См. первое возражение.

И мы видим, что не защищает эта проверка на 0 на 100% даже в этом случае.

О чём и речь, оно не для этого придумано.

vodz ★★★★★
()
Ответ на: комментарий от aureliano15

Тут считать такты это сизифов труд, когда из-за общей программной архитектуры - абстракций, врапперов едущих через врапперы и вложенных на каждом их уровне проверок потеряно на порядки больше. Нужно что-то глобальное вроде допиленного LTO+PGO, которое будет сокращать/оптимизировать весь код включая вызываемый из внешних библиотек.

anonymous
()
Ответ на: комментарий от vodz

Я не про сравнение тактов if(ptr==0) vs ptr=0, а про то, что писать алгоритм надо так, чтобы вообще этого ничего не надо было делать.

Это уже дело вкуса. Рациональных причин, однозначно свидетельствующих за выбор того или иного подхода, я не вижу. В случае с отсутствием такой проверки в delete и во free, мы в ряде случаев ускорим код на несколько тактов, а в других не ускорим, но и не замедлим. В случае проверки мы сократим исходник и бинарник на несколько байт и на несколько процентов увеличим защиту от индуса. В обоих случаях и потери, и выгоды копеечные, поэтому ломать копья по этому поводу не вижу смысла. Но остаюсь при своей точке зрения.

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

Тоже спорно. Если в указателе будет рандомное значение (например, взятое из стека, если указатель не был явно инициализирован 0), то оно может случайно попасть в нужный диапазон, а может не попасть. С другой стороны, при повторном освобождении, указатель (если он не был обнулён) будет указывать на допустимый диапазон, и проверка ничего криминального не обнаружит. В итоге мы получим более сложную проверку, которая случайным образом то будет срабатывать как надо, то нет. Уж лучше пусть тогда всегда падает. В этом случае ошибку будет легче выявить и локализовать на этапе отладки. Поэтому, имхо, проверки на 0 достаточно.

aureliano15 ★★
()
Ответ на: комментарий от anonymous

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

Полностью согласен, поэтому и оговорился, что

коли сделали, то не страшно, не умрём от потери нескольких тактов. :-)

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

aureliano15 ★★
()
Ответ на: комментарий от aureliano15

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

Ты ведь осознаёшь, что если у тебя вызов free() в горячем цикле, то ты что-то делаешь не так?

i-rinat ★★★★★
()
Ответ на: комментарий от aureliano15

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

Скорее «так уж получилось», чем «мы тщательно всё обдумали, и решили спроектировать вот так».

i-rinat ★★★★★
()
Ответ на: комментарий от i-rinat

Ты ведь осознаёшь, что если у тебя вызов free() в горячем цикле, то ты что-то делаешь не так?

Безусловно. Или это трэш-тест malloc/free. Но ведь я и говорил, что несколько тактов в данном случае не принципиальны.

Скорее «так уж получилось», чем «мы тщательно всё обдумали, и решили спроектировать вот так».

Скорее всего так и было.

aureliano15 ★★
()
Ответ на: комментарий от aureliano15

Скорее всего так и было.

Вот есть busybox, там и я и все придерживаются соглашения, что где есть дилемма size vs speed, она должна разрешаться в сторону уменьшения размера, если только потеря производительности не будет катастрофическая. Потому там вполне можно не обращать внимания на обходные маневры в алгоритмах по увеличению скорости в недопущении лишних вызовов free, так как зачастую это приводит к излишнему повторению кода «с» и «без» free, то есть к увеличению объема. И только на первый взгляд, оно кажется, что «так уж получилось».

vodz ★★★★★
()
Последнее исправление: vodz (всего исправлений: 1)
Ответ на: комментарий от vodz

Вот есть busybox, там и я и все придерживаются соглашения, что где есть дилемма size vs speed, она должна разрешаться в сторону уменьшения размера [skip] маневры в алгоритмах по увеличению скорости в недопущении лишних вызовов free, так как зачастую это приводит к излишнему повторению кода «с» и «без» free

Может быть и такое. Но мне кажется, что если уж считать каждый байт, одновременно, естественно, стремясь к надёжности программы, то множественное освобождение одной и той же области памяти в разных ветках программы потребует для обеспечения безопасности обнуления освобождённого указателя каждый раз. В результате сэкономленные при проверке на 0 байты мы потратим на присвоение 0 указателю во многих местах. Такой алгоритм, ориентированный на уменьшение кода любой ценой при сохранении его надёжности, ещё больше нуждается в аккуратном программировании и создании такого алгоритма, который высвобождает выделенную память по возможности только в одном месте. Хотя, оговорюсь, ситуации, конечно, могут быть разные.

А что касается скорости, то применительно к задачам реального времени алгоритм выделения памяти, кстати говоря, не оптимальный, т. к. не гарантирует одинаковое время работы malloc при любом состоянии кучи, хотя в среднем он работает быстрее других алгоритмов. Так что для rt-задач, пользующихся кучей, лучше создать свою библиотеку (или найти готовую реализацию), которая, может, в среднем не так быстра, но зато более стабильна в плане времени выполнения.

aureliano15 ★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.