Hello! On Tue, Oct 04, 2022 at 11:33:14PM +0700, Eugene Grosbein wrote:
> 04.10.2022 20:11, Evgeniy Berdnikov пишет: > > On Tue, Oct 04, 2022 at 12:00:57PM +0000, Korobov Vladimir via nginx-ru > > wrote: > >> После проверки исходного кода статическим анализатором (Svace > >> https://www.ispras.ru/technologies/svace/) выделено несколько > >> потенциально > >> опасных мест, закрывающихся приложенным патчем. > > > > Тупое выбрасывание кусков кода при проверке указателя на NULL не только > > не решает проблему, но создаёт более опасную ситуацию, когда код приложения > > может работать неверно, но ничего об этом не сообщать, так что поймать баг > > станет очень трудно. Лучше сегфолт в в точно локализованном месте, чем > > глюки непонятно где и непонятно отчего. > > > > При потенциальной возможности зануления указателя следует ловить и > > обрабатывать такое исключение. В противном случае нет смысла в проверке. > > Задача же не в ублажении тупых анализаторов, а в правильной работе кода. > > Как пользователь разнообразного софта, могу доложить, что сегфолт очень > фиговая обработка исключений. > Проверка указателя на NULL перед разадресацией в том случае, когда нельзя > гарантировать что он не NULL, > практически всегда благо. Другой вопрос, что потом делать, если вдруг: молча > восстановиться и ехать дальше, > или не молча, а с сообщением в лог, или выдать даже stack trace и выйти. Но > что угодно лучше сырого сегфолта. Так о том и речь, что в данном случае есть, потенциально, два варианта: - Статический анализатор прав, и в рассматриваемых местах возможно появление нулевых указателей. Тогда ситуацию надо обрабатывать, а не тупо игнорировать кусок кода. То есть патч плохой. - Статический анализатор не прав, и в рассматриваемых местах появление нулевых указателей невозможно (ну, кроме случая memory corruption). Тогда добавлять проверки с игнорированием кода опять же неправильно: они ничего не будут делать в штатном режиме, а в случае memory corruption - ещё и увеличивают шанс получить code execution вместо segmentation fault. То есть патч опять же плохой. Ситуаций, когда предлагаемый патч может быть хорошим - не прослеживается. Вообще, по моему опыту - в большинстве случаев статические анализаторы, даже лучшие, приносят мусор. Но иногда случается и что-нибудь полезное, поэтому жалобы какого-нибудь Coverity - регулярно отсматриваются. Но надо понимать, что анализ жалоб какого-либо статического анализатора - это кропотливый труда, а не взять и заткнуть тупыми проверками всё, на что оно вдруг решило пожаловаться по каким-то своим внутренним причинам. -- Maxim Dounin http://mdounin.ru/ _______________________________________________ nginx-ru mailing list -- nginx-ru@nginx.org To unsubscribe send an email to nginx-ru-le...@nginx.org